Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 58c114aecb | |||
| 4842ac64cc | |||
| a414509a94 |
@@ -19,7 +19,7 @@ jobs:
|
||||
- run: go build -o review-bot ./cmd/review-bot
|
||||
|
||||
# Self-review: builds from source since we're pre-release
|
||||
# Models configured to match SAP AI Core deployments
|
||||
# Models must match SAP AI Core deployments (use 'anthropic--' prefix for Claude)
|
||||
review:
|
||||
runs-on: ubuntu-24.04
|
||||
if: github.event_name == 'pull_request'
|
||||
|
||||
@@ -1,32 +0,0 @@
|
||||
name: PR Ready Gate
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [synchronize]
|
||||
|
||||
jobs:
|
||||
clear-labels:
|
||||
runs-on: ubuntu-24.04
|
||||
if: contains(github.event.pull_request.labels.*.name, 'self-reviewed')
|
||||
steps:
|
||||
- name: Remove self-reviewed label, reassign to author
|
||||
env:
|
||||
GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
|
||||
run: |
|
||||
PR_NUMBER=${{ github.event.pull_request.number }}
|
||||
AUTHOR=${{ github.event.pull_request.user.login }}
|
||||
SELF_REVIEWED_LABEL_ID=37
|
||||
|
||||
# Remove self-reviewed label if present
|
||||
curl -sS -X DELETE \
|
||||
-H "Authorization: token $GITEA_TOKEN" \
|
||||
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/issues/${PR_NUMBER}/labels/${SELF_REVIEWED_LABEL_ID}" || true
|
||||
|
||||
# Reassign to author
|
||||
curl -sS -X PATCH \
|
||||
-H "Authorization: token $GITEA_TOKEN" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d "{\"assignees\": [\"${AUTHOR}\"]}" \
|
||||
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/pulls/${PR_NUMBER}"
|
||||
|
||||
echo "Cleared self-reviewed label and reassigned PR #${PR_NUMBER} to ${AUTHOR}"
|
||||
+6
-20
@@ -316,20 +316,19 @@ func main() {
|
||||
sentinel := fmt.Sprintf("<!-- review-bot:%s -->", *reviewerName)
|
||||
|
||||
// Stale check: verify HEAD hasn't moved since we started
|
||||
// Re-fetch PR metadata to get the current HEAD SHA
|
||||
evaluatedSHA := pr.Head.Sha
|
||||
var currentSHA string
|
||||
currentPR, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber)
|
||||
if err != nil {
|
||||
slog.Warn("could not re-fetch PR for stale check", "pr", prNumber, "error", err)
|
||||
// currentSHA stays empty — shouldSkipStaleReview will return false
|
||||
} else {
|
||||
currentSHA = currentPR.Head.Sha
|
||||
}
|
||||
if shouldSkipStaleReview(evaluatedSHA, currentSHA) {
|
||||
// Continue anyway — better to post a potentially stale review than fail
|
||||
} else if currentPR.Head.Sha != evaluatedSHA {
|
||||
slog.Warn("HEAD moved during review — skipping stale review",
|
||||
"evaluated", evaluatedSHA,
|
||||
"current", currentSHA,
|
||||
"current", currentPR.Head.Sha,
|
||||
"pr", prNumber)
|
||||
// Exit successfully — this isn't an error, just outdated work
|
||||
// A new workflow run should already be in progress for the new HEAD
|
||||
return
|
||||
}
|
||||
|
||||
@@ -684,16 +683,3 @@ func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review {
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// shouldSkipStaleReview reports whether to skip posting because HEAD moved.
|
||||
// Returns true (skip) if evaluatedSHA differs from currentSHA.
|
||||
// Returns false (don't skip) if:
|
||||
// - SHAs match (no movement)
|
||||
// - currentSHA is empty (re-fetch failed; prefer posting stale over failing)
|
||||
func shouldSkipStaleReview(evaluatedSHA, currentSHA string) bool {
|
||||
if currentSHA == "" {
|
||||
// Re-fetch failed; better to post potentially stale than fail
|
||||
return false
|
||||
}
|
||||
return evaluatedSHA != currentSHA
|
||||
}
|
||||
|
||||
@@ -862,53 +862,3 @@ func TestFindAllOwnReviews(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestShouldSkipStaleReview(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
evaluatedSHA string
|
||||
currentSHA string
|
||||
wantSkip bool
|
||||
}{
|
||||
{
|
||||
name: "matching SHAs",
|
||||
evaluatedSHA: "abc123def456",
|
||||
currentSHA: "abc123def456",
|
||||
wantSkip: false,
|
||||
},
|
||||
{
|
||||
name: "different SHAs",
|
||||
evaluatedSHA: "abc123def456",
|
||||
currentSHA: "xyz789abc123",
|
||||
wantSkip: true,
|
||||
},
|
||||
{
|
||||
name: "empty current SHA (re-fetch failed)",
|
||||
evaluatedSHA: "abc123def456",
|
||||
currentSHA: "",
|
||||
wantSkip: false,
|
||||
},
|
||||
{
|
||||
name: "both empty (edge case)",
|
||||
evaluatedSHA: "",
|
||||
currentSHA: "",
|
||||
wantSkip: false,
|
||||
},
|
||||
{
|
||||
name: "only current empty",
|
||||
evaluatedSHA: "abc123",
|
||||
currentSHA: "",
|
||||
wantSkip: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := shouldSkipStaleReview(tc.evaluatedSHA, tc.currentSHA)
|
||||
if got != tc.wantSkip {
|
||||
t.Errorf("shouldSkipStaleReview(%q, %q) = %v, want %v",
|
||||
tc.evaluatedSHA, tc.currentSHA, got, tc.wantSkip)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user