Compare commits

..

3 Commits

Author SHA1 Message Date
Rodin 58c114aecb ci: use SAP AI Core model names and remove unavailable models
CI / test (pull_request) Successful in 15s
CI / review (/anthropic/v1, anthropic--claude-4.6-sonnet, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Successful in 21s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 34s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 55s
- Change claude-sonnet-4-6 to anthropic--claude-4.6-sonnet (SAP AI Core naming)
- Remove gpt-4.1, gpt-5-mini, gpt-4.1-mini (not deployed on SAP AI Core)
- Keep core reviewers: sonnet, gpt, security
2026-05-09 22:12:37 -07:00
Rodin 4842ac64cc chore: retrigger CI after LLM_BASE_URL fix
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 11s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 38s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m4s
2026-05-09 21:53:56 -07:00
Rodin a414509a94 fix: skip posting review when HEAD moves during evaluation
CI / test (pull_request) Successful in 15s
CI / review (/openai/v1, gpt-4.1-mini, gpt41-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 14s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 14s
CI / review (/anthropic/v1, claude-sonnet-4-6, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Failing after 15s
CI / review (/openai/v1, gpt-4.1, gpt41, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 14s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_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
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
2026-05-09 18:08:06 -07:00
4 changed files with 7 additions and 103 deletions
+1 -1
View File
@@ -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'
-32
View File
@@ -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
View File
@@ -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
}
-50
View File
@@ -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)
}
})
}
}