Compare commits
4 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| ced1fa7ffd | |||
| 6b615c77d5 | |||
| b43b86a4a5 | |||
| 2089ca0f2d |
+2
-16
@@ -19,6 +19,7 @@ jobs:
|
|||||||
- run: go build -o review-bot ./cmd/review-bot
|
- run: go build -o review-bot ./cmd/review-bot
|
||||||
|
|
||||||
# Self-review: builds from source since we're pre-release
|
# Self-review: builds from source since we're pre-release
|
||||||
|
# Models configured to match SAP AI Core deployments
|
||||||
review:
|
review:
|
||||||
runs-on: ubuntu-24.04
|
runs-on: ubuntu-24.04
|
||||||
if: github.event_name == 'pull_request'
|
if: github.event_name == 'pull_request'
|
||||||
@@ -30,27 +31,12 @@ jobs:
|
|||||||
token_secret: SONNET_REVIEW_TOKEN
|
token_secret: SONNET_REVIEW_TOKEN
|
||||||
provider: anthropic
|
provider: anthropic
|
||||||
llm_path: /anthropic/v1
|
llm_path: /anthropic/v1
|
||||||
model: claude-sonnet-4-6
|
model: anthropic--claude-4.6-sonnet
|
||||||
- name: gpt
|
- name: gpt
|
||||||
token_secret: GPT_REVIEW_TOKEN
|
token_secret: GPT_REVIEW_TOKEN
|
||||||
provider: openai
|
provider: openai
|
||||||
llm_path: /openai/v1
|
llm_path: /openai/v1
|
||||||
model: gpt-5
|
model: gpt-5
|
||||||
- name: gpt41
|
|
||||||
token_secret: GPT_REVIEW_TOKEN
|
|
||||||
provider: openai
|
|
||||||
llm_path: /openai/v1
|
|
||||||
model: gpt-4.1
|
|
||||||
- name: gpt5-mini
|
|
||||||
token_secret: GPT_REVIEW_TOKEN
|
|
||||||
provider: openai
|
|
||||||
llm_path: /openai/v1
|
|
||||||
model: gpt-5-mini
|
|
||||||
- name: gpt41-mini
|
|
||||||
token_secret: GPT_REVIEW_TOKEN
|
|
||||||
provider: openai
|
|
||||||
llm_path: /openai/v1
|
|
||||||
model: gpt-4.1-mini
|
|
||||||
- name: security
|
- name: security
|
||||||
token_secret: SECURITY_REVIEW_TOKEN
|
token_secret: SECURITY_REVIEW_TOKEN
|
||||||
provider: openai
|
provider: openai
|
||||||
|
|||||||
@@ -315,6 +315,24 @@ func main() {
|
|||||||
|
|
||||||
sentinel := fmt.Sprintf("<!-- review-bot:%s -->", *reviewerName)
|
sentinel := fmt.Sprintf("<!-- review-bot:%s -->", *reviewerName)
|
||||||
|
|
||||||
|
// Stale check: verify HEAD hasn't moved since we started
|
||||||
|
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) {
|
||||||
|
slog.Warn("HEAD moved during review — skipping stale review",
|
||||||
|
"evaluated", evaluatedSHA,
|
||||||
|
"current", currentSHA,
|
||||||
|
"pr", prNumber)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Map findings to inline comments for lines present in the diff
|
// Map findings to inline comments for lines present in the diff
|
||||||
diffRanges := gitea.ParseDiffNewLines(diff)
|
diffRanges := gitea.ParseDiffNewLines(diff)
|
||||||
var inlineComments []gitea.ReviewComment
|
var inlineComments []gitea.ReviewComment
|
||||||
@@ -666,3 +684,16 @@ func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review {
|
|||||||
}
|
}
|
||||||
return result
|
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,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