fix: skip posting review when HEAD moves during evaluation
CI / test (pull_request) Successful in 13s
CI / review (/anthropic/v1, claude-sonnet-4-6, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Failing after 13s
CI / review (/openai/v1, gpt-4.1, gpt41, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 13s
CI / review (/openai/v1, gpt-4.1-mini, gpt41-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 13s
CI / review (/openai/v1, gpt-5-mini, gpt5-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 13s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 53s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m3s
CI / test (pull_request) Successful in 13s
CI / review (/anthropic/v1, claude-sonnet-4-6, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Failing after 13s
CI / review (/openai/v1, gpt-4.1, gpt41, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 13s
CI / review (/openai/v1, gpt-4.1-mini, gpt41-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 13s
CI / review (/openai/v1, gpt-5-mini, gpt5-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 13s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 53s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m3s
When a new push arrives while review-bot is processing, the review would be posted against a stale commit. This causes noise in the PR timeline with findings that reference code that no longer exists. Before posting, re-fetch PR metadata and compare HEAD SHA with the commit we evaluated against. If they differ, log a warning and exit successfully — a new workflow run should already be processing the new HEAD. Fixes #52
This commit is contained in:
@@ -862,3 +862,53 @@ 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