Compare commits

..

10 Commits

Author SHA1 Message Date
Rodin 5132a2028d fix: address review findings (body truncation, unused field, whitespace)
CI / test (pull_request) Successful in 15s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 32s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m34s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 2m28s
Self-review fixes for PR #54:

- Add truncateBody helper to limit error message body length (200 chars)
  Addresses security bot finding about potential information leakage
  in error messages that include upstream response bodies

- Remove unused deployment.ID field from deployment struct
  Now stores just the URL string directly in the deployments map
  Addresses sonnet finding about unused struct field

- Add doc comment noting deployment cache limitation
  Documents that cache is never invalidated, acceptable for CI use case

- Fix trailing whitespace in action.yml aicore-resource-group default

All existing tests pass.
2026-05-10 08:28:49 -07:00
Rodin 065b8417dd ci: remove GPT models not deployed on AI Core
gpt-4.1, gpt-4.1-mini, and gpt-5-mini are not deployed on SAP AI Core.
Only gpt-5 and anthropic--claude-4.6-sonnet are available.

Removed matrix entries for non-existent deployments to fix CI failures.
2026-05-10 08:28:49 -07:00
Rodin 9a40b1baf4 docs: update README with AI Core configuration 2026-05-10 08:28:49 -07:00
Rodin f40584e1de ci: switch to native AI Core provider
- Remove HAI proxy dependency
- Use aicore provider with secrets
- Update model names to AI Core format (anthropic--claude-4.6-sonnet)
2026-05-10 08:28:49 -07:00
Rodin b0ff007aa2 feat: integrate AI Core client with review-bot
Adds aicore provider option that uses native SAP AI Core instead of
OpenAI-compatible proxy. Configurable via:
- AICORE_CLIENT_ID
- AICORE_CLIENT_SECRET
- AICORE_AUTH_URL
- AICORE_API_URL
- AICORE_RESOURCE_GROUP
2026-05-10 08:28:25 -07:00
Rodin 6379e303ba feat: add SAP AI Core client for Anthropic models
Implements native AI Core support with:
- OAuth2 token refresh
- Deployment discovery via /v2/lm/deployments
- Anthropic Messages API via /invoke endpoint
- Uses bedrock-2023-05-31 API version (AI Core uses Bedrock format)
- Model field omitted from body (deployment URL specifies model)
- Retry logic with exponential backoff

Tested via integration tests against live AI Core endpoint.
2026-05-10 08:28:25 -07:00
aweiker 4bb3a2f960 Merge pull request 'fix: skip posting review when HEAD moves during evaluation' (#53) from fix/stale-commit-check into main
CI / test (push) Successful in 15s
CI / review (/anthropic/v1, anthropic--claude-4.6-sonnet, sonnet, anthropic, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
Reviewed-on: #53
Reviewed-by: Aaron Weiker <aaron@weiker.org>
Reviewed-by: security-review-bot <10+security-review-bot@noreply.gitea.weiker.me>
2026-05-10 15:26:11 +00:00
Rodin ced1fa7ffd ci: fix model names to match SAP AI Core deployments
CI / test (pull_request) Successful in 14s
CI / review (/anthropic/v1, anthropic--claude-4.6-sonnet, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Successful in 26s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 35s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 50s
- Restore sonnet reviewer with correct model name (anthropic--claude-4.6-sonnet)
- Remove gpt-4.1, gpt-4.1-mini, gpt-5-mini (not deployed on SAP AI Core)
- Keep gpt-5 and security reviewers

The previous model names (claude-sonnet-4-6, etc.) were incorrect —
SAP AI Core uses 'anthropic--claude-4.6-sonnet' format.
2026-05-10 08:23:10 -07:00
Rodin 6b615c77d5 ci: remove unavailable models from review matrix
CI / test (pull_request) Successful in 15s
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 49s
Models claude-sonnet-4-6, gpt-4.1, gpt-4.1-mini, and gpt-5-mini are not
deployed on the LLM proxy, causing 502 errors. Keep only gpt-5 which
is the only available model.
2026-05-10 03:15:04 -07:00
Rodin b43b86a4a5 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
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 23:18:13 -07:00
2 changed files with 81 additions and 0 deletions
+31
View File
@@ -340,6 +340,24 @@ func main() {
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
diffRanges := gitea.ParseDiffNewLines(diff)
var inlineComments []gitea.ReviewComment
@@ -691,3 +709,16 @@ 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,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)
}
})
}
}