feat: native SAP AI Core support

Add native SAP AI Core provider that handles OAuth token management and
deployment discovery automatically. This eliminates the need for the
external LLM proxy when running in SAP environments.

Changes:
- Add AICoreClient with OAuth token caching and deployment URL discovery
- Support both Anthropic and OpenAI models via AI Core deployments
- Update CI to use native AI Core provider
- Update action inputs to accept AI Core credentials
- Update README with AI Core configuration examples

Model names must match AI Core deployment names (e.g. anthropic--claude-4.6-sonnet, gpt-5).

.gitea/actions/review/action.yml

@@ -26,18 +26,40 @@ inputs:
     required: false
     default: ''
   llm-base-url:
-    description: 'OpenAI-compatible LLM API base URL'
+    description: 'OpenAI-compatible LLM API base URL (not required for aicore provider)'
-    required: true
+    required: false
+    default: ''
   llm-api-key:
-    description: 'LLM API key'
+    description: 'LLM API key (not required for aicore provider)'
-    required: true
+    required: false
+    default: ''
   llm-model:
     description: 'LLM model name'
     required: true
   llm-provider:
-    description: 'LLM API provider: openai or anthropic (default openai)'
+    description: 'LLM API provider: openai, anthropic, or aicore (default openai)'
     required: false
     default: 'openai'
+  aicore-client-id:
+    description: 'SAP AI Core client ID (required for aicore provider)'
+    required: false
+    default: ''
+  aicore-client-secret:
+    description: 'SAP AI Core client secret (required for aicore provider)'
+    required: false
+    default: ''
+  aicore-auth-url:
+    description: 'SAP AI Core authentication URL (required for aicore provider)'
+    required: false
+    default: ''
+  aicore-api-url:
+    description: 'SAP AI Core API URL (required for aicore provider)'
+    required: false
+    default: ''
+  aicore-resource-group:
+    description: 'SAP AI Core resource group (default: default)'
+    required: false
+    default: 'default'
   conventions-file:
     description: 'Path to conventions file in the repo (e.g. CLAUDE.md)'
     required: false
@@ -155,6 +177,11 @@ runs:
         LLM_PROVIDER: ${{ inputs.llm-provider }}
         UPDATE_EXISTING: ${{ inputs.update-existing }}
         SYSTEM_PROMPT_FILE: ${{ inputs.system-prompt-file }}
+        AICORE_CLIENT_ID: ${{ inputs.aicore-client-id }}
+        AICORE_CLIENT_SECRET: ${{ inputs.aicore-client-secret }}
+        AICORE_AUTH_URL: ${{ inputs.aicore-auth-url }}
+        AICORE_API_URL: ${{ inputs.aicore-api-url }}
+        AICORE_RESOURCE_GROUP: ${{ inputs.aicore-resource-group }}
       run: |
         ARGS=""
         if [ "${{ inputs.dry-run }}" = "true" ]; then

.gitea/workflows/ci.yml

@@ -18,7 +18,10 @@ jobs:
       - run: go vet ./...
       - run: go build -o review-bot ./cmd/review-bot
 
-  # Self-review: builds from source since we're pre-release
+  # Self-review using native SAP AI Core provider
+  # Models must match SAP AI Core deployments
+  # Available models: gpt-5, anthropic--claude-4.6-sonnet, anthropic--claude-4.6-opus
+  # Removed gpt-4.1, gpt-5-mini, gpt-4.1-mini - not deployed on AI Core
   review:
     runs-on: ubuntu-24.04
     if: github.event_name == 'pull_request'
@@ -28,12 +31,12 @@ jobs:
         include:
           - name: sonnet
             token_secret: SONNET_REVIEW_TOKEN
-            model: gpt-5
+            model: anthropic--claude-4.6-sonnet
           - name: gpt
             token_secret: GPT_REVIEW_TOKEN
-            model: gpt-4.1
+            model: gpt-5
           - name: security
-            token_secret: SONNET_REVIEW_TOKEN
+            token_secret: SECURITY_REVIEW_TOKEN
             model: gpt-5
             system_prompt_file: SECURITY_REVIEW.md
     steps:
@@ -49,9 +52,13 @@ jobs:
           PR_NUMBER: ${{ github.event.pull_request.number }}
           REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }}
           REVIEWER_NAME: ${{ matrix.name }}
-          LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }}
+          LLM_PROVIDER: aicore
-          LLM_API_KEY: ${{ secrets.LLM_API_KEY }}
           LLM_MODEL: ${{ matrix.model }}
+          AICORE_CLIENT_ID: ${{ secrets.AICORE_CLIENT_ID }}
+          AICORE_CLIENT_SECRET: ${{ secrets.AICORE_CLIENT_SECRET }}
+          AICORE_AUTH_URL: ${{ secrets.AICORE_AUTH_URL }}
+          AICORE_API_URL: ${{ secrets.AICORE_API_URL }}
+          AICORE_RESOURCE_GROUP: ${{ secrets.AICORE_RESOURCE_GROUP }}
           CONVENTIONS_FILE: "CONVENTIONS.md"
           PATTERNS_REPO: "rodin/go-patterns"
           PATTERNS_FILES: "README.md,patterns/"

.gitea/workflows/pr-ready-gate.yml

@@ -0,0 +1,32 @@
+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}"

.gitea/workflows/release.yml

@@ -69,14 +69,28 @@ jobs:
 
           echo "Release ID: ${RELEASE_ID}"
 
-          # Upload each asset
+          # Upload each asset (idempotent: delete existing asset with same name first)
           for file in dist/*; do
             filename=$(basename "$file")
             echo "Uploading ${filename}..."
+
+            # Check if asset already exists and delete it
+            EXISTING_ID=$(export ASSET_NAME="${filename}"; curl -sS \
+              -H "Authorization: token ${GITEA_TOKEN}" \
+              "${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets" \
+              | python3 -c "import json,sys,os; name=os.environ['ASSET_NAME']; assets=json.load(sys.stdin); print(next((str(a['id']) for a in assets if a['name']==name),''))" 2>/dev/null)
+
+            if [ -n "$EXISTING_ID" ]; then
+              echo "  Asset ${filename} already exists (id=${EXISTING_ID}), deleting..."
+              curl -sSf -X DELETE \
+                -H "Authorization: token ${GITEA_TOKEN}" \
+                "${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets/${EXISTING_ID}"
+            fi
+
             curl -sSf -X POST \
               -H "Authorization: token ${GITEA_TOKEN}" \
               -H "Content-Type: application/octet-stream" \
-              "${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=${filename}" \
+              "${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=$(printf '%s' "${filename}" | jq -sRr @uri)" \
               --data-binary "@${file}"
           done
 

.gitignore

@@ -1 +1,2 @@
 /review-bot
+coverage.out

Makefile

@@ -0,0 +1,20 @@
+.PHONY: build test test-integration lint clean coverage
+
+build:
+	go build -o review-bot ./cmd/review-bot/
+
+test:
+	go test ./...
+
+test-integration:
+	go test -tags integration -v ./cmd/review-bot/
+
+lint:
+	go vet ./...
+
+clean:
+	rm -f review-bot
+
+coverage:
+	go test -coverprofile=coverage.out ./...
+	go tool cover -func=coverage.out

README.md

@@ -4,7 +4,7 @@ AI-powered code review bot for Gitea pull requests. Fetches diff + context, send
 
 ## Features
 
-- **Multi-provider**: OpenAI-compatible and Anthropic Messages API
+- **Multi-provider**: OpenAI-compatible, Anthropic Messages API, and SAP AI Core
 - **Context-aware**: Fetches full file content, conventions, language patterns, CI status
 - **Smart budget**: Automatically trims context to fit model token limits
 - **Idempotent reviews**: Posts new review, then cleans up stale ones (one review per bot)
@@ -168,16 +168,41 @@ Prints the review to CI logs without posting to the PR. Useful for testing promp
     llm-provider: anthropic
 ```
 
+### Using SAP AI Core
+
+For SAP environments with AI Core deployments, use the `aicore` provider for native authentication:
+
+```yaml
+- uses: https://gitea.weiker.me/rodin/review-bot/.gitea/actions/review@v0.1.0
+  with:
+    reviewer-token: ${{ secrets.REVIEW_TOKEN }}
+    reviewer-name: aicore-review
+    llm-model: anthropic--claude-4.6-sonnet  # or gpt-5
+    llm-provider: aicore
+    aicore-client-id: ${{ secrets.AICORE_CLIENT_ID }}
+    aicore-client-secret: ${{ secrets.AICORE_CLIENT_SECRET }}
+    aicore-auth-url: ${{ secrets.AICORE_AUTH_URL }}
+    aicore-api-url: ${{ secrets.AICORE_API_URL }}
+    aicore-resource-group: default
+```
+
+AI Core handles OAuth token management and deployment discovery automatically. Model names must match the deployment name in AI Core (e.g. `anthropic--claude-4.6-sonnet`, `gpt-5`).
+
 ## Action Inputs
 
 | Input | Required | Default | Description |
 |-------|----------|---------|-------------|
 | `reviewer-token` | Yes | — | Gitea token for posting reviews (needs `write:issue`, `write:repository`) |
 | `reviewer-name` | No | `""` | Logical identity for this reviewer. Used as sentinel for idempotent cleanup. Set this when running multiple review bots on the same PR. |
-| `llm-base-url` | Yes | — | LLM API base URL |
+| `llm-base-url` | No* | `""` | LLM API base URL (required unless using aicore provider) |
-| `llm-api-key` | Yes | — | LLM API key |
+| `llm-api-key` | No* | `""` | LLM API key (required unless using aicore provider) |
 | `llm-model` | Yes | — | Model name |
-| `llm-provider` | No | `openai` | API provider: `openai` or `anthropic` |
+| `llm-provider` | No | `openai` | API provider: `openai`, `anthropic`, or `aicore` |
+| `aicore-client-id` | No** | `""` | SAP AI Core client ID |
+| `aicore-client-secret` | No** | `""` | SAP AI Core client secret |
+| `aicore-auth-url` | No** | `""` | SAP AI Core authentication URL |
+| `aicore-api-url` | No** | `""` | SAP AI Core API URL |
+| `aicore-resource-group` | No | `default` | SAP AI Core resource group |
 | `conventions-file` | No | `""` | Path to coding conventions file in the repo |
 | `patterns-repo` | No | `""` | Comma-separated repos with language patterns (e.g. `rodin/go-patterns`) |
 | `patterns-files` | No | `README.md` | Files/directories to fetch from pattern repos |
@@ -188,6 +213,21 @@ Prints the review to CI logs without posting to the PR. Useful for testing promp
 | `update-existing` | No | `true` | Delete previous review from same bot before posting. Accepts: true/1/yes or false/0/no |
 | `version` | No | `latest` | review-bot version to install |
 
+*Required for `openai` and `anthropic` providers, not for `aicore`.
+**Required only for `aicore` provider.
+
+## Runner Requirements
+
+The composite action requires these tools on the runner:
+
+| Tool | Used For |
+|------|----------|
+| `python3` | JSON parsing during version detection |
+| `sha256sum` | Checksum verification of downloaded binary |
+| `curl` | Downloading releases and querying the API |
+
+All three are pre-installed on `ubuntu-*` runners (e.g. `ubuntu-24.04`). If you use a custom runner image, ensure these are available.
+
 ## How Review Cleanup Works
 
 When `reviewer-name` is set, the bot embeds a hidden sentinel in each review:

cmd/review-bot/integration_test.go

@@ -0,0 +1,161 @@
+//go:build integration
+
+package main
+
+import (
+	"context"
+	"os"
+	"strconv"
+	"strings"
+	"testing"
+
+	"gitea.weiker.me/rodin/review-bot/gitea"
+	"gitea.weiker.me/rodin/review-bot/llm"
+	"gitea.weiker.me/rodin/review-bot/review"
+)
+
+// Integration test requires a running Gitea instance and LLM endpoint.
+// Set environment variables:
+//
+//	INTEGRATION_GITEA_URL     - Gitea base URL
+//	INTEGRATION_GITEA_TOKEN   - Gitea API token with repo access
+//	INTEGRATION_GITEA_REPO    - owner/repo with an open PR
+//	INTEGRATION_PR_NUMBER     - PR number to test against
+//	INTEGRATION_LLM_BASE_URL  - LLM API base URL
+//	INTEGRATION_LLM_API_KEY   - LLM API key
+//	INTEGRATION_LLM_MODEL     - Model name
+func TestIntegration_FullReviewFlow(t *testing.T) {
+	giteaURL := os.Getenv("INTEGRATION_GITEA_URL")
+	giteaToken := os.Getenv("INTEGRATION_GITEA_TOKEN")
+	giteaRepo := os.Getenv("INTEGRATION_GITEA_REPO")
+	prNumStr := os.Getenv("INTEGRATION_PR_NUMBER")
+	llmBaseURL := os.Getenv("INTEGRATION_LLM_BASE_URL")
+	llmAPIKey := os.Getenv("INTEGRATION_LLM_API_KEY")
+	llmModel := os.Getenv("INTEGRATION_LLM_MODEL")
+
+	if giteaURL == "" || giteaToken == "" || giteaRepo == "" || prNumStr == "" ||
+		llmBaseURL == "" || llmAPIKey == "" || llmModel == "" {
+		t.Skip("Integration test env vars not set, skipping")
+	}
+
+	prNumber, err := strconv.Atoi(prNumStr)
+	if err != nil {
+		t.Fatalf("Invalid PR number %q: %v", prNumStr, err)
+	}
+
+	// Parse owner/repo
+	parts := strings.SplitN(giteaRepo, "/", 2)
+	if len(parts) != 2 {
+		t.Fatalf("Invalid repo format %q", giteaRepo)
+	}
+	owner, repoName := parts[0], parts[1]
+	if owner == "" || repoName == "" {
+		t.Fatalf("Invalid repo format %q", giteaRepo)
+	}
+
+	ctx := context.Background()
+
+	// Step 1: Fetch PR
+	giteaClient := gitea.NewClient(giteaURL, giteaToken)
+	pr, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber)
+	if err != nil {
+		t.Fatalf("GetPullRequest: %v", err)
+	}
+	t.Logf("PR: %s (sha: %s)", pr.Title, pr.Head.Sha)
+
+	// Step 2: Fetch diff
+	diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber)
+	if err != nil {
+		t.Fatalf("GetPullRequestDiff: %v", err)
+	}
+	if diff == "" {
+		t.Fatal("diff is empty")
+	}
+	t.Logf("Diff size: %d bytes", len(diff))
+
+	// Step 3: Build prompts
+	systemPrompt := review.BuildSystemPrompt("", "")
+	userPrompt := review.BuildUserPrompt(pr.Title, pr.Body, diff, "", true, "")
+
+	// Step 4: Call LLM
+	llmClient := llm.NewClient(llmBaseURL, llmAPIKey, llmModel)
+	response, err := llmClient.Complete(ctx, []llm.Message{
+		{Role: "system", Content: systemPrompt},
+		{Role: "user", Content: userPrompt},
+	})
+	if err != nil {
+		t.Fatalf("LLM Complete: %v", err)
+	}
+	t.Logf("LLM response: %d bytes", len(response))
+
+	// Step 5: Parse response
+	result, err := review.ParseResponse(response)
+	if err != nil {
+		t.Fatalf("ParseResponse: %v", err)
+	}
+	t.Logf("Verdict: %s, Findings: %d", result.Verdict, len(result.Findings))
+
+	// Step 6: Format (dry-run validation)
+	body := review.FormatMarkdown(result, "integration-test")
+	if body == "" {
+		t.Fatal("formatted review body is empty")
+	}
+	t.Logf("Review body:\n%s", body)
+}
+
+func TestIntegration_PostAndCleanup(t *testing.T) {
+	giteaURL := os.Getenv("INTEGRATION_GITEA_URL")
+	giteaToken := os.Getenv("INTEGRATION_GITEA_TOKEN")
+	giteaRepo := os.Getenv("INTEGRATION_GITEA_REPO")
+	prNumStr := os.Getenv("INTEGRATION_PR_NUMBER")
+
+	if giteaURL == "" || giteaToken == "" || giteaRepo == "" || prNumStr == "" {
+		t.Skip("Integration test env vars not set, skipping")
+	}
+
+	prNumber, err := strconv.Atoi(prNumStr)
+	if err != nil {
+		t.Fatalf("Invalid PR number %q: %v", prNumStr, err)
+	}
+
+	parts := strings.SplitN(giteaRepo, "/", 2)
+	if len(parts) != 2 {
+		t.Fatalf("Invalid repo format %q", giteaRepo)
+	}
+	owner, repoName := parts[0], parts[1]
+
+	ctx := context.Background()
+	giteaClient := gitea.NewClient(giteaURL, giteaToken)
+
+	// Post a test review
+	sentinel := "<!-- review-bot:integration-test -->"
+	testBody := "# Integration Test Review\n\nThis is a test review.\n\n" + sentinel
+	posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, "COMMENT", testBody, nil)
+	if err != nil {
+		t.Fatalf("PostReview: %v", err)
+	}
+	t.Logf("Posted review ID: %d", posted.ID)
+
+	// Verify it appears in listing
+	reviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
+	if err != nil {
+		t.Fatalf("ListReviews: %v", err)
+	}
+
+	found := false
+	for _, r := range reviews {
+		if r.ID == posted.ID && strings.Contains(r.Body, sentinel) {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Error("posted review not found in listing")
+	}
+
+	// Cleanup: delete the test review
+	err = giteaClient.DeleteReview(ctx, owner, repoName, prNumber, posted.ID)
+	if err != nil {
+		t.Logf("Warning: could not delete test review %d: %v", posted.ID, err)
+	}
+}

cmd/review-bot/main.go

@@ -4,7 +4,7 @@ import (
 	"context"
 	"flag"
 	"fmt"
-	"log"
+	"log/slog"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -19,8 +19,40 @@ import (
 
 var version = "dev"
 
+// setupLogger configures the global slog default logger based on format and verbosity.
+func setupLogger(format, verbosity string) {
+	var level slog.Level
+	switch strings.ToLower(verbosity) {
+	case "debug":
+		level = slog.LevelDebug
+	case "info":
+		level = slog.LevelInfo
+	case "warn":
+		level = slog.LevelWarn
+	case "error":
+		level = slog.LevelError
+	default:
+		level = slog.LevelInfo
+	}
+
+	opts := &slog.HandlerOptions{Level: level}
+
+	var handler slog.Handler
+	switch strings.ToLower(format) {
+	case "json":
+		handler = slog.NewJSONHandler(os.Stderr, opts)
+	default:
+		handler = slog.NewTextHandler(os.Stderr, opts)
+	}
+
+	slog.SetDefault(slog.New(handler))
+}
+
 func main() {
 	versionFlag := flag.Bool("version", false, "Print version and exit")
+	// Logging flags
+	logFormat := flag.String("log-format", envOrDefault("LOG_FORMAT", "text"), "Log output format: text or json")
+	verbosity := flag.String("verbosity", envOrDefault("LOG_VERBOSITY", "info"), "Log verbosity: debug, info, warn, error")
 	// CLI flags
 	giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL")
 	repo := flag.String("repo", envOrDefault("GITEA_REPO", ""), "Repository (owner/name)")
@@ -35,10 +67,15 @@ func main() {
 	patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)")
 	patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", "README.md"), "Comma-separated file paths to fetch from patterns repo")
 	dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting")
-	updateExisting := flag.Bool("update-existing", envOrDefaultBool("UPDATE_EXISTING", true), "Delete previous review from same bot before posting (default true)")
 	llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
 	llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)")
-	llmProvider := flag.String("llm-provider", envOrDefault("LLM_PROVIDER", "openai"), "LLM API provider: openai or anthropic")
+	llmProvider := flag.String("llm-provider", envOrDefault("LLM_PROVIDER", "openai"), "LLM API provider: openai, anthropic, or aicore")
+	// AI Core specific flags (only used when provider=aicore)
+	aicoreClientID := flag.String("aicore-client-id", envOrDefault("AICORE_CLIENT_ID", ""), "SAP AI Core client ID (for provider=aicore)")
+	aicoreClientSecret := flag.String("aicore-client-secret", envOrDefault("AICORE_CLIENT_SECRET", ""), "SAP AI Core client secret (for provider=aicore)")
+	aicoreAuthURL := flag.String("aicore-auth-url", envOrDefault("AICORE_AUTH_URL", ""), "SAP AI Core auth URL (for provider=aicore)")
+	aicoreAPIURL := flag.String("aicore-api-url", envOrDefault("AICORE_API_URL", ""), "SAP AI Core API URL (for provider=aicore)")
+	aicoreResourceGroup := flag.String("aicore-resource-group", envOrDefault("AICORE_RESOURCE_GROUP", "default"), "SAP AI Core resource group (for provider=aicore)")
 
 	flag.Parse()
 
@@ -47,39 +84,56 @@ func main() {
 		os.Exit(0)
 	}
 
-	log.Printf("review-bot %s", version)
+	// Initialize structured logger
+	setupLogger(*logFormat, *verbosity)
+
+	slog.Info("review-bot starting", "version", version)
 
 	// Validate required fields
-	if *giteaURL == "" || *repo == "" || *prNum == "" || *reviewerToken == "" ||
+	// For aicore provider, llm-base-url and llm-api-key are not required
-		*llmBaseURL == "" || *llmAPIKey == "" || *llmModel == "" {
+	isAICore := llm.Provider(*llmProvider) == llm.ProviderAICore
+	if *giteaURL == "" || *repo == "" || *prNum == "" || *reviewerToken == "" || *llmModel == "" {
 		fmt.Fprintf(os.Stderr, "Error: missing required flags or environment variables\n\n")
-		fmt.Fprintf(os.Stderr, "Required: --gitea-url, --repo, --pr, --reviewer-token, --llm-base-url, --llm-api-key, --llm-model\n")
+		fmt.Fprintf(os.Stderr, "Required: --gitea-url, --repo, --pr, --reviewer-token, --llm-model\n")
+		os.Exit(1)
+	}
+	if !isAICore && (*llmBaseURL == "" || *llmAPIKey == "") {
+		fmt.Fprintf(os.Stderr, "Error: --llm-base-url and --llm-api-key are required for provider=%s\n", *llmProvider)
+		os.Exit(1)
+	}
+	if isAICore && (*aicoreClientID == "" || *aicoreClientSecret == "" || *aicoreAuthURL == "" || *aicoreAPIURL == "") {
+		fmt.Fprintf(os.Stderr, "Error: AI Core credentials required for provider=aicore\n\n")
+		fmt.Fprintf(os.Stderr, "Required: --aicore-client-id, --aicore-client-secret, --aicore-auth-url, --aicore-api-url\n")
 		os.Exit(1)
 	}
 
 	// Validate reviewer-name: only safe characters allowed in sentinel
 	if err := validateReviewerName(*reviewerName); err != nil {
-		log.Fatalf("%v", err)
+		slog.Error("invalid reviewer name", "error", err)
+		os.Exit(1)
 	}
 
 	// Parse repo owner/name
 	parts := strings.SplitN(*repo, "/", 2)
 	if len(parts) != 2 {
-		log.Fatalf("Invalid repo format %q, expected owner/name", *repo)
+		slog.Error("invalid repo format", "repo", *repo, "expected", "owner/name")
+		os.Exit(1)
 	}
 	owner, repoName := parts[0], parts[1]
 
 	// Parse PR number
 	prNumber, err := strconv.Atoi(*prNum)
 	if err != nil {
-		log.Fatalf("Invalid PR number %q: %v", *prNum, err)
+		slog.Error("invalid PR number", "pr", *prNum, "error", err)
+		os.Exit(1)
 	}
 
 	// Initialize clients
 	giteaClient := gitea.NewClient(*giteaURL, *reviewerToken)
 	llmClient := llm.NewClient(*llmBaseURL, *llmAPIKey, *llmModel)
 	if *llmTemp < 0 || *llmTemp > 2 {
-		log.Fatal("--llm-temperature must be between 0 and 2")
+		slog.Error("invalid LLM temperature", "temperature", *llmTemp, "range", "0-2")
+		os.Exit(1)
 	}
 	if *llmTemp > 0 {
 		llmClient.WithTemperature(*llmTemp)
@@ -87,8 +141,18 @@ func main() {
 	switch llm.Provider(*llmProvider) {
 	case llm.ProviderOpenAI, llm.ProviderAnthropic:
 		llmClient.WithProvider(llm.Provider(*llmProvider))
+	case llm.ProviderAICore:
+		llmClient.WithAICore(llm.AICoreConfig{
+			ClientID:      *aicoreClientID,
+			ClientSecret:  *aicoreClientSecret,
+			AuthURL:       *aicoreAuthURL,
+			APIURL:        *aicoreAPIURL,
+			ResourceGroup: *aicoreResourceGroup,
+		})
+		slog.Info("using SAP AI Core provider", "resource_group", *aicoreResourceGroup)
 	default:
-		log.Fatalf("Invalid --llm-provider %q, must be openai or anthropic", *llmProvider)
+		slog.Error("invalid LLM provider", "provider", *llmProvider, "valid", "openai, anthropic, aicore")
+		os.Exit(1)
 	}
 	if *llmTimeout > 0 {
 		llmClient.WithTimeout(time.Duration(*llmTimeout) * time.Second)
@@ -99,30 +163,32 @@ func main() {
 	ctx, cancel := context.WithTimeout(context.Background(), overallTimeout)
 	defer cancel()
 
-	log.Printf("Reviewing PR #%d on %s/%s", prNumber, owner, repoName)
+	slog.Info("reviewing pull request", "pr", prNumber, "repo", fmt.Sprintf("%s/%s", owner, repoName))
 
 	// Step 1: Fetch PR metadata
 	pr, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber)
 	if err != nil {
-		log.Fatalf("Failed to fetch PR: %v", err)
+		slog.Error("failed to fetch PR", "pr", prNumber, "error", err)
+		os.Exit(1)
 	}
-	log.Printf("PR: %s", pr.Title)
+	slog.Info("fetched PR metadata", "pr", prNumber, "title", pr.Title)
 
 	// Step 2: Fetch diff
 	diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber)
 	if err != nil {
-		log.Fatalf("Failed to fetch diff: %v", err)
+		slog.Error("failed to fetch diff", "pr", prNumber, "error", err)
+		os.Exit(1)
 	}
-	log.Printf("Diff size: %d bytes", len(diff))
+	slog.Info("fetched diff", "bytes", len(diff))
 
 	// Step 3: Fetch full file content for modified files
 	fileContext := ""
 	files, err := giteaClient.GetPullRequestFiles(ctx, owner, repoName, prNumber)
 	if err != nil {
-		log.Printf("Warning: could not fetch PR files list: %v", err)
+		slog.Warn("could not fetch PR files list", "pr", prNumber, "error", err)
 	} else {
 		fileContext = fetchFileContext(ctx, giteaClient, owner, repoName, pr.Head.Ref, files)
-		log.Printf("Fetched full context for %d files", len(files))
+		slog.Debug("fetched file context", "files", len(files))
 	}
 
 	// Step 4: Check CI status
@@ -131,10 +197,10 @@ func main() {
 	if pr.Head.Sha != "" {
 		statuses, err := giteaClient.GetCommitStatuses(ctx, owner, repoName, pr.Head.Sha)
 		if err != nil {
-			log.Printf("Warning: could not fetch CI status: %v", err)
+			slog.Warn("could not fetch CI status", "sha", pr.Head.Sha, "error", err)
 		} else {
 			ciPassed, ciDetails = evaluateCIStatus(statuses)
-			log.Printf("CI status: passed=%v", ciPassed)
+			slog.Info("CI status checked", "passed", ciPassed)
 		}
 	}
 
@@ -143,10 +209,10 @@ func main() {
 	if *conventionsFile != "" {
 		content, err := giteaClient.GetFileContent(ctx, owner, repoName, *conventionsFile)
 		if err != nil {
-			log.Printf("Warning: could not load conventions file %q: %v", *conventionsFile, err)
+			slog.Warn("could not load conventions file", "file", *conventionsFile, "error", err)
 		} else {
 			conventions = content
-			log.Printf("Loaded conventions file: %s (%d bytes)", *conventionsFile, len(conventions))
+			slog.Debug("loaded conventions file", "file", *conventionsFile, "bytes", len(conventions))
 		}
 	}
 
@@ -154,7 +220,7 @@ func main() {
 	patterns := ""
 	if *patternsRepo != "" {
 		patterns = fetchPatterns(ctx, giteaClient, *patternsRepo, *patternsFiles)
-		log.Printf("Loaded patterns from %s (%d bytes)", *patternsRepo, len(patterns))
+		slog.Debug("loaded patterns", "repo", *patternsRepo, "bytes", len(patterns))
 	}
 
 	// Step 6b: Load additional system prompt if specified
@@ -166,27 +232,32 @@ func main() {
 		}
 		absWorkspace, err := filepath.Abs(workspace)
 		if err != nil {
-			log.Fatalf("Failed to resolve workspace path: %v", err)
+			slog.Error("failed to resolve workspace path", "error", err)
+			os.Exit(1)
 		}
 		promptPath := filepath.Join(absWorkspace, *systemPromptFile)
 		promptPath = filepath.Clean(promptPath)
 		if !strings.HasPrefix(promptPath, absWorkspace+string(filepath.Separator)) && promptPath != absWorkspace {
-			log.Fatalf("system-prompt-file resolves outside workspace (got %q, workspace %q)", promptPath, absWorkspace)
+			slog.Error("system-prompt-file resolves outside workspace", "path", promptPath, "workspace", absWorkspace)
+			os.Exit(1)
 		}
 		// Resolve symlinks and re-validate to prevent symlink traversal
 		resolvedPath, err := filepath.EvalSymlinks(promptPath)
 		if err != nil {
-			log.Fatalf("Failed to resolve system prompt file %q: %v", promptPath, err)
+			slog.Error("failed to resolve system prompt file", "path", promptPath, "error", err)
+			os.Exit(1)
 		}
 		if !strings.HasPrefix(resolvedPath, absWorkspace+string(filepath.Separator)) && resolvedPath != absWorkspace {
-			log.Fatalf("system-prompt-file symlink resolves outside workspace (got %q, workspace %q)", resolvedPath, absWorkspace)
+			slog.Error("system-prompt-file symlink resolves outside workspace", "resolved", resolvedPath, "workspace", absWorkspace)
+			os.Exit(1)
 		}
 		data, err := os.ReadFile(resolvedPath)
 		if err != nil {
-			log.Fatalf("Failed to read system prompt file %q: %v", promptPath, err)
+			slog.Error("failed to read system prompt file", "path", promptPath, "error", err)
+			os.Exit(1)
 		}
 		additionalPrompt = string(data)
-		log.Printf("Loaded system prompt file: %s (%d bytes)", *systemPromptFile, len(additionalPrompt))
+		slog.Debug("loaded system prompt file", "file", *systemPromptFile, "bytes", len(additionalPrompt))
 	}
 
 	// Step 7: Budget-aware prompt assembly
@@ -203,33 +274,61 @@ func main() {
 		UserMeta:    review.BuildUserMeta(pr.Title, pr.Body, ciPassed, ciDetails),
 	}
 	budgetResult := budget.Fit(*llmModel, sections)
-	log.Printf("Token estimate: ~%dK (limit: %dK)", budgetResult.EstTokens/1000, budget.LimitForModel(*llmModel)/1000)
+	slog.Info("token budget calculated", "tokens", budgetResult.EstTokens, "limit", budget.LimitForModel(*llmModel), "model", *llmModel)
 	if len(budgetResult.Trimmed) > 0 {
-		log.Printf("Context trimmed: %v", budgetResult.Trimmed)
+		slog.Warn("context trimmed to fit budget", "trimmed", budgetResult.Trimmed)
 	}
 
-	// Step 8: Call LLM
+	// Step 8: Call LLM (with retry on parse failure)
-	log.Printf("Sending to LLM (%s)...", *llmModel)
+	slog.Info("sending request to LLM", "model", *llmModel)
 	messages := []llm.Message{
 		{Role: "system", Content: budgetResult.SystemPrompt},
 		{Role: "user", Content: budgetResult.UserPrompt},
 	}
 
-	response, err := llmClient.Complete(ctx, messages)
+	var response string
-	if err != nil {
+	var result *review.ReviewResult
-		log.Fatalf("LLM request failed: %v", err)
+	for attempt := 1; attempt <= 2; attempt++ {
-	}
+		if attempt > 1 {
-	log.Printf("LLM response received (%d bytes)", len(response))
+			slog.Warn("retrying LLM request after parse failure", "attempt", attempt)
+			time.Sleep(time.Second)
+		}
 
-	// Step 9: Parse response
+		response, err = llmClient.Complete(ctx, messages)
-	result, err := review.ParseResponse(response)
+		if err != nil {
-	if err != nil {
+			slog.Error("LLM request failed", "model", *llmModel, "error", err, "attempt", attempt)
-		log.Fatalf("Failed to parse LLM response: %v", err)
+			if attempt == 2 {
+				os.Exit(1)
+			}
+			continue
+		}
+		slog.Info("LLM response received", "bytes", len(response), "attempt", attempt)
+
+		// Step 9: Parse response
+		result, err = review.ParseResponse(response)
+		if err != nil {
+			slog.Error("failed to parse LLM response", "error", err, "attempt", attempt)
+			if attempt == 2 {
+				os.Exit(1)
+			}
+			continue
+		}
+		break
 	}
-	log.Printf("Verdict: %s (%d findings)", result.Verdict, len(result.Findings))
+	slog.Info("review parsed", "verdict", result.Verdict, "findings", len(result.Findings))
 
 	// Step 10: Format and post review
 	reviewBody := review.FormatMarkdown(result, *reviewerName)
+
+	// Add commit footer so readers know which commit was evaluated
+	if pr.Head.Sha != "" {
+		shortSHA := pr.Head.Sha
+		if len(shortSHA) > 8 {
+			shortSHA = shortSHA[:8]
+		}
+		reviewBody += fmt.Sprintf("\n\n---\n*Evaluated against %s*", shortSHA)
+	}
+
 	event := review.GiteaEvent(result.Verdict)
 
 	if *dryRun {
@@ -241,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
@@ -254,97 +371,91 @@ func main() {
 		}
 	}
 	if len(inlineComments) > 0 {
-		log.Printf("Attaching %d inline comments", len(inlineComments))
+		slog.Debug("attaching inline comments", "count", len(inlineComments))
 	}
 
 	// --- Review update strategy ---
-	// 1. No existing review → POST new
+	// 1. POST new review first (gets non-stale approval badge on HEAD)
-	// 2. Existing review, same state → PATCH body in place (preserves threads)
+	// 2. Then supersede old review with link to the new one
-	// 3. Existing review, state change → PATCH old to "Superseded", POST new
+	// Order matters: post first so we have the new review's URL for the supersede message.
-	if *updateExisting && *reviewerName != "" {
+	var oldReviews []gitea.Review
+	if *reviewerName != "" {
 		existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
 		if err != nil {
-			log.Printf("Warning: could not list existing reviews: %v", err)
+			slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
 		} else {
-			// Worst-wins: escalate if a sibling blocks (need own login from existing review)
+			if hasSharedToken(existingReviews, sentinel) {
-			ownLogin := ""
+				slog.Warn("shared token mode: skipping supersede to avoid clobbering sibling review")
-			existing := findOwnReview(existingReviews, sentinel)
+			} else {
-			if existing != nil {
+				oldReviews = findAllOwnReviews(existingReviews, sentinel)
-				ownLogin = existing.User.Login
-			}
-			if event == "APPROVED" && shouldEscalate(existingReviews, 0, ownLogin, sentinel) {
-				log.Printf("Sibling review has REQUEST_CHANGES; escalating to REQUEST_CHANGES")
-				event = "REQUEST_CHANGES"
-			}
-
-			if existing != nil {
-				if reviewUnchanged(existingReviews, reviewBody, event, sentinel) {
-					log.Printf("Review unchanged from previous run; skipping to preserve threads")
-					return
-				}
-
-				// Same state → PATCH in place
-				if existing.State == event {
-					commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
-					if err != nil {
-						log.Printf("Warning: could not find review comment ID, falling back to new post: %v", err)
-					} else {
-						if err := giteaClient.EditComment(ctx, owner, repoName, commentID, reviewBody); err != nil {
-							log.Printf("Warning: could not edit review, falling back to new post: %v", err)
-						} else {
-							log.Printf("Review updated in place (comment_id=%d)", commentID)
-							return
-						}
-					}
-				} else {
-					// State change → mark old as superseded, post new below
-					commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
-					if err != nil {
-						log.Printf("Warning: could not find old review comment ID: %v", err)
-					} else {
-						supersededBody := fmt.Sprintf("~~*This review has been superseded by a newer review below.*~~\n\n%s", sentinel)
-						if err := giteaClient.EditComment(ctx, owner, repoName, commentID, supersededBody); err != nil {
-							log.Printf("Warning: could not mark old review as superseded: %v", err)
-						} else {
-							log.Printf("Marked old review as superseded (state was %s, now %s)", existing.State, event)
-						}
-					}
-				}
 			}
 		}
 	}
 
-	// POST new review (first run, or state transition fallthrough)
+	// Self-request as reviewer (ensures we appear in required-reviewer checks)
-	log.Printf("Posting review (event=%s)...", event)
+	authUser, err := giteaClient.GetAuthenticatedUser(ctx)
+	if err != nil {
+		slog.Warn("could not determine authenticated user for reviewer self-request", "error", err)
+	} else if authUser != "" {
+		if err := giteaClient.RequestReviewer(ctx, owner, repoName, prNumber, authUser); err != nil {
+			slog.Warn("could not self-request as reviewer", "user", authUser, "error", err)
+		} else {
+			slog.Debug("self-requested as reviewer", "user", authUser, "pr", prNumber)
+		}
+	}
+
+	// POST new review
+	slog.Info("posting review", "event", event, "pr", prNumber)
 	posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, inlineComments)
 	if err != nil {
-		log.Fatalf("Failed to post review: %v", err)
+		slog.Error("failed to post review", "pr", prNumber, "event", event, "error", err)
+		os.Exit(1)
 	}
-	log.Printf("Review posted (id=%d, user=%s)", posted.ID, posted.User.Login)
+	slog.Info("review posted", "review_id", posted.ID, "user", posted.User.Login, "pr", prNumber)
 
-	// Post-posting escalation: if we just posted APPROVED but a sibling
+	// Supersede all old reviews with link to the new one
-	// from the same user has REQUEST_CHANGES, mark ours as superseded and
+	if len(oldReviews) > 0 {
-	// re-post as REQUEST_CHANGES. This handles the first-run case where
+		newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*giteaURL, "/"), owner, repoName, prNumber, posted.ID)
-	// we don't know our login until after posting.
+		for _, oldReview := range oldReviews {
-	if event == "APPROVED" && *updateExisting && *reviewerName != "" {
+			cid, err := giteaClient.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID)
-		reviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
-		if err == nil && shouldEscalate(reviews, posted.ID, posted.User.Login, sentinel) {
-			log.Printf("Post-posting escalation: sibling has REQUEST_CHANGES")
-			// Mark our just-posted review as superseded
-			commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
-			if err == nil {
-				supersededBody := fmt.Sprintf("~~*This review has been superseded by a newer review below.*~~\n\n%s", sentinel)
-				giteaClient.EditComment(ctx, owner, repoName, commentID, supersededBody)
-			}
-			// Re-post as REQUEST_CHANGES
-			_, err = giteaClient.PostReview(ctx, owner, repoName, prNumber, "REQUEST_CHANGES", reviewBody, inlineComments)
 			if err != nil {
-				log.Printf("Warning: could not re-post as REQUEST_CHANGES: %v", err)
+				slog.Warn("could not find comment ID for old review", "review_id", oldReview.ID, "error", err)
-			} else {
+				continue
-				log.Printf("Review escalated to REQUEST_CHANGES")
+			}
+			supersededBody := buildSupersededBody(oldReview.Body, oldReview.CommitID, newReviewURL, sentinel)
+			if err := giteaClient.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil {
+				slog.Warn("could not mark old review as superseded", "review_id", oldReview.ID, "comment_id", cid, "error", err)
+				continue
+			}
+			slog.Info("marked old review as superseded", "review_id", oldReview.ID, "new_review_id", posted.ID, "pr", prNumber)
+
+			// Resolve old review's inline comments
+			oldComments, err := giteaClient.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID)
+			if err != nil {
+				slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err)
+				continue
+			}
+			resolved, failed := 0, 0
+			for _, c := range oldComments {
+				if c.ID == 0 {
+					continue
+				}
+				if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
+					slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
+					failed++
+				} else {
+					resolved++
+				}
+			}
+			if resolved > 0 {
+				slog.Info("resolved old inline comments", "review_id", oldReview.ID, "count", resolved, "pr", prNumber)
+			}
+			if failed > 0 {
+				slog.Warn("some inline comments could not be resolved", "review_id", oldReview.ID, "failed", failed, "pr", prNumber)
 			}
 		}
 	}
+
 }
 
 // fetchFileContext fetches the full content of modified files from the PR branch.
@@ -359,7 +470,7 @@ func fetchFileContext(ctx context.Context, client *gitea.Client, owner, repo, re
 		}
 		content, err := client.GetFileContentRef(ctx, owner, repo, f.Filename, ref)
 		if err != nil {
-			log.Printf("Warning: could not fetch %s: %v", f.Filename, err)
+			slog.Warn("could not fetch file content", "file", f.Filename, "error", err)
 			continue
 		}
 		sb.WriteString(fmt.Sprintf("--- %s ---\n", f.Filename))
@@ -390,7 +501,7 @@ func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patt
 		}
 		parts := strings.SplitN(repoRef, "/", 2)
 		if len(parts) != 2 {
-			log.Printf("Warning: invalid patterns-repo format %q, expected owner/name", repoRef)
+			slog.Warn("invalid patterns-repo format", "repo", repoRef, "expected", "owner/name")
 			continue
 		}
 		owner, repo := parts[0], parts[1]
@@ -403,7 +514,7 @@ func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patt
 
 			files, err := client.GetAllFilesInPath(ctx, owner, repo, path)
 			if err != nil {
-				log.Printf("Warning: could not fetch %s from %s: %v", path, repoRef, err)
+				slog.Warn("could not fetch patterns", "path", path, "repo", repoRef, "error", err)
 				continue
 			}
 
@@ -501,50 +612,113 @@ func validateReviewerName(name string) error {
 	return nil
 }
 
-// shouldEscalate checks if any sibling bot review from the same user
+// buildSupersededBody creates the body for a superseded review: struck-through banner
-// (different sentinel, same token) has REQUEST_CHANGES.
+// with collapsed original content and the commit it was evaluated against.
-// ownLogin is the bot user login; if empty, escalation check is skipped.
+func buildSupersededBody(originalBody, commitSHA, newReviewURL, sentinel string) string {
-// postedID is excluded from consideration (0 means no exclusion needed).
+	shortSHA := commitSHA
-func shouldEscalate(reviews []gitea.Review, postedID int64, ownLogin, ownSentinel string) bool {
+	if len(shortSHA) > 8 {
+		shortSHA = shortSHA[:8]
+	}
+	var sb strings.Builder
+	sb.WriteString("~~Original review~~\n\n")
+	sb.WriteString("**Superseded** \u2014 [see current review](")
+	sb.WriteString(newReviewURL)
+	sb.WriteString(") for up-to-date findings.\n\n")
+	if shortSHA != "" {
+		sb.WriteString("<details><summary>Previous findings (commit ")
+		sb.WriteString(shortSHA)
+		sb.WriteString(")</summary>\n\n")
+	} else {
+		sb.WriteString("<details><summary>Previous findings</summary>\n\n")
+	}
+	sb.WriteString(originalBody)
+	sb.WriteString("\n\n</details>\n\n")
+	sb.WriteString(sentinel)
+	return sb.String()
+}
+
+// hasSharedToken detects if another review-bot role posted under the same
+// Gitea user. This indicates misconfiguration where two roles share a token
+// instead of having separate Gitea accounts. Returns true if shared token
+// detected (caller should skip update-in-place logic to avoid clobbering).
+func hasSharedToken(reviews []gitea.Review, ownSentinel string) bool {
+	ownLogin := ""
+	for _, r := range reviews {
+		if strings.Contains(r.Body, ownSentinel) {
+			ownLogin = r.User.Login
+			break
+		}
+	}
 	if ownLogin == "" {
 		return false
 	}
 	for _, r := range reviews {
-		if r.ID == postedID || r.Stale {
+		if r.User.Login == ownLogin && strings.Contains(r.Body, "<!-- review-bot:") && !strings.Contains(r.Body, ownSentinel) {
-			continue
+			slog.Warn("shared token detected — another review-bot role is using the same Gitea user",
-		}
+				"sibling_role", extractSentinelName(r.Body), "user", ownLogin)
-		// Sibling = same user, has a review-bot sentinel, but not OUR sentinel
-		if r.User.Login == ownLogin && r.State == "REQUEST_CHANGES" && strings.Contains(r.Body, "<!-- review-bot:") && !strings.Contains(r.Body, ownSentinel) {
 			return true
 		}
 	}
 	return false
 }
 
-// reviewUnchanged checks if an existing review with the same sentinel
+// extractSentinelName pulls the reviewer name from a sentinel comment.
-// already has identical body and state. Returns true if a re-post would
+func extractSentinelName(body string) string {
-// produce the same result (skip to preserve conversation threads).
+	const prefix = "<!-- review-bot:"
-func reviewUnchanged(reviews []gitea.Review, newBody, newEvent, sentinel string) bool {
+	const suffix = " -->"
-	for _, r := range reviews {
+	idx := strings.Index(body, prefix)
-		if r.Stale {
+	if idx < 0 {
-			continue
+		return "unknown"
-		}
-		if !strings.Contains(r.Body, sentinel) {
-			continue
-		}
-		if r.State == newEvent && r.Body == newBody {
-			return true
-		}
 	}
-	return false
+	rest := body[idx+len(prefix):]
+	end := strings.Index(rest, suffix)
+	if end < 0 {
+		return "unknown"
+	}
+	return rest[:end]
 }
 
-// findOwnReview locates a review matching the given sentinel in its body.
+// findOwnReview locates the most recent non-superseded review matching the sentinel.
 func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
+	var best *gitea.Review
 	for i := range reviews {
-		if strings.Contains(reviews[i].Body, sentinel) {
+		if !strings.Contains(reviews[i].Body, sentinel) {
-			return &reviews[i]
+			continue
+		}
+		if strings.Contains(reviews[i].Body, "~~Original review~~") {
+			continue
+		}
+		if best == nil || reviews[i].ID > best.ID {
+			best = &reviews[i]
 		}
 	}
-	return nil
+	return best
+}
+
+// findAllOwnReviews returns all non-superseded reviews matching the sentinel.
+func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review {
+	var result []gitea.Review
+	for i := range reviews {
+		if !strings.Contains(reviews[i].Body, sentinel) {
+			continue
+		}
+		if strings.Contains(reviews[i].Body, "~~Original review~~") {
+			continue
+		}
+		result = append(result, reviews[i])
+	}
+	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
 }

cmd/review-bot/main_test.go

@@ -1,6 +1,12 @@
 package main
 
 import (
+	"bytes"
+	"flag"
+	"log/slog"
+	"os"
+	"os/exec"
+	"strings"
 	"testing"
 
 	"gitea.weiker.me/rodin/review-bot/gitea"
@@ -50,182 +56,52 @@ func makeReview(id int64, login, state string, stale bool, body string) gitea.Re
 	return r
 }
 
-func TestShouldEscalate(t *testing.T) {
-	tests := []struct {
-		name        string
-		reviews     []gitea.Review
-		postedID    int64
-		ownLogin    string
 
-		ownSentinel string
+func TestBuildSupersededBody(t *testing.T) {
-		want        bool
+	original := "# Review\n\nLooks good.\n\n<!-- review-bot:sonnet -->"
-	}{
+	sentinel := "<!-- review-bot:sonnet -->"
-		{
+	newURL := "https://gitea.example.com/owner/repo/pulls/1#pullrequestreview-99"
-			name:        "no reviews",
+
-			reviews:     nil,
+	result := buildSupersededBody(original, "abcdef1234567890", newURL, sentinel)
-			postedID:    100,
+
-			ownLogin:    "bot",
+	// Should contain the struck-through banner
-			ownSentinel: "<!-- review-bot:sonnet -->",
+	if !strings.Contains(result, "~~Original review~~") {
-			want:        false,
+		t.Error("missing struck-through banner")
-		},
-		{
-			name: "sibling same user has REQUEST_CHANGES",
-			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", false, "bad\n<!-- review-bot:security -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        true,
-		},
-		{
-			name: "sibling different user has REQUEST_CHANGES (should NOT escalate)",
-			reviews: []gitea.Review{
-				makeReview(101, "other-bot", "REQUEST_CHANGES", false, "bad\n<!-- review-bot:gpt -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "same user REQUEST_CHANGES but stale (should NOT escalate)",
-			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", true, "old\n<!-- review-bot:security -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "same user same sentinel (own stale review, should NOT escalate)",
-			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", false, "old\n<!-- review-bot:sonnet -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "same user APPROVED sibling (should NOT escalate)",
-			reviews: []gitea.Review{
-				makeReview(101, "bot", "APPROVED", false, "good\n<!-- review-bot:security -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "human REQUEST_CHANGES no sentinel (should NOT escalate)",
-			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", false, "please fix this"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "skip own posted ID",
-			reviews: []gitea.Review{
-				makeReview(100, "bot", "REQUEST_CHANGES", false, "x\n<!-- review-bot:security -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
 	}
-
+	// Should contain superseded notice with link
-	for _, tc := range tests {
+	if !strings.Contains(result, "**Superseded**") {
-		t.Run(tc.name, func(t *testing.T) {
+		t.Error("missing superseded notice")
-			got := shouldEscalate(tc.reviews, tc.postedID, tc.ownLogin, tc.ownSentinel)
+	}
-			if got != tc.want {
+	if !strings.Contains(result, "[see current review]("+newURL+")") {
-				t.Errorf("shouldEscalate() = %v, want %v", got, tc.want)
+		t.Error("missing link to new review")
-			}
+	}
-		})
+	// Should contain collapsed original
+	if !strings.Contains(result, "<details>") {
+		t.Error("missing details/collapse")
+	}
+	// Should contain short commit SHA
+	if !strings.Contains(result, "abcdef12") {
+		t.Error("missing short SHA")
+	}
+	// Should NOT contain full SHA
+	if strings.Contains(result, "abcdef1234567890") {
+		t.Error("should truncate SHA to 8 chars")
+	}
+	// Should contain the original body inside details
+	if !strings.Contains(result, original) {
+		t.Error("original body not preserved in collapsed section")
+	}
+	// Should end with sentinel
+	if !strings.Contains(result, sentinel) {
+		t.Error("missing sentinel")
 	}
 }
 
-func TestReviewUnchanged(t *testing.T) {
+func TestBuildSupersededBodyShortSHA(t *testing.T) {
-	tests := []struct {
+	// Short SHA should pass through without panic
-		name     string
+	result := buildSupersededBody("body", "abc", "https://example.com/review", "<!-- review-bot:x -->")
-		existing []gitea.Review
+	if !strings.Contains(result, "abc") {
-		newBody  string
+		t.Error("short SHA not preserved")
-		newEvent string
-		sentinel string
-		want     bool
-	}{
-		{
-			name:     "no existing review",
-			existing: nil,
-			newBody:  "new review",
-			newEvent: "APPROVED",
-			sentinel: "<!-- review-bot:sonnet -->",
-			want:     false,
-		},
-		{
-			name: "identical body and state",
-			existing: []gitea.Review{
-				makeReview(100, "bot", "APPROVED", false, "same body\n<!-- review-bot:sonnet -->"),
-			},
-			newBody:  "same body\n<!-- review-bot:sonnet -->",
-			newEvent: "APPROVED",
-			sentinel: "<!-- review-bot:sonnet -->",
-			want:     true,
-		},
-		{
-			name: "same body but different state",
-			existing: []gitea.Review{
-				makeReview(100, "bot", "APPROVED", false, "body\n<!-- review-bot:sonnet -->"),
-			},
-			newBody:  "body\n<!-- review-bot:sonnet -->",
-			newEvent: "REQUEST_CHANGES",
-			sentinel: "<!-- review-bot:sonnet -->",
-			want:     false,
-		},
-		{
-			name: "different body same state",
-			existing: []gitea.Review{
-				makeReview(100, "bot", "APPROVED", false, "old body\n<!-- review-bot:sonnet -->"),
-			},
-			newBody:  "new body\n<!-- review-bot:sonnet -->",
-			newEvent: "APPROVED",
-			sentinel: "<!-- review-bot:sonnet -->",
-			want:     false,
-		},
-		{
-			name: "stale review with same body (should still post)",
-			existing: []gitea.Review{
-				makeReview(100, "bot", "APPROVED", true, "same\n<!-- review-bot:sonnet -->"),
-			},
-			newBody:  "same\n<!-- review-bot:sonnet -->",
-			newEvent: "APPROVED",
-			sentinel: "<!-- review-bot:sonnet -->",
-			want:     false,
-		},
-		{
-			name: "different sentinel (not our review)",
-			existing: []gitea.Review{
-				makeReview(100, "bot", "APPROVED", false, "body\n<!-- review-bot:gpt -->"),
-			},
-			newBody:  "body\n<!-- review-bot:sonnet -->",
-			newEvent: "APPROVED",
-			sentinel: "<!-- review-bot:sonnet -->",
-			want:     false,
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			got := reviewUnchanged(tc.existing, tc.newBody, tc.newEvent, tc.sentinel)
-			if got != tc.want {
-				t.Errorf("reviewUnchanged() = %v, want %v", got, tc.want)
-			}
-		})
 	}
 }
 
@@ -268,6 +144,32 @@ func TestFindOwnReview(t *testing.T) {
 			sentinel: "<!-- review-bot:sonnet -->",
 			wantID:   20,
 		},
+		{
+			name: "skips superseded review",
+			reviews: []gitea.Review{
+				makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n**Superseded**\n<!-- review-bot:sonnet -->"),
+				makeReview(20, "bot", "APPROVED", false, "fresh review\n<!-- review-bot:sonnet -->"),
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			wantID:   20,
+		},
+		{
+			name: "only superseded reviews exist",
+			reviews: []gitea.Review{
+				makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n<!-- review-bot:sonnet -->"),
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			wantNil:  true,
+		},
+		{
+			name: "picks highest ID among matches",
+			reviews: []gitea.Review{
+				makeReview(50, "bot", "APPROVED", false, "v1\n<!-- review-bot:sonnet -->"),
+				makeReview(30, "bot", "APPROVED", false, "v0\n<!-- review-bot:sonnet -->"),
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			wantID:   50,
+		},
 	}
 
 	for _, tc := range tests {
@@ -288,3 +190,725 @@ func TestFindOwnReview(t *testing.T) {
 		})
 	}
 }
+
+func TestHasSharedToken(t *testing.T) {
+	tests := []struct {
+		name     string
+		reviews  []gitea.Review
+		sentinel string
+		want     bool
+	}{
+		{
+			name:     "no reviews",
+			reviews:  nil,
+			sentinel: "<!-- review-bot:sonnet -->",
+			want:     false,
+		},
+		{
+			name: "no own review yet - cannot detect",
+			reviews: []gitea.Review{
+				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "other"}, Body: "<!-- review-bot:gpt --> body"},
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			want:     false,
+		},
+		{
+			name: "separate users - no shared token",
+			reviews: []gitea.Review{
+				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: struct{ Login string `json:"login"` }{Login: "security-review-bot"}, Body: "<!-- review-bot:security --> body"},
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			want:     false,
+		},
+		{
+			name: "shared token detected - same user different sentinels",
+			reviews: []gitea.Review{
+				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:security --> body"},
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			want:     true,
+		},
+		{
+			name: "three roles same user",
+			reviews: []gitea.Review{
+				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:security --> body"},
+				{ID: 3, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:gpt --> body"},
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			want:     true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := hasSharedToken(tc.reviews, tc.sentinel)
+			if got != tc.want {
+				t.Errorf("hasSharedToken() = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestExtractSentinelName(t *testing.T) {
+	tests := []struct {
+		body string
+		want string
+	}{
+		{"<!-- review-bot:sonnet --> rest", "sonnet"},
+		{"<!-- review-bot:security --> rest", "security"},
+		{"no sentinel here", "unknown"},
+		{"<!-- review-bot:gpt-review --> rest", "gpt-review"},
+	}
+
+	for _, tc := range tests {
+		got := extractSentinelName(tc.body)
+		if got != tc.want {
+			t.Errorf("extractSentinelName(%q) = %q, want %q", tc.body, got, tc.want)
+		}
+	}
+}
+
+func TestSetupLogger_JSONFormat(t *testing.T) {
+	// Capture output by creating a logger manually with the same logic
+	var buf bytes.Buffer
+	opts := &slog.HandlerOptions{Level: slog.LevelInfo}
+	handler := slog.NewJSONHandler(&buf, opts)
+	logger := slog.New(handler)
+
+	logger.Info("test message", "key", "value")
+
+	output := buf.String()
+	if !strings.Contains(output, `"msg":"test message"`) {
+		t.Errorf("expected JSON msg field, got: %s", output)
+	}
+	if !strings.Contains(output, `"key":"value"`) {
+		t.Errorf("expected JSON key field, got: %s", output)
+	}
+	if !strings.Contains(output, `"level":"INFO"`) {
+		t.Errorf("expected JSON level field, got: %s", output)
+	}
+}
+
+func TestSetupLogger_TextFormat(t *testing.T) {
+	var buf bytes.Buffer
+	opts := &slog.HandlerOptions{Level: slog.LevelInfo}
+	handler := slog.NewTextHandler(&buf, opts)
+	logger := slog.New(handler)
+
+	logger.Info("test message", "key", "value")
+
+	output := buf.String()
+	if !strings.Contains(output, "msg=\"test message\"") && !strings.Contains(output, "msg=test") {
+		t.Errorf("expected text msg field, got: %s", output)
+	}
+	if !strings.Contains(output, "key=value") {
+		t.Errorf("expected text key field, got: %s", output)
+	}
+}
+
+func TestSetupLogger_LevelFiltering(t *testing.T) {
+	tests := []struct {
+		name      string
+		verbosity string
+		logLevel  slog.Level
+		expected  bool // should the message appear
+	}{
+		{"info logger shows info", "info", slog.LevelInfo, true},
+		{"info logger hides debug", "info", slog.LevelDebug, false},
+		{"debug logger shows debug", "debug", slog.LevelDebug, true},
+		{"warn logger hides info", "warn", slog.LevelInfo, false},
+		{"warn logger shows warn", "warn", slog.LevelWarn, true},
+		{"error logger hides warn", "error", slog.LevelWarn, false},
+		{"error logger shows error", "error", slog.LevelError, true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			var level slog.Level
+			switch tc.verbosity {
+			case "debug":
+				level = slog.LevelDebug
+			case "info":
+				level = slog.LevelInfo
+			case "warn":
+				level = slog.LevelWarn
+			case "error":
+				level = slog.LevelError
+			}
+
+			var buf bytes.Buffer
+			opts := &slog.HandlerOptions{Level: level}
+			handler := slog.NewTextHandler(&buf, opts)
+			logger := slog.New(handler)
+
+			logger.Log(nil, tc.logLevel, "test")
+
+			hasOutput := buf.Len() > 0
+			if hasOutput != tc.expected {
+				t.Errorf("verbosity=%s, logLevel=%s: got output=%v, want %v",
+					tc.verbosity, tc.logLevel, hasOutput, tc.expected)
+			}
+		})
+	}
+}
+
+func TestSetupLogger_Integration(t *testing.T) {
+	// Test that setupLogger doesn't panic for valid inputs
+	setupLogger("text", "info")
+	setupLogger("json", "debug")
+	setupLogger("text", "warn")
+	setupLogger("json", "error")
+	setupLogger("text", "unknown") // should default to info
+	setupLogger("invalid", "info") // should default to text
+}
+
+func TestIsPatternFile(t *testing.T) {
+	tests := []struct {
+		path string
+		want bool
+	}{
+		{"README.md", true},
+		{"docs/GUIDE.MD", true},
+		{"config.yml", true},
+		{"config.yaml", true},
+		{"notes.txt", true},
+		{"NOTES.TXT", true},
+		{"main.go", false},
+		{"lib.rs", false},
+		{"index.js", false},
+		{"Makefile", false},
+		{"", false},
+		{"doc.pdf", false},
+		{"patterns.Yml", true},
+		{"deep/path/file.yaml", true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.path, func(t *testing.T) {
+			got := isPatternFile(tc.path)
+			if got != tc.want {
+				t.Errorf("isPatternFile(%q) = %v, want %v", tc.path, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestEvaluateCIStatus(t *testing.T) {
+	tests := []struct {
+		name       string
+		statuses   []gitea.CommitStatus
+		wantPassed bool
+		wantSubstr string
+	}{
+		{
+			name:       "empty statuses",
+			statuses:   nil,
+			wantPassed: true,
+			wantSubstr: "no CI statuses",
+		},
+		{
+			name: "all success",
+			statuses: []gitea.CommitStatus{
+				{Status: "success", Context: "ci/build", Description: "Build passed"},
+				{Status: "success", Context: "ci/test", Description: "Tests passed"},
+			},
+			wantPassed: true,
+			wantSubstr: "all checks passed",
+		},
+		{
+			name: "one failure",
+			statuses: []gitea.CommitStatus{
+				{Status: "success", Context: "ci/build", Description: "Build passed"},
+				{Status: "failure", Context: "ci/test", Description: "Tests failed"},
+			},
+			wantPassed: false,
+			wantSubstr: "ci/test",
+		},
+		{
+			name: "error status",
+			statuses: []gitea.CommitStatus{
+				{Status: "error", Context: "ci/lint", Description: "Lint error"},
+			},
+			wantPassed: false,
+			wantSubstr: "ci/lint",
+		},
+		{
+			name: "pending treated as not-failed",
+			statuses: []gitea.CommitStatus{
+				{Status: "pending", Context: "ci/build", Description: "In progress"},
+				{Status: "success", Context: "ci/test", Description: "Tests passed"},
+			},
+			wantPassed: true,
+			wantSubstr: "all checks passed",
+		},
+		{
+			name: "multiple failures",
+			statuses: []gitea.CommitStatus{
+				{Status: "failure", Context: "ci/build", Description: "Build failed"},
+				{Status: "failure", Context: "ci/test", Description: "Tests failed"},
+			},
+			wantPassed: false,
+			wantSubstr: "ci/build",
+		},
+		{
+			name: "mixed with pending and failure",
+			statuses: []gitea.CommitStatus{
+				{Status: "success", Context: "ci/build", Description: "Build passed"},
+				{Status: "pending", Context: "ci/deploy", Description: "Deploying"},
+				{Status: "failure", Context: "ci/test", Description: "Tests failed"},
+			},
+			wantPassed: false,
+			wantSubstr: "ci/test",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			passed, details := evaluateCIStatus(tc.statuses)
+			if passed != tc.wantPassed {
+				t.Errorf("evaluateCIStatus() passed = %v, want %v", passed, tc.wantPassed)
+			}
+			if !strings.Contains(details, tc.wantSubstr) {
+				t.Errorf("evaluateCIStatus() details = %q, want substring %q", details, tc.wantSubstr)
+			}
+		})
+	}
+}
+
+func TestEnvOrDefault(t *testing.T) {
+	// Test with unset env var
+	os.Unsetenv("TEST_ENV_OR_DEFAULT_UNSET")
+	got := envOrDefault("TEST_ENV_OR_DEFAULT_UNSET", "fallback")
+	if got != "fallback" {
+		t.Errorf("envOrDefault(unset) = %q, want %q", got, "fallback")
+	}
+
+	// Test with set env var
+	os.Setenv("TEST_ENV_OR_DEFAULT_SET", "custom")
+	defer os.Unsetenv("TEST_ENV_OR_DEFAULT_SET")
+	got = envOrDefault("TEST_ENV_OR_DEFAULT_SET", "fallback")
+	if got != "custom" {
+		t.Errorf("envOrDefault(set) = %q, want %q", got, "custom")
+	}
+
+	// Test with empty env var (should return default)
+	os.Setenv("TEST_ENV_OR_DEFAULT_EMPTY", "")
+	defer os.Unsetenv("TEST_ENV_OR_DEFAULT_EMPTY")
+	got = envOrDefault("TEST_ENV_OR_DEFAULT_EMPTY", "fallback")
+	if got != "fallback" {
+		t.Errorf("envOrDefault(empty) = %q, want %q", got, "fallback")
+	}
+}
+
+func TestEnvOrDefaultFloat(t *testing.T) {
+	// Test with unset env var
+	os.Unsetenv("TEST_ENV_FLOAT_UNSET")
+	got := envOrDefaultFloat("TEST_ENV_FLOAT_UNSET", 1.5)
+	if got != 1.5 {
+		t.Errorf("envOrDefaultFloat(unset) = %f, want %f", got, 1.5)
+	}
+
+	// Test with valid float
+	os.Setenv("TEST_ENV_FLOAT_SET", "2.7")
+	defer os.Unsetenv("TEST_ENV_FLOAT_SET")
+	got = envOrDefaultFloat("TEST_ENV_FLOAT_SET", 1.5)
+	if got != 2.7 {
+		t.Errorf("envOrDefaultFloat(set) = %f, want %f", got, 2.7)
+	}
+
+	// Test with invalid float (should return default)
+	os.Setenv("TEST_ENV_FLOAT_INVALID", "not-a-number")
+	defer os.Unsetenv("TEST_ENV_FLOAT_INVALID")
+	got = envOrDefaultFloat("TEST_ENV_FLOAT_INVALID", 3.14)
+	if got != 3.14 {
+		t.Errorf("envOrDefaultFloat(invalid) = %f, want %f", got, 3.14)
+	}
+
+	// Test with empty string (should return default)
+	os.Setenv("TEST_ENV_FLOAT_EMPTY", "")
+	defer os.Unsetenv("TEST_ENV_FLOAT_EMPTY")
+	got = envOrDefaultFloat("TEST_ENV_FLOAT_EMPTY", 0.5)
+	if got != 0.5 {
+		t.Errorf("envOrDefaultFloat(empty) = %f, want %f", got, 0.5)
+	}
+}
+
+func TestEnvOrDefaultInt(t *testing.T) {
+	// Test with unset env var
+	os.Unsetenv("TEST_ENV_INT_UNSET")
+	got := envOrDefaultInt("TEST_ENV_INT_UNSET", 42)
+	if got != 42 {
+		t.Errorf("envOrDefaultInt(unset) = %d, want %d", got, 42)
+	}
+
+	// Test with valid int
+	os.Setenv("TEST_ENV_INT_SET", "100")
+	defer os.Unsetenv("TEST_ENV_INT_SET")
+	got = envOrDefaultInt("TEST_ENV_INT_SET", 42)
+	if got != 100 {
+		t.Errorf("envOrDefaultInt(set) = %d, want %d", got, 100)
+	}
+
+	// Test with invalid int (should return default)
+	os.Setenv("TEST_ENV_INT_INVALID", "abc")
+	defer os.Unsetenv("TEST_ENV_INT_INVALID")
+	got = envOrDefaultInt("TEST_ENV_INT_INVALID", 42)
+	if got != 42 {
+		t.Errorf("envOrDefaultInt(invalid) = %d, want %d", got, 42)
+	}
+
+	// Test with empty string (should return default)
+	os.Setenv("TEST_ENV_INT_EMPTY", "")
+	defer os.Unsetenv("TEST_ENV_INT_EMPTY")
+	got = envOrDefaultInt("TEST_ENV_INT_EMPTY", 99)
+	if got != 99 {
+		t.Errorf("envOrDefaultInt(empty) = %d, want %d", got, 99)
+	}
+
+	// Test with negative int
+	os.Setenv("TEST_ENV_INT_NEG", "-5")
+	defer os.Unsetenv("TEST_ENV_INT_NEG")
+	got = envOrDefaultInt("TEST_ENV_INT_NEG", 42)
+	if got != -5 {
+		t.Errorf("envOrDefaultInt(negative) = %d, want %d", got, -5)
+	}
+}
+
+func TestEnvOrDefaultBool(t *testing.T) {
+	tests := []struct {
+		name       string
+		envVal     string
+		setEnv     bool
+		defaultVal bool
+		want       bool
+	}{
+		{"unset returns default true", "", false, true, true},
+		{"unset returns default false", "", false, false, false},
+		{"true", "true", true, false, true},
+		{"TRUE", "TRUE", true, false, true},
+		{"True", "True", true, false, true},
+		{"1", "1", true, false, true},
+		{"yes", "yes", true, false, true},
+		{"YES", "YES", true, false, true},
+		{"false", "false", true, true, false},
+		{"0", "0", true, true, false},
+		{"no", "no", true, true, false},
+		{"random string", "random", true, true, false},
+		{"empty string returns default", "", true, true, true},
+		{"whitespace true", "  true  ", true, false, true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			envKey := "TEST_ENV_BOOL_" + strings.ReplaceAll(tc.name, " ", "_")
+			if tc.setEnv {
+				os.Setenv(envKey, tc.envVal)
+				defer os.Unsetenv(envKey)
+			} else {
+				os.Unsetenv(envKey)
+			}
+			got := envOrDefaultBool(envKey, tc.defaultVal)
+			if got != tc.want {
+				t.Errorf("envOrDefaultBool(%q, %v) = %v, want %v", tc.envVal, tc.defaultVal, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestExtractSentinelName_EdgeCases(t *testing.T) {
+	tests := []struct {
+		body string
+		want string
+	}{
+		{"<!-- review-bot:sonnet --> rest", "sonnet"},
+		{"<!-- review-bot:gpt-review --> rest", "gpt-review"},
+		{"no sentinel here", "unknown"},
+		{"<!-- review-bot:", "unknown"},       // prefix but no suffix
+		{"prefix <!-- review-bot:abc --> end", "abc"}, // embedded in text
+	}
+
+	for _, tc := range tests {
+		got := extractSentinelName(tc.body)
+		if got != tc.want {
+			t.Errorf("extractSentinelName(%q) = %q, want %q", tc.body, got, tc.want)
+		}
+	}
+}
+
+// TestMainSubprocess runs main() as a subprocess using the test binary itself.
+// This allows coverage to be captured for main() code paths.
+func TestMainSubprocess_Version(t *testing.T) {
+	if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
+		// Reset flags for main()
+		flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+		os.Args = []string{"review-bot", "--version"}
+		main()
+		return
+	}
+
+	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_Version")
+	cmd.Env = append(os.Environ(), "TEST_SUBPROCESS_MAIN=1")
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("--version subprocess failed: %v\n%s", err, out)
+	}
+	if !strings.Contains(string(out), "review-bot") {
+		t.Errorf("--version output = %q, want to contain 'review-bot'", string(out))
+	}
+}
+
+func TestMainSubprocess_MissingFlags(t *testing.T) {
+	if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
+		// Reset flags for main()
+		flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+		os.Args = []string{"review-bot"}
+		main()
+		return
+	}
+
+	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_MissingFlags")
+	cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatal("expected non-zero exit with no flags, got success")
+	}
+	if !strings.Contains(string(out), "missing required") {
+		t.Errorf("expected error about missing flags, got: %s", out)
+	}
+}
+
+func TestMainSubprocess_InvalidReviewerName(t *testing.T) {
+	if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
+		flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+		os.Args = []string{"review-bot",
+			"--gitea-url", "http://localhost",
+			"--repo", "owner/repo",
+			"--pr", "1",
+			"--reviewer-name", "invalid name",
+			"--reviewer-token", "tok",
+			"--llm-base-url", "http://localhost",
+			"--llm-api-key", "key",
+			"--llm-model", "model",
+		}
+		main()
+		return
+	}
+
+	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidReviewerName")
+	cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatal("expected non-zero exit with invalid reviewer-name, got success")
+	}
+	if !strings.Contains(string(out), "invalid reviewer name") {
+		t.Errorf("expected error about invalid reviewer name, got: %s", out)
+	}
+}
+
+func TestMainSubprocess_InvalidRepo(t *testing.T) {
+	if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
+		flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+		os.Args = []string{"review-bot",
+			"--gitea-url", "http://localhost",
+			"--repo", "invalidrepo",
+			"--pr", "1",
+			"--reviewer-token", "tok",
+			"--llm-base-url", "http://localhost",
+			"--llm-api-key", "key",
+			"--llm-model", "model",
+		}
+		main()
+		return
+	}
+
+	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidRepo")
+	cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatal("expected non-zero exit with invalid repo format")
+	}
+	if !strings.Contains(string(out), "invalid repo format") {
+		t.Errorf("expected error about invalid repo, got: %s", out)
+	}
+}
+
+func TestMainSubprocess_InvalidPRNumber(t *testing.T) {
+	if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
+		flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+		os.Args = []string{"review-bot",
+			"--gitea-url", "http://localhost",
+			"--repo", "owner/repo",
+			"--pr", "notanumber",
+			"--reviewer-token", "tok",
+			"--llm-base-url", "http://localhost",
+			"--llm-api-key", "key",
+			"--llm-model", "model",
+		}
+		main()
+		return
+	}
+
+	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidPRNumber")
+	cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatal("expected non-zero exit with invalid PR number")
+	}
+	if !strings.Contains(string(out), "invalid PR number") {
+		t.Errorf("expected error about invalid PR number, got: %s", out)
+	}
+}
+
+func TestMainSubprocess_InvalidTemperature(t *testing.T) {
+	if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
+		flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+		os.Args = []string{"review-bot",
+			"--gitea-url", "http://localhost",
+			"--repo", "owner/repo",
+			"--pr", "1",
+			"--reviewer-token", "tok",
+			"--llm-base-url", "http://localhost",
+			"--llm-api-key", "key",
+			"--llm-model", "model",
+			"--llm-temperature", "5.0",
+		}
+		main()
+		return
+	}
+
+	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidTemperature")
+	cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatal("expected non-zero exit with invalid temperature")
+	}
+	if !strings.Contains(string(out), "invalid LLM temperature") {
+		t.Errorf("expected error about invalid temperature, got: %s", out)
+	}
+}
+
+func TestMainSubprocess_InvalidProvider(t *testing.T) {
+	if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
+		flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+		os.Args = []string{"review-bot",
+			"--gitea-url", "http://localhost",
+			"--repo", "owner/repo",
+			"--pr", "1",
+			"--reviewer-token", "tok",
+			"--llm-base-url", "http://localhost",
+			"--llm-api-key", "key",
+			"--llm-model", "model",
+			"--llm-provider", "invalid-provider",
+		}
+		main()
+		return
+	}
+
+	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidProvider")
+	cmd.Env = append(cleanEnv(), "TEST_SUBPROCESS_MAIN=1")
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatal("expected non-zero exit with invalid provider")
+	}
+	if !strings.Contains(string(out), "invalid LLM provider") {
+		t.Errorf("expected error about invalid provider, got: %s", out)
+	}
+}
+
+// cleanEnv returns environ without any GITEA/LLM/REVIEWER env vars that would
+// interfere with testing missing-flag scenarios.
+func cleanEnv() []string {
+	var env []string
+	for _, e := range os.Environ() {
+		key := strings.SplitN(e, "=", 2)[0]
+		switch {
+		case strings.HasPrefix(key, "GITEA_"),
+			strings.HasPrefix(key, "LLM_"),
+			strings.HasPrefix(key, "REVIEWER_"),
+			strings.HasPrefix(key, "PR_"),
+			strings.HasPrefix(key, "LOG_"),
+			strings.HasPrefix(key, "CONVENTIONS_"),
+			strings.HasPrefix(key, "SYSTEM_PROMPT_"),
+			strings.HasPrefix(key, "PATTERNS_"),
+			strings.HasPrefix(key, "UPDATE_"):
+			continue
+		default:
+			env = append(env, e)
+		}
+	}
+	return env
+}
+
+func TestFindAllOwnReviews(t *testing.T) {
+	reviews := []gitea.Review{
+		{ID: 1, Body: "<!-- review-bot:sonnet -->\nfirst review"},
+		{ID: 2, Body: "<!-- review-bot:gpt -->\nother bot"},
+		{ID: 3, Body: "<!-- review-bot:sonnet -->\nsecond review"},
+		{ID: 4, Body: "~~Original review~~\n<!-- review-bot:sonnet -->\nsuperseded"},
+		{ID: 5, Body: "<!-- review-bot:sonnet -->\nthird review"},
+	}
+
+	got := findAllOwnReviews(reviews, "<!-- review-bot:sonnet -->")
+	if len(got) != 3 {
+		t.Fatalf("findAllOwnReviews() returned %d, want 3", len(got))
+	}
+	wantIDs := []int64{1, 3, 5}
+	for i, r := range got {
+		if r.ID != wantIDs[i] {
+			t.Errorf("got[%d].ID = %d, want %d", i, r.ID, wantIDs[i])
+		}
+	}
+}
+
+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)
+			}
+		})
+	}
+}

gitea/client.go

@@ -7,15 +7,38 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
-	"log"
+	"log/slog"
 	"net/http"
 	"net/url"
 	"strings"
 	"time"
 )
 
+// APIError represents an HTTP error response from the Gitea API.
+// It carries the status code so callers can distinguish between
+// different failure modes (e.g. 404 vs 500).
+type APIError struct {
+	StatusCode int
+	Body       string
+}
+
+func (e *APIError) Error() string {
+	body := e.Body
+	if len(body) > 200 {
+		body = body[:200] + "...(truncated)"
+	}
+	return fmt.Sprintf("HTTP %d: %s", e.StatusCode, body)
+}
+
+// IsNotFound reports whether an error is an API 404 response.
+func IsNotFound(err error) bool {
+	var apiErr *APIError
+	return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound
+}
+
 // Client interacts with the Gitea API.
 // A Client is safe for concurrent use by multiple goroutines.
 type Client struct {
@@ -59,6 +82,7 @@ type ChangedFile struct {
 
 // ReviewComment represents an inline comment to attach to a review.
 type ReviewComment struct {
+	ID          int64  `json:"id,omitempty"`
 	Path        string `json:"path"`
 	NewPosition int64  `json:"new_position"`
 	Body        string `json:"body"`
@@ -201,7 +225,7 @@ func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		body, _ := io.ReadAll(resp.Body)
-		return nil, fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(body))
+		return nil, &APIError{StatusCode: resp.StatusCode, Body: string(body)}
 	}
 	return io.ReadAll(resp.Body)
 }
@@ -254,10 +278,15 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
 	// Try listing as directory first
 	entries, err := c.ListContents(ctx, owner, repo, path)
 	if err != nil {
-		// Might be a file, try fetching directly
+		// Only fall back to single-file fetch on 404 (path is a file, not a dir).
+		// Propagate all other errors (auth failures, server errors, rate limits).
+		if !IsNotFound(err) {
+			return nil, fmt.Errorf("list contents %q: %w", path, err)
+		}
+		// 404 means the path might be a file — try fetching directly
 		content, fileErr := c.GetFileContent(ctx, owner, repo, path)
 		if fileErr != nil {
-			return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, err)
+			return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, fileErr)
 		}
 		results[path] = content
 		return results, nil
@@ -268,14 +297,14 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
 		case "file":
 			content, err := c.GetFileContent(ctx, owner, repo, entry.Path)
 			if err != nil {
-				log.Printf("Warning: could not fetch file %s: %v", entry.Path, err)
+				slog.Warn("could not fetch file from patterns repo", "file", entry.Path, "error", err)
 				continue
 			}
 			results[entry.Path] = content
 		case "dir":
 			subResults, err := c.GetAllFilesInPath(ctx, owner, repo, entry.Path)
 			if err != nil {
-				log.Printf("Warning: could not recurse into %s: %v", entry.Path, err)
+				slog.Warn("could not recurse into directory", "dir", entry.Path, "error", err)
 				continue
 			}
 			for k, v := range subResults {
@@ -288,13 +317,14 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
 
 // Review represents a pull request review from the Gitea API.
 type Review struct {
-	ID   int64  `json:"id"`
+	ID       int64  `json:"id"`
-	Body string `json:"body"`
+	Body     string `json:"body"`
-	User struct {
+	User     struct {
 		Login string `json:"login"`
 	} `json:"user"`
-	State string `json:"state"`
+	State    string `json:"state"`
-	Stale bool   `json:"stale"`
+	Stale    bool   `json:"stale"`
+	CommitID string `json:"commit_id"`
 }
 
 // ListReviews returns all reviews on a pull request.
@@ -396,6 +426,68 @@ func (c *Client) GetTimelineReviewCommentID(ctx context.Context, owner, repo str
 	return 0, fmt.Errorf("no timeline event found with sentinel")
 }
 
+// GetTimelineReviewCommentIDForReview finds the timeline comment ID for a
+// specific review by matching its body content in the timeline.
+func (c *Client) GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, number int, reviewID int64) (int64, error) {
+	// Use the reviews API to get the review body, then find in timeline
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d",
+		c.baseURL,
+		url.PathEscape(owner),
+		url.PathEscape(repo),
+		number,
+		reviewID)
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return 0, fmt.Errorf("get review %d: %w", reviewID, err)
+	}
+	var review struct {
+		Body string `json:"body"`
+		User struct {
+			Login string `json:"login"`
+		} `json:"user"`
+	}
+	if err := json.Unmarshal(body, &review); err != nil {
+		return 0, fmt.Errorf("parse review %d: %w", reviewID, err)
+	}
+	if review.Body == "" {
+		return 0, fmt.Errorf("review %d has empty body", reviewID)
+	}
+
+	// Use a prefix for matching (handles minor trailing whitespace differences)
+	matchPrefix := review.Body
+	if len(matchPrefix) > 200 {
+		matchPrefix = matchPrefix[:200]
+	}
+
+	const pageSize = 50
+	for page := 1; ; page++ {
+		timelineURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues/%d/timeline?limit=%d&page=%d",
+			c.baseURL,
+			url.PathEscape(owner),
+			url.PathEscape(repo),
+			number,
+			pageSize,
+			page)
+		tlBody, err := c.doGet(ctx, timelineURL)
+		if err != nil {
+			return 0, fmt.Errorf("get timeline (page %d): %w", page, err)
+		}
+		var events []TimelineEvent
+		if err := json.Unmarshal(tlBody, &events); err != nil {
+			return 0, fmt.Errorf("parse timeline (page %d): %w", page, err)
+		}
+		for _, ev := range events {
+			if ev.Type == "review" && ev.User.Login == review.User.Login && strings.HasPrefix(ev.Body, matchPrefix) {
+				return ev.ID, nil
+			}
+		}
+		if len(events) < pageSize {
+			break
+		}
+	}
+	return 0, fmt.Errorf("no timeline event found for review %d", reviewID)
+}
+
 // EditComment updates the body of an issue/review comment.
 func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID int64, newBody string) error {
 	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues/comments/%d",
@@ -431,3 +523,113 @@ func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID
 	}
 	return nil
 }
+
+// GetAuthenticatedUser returns the login of the user authenticated by the token.
+func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) {
+	reqURL := fmt.Sprintf("%s/api/v1/user", c.baseURL)
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return "", fmt.Errorf("get authenticated user: %w", err)
+	}
+	var result struct {
+		Login string `json:"login"`
+	}
+	if err := json.Unmarshal(body, &result); err != nil {
+		return "", fmt.Errorf("parse user response: %w", err)
+	}
+	return result.Login, nil
+}
+
+// RequestReviewer adds the given user as a requested reviewer on a pull request.
+// This is idempotent — requesting an already-requested reviewer is a no-op.
+func (c *Client) RequestReviewer(ctx context.Context, owner, repo string, number int, reviewer string) error {
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/requested_reviewers",
+		c.baseURL,
+		url.PathEscape(owner),
+		url.PathEscape(repo),
+		number)
+
+	payload := struct {
+		Reviewers []string `json:"reviewers"`
+	}{Reviewers: []string{reviewer}}
+	data, err := json.Marshal(payload)
+	if err != nil {
+		return fmt.Errorf("marshal reviewer request: %w", err)
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data))
+	if err != nil {
+		return fmt.Errorf("create reviewer request: %w", err)
+	}
+	req.Header.Set("Authorization", "token "+c.token)
+	req.Header.Set("Content-Type", "application/json")
+
+	resp, err := c.http.Do(req)
+	if err != nil {
+		return fmt.Errorf("request reviewer: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent {
+		body, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
+		return fmt.Errorf("request reviewer failed (status %d): %s", resp.StatusCode, body)
+	}
+	return nil
+}
+
+// ListReviewComments returns the inline comments attached to a specific review.
+// Paginates through all pages.
+func (c *Client) ListReviewComments(ctx context.Context, owner, repo string, prNumber int, reviewID int64) ([]ReviewComment, error) {
+	const pageSize = 50
+	var all []ReviewComment
+	for page := 1; ; page++ {
+		reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d/comments?limit=%d&page=%d",
+			c.baseURL,
+			url.PathEscape(owner),
+			url.PathEscape(repo),
+			prNumber,
+			reviewID,
+			pageSize,
+			page)
+		body, err := c.doGet(ctx, reqURL)
+		if err != nil {
+			return nil, fmt.Errorf("list review comments (page %d): %w", page, err)
+		}
+		var batch []ReviewComment
+		if err := json.Unmarshal(body, &batch); err != nil {
+			return nil, fmt.Errorf("parse review comments (page %d): %w", page, err)
+		}
+		all = append(all, batch...)
+		if len(batch) < pageSize {
+			break
+		}
+	}
+	return all, nil
+}
+
+// ResolveComment marks an inline review comment as resolved.
+func (c *Client) ResolveComment(ctx context.Context, owner, repo string, commentID int64) error {
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/comments/%d/resolve",
+		c.baseURL,
+		url.PathEscape(owner),
+		url.PathEscape(repo),
+		commentID)
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, nil)
+	if err != nil {
+		return fmt.Errorf("create resolve request: %w", err)
+	}
+	req.Header.Set("Authorization", "token "+c.token)
+
+	resp, err := c.http.Do(req)
+	if err != nil {
+		return fmt.Errorf("resolve comment: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent {
+		body, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
+		return fmt.Errorf("resolve comment failed (status %d): %s", resp.StatusCode, body)
+	}
+	return nil
+}

gitea/client_test.go

@@ -3,9 +3,12 @@ package gitea
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 )
 
@@ -505,3 +508,238 @@ func TestGetTimelineReviewCommentID_NotFound(t *testing.T) {
 		t.Fatal("expected error when sentinel not found")
 	}
 }
+
+func TestGetAllFilesInPath_404FallsBackToFile(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/api/v1/repos/owner/repo/contents/README.md":
+			// Contents API returns 404 for files (not a directory)
+			w.WriteHeader(http.StatusNotFound)
+			w.Write([]byte(`{"message":"not found"}`))
+		case "/api/v1/repos/owner/repo/raw/README.md":
+			w.Write([]byte("# Hello\n"))
+		default:
+			w.WriteHeader(http.StatusNotFound)
+			w.Write([]byte(`{"message":"not found"}`))
+		}
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	files, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
+	if err != nil {
+		t.Fatalf("expected fallback to file on 404, got error: %v", err)
+	}
+	if len(files) != 1 {
+		t.Fatalf("expected 1 file, got %d", len(files))
+	}
+	if files["README.md"] != "# Hello\n" {
+		t.Errorf("unexpected content: %q", files["README.md"])
+	}
+}
+
+func TestGetAllFilesInPath_500Propagates(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Simulate a server error from ListContents
+		w.WriteHeader(http.StatusInternalServerError)
+		w.Write([]byte(`{"message":"internal server error"}`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	_, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "somepath")
+	if err == nil {
+		t.Fatal("expected error to propagate for 500, got nil")
+	}
+	// Should NOT fall back to file fetch — error should propagate
+	var apiErr *APIError
+	if !errors.As(err, &apiErr) {
+		t.Fatalf("expected APIError in chain, got: %v", err)
+	}
+	if apiErr.StatusCode != http.StatusInternalServerError {
+		t.Errorf("expected status 500, got %d", apiErr.StatusCode)
+	}
+}
+
+func TestGetAllFilesInPath_403Propagates(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+		w.Write([]byte(`{"message":"token has insufficient scope"}`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	_, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "private/stuff")
+	if err == nil {
+		t.Fatal("expected error to propagate for 403, got nil")
+	}
+	var apiErr *APIError
+	if !errors.As(err, &apiErr) {
+		t.Fatalf("expected APIError in chain, got: %v", err)
+	}
+	if apiErr.StatusCode != http.StatusForbidden {
+		t.Errorf("expected status 403, got %d", apiErr.StatusCode)
+	}
+}
+
+func TestIsNotFound(t *testing.T) {
+	tests := []struct {
+		name   string
+		err    error
+		want   bool
+	}{
+		{"nil error", nil, false},
+		{"non-API error", fmt.Errorf("network timeout"), false},
+		{"404 APIError", &APIError{StatusCode: 404, Body: "not found"}, true},
+		{"500 APIError", &APIError{StatusCode: 500, Body: "server error"}, false},
+		{"wrapped 404", fmt.Errorf("list contents: %w", &APIError{StatusCode: 404, Body: "not found"}), true},
+		{"wrapped 500", fmt.Errorf("list contents: %w", &APIError{StatusCode: 500, Body: "err"}), false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := IsNotFound(tt.err)
+			if got != tt.want {
+				t.Errorf("IsNotFound(%v) = %v, want %v", tt.err, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestGetAuthenticatedUser(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/api/v1/user" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			http.NotFound(w, r)
+			return
+		}
+		if r.Header.Get("Authorization") != "token test-token" {
+			t.Error("missing or wrong auth header")
+		}
+		w.Header().Set("Content-Type", "application/json")
+		fmt.Fprint(w, `{"login":"my-bot","id":42}`)
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	login, err := client.GetAuthenticatedUser(context.Background())
+	if err != nil {
+		t.Fatalf("GetAuthenticatedUser() error = %v", err)
+	}
+	if login != "my-bot" {
+		t.Errorf("login = %q, want %q", login, "my-bot")
+	}
+}
+
+func TestRequestReviewer(t *testing.T) {
+	var gotBody []byte
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			t.Errorf("expected POST, got %s", r.Method)
+		}
+		expected := "/api/v1/repos/owner/repo/pulls/7/requested_reviewers"
+		if r.URL.Path != expected {
+			t.Errorf("path = %q, want %q", r.URL.Path, expected)
+		}
+		gotBody, _ = io.ReadAll(r.Body)
+		w.WriteHeader(http.StatusCreated)
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	err := client.RequestReviewer(context.Background(), "owner", "repo", 7, "bot-user")
+	if err != nil {
+		t.Fatalf("RequestReviewer() error = %v", err)
+	}
+	if !strings.Contains(string(gotBody), `"bot-user"`) {
+		t.Errorf("body = %s, want to contain bot-user", gotBody)
+	}
+}
+
+func TestRequestReviewer_204(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNoContent)
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
+	if err != nil {
+		t.Fatalf("RequestReviewer() should accept 204, got error = %v", err)
+	}
+}
+
+func TestRequestReviewer_Error(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+		fmt.Fprint(w, "no permission")
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
+	if err == nil {
+		t.Fatal("expected error for 403 response")
+	}
+	if !strings.Contains(err.Error(), "403") {
+		t.Errorf("error should mention status code: %v", err)
+	}
+}
+
+func TestListReviewComments(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if !strings.Contains(r.URL.Path, "/pulls/1/reviews/42/comments") {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		fmt.Fprint(w, `[{"id":100,"path":"main.go","new_position":5,"body":"finding"},{"id":101,"path":"lib.go","new_position":10,"body":"another"}]`)
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	comments, err := client.ListReviewComments(context.Background(), "owner", "repo", 1, 42)
+	if err != nil {
+		t.Fatalf("ListReviewComments() error = %v", err)
+	}
+	if len(comments) != 2 {
+		t.Fatalf("got %d comments, want 2", len(comments))
+	}
+	if comments[0].ID != 100 {
+		t.Errorf("comments[0].ID = %d, want 100", comments[0].ID)
+	}
+	if comments[1].Path != "lib.go" {
+		t.Errorf("comments[1].Path = %q, want %q", comments[1].Path, "lib.go")
+	}
+}
+
+func TestResolveComment(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			t.Errorf("expected POST, got %s", r.Method)
+		}
+		if !strings.Contains(r.URL.Path, "/pulls/comments/99/resolve") {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+		}
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	err := client.ResolveComment(context.Background(), "owner", "repo", 99)
+	if err != nil {
+		t.Fatalf("ResolveComment() error = %v", err)
+	}
+}
+
+func TestResolveComment_Error(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+		fmt.Fprint(w, "not found")
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	err := client.ResolveComment(context.Background(), "owner", "repo", 99)
+	if err == nil {
+		t.Fatal("expected error for 404 response")
+	}
+}

llm/aicore.go

@@ -0,0 +1,391 @@
+package llm
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"net/url"
+	"strings"
+	"sync"
+	"time"
+)
+
+// AICoreOpenAIAPIVersion is the API version used for OpenAI models through AI Core.
+// Update this when SAP AI Core releases a new stable version.
+const AICoreOpenAIAPIVersion = "2024-12-01-preview"
+
+// maxErrorBodyLen limits the length of response bodies included in error messages
+// to prevent leaking potentially sensitive upstream details in logs.
+const maxErrorBodyLen = 200
+
+// AICoreConfig holds SAP AI Core authentication and connection settings.
+type AICoreConfig struct {
+	ClientID      string
+	ClientSecret  string
+	AuthURL       string
+	APIURL        string
+	ResourceGroup string
+}
+
+// AICoreClient wraps AI Core authentication and deployment discovery.
+// Thread-safe for concurrent use after construction.
+//
+// Design: The deployment cache is populated once and never invalidated. This is
+// acceptable for short-lived CI runner processes, but longer-lived deployments
+// may want to add a TTL or re-fetch on errors.
+type AICoreClient struct {
+	config AICoreConfig
+	http   *http.Client
+
+	mu          sync.RWMutex
+	token       string
+	tokenExpiry time.Time
+	deployments map[string]string // model name -> deployment URL
+}
+
+// NewAICoreClient creates a new AI Core client with the given configuration.
+// The client uses a default 5-minute timeout; use WithTimeout to customize.
+func NewAICoreClient(cfg AICoreConfig) *AICoreClient {
+	return &AICoreClient{
+		config:      cfg,
+		http:        &http.Client{Timeout: 5 * time.Minute},
+		deployments: make(map[string]string),
+	}
+}
+
+// WithTimeout sets the HTTP request timeout for AI Core calls.
+// This should be called during construction, before concurrent use.
+func (c *AICoreClient) WithTimeout(d time.Duration) *AICoreClient {
+	c.http.Timeout = d
+	return c
+}
+
+// truncateBody truncates a response body for inclusion in error messages.
+// This prevents leaking potentially sensitive upstream response details in logs.
+func truncateBody(body []byte) string {
+	if len(body) <= maxErrorBodyLen {
+		return string(body)
+	}
+	return string(body[:maxErrorBodyLen]) + "..."
+}
+
+// getToken returns a valid OAuth token, refreshing if necessary.
+func (c *AICoreClient) getToken(ctx context.Context) (string, error) {
+	c.mu.RLock()
+	if c.token != "" && time.Now().Add(5*time.Minute).Before(c.tokenExpiry) {
+		token := c.token
+		c.mu.RUnlock()
+		return token, nil
+	}
+	c.mu.RUnlock()
+
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	// Double-check after acquiring write lock
+	if c.token != "" && time.Now().Add(5*time.Minute).Before(c.tokenExpiry) {
+		return c.token, nil
+	}
+
+	token, expiry, err := c.fetchToken(ctx)
+	if err != nil {
+		return "", err
+	}
+	c.token = token
+	c.tokenExpiry = expiry
+	return token, nil
+}
+
+func (c *AICoreClient) fetchToken(ctx context.Context) (string, time.Time, error) {
+	tokenURL := strings.TrimRight(c.config.AuthURL, "/") + "/oauth/token"
+
+	data := url.Values{}
+	data.Set("grant_type", "client_credentials")
+	data.Set("client_id", c.config.ClientID)
+	data.Set("client_secret", c.config.ClientSecret)
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, tokenURL, strings.NewReader(data.Encode()))
+	if err != nil {
+		return "", time.Time{}, fmt.Errorf("create token request: %w", err)
+	}
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	resp, err := c.http.Do(req)
+	if err != nil {
+		return "", time.Time{}, fmt.Errorf("token request: %w", err)
+	}
+	defer resp.Body.Close()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", time.Time{}, fmt.Errorf("read token response: %w", err)
+	}
+
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return "", time.Time{}, fmt.Errorf("token request failed (status %d): %s", resp.StatusCode, truncateBody(body))
+	}
+
+	var tokenResp struct {
+		AccessToken string `json:"access_token"`
+		ExpiresIn   int    `json:"expires_in"`
+	}
+	if err := json.Unmarshal(body, &tokenResp); err != nil {
+		return "", time.Time{}, fmt.Errorf("parse token response: %w", err)
+	}
+
+	if tokenResp.AccessToken == "" {
+		return "", time.Time{}, fmt.Errorf("empty access token in response")
+	}
+
+	expiry := time.Now().Add(time.Duration(tokenResp.ExpiresIn) * time.Second)
+	return tokenResp.AccessToken, expiry, nil
+}
+
+// getDeploymentURL returns the deployment URL for a model, fetching deployments if needed.
+// getDeploymentURL returns the deployment URL for a model, fetching deployments if needed.
+// Also returns a valid token for use by the caller, avoiding redundant getToken calls.
+//
+// Note: The token is fetched before acquiring the write lock to avoid holding the lock
+// during network I/O. In rare cases where multiple goroutines race and one waits a long
+// time for the write lock, the token could theoretically expire. The 5-minute refresh
+// buffer in getToken makes this extremely unlikely in practice.
+func (c *AICoreClient) getDeploymentURL(ctx context.Context, model string) (deployURL, token string, err error) {
+	c.mu.RLock()
+	if u, ok := c.deployments[model]; ok {
+		c.mu.RUnlock()
+		// Still need a token for the caller
+		token, err = c.getToken(ctx)
+		if err != nil {
+			return "", "", fmt.Errorf("get token: %w", err)
+		}
+		return u, token, nil
+	}
+	c.mu.RUnlock()
+
+	// Fetch token first (before acquiring write lock to avoid holding lock during I/O)
+	token, err = c.getToken(ctx)
+	if err != nil {
+		return "", "", fmt.Errorf("get token for deployments: %w", err)
+	}
+
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	// Double-check after acquiring write lock
+	if u, ok := c.deployments[model]; ok {
+		return u, token, nil
+	}
+
+	if err := c.fetchDeployments(ctx, token); err != nil {
+		return "", "", err
+	}
+
+	if u, ok := c.deployments[model]; ok {
+		return u, token, nil
+	}
+	return "", "", fmt.Errorf("no deployment found for model %q", model)
+}
+
+func (c *AICoreClient) fetchDeployments(ctx context.Context, token string) error {
+	deployURL := strings.TrimRight(c.config.APIURL, "/") + "/v2/lm/deployments"
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, deployURL, nil)
+	if err != nil {
+		return fmt.Errorf("create deployments request: %w", err)
+	}
+	req.Header.Set("Authorization", "Bearer "+token)
+	req.Header.Set("AI-Resource-Group", c.config.ResourceGroup)
+
+	resp, err := c.http.Do(req)
+	if err != nil {
+		return fmt.Errorf("deployments request: %w", err)
+	}
+	defer resp.Body.Close()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return fmt.Errorf("read deployments response: %w", err)
+	}
+
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return fmt.Errorf("deployments request failed (status %d): %s", resp.StatusCode, truncateBody(body))
+	}
+
+	var deployResp struct {
+		Resources []struct {
+			DeploymentURL string `json:"deploymentUrl"`
+			Status        string `json:"status"`
+			Details       struct {
+				Resources struct {
+					BackendDetails struct {
+						Model struct {
+							Name string `json:"name"`
+						} `json:"model"`
+					} `json:"backend_details"`
+				} `json:"resources"`
+			} `json:"details"`
+		} `json:"resources"`
+	}
+	if err := json.Unmarshal(body, &deployResp); err != nil {
+		return fmt.Errorf("parse deployments response: %w", err)
+	}
+
+	for _, r := range deployResp.Resources {
+		if r.Status != "RUNNING" {
+			continue
+		}
+		modelName := r.Details.Resources.BackendDetails.Model.Name
+		if modelName == "" {
+			continue
+		}
+		c.deployments[modelName] = r.DeploymentURL
+	}
+
+	return nil
+}
+
+// CompleteAnthropic sends a request to an Anthropic model via AI Core.
+func (c *AICoreClient) CompleteAnthropic(ctx context.Context, model string, messages []Message, maxTokens int, temperature float64) (string, error) {
+	deployURL, token, err := c.getDeploymentURL(ctx, model)
+	if err != nil {
+		return "", err
+	}
+
+	// Extract system message
+	var system string
+	var userMessages []anthropicMsg
+	for _, m := range messages {
+		if m.Role == "system" {
+			system = m.Content
+		} else {
+			userMessages = append(userMessages, anthropicMsg{
+				Role:    m.Role,
+				Content: m.Content,
+			})
+		}
+	}
+
+	reqBody := anthropicRequest{
+		AnthropicVersion: "bedrock-2023-05-31", // SAP AI Core uses Bedrock format
+		// Model omitted - AI Core deployment already specifies model
+		MaxTokens: maxTokens,
+		System:    system,
+		Messages:  userMessages,
+	}
+	if temperature > 0 {
+		reqBody.Temperature = temperature
+	}
+
+	data, err := json.Marshal(reqBody)
+	if err != nil {
+		return "", fmt.Errorf("marshal request: %w", err)
+	}
+
+	// AI Core uses /invoke for Anthropic models
+	invokeURL := strings.TrimRight(deployURL, "/") + "/invoke"
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, invokeURL, bytes.NewReader(data))
+	if err != nil {
+		return "", fmt.Errorf("create request: %w", err)
+	}
+	req.Header.Set("Authorization", "Bearer "+token)
+	req.Header.Set("AI-Resource-Group", c.config.ResourceGroup)
+	req.Header.Set("Content-Type", "application/json")
+
+	resp, err := c.http.Do(req)
+	if err != nil {
+		return "", fmt.Errorf("AI Core request: %w", err)
+	}
+	defer resp.Body.Close()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", fmt.Errorf("read response: %w", err)
+	}
+
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return "", fmt.Errorf("AI Core API error (status %d): %s", resp.StatusCode, truncateBody(body))
+	}
+
+	var anthropicResp anthropicResponse
+	if err := json.Unmarshal(body, &anthropicResp); err != nil {
+		return "", fmt.Errorf("parse response: %w", err)
+	}
+
+	if len(anthropicResp.Content) == 0 {
+		return "", fmt.Errorf("no content in response")
+	}
+
+	var sb strings.Builder
+	for _, block := range anthropicResp.Content {
+		if block.Type == "text" {
+			sb.WriteString(block.Text)
+		}
+	}
+	result := sb.String()
+	if result == "" {
+		return "", fmt.Errorf("no text content in response")
+	}
+	return result, nil
+}
+
+// CompleteOpenAI sends a request to an OpenAI model via AI Core.
+func (c *AICoreClient) CompleteOpenAI(ctx context.Context, model string, messages []Message, temperature float64) (string, error) {
+	deployURL, token, err := c.getDeploymentURL(ctx, model)
+	if err != nil {
+		return "", err
+	}
+
+	reqBody := ChatRequest{
+		Model:       model,
+		Temperature: temperature,
+		Messages:    messages,
+	}
+
+	data, err := json.Marshal(reqBody)
+	if err != nil {
+		return "", fmt.Errorf("marshal request: %w", err)
+	}
+
+	// AI Core uses /chat/completions?api-version=<version> for OpenAI models
+	chatURL := strings.TrimRight(deployURL, "/") + "/chat/completions?api-version=" + AICoreOpenAIAPIVersion
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, chatURL, bytes.NewReader(data))
+	if err != nil {
+		return "", fmt.Errorf("create request: %w", err)
+	}
+	req.Header.Set("Authorization", "Bearer "+token)
+	req.Header.Set("AI-Resource-Group", c.config.ResourceGroup)
+	req.Header.Set("Content-Type", "application/json")
+
+	resp, err := c.http.Do(req)
+	if err != nil {
+		return "", fmt.Errorf("AI Core request: %w", err)
+	}
+	defer resp.Body.Close()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", fmt.Errorf("read response: %w", err)
+	}
+
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return "", fmt.Errorf("AI Core API error (status %d): %s", resp.StatusCode, truncateBody(body))
+	}
+
+	var openaiResp ChatResponse
+	if err := json.Unmarshal(body, &openaiResp); err != nil {
+		return "", fmt.Errorf("parse response: %w", err)
+	}
+
+	if len(openaiResp.Choices) == 0 {
+		return "", fmt.Errorf("no choices in response")
+	}
+	return openaiResp.Choices[0].Message.Content, nil
+}
+
+// IsAnthropicModel returns true if the model name indicates an Anthropic model.
+// SAP AI Core uses "anthropic--" prefix for Anthropic models (e.g., "anthropic--claude-3-5-sonnet").
+func IsAnthropicModel(model string) bool {
+	return strings.HasPrefix(model, "anthropic--")
+}

llm/aicore_test.go

@@ -0,0 +1,535 @@
+package llm
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync/atomic"
+	"testing"
+	"time"
+)
+
+func TestAICoreClient_TokenFetch(t *testing.T) {
+	tokenCalls := int32(0)
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/oauth/token" {
+			atomic.AddInt32(&tokenCalls, 1)
+			if r.Method != http.MethodPost {
+				t.Errorf("expected POST for token, got %s", r.Method)
+			}
+			if r.Header.Get("Content-Type") != "application/x-www-form-urlencoded" {
+				t.Errorf("expected form content type")
+			}
+			w.Header().Set("Content-Type", "application/json")
+			json.NewEncoder(w).Encode(map[string]interface{}{
+				"access_token": "test-token-123",
+				"expires_in":   3600,
+			})
+			return
+		}
+		t.Errorf("unexpected path: %s", r.URL.Path)
+	}))
+	defer server.Close()
+
+	client := NewAICoreClient(AICoreConfig{
+		ClientID:      "test-id",
+		ClientSecret:  "test-secret",
+		AuthURL:       server.URL,
+		APIURL:        server.URL,
+		ResourceGroup: "default",
+	})
+
+	token, err := client.getToken(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if token != "test-token-123" {
+		t.Errorf("expected token 'test-token-123', got %q", token)
+	}
+
+	// Second call should use cached token
+	token2, err := client.getToken(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if token2 != "test-token-123" {
+		t.Errorf("expected cached token")
+	}
+	if atomic.LoadInt32(&tokenCalls) != 1 {
+		t.Errorf("expected 1 token call (cached), got %d", tokenCalls)
+	}
+}
+
+func TestAICoreClient_DeploymentFetch(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/oauth/token" {
+			w.Header().Set("Content-Type", "application/json")
+			json.NewEncoder(w).Encode(map[string]interface{}{
+				"access_token": "test-token",
+				"expires_in":   3600,
+			})
+			return
+		}
+		if r.URL.Path == "/v2/lm/deployments" {
+			if r.Header.Get("Authorization") != "Bearer test-token" {
+				t.Errorf("expected Bearer auth")
+			}
+			if r.Header.Get("AI-Resource-Group") != "default" {
+				t.Errorf("expected resource group header")
+			}
+			w.Header().Set("Content-Type", "application/json")
+			json.NewEncoder(w).Encode(map[string]interface{}{
+				"resources": []map[string]interface{}{
+					{
+						"id":            "deploy-123",
+						"deploymentUrl": "https://example.com/v2/inference/deployments/deploy-123",
+						"status":        "RUNNING",
+						"details": map[string]interface{}{
+							"resources": map[string]interface{}{
+								"backend_details": map[string]interface{}{
+									"model": map[string]interface{}{
+										"name": "anthropic--claude-4.6-sonnet",
+									},
+								},
+							},
+						},
+					},
+					{
+						"id":            "deploy-456",
+						"deploymentUrl": "https://example.com/v2/inference/deployments/deploy-456",
+						"status":        "STOPPED",
+						"details": map[string]interface{}{
+							"resources": map[string]interface{}{
+								"backend_details": map[string]interface{}{
+									"model": map[string]interface{}{
+										"name": "gpt-5",
+									},
+								},
+							},
+						},
+					},
+					{
+						"id":            "deploy-789",
+						"deploymentUrl": "https://example.com/v2/inference/deployments/deploy-789",
+						"status":        "RUNNING",
+						"details": map[string]interface{}{
+							"resources": map[string]interface{}{
+								"backend_details": map[string]interface{}{
+									"model": map[string]interface{}{
+										"name": "gpt-5",
+									},
+								},
+							},
+						},
+					},
+				},
+			})
+			return
+		}
+		t.Errorf("unexpected path: %s", r.URL.Path)
+	}))
+	defer server.Close()
+
+	client := NewAICoreClient(AICoreConfig{
+		ClientID:      "test-id",
+		ClientSecret:  "test-secret",
+		AuthURL:       server.URL,
+		APIURL:        server.URL,
+		ResourceGroup: "default",
+	})
+
+	// Should find running deployment
+	url, _, err := client.getDeploymentURL(context.Background(), "anthropic--claude-4.6-sonnet")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if url != "https://example.com/v2/inference/deployments/deploy-123" {
+		t.Errorf("unexpected URL: %s", url)
+	}
+
+	// Should find running gpt-5, not stopped one
+	url, _, err = client.getDeploymentURL(context.Background(), "gpt-5")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if url != "https://example.com/v2/inference/deployments/deploy-789" {
+		t.Errorf("unexpected URL: %s", url)
+	}
+
+	// Should error on unknown model
+	_, _, err = client.getDeploymentURL(context.Background(), "unknown-model")
+	if err == nil {
+		t.Error("expected error for unknown model")
+	}
+}
+
+func TestAICoreClient_CompleteAnthropic(t *testing.T) {
+	// baseURL is set after server creation; captured by closure in handlers
+	var baseURL string
+	mux := http.NewServeMux()
+	mux.HandleFunc("/oauth/token", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]interface{}{
+			"access_token": "test-token",
+			"expires_in":   3600,
+		})
+	})
+	mux.HandleFunc("/v2/lm/deployments", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]interface{}{
+			"resources": []map[string]interface{}{
+				{
+					"id":            "deploy-anthropic",
+					"deploymentUrl": baseURL + "/deployments/anthropic",
+					"status":        "RUNNING",
+					"details": map[string]interface{}{
+						"resources": map[string]interface{}{
+							"backend_details": map[string]interface{}{
+								"model": map[string]interface{}{
+									"name": "anthropic--claude-4.6-sonnet",
+								},
+							},
+						},
+					},
+				},
+			},
+		})
+	})
+	mux.HandleFunc("/deployments/anthropic/invoke", func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("Authorization") != "Bearer test-token" {
+			t.Errorf("expected Bearer auth on invoke")
+		}
+		var req anthropicRequest
+		if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+			t.Fatalf("decode request: %v", err)
+		}
+		if req.AnthropicVersion != "bedrock-2023-05-31" {
+			t.Errorf("expected bedrock anthropic_version in request")
+		}
+		if req.System != "You are helpful" {
+			t.Errorf("expected system prompt: %q", req.System)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]interface{}{
+			"content": []map[string]interface{}{
+				{"type": "text", "text": "Hello from AI Core!"},
+			},
+		})
+	})
+
+	server := httptest.NewServer(mux)
+	baseURL = server.URL
+	defer server.Close()
+
+	client := NewAICoreClient(AICoreConfig{
+		ClientID:      "test-id",
+		ClientSecret:  "test-secret",
+		AuthURL:       server.URL,
+		APIURL:        server.URL,
+		ResourceGroup: "default",
+	})
+
+	result, err := client.CompleteAnthropic(context.Background(), "anthropic--claude-4.6-sonnet", []Message{
+		{Role: "system", Content: "You are helpful"},
+		{Role: "user", Content: "Hello"},
+	}, 8192, 0)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if result != "Hello from AI Core!" {
+		t.Errorf("expected 'Hello from AI Core!', got %q", result)
+	}
+}
+
+func TestAICoreClient_CompleteOpenAI(t *testing.T) {
+	var baseURL string
+	mux := http.NewServeMux()
+	mux.HandleFunc("/oauth/token", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]interface{}{
+			"access_token": "test-token",
+			"expires_in":   3600,
+		})
+	})
+	mux.HandleFunc("/v2/lm/deployments", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]interface{}{
+			"resources": []map[string]interface{}{
+				{
+					"id":            "deploy-openai",
+					"deploymentUrl": baseURL + "/deployments/openai",
+					"status":        "RUNNING",
+					"details": map[string]interface{}{
+						"resources": map[string]interface{}{
+							"backend_details": map[string]interface{}{
+								"model": map[string]interface{}{
+									"name": "gpt-5",
+								},
+							},
+						},
+					},
+				},
+			},
+		})
+	})
+	mux.HandleFunc("/deployments/openai/chat/completions", func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Query().Get("api-version") != AICoreOpenAIAPIVersion {
+			t.Errorf("expected api-version %s, got %s", AICoreOpenAIAPIVersion, r.URL.Query().Get("api-version"))
+		}
+		var req ChatRequest
+		if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+			t.Fatalf("decode request: %v", err)
+		}
+		if req.Model != "gpt-5" {
+			t.Errorf("expected model gpt-5, got %s", req.Model)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(ChatResponse{
+			Choices: []struct {
+				Message struct {
+					Content string `json:"content"`
+				} `json:"message"`
+			}{
+				{Message: struct {
+					Content string `json:"content"`
+				}{Content: "Hello from GPT-5!"}},
+			},
+		})
+	})
+
+	server := httptest.NewServer(mux)
+	baseURL = server.URL
+	defer server.Close()
+
+	client := NewAICoreClient(AICoreConfig{
+		ClientID:      "test-id",
+		ClientSecret:  "test-secret",
+		AuthURL:       server.URL,
+		APIURL:        server.URL,
+		ResourceGroup: "default",
+	})
+
+	result, err := client.CompleteOpenAI(context.Background(), "gpt-5", []Message{
+		{Role: "user", Content: "Hello"},
+	}, 0)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if result != "Hello from GPT-5!" {
+		t.Errorf("expected 'Hello from GPT-5!', got %q", result)
+	}
+}
+
+func TestIsAnthropicModel(t *testing.T) {
+	tests := []struct {
+		model    string
+		expected bool
+	}{
+		// SAP AI Core uses "anthropic--" prefix for Anthropic models
+		{"anthropic--claude-4.6-sonnet", true},
+		{"anthropic--claude-4.6-opus", true},
+		{"anthropic--claude-3-5-sonnet", true},
+		// Non-prefixed model names are not detected as Anthropic
+		// (SAP AI Core always uses the prefix for Anthropic models)
+		{"claude-sonnet-4", false},
+		{"gpt-5", false},
+		{"gpt-4.1", false},
+		{"llama-3", false},
+		{"my-claude-model", false}, // Avoid false positives on "claude" substring
+	}
+
+	for _, tt := range tests {
+		got := IsAnthropicModel(tt.model)
+		if got != tt.expected {
+			t.Errorf("IsAnthropicModel(%q) = %v, want %v", tt.model, got, tt.expected)
+		}
+	}
+}
+
+func TestAICoreClient_TokenExpiry(t *testing.T) {
+	tokenCalls := int32(0)
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/oauth/token" {
+			call := atomic.AddInt32(&tokenCalls, 1)
+			w.Header().Set("Content-Type", "application/json")
+			json.NewEncoder(w).Encode(map[string]interface{}{
+				"access_token": fmt.Sprintf("token-%d", call),
+				"expires_in":   1, // 1 second expiry
+			})
+			return
+		}
+	}))
+	defer server.Close()
+
+	client := NewAICoreClient(AICoreConfig{
+		ClientID:      "test-id",
+		ClientSecret:  "test-secret",
+		AuthURL:       server.URL,
+		APIURL:        server.URL,
+		ResourceGroup: "default",
+	})
+
+	// First call
+	token1, err := client.getToken(context.Background())
+	if err != nil {
+		t.Fatalf("first getToken: %v", err)
+	}
+
+	// Force token expiry by manipulating expiry time
+	client.mu.Lock()
+	client.tokenExpiry = time.Now().Add(-time.Hour)
+	client.mu.Unlock()
+
+	// Should fetch new token
+	token2, err := client.getToken(context.Background())
+	if err != nil {
+		t.Fatalf("second getToken: %v", err)
+	}
+
+	if token1 == token2 {
+		t.Error("expected different tokens after expiry")
+	}
+	if atomic.LoadInt32(&tokenCalls) != 2 {
+		t.Errorf("expected 2 token calls, got %d", tokenCalls)
+	}
+}
+
+func TestAICoreClient_WithTimeout(t *testing.T) {
+	client := NewAICoreClient(AICoreConfig{
+		ClientID:      "test-id",
+		ClientSecret:  "test-secret",
+		AuthURL:       "https://auth.example.com",
+		APIURL:        "https://api.example.com",
+		ResourceGroup: "default",
+	})
+
+	// Default timeout is 5 minutes
+	if client.http.Timeout != 5*time.Minute {
+		t.Errorf("expected default timeout 5m, got %v", client.http.Timeout)
+	}
+
+	// WithTimeout should update the timeout
+	client.WithTimeout(10 * time.Minute)
+	if client.http.Timeout != 10*time.Minute {
+		t.Errorf("expected timeout 10m, got %v", client.http.Timeout)
+	}
+}
+
+func TestClient_WithAICore(t *testing.T) {
+	client := NewClient("http://example.com", "key", "model")
+	if client.provider != ProviderOpenAI {
+		t.Errorf("expected default provider openai, got %s", client.provider)
+	}
+
+	client.WithAICore(AICoreConfig{
+		ClientID:      "id",
+		ClientSecret:  "secret",
+		AuthURL:       "https://auth.example.com",
+		APIURL:        "https://api.example.com",
+		ResourceGroup: "default",
+	})
+
+	if client.provider != ProviderAICore {
+		t.Errorf("expected provider aicore, got %s", client.provider)
+	}
+	if client.aicore == nil {
+		t.Error("expected aicore client to be set")
+	}
+}
+
+func TestClient_WithTimeout_PropagatestoAICore(t *testing.T) {
+	client := NewClient("http://example.com", "key", "model").
+		WithAICore(AICoreConfig{
+			ClientID:      "id",
+			ClientSecret:  "secret",
+			AuthURL:       "https://auth.example.com",
+			APIURL:        "https://api.example.com",
+			ResourceGroup: "default",
+		})
+
+	// Default should be 5 minutes (inherited from parent client)
+	if client.aicore.http.Timeout != 5*time.Minute {
+		t.Errorf("expected aicore default timeout 5m, got %v", client.aicore.http.Timeout)
+	}
+
+	// WithTimeout should propagate to AI Core client
+	client.WithTimeout(15 * time.Minute)
+	if client.http.Timeout != 15*time.Minute {
+		t.Errorf("expected parent timeout 15m, got %v", client.http.Timeout)
+	}
+	if client.aicore.http.Timeout != 15*time.Minute {
+		t.Errorf("expected aicore timeout 15m, got %v", client.aicore.http.Timeout)
+	}
+}
+
+func TestClient_CompleteAICore(t *testing.T) {
+	var baseURL string
+	mux := http.NewServeMux()
+	mux.HandleFunc("/oauth/token", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]interface{}{
+			"access_token": "test-token",
+			"expires_in":   3600,
+		})
+	})
+	mux.HandleFunc("/v2/lm/deployments", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]interface{}{
+			"resources": []map[string]interface{}{
+				{
+					"id":            "deploy-test",
+					"deploymentUrl": baseURL + "/deployments/test",
+					"status":        "RUNNING",
+					"details": map[string]interface{}{
+						"resources": map[string]interface{}{
+							"backend_details": map[string]interface{}{
+								"model": map[string]interface{}{
+									"name": "gpt-5",
+								},
+							},
+						},
+					},
+				},
+			},
+		})
+	})
+	mux.HandleFunc("/deployments/test/chat/completions", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(ChatResponse{
+			Choices: []struct {
+				Message struct {
+					Content string `json:"content"`
+				} `json:"message"`
+			}{
+				{Message: struct {
+					Content string `json:"content"`
+				}{Content: "AI Core via Client works!"}},
+			},
+		})
+	})
+
+	server := httptest.NewServer(mux)
+	baseURL = server.URL
+	defer server.Close()
+
+	client := NewClient("", "", "gpt-5").WithAICore(AICoreConfig{
+		ClientID:      "test-id",
+		ClientSecret:  "test-secret",
+		AuthURL:       server.URL,
+		APIURL:        server.URL,
+		ResourceGroup: "default",
+	})
+
+	result, err := client.Complete(context.Background(), []Message{
+		{Role: "user", Content: "Hello"},
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !strings.Contains(result, "AI Core via Client works!") {
+		t.Errorf("unexpected result: %s", result)
+	}
+}

llm/client.go

@@ -1,6 +1,6 @@
 // Package llm provides clients for LLM chat completion APIs.
 //
-// Supports OpenAI-compatible (default) and Anthropic Messages API providers.
+// Supports OpenAI-compatible (default), Anthropic Messages API, and SAP AI Core providers.
 package llm
 
 import (
@@ -22,6 +22,8 @@ const (
 	ProviderOpenAI Provider = "openai"
 	// ProviderAnthropic uses the Anthropic Messages API endpoint.
 	ProviderAnthropic Provider = "anthropic"
+	// ProviderAICore uses SAP AI Core with OAuth authentication.
+	ProviderAICore Provider = "aicore"
 )
 
 // Client calls an LLM chat completion API.
@@ -35,6 +37,7 @@ type Client struct {
 	temperature float64
 	provider    Provider
 	http        *http.Client
+	aicore      *AICoreClient // Only set when provider is aicore
 }
 
 // NewClient creates a new LLM client. Default provider is OpenAI-compatible.
@@ -49,8 +52,12 @@ func NewClient(baseURL, apiKey, model string) *Client {
 }
 
 // WithTimeout sets the HTTP request timeout for LLM calls (default 5 minutes).
+// When using AI Core, this also sets the timeout on the AI Core client.
 func (c *Client) WithTimeout(d time.Duration) *Client {
 	c.http.Timeout = d
+	if c.aicore != nil {
+		c.aicore.WithTimeout(d)
+	}
 	return c
 }
 
@@ -60,12 +67,21 @@ func (c *Client) WithTemperature(t float64) *Client {
 	return c
 }
 
-// WithProvider sets the API provider format (openai or anthropic).
+// WithProvider sets the API provider format (openai, anthropic, or aicore).
 func (c *Client) WithProvider(p Provider) *Client {
 	c.provider = p
 	return c
 }
 
+// WithAICore configures the client to use SAP AI Core for authentication.
+// This sets the provider to aicore automatically.
+// The AI Core client inherits the current HTTP timeout from this client.
+func (c *Client) WithAICore(cfg AICoreConfig) *Client {
+	c.provider = ProviderAICore
+	c.aicore = NewAICoreClient(cfg).WithTimeout(c.http.Timeout)
+	return c
+}
+
 // Message represents a chat message.
 type Message struct {
 	Role    string `json:"role"`
@@ -75,12 +91,66 @@ type Message struct {
 // Complete sends a chat completion request and returns the assistant's response content.
 // The first message with role "system" is treated as the system prompt.
 func (c *Client) Complete(ctx context.Context, messages []Message) (string, error) {
-	switch c.provider {
+	var result string
-	case ProviderAnthropic:
+	var err error
-		return c.completeAnthropic(ctx, messages)
+
-	default:
+	for attempt := 0; attempt < 2; attempt++ {
-		return c.completeOpenAI(ctx, messages)
+		switch c.provider {
+		case ProviderAnthropic:
+			result, err = c.completeAnthropic(ctx, messages)
+		case ProviderAICore:
+			result, err = c.completeAICore(ctx, messages)
+		default:
+			result, err = c.completeOpenAI(ctx, messages)
+		}
+
+		if err == nil {
+			return result, nil
+		}
+
+		// Only retry on response body read errors (transient network issues).
+		// Do not retry on context cancellation, status errors, or parse errors
+		// that indicate a structural API problem.
+		if !isRetryableError(err) {
+			return "", err
+		}
+
+		if attempt == 0 && ctx.Err() == nil {
+			// Brief pause before retry to allow transient issues to resolve.
+			time.Sleep(500 * time.Millisecond)
+		}
 	}
+
+	return "", err
+}
+
+// completeAICore routes to AI Core using the appropriate endpoint based on model type.
+func (c *Client) completeAICore(ctx context.Context, messages []Message) (string, error) {
+	if c.aicore == nil {
+		return "", fmt.Errorf("AI Core client not configured")
+	}
+
+	if IsAnthropicModel(c.model) {
+		return c.aicore.CompleteAnthropic(ctx, c.model, messages, 8192, c.temperature)
+	}
+	return c.aicore.CompleteOpenAI(ctx, c.model, messages, c.temperature)
+}
+
+// isRetryableError returns true for transient errors worth retrying.
+func isRetryableError(err error) bool {
+	if err == nil {
+		return false
+	}
+	s := err.Error()
+	// Body read failures (connection reset, truncation)
+	if strings.Contains(s, "read response") {
+		return true
+	}
+	// Unexpected body length (our content-length validation)
+	if strings.Contains(s, "body length mismatch") {
+		return true
+	}
+	return false
 }
 
 // --- OpenAI-compatible implementation ---
@@ -136,11 +206,12 @@ func (c *Client) completeOpenAI(ctx context.Context, messages []Message) (string
 // --- Anthropic Messages API implementation ---
 
 type anthropicRequest struct {
-	Model       string            `json:"model"`
+	AnthropicVersion string         `json:"anthropic_version,omitempty"`
-	MaxTokens   int               `json:"max_tokens"`
+	Model       string         `json:"model,omitempty"`
-	System      string            `json:"system,omitempty"`
+	MaxTokens   int            `json:"max_tokens"`
-	Messages    []anthropicMsg    `json:"messages"`
+	System      string         `json:"system,omitempty"`
-	Temperature float64           `json:"temperature,omitempty"`
+	Messages    []anthropicMsg `json:"messages"`
+	Temperature float64        `json:"temperature,omitempty"`
 }
 
 type anthropicMsg struct {
@@ -231,6 +302,12 @@ func (c *Client) doRequest(req *http.Request, parse func([]byte) (string, error)
 		return "", fmt.Errorf("read response: %w", err)
 	}
 
+	// Validate body length against Content-Length header when present.
+	// A mismatch indicates the response was truncated in transit.
+	if cl := resp.ContentLength; cl > 0 && int64(len(body)) < cl {
+		return "", fmt.Errorf("body length mismatch: Content-Length=%d, received=%d", cl, len(body))
+	}
+
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		return "", fmt.Errorf("LLM API error (status %d): %s", resp.StatusCode, string(body))
 	}

llm/client_test.go

@@ -3,6 +3,7 @@ package llm
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -295,3 +296,131 @@ func TestWithProvider(t *testing.T) {
 		t.Errorf("expected provider anthropic, got %s", client.provider)
 	}
 }
+
+func TestComplete_RetryOnBodyReadError(t *testing.T) {
+	attempts := 0
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		attempts++
+		if attempts == 1 {
+			// First attempt: send headers then close connection abruptly
+			// Simulate by writing partial response and flushing with wrong Content-Length
+			w.Header().Set("Content-Length", "1000")
+			w.WriteHeader(http.StatusOK)
+			w.Write([]byte(`{"choices":[{"message":{"con`))
+			// The test HTTP server will close the connection after handler returns,
+			// but Content-Length mismatch means client gets fewer bytes than expected
+			return
+		}
+		// Second attempt: succeed
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(ChatResponse{
+			Choices: []struct {
+				Message struct {
+					Content string `json:"content"`
+				} `json:"message"`
+			}{{Message: struct {
+				Content string `json:"content"`
+			}{Content: "success"}}},
+		})
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "key", "model")
+	got, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
+	if err != nil {
+		t.Fatalf("expected retry to succeed, got error: %v", err)
+	}
+	if got != "success" {
+		t.Errorf("expected %q, got %q", "success", got)
+	}
+	if attempts != 2 {
+		t.Errorf("expected 2 attempts, got %d", attempts)
+	}
+}
+
+func TestComplete_ContentLengthMismatch(t *testing.T) {
+	attempts := 0
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		attempts++
+		if attempts == 1 {
+			// Claim Content-Length is larger than actual body
+			w.Header().Set("Content-Length", "500")
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusOK)
+			// Write less than 500 bytes
+			w.Write([]byte(`{"choices":[{"message":{"content":"partial"}}]}`))
+			return
+		}
+		// Second attempt succeeds
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(ChatResponse{
+			Choices: []struct {
+				Message struct {
+					Content string `json:"content"`
+				} `json:"message"`
+			}{{Message: struct {
+				Content string `json:"content"`
+			}{Content: "complete"}}},
+		})
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "key", "model")
+	got, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
+	if err != nil {
+		t.Fatalf("expected retry to succeed on content-length mismatch, got: %v", err)
+	}
+	if got != "complete" {
+		t.Errorf("expected %q, got %q", "complete", got)
+	}
+}
+
+func TestComplete_NoRetryOnAPIError(t *testing.T) {
+	attempts := 0
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		attempts++
+		w.WriteHeader(http.StatusBadRequest)
+		w.Write([]byte(`{"error":"bad request"}`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "key", "model")
+	_, err := client.Complete(context.Background(), []Message{{Role: "user", Content: "Hi"}})
+	if err == nil {
+		t.Fatal("expected error for 400, got nil")
+	}
+	if attempts != 1 {
+		t.Errorf("should not retry on API errors, got %d attempts", attempts)
+	}
+}
+
+func TestIsRetryableError(t *testing.T) {
+	tests := []struct {
+		name     string
+		err      string
+		expected bool
+	}{
+		{"nil formatted", "", false},
+		{"read response error", "read response: unexpected EOF", true},
+		{"body length mismatch", "body length mismatch: Content-Length=1000, received=500", true},
+		{"API error", "LLM API error (status 400): bad request", false},
+		{"parse error", "parse response: unexpected end of JSON input", false},
+		{"request error", "LLM request: connection refused", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.err == "" {
+				if isRetryableError(nil) {
+					t.Error("nil error should not be retryable")
+				}
+				return
+			}
+			err := fmt.Errorf("%s", tt.err)
+			got := isRetryableError(err)
+			if got != tt.expected {
+				t.Errorf("isRetryableError(%q) = %v, want %v", tt.err, got, tt.expected)
+			}
+		})
+	}
+}

review/parser.go

@@ -29,7 +29,19 @@ func ParseResponse(response string) (*ReviewResult, error) {
 
 	var result ReviewResult
 	if err := json.Unmarshal([]byte(cleaned), &result); err != nil {
-		return nil, fmt.Errorf("parse LLM response as JSON: %w\nRaw response: %s", err, response)
+		// LLMs sometimes produce JSON with unescaped quotes inside string values.
+		// Try to repair before giving up.
+		repaired := repairJSON(cleaned)
+		if err2 := json.Unmarshal([]byte(repaired), &result); err2 != nil {
+			// Include diagnostic info: lengths help identify truncation
+			rawLen := len(response)
+			cleanedLen := len(cleaned)
+			preview := cleaned
+			if len(preview) > 200 {
+				preview = preview[:100] + "..." + preview[len(preview)-100:]
+			}
+			return nil, fmt.Errorf("parse LLM response as JSON: %w\nRaw length: %d, cleaned length: %d\nCleaned preview: %s", err, rawLen, cleanedLen, preview)
+		}
 	}
 
 	// Validate verdict
@@ -74,3 +86,230 @@ func extractJSON(s string) string {
 	s = strings.TrimSpace(s)
 	return s
 }
+
+// repairJSON attempts to fix common LLM JSON issues:
+// - Unescaped double quotes inside string values
+//
+// Strategy: walk the JSON structurally. Object keys are parsed normally (LLMs
+// get those right). For string VALUES, we find all candidate closing quotes and
+// pick the LAST one that leaves valid JSON structure afterward — maximizing
+// string content, which is the correct bias for the "LLM put unescaped quotes
+// in a string value" failure mode.
+func repairJSON(s string) string {
+	runes := []rune(s)
+	var out strings.Builder
+	out.Grow(len(s) + 64)
+
+	i := 0
+	for i < len(runes) {
+		c := runes[i]
+
+		if c != '"' {
+			out.WriteRune(c)
+			i++
+			continue
+		}
+
+		// We hit an opening quote. Determine if this is a key or a value.
+		// Keys: the standard JSON parser in LLMs gets keys right, so we parse
+		// them normally (first unescaped quote closes).
+		// Values: may contain unescaped quotes — use the repair heuristic.
+		isValue := isValuePosition(runes, i)
+
+		if !isValue {
+			// Parse key/simple string normally
+			out.WriteRune('"')
+			i++
+			for i < len(runes) {
+				ch := runes[i]
+				if ch == '\\' && i+1 < len(runes) {
+					out.WriteRune(ch)
+					i++
+					out.WriteRune(runes[i])
+					i++
+					continue
+				}
+				if ch == '"' {
+					out.WriteRune('"')
+					i++
+					break
+				}
+				out.WriteRune(ch)
+				i++
+			}
+			continue
+		}
+
+		// Value string — find the correct close using last-valid-candidate heuristic
+		out.WriteRune('"')
+		i++
+
+		closeIdx := findClosingQuote(runes, i)
+
+		// Write everything between open and close, escaping interior quotes
+		for j := i; j < closeIdx; j++ {
+			ch := runes[j]
+			if ch == '\\' && j+1 < closeIdx {
+				// Already-escaped sequence — pass through
+				out.WriteRune(ch)
+				j++
+				out.WriteRune(runes[j])
+			} else if ch == '"' {
+				out.WriteRune('\\')
+				out.WriteRune('"')
+			} else {
+				out.WriteRune(ch)
+			}
+		}
+
+		// Write the closing quote
+		out.WriteRune('"')
+		i = closeIdx + 1
+	}
+
+	return out.String()
+}
+
+// isValuePosition determines if the quote at position i is opening a JSON value
+// string (as opposed to an object key). We only apply repair to values that
+// follow ':' since those are the free-text fields where LLMs produce unescaped
+// quotes. Array elements and keys are left alone (parsed normally).
+func isValuePosition(runes []rune, i int) bool {
+	// Look backward, skipping whitespace, for the preceding structural char
+	j := i - 1
+	for j >= 0 && (runes[j] == ' ' || runes[j] == '\t' || runes[j] == '\n' || runes[j] == '\r') {
+		j--
+	}
+	if j < 0 {
+		return false
+	}
+	// After ':' → definitely a value
+	return runes[j] == ':'
+}
+
+// findClosingQuote finds the index of the true closing quote for a JSON string
+// value starting at position start (the character after the opening quote).
+// It collects all unescaped quote candidates and returns the FIRST one that
+// produces valid JSON continuation (deeper lookahead verifies the next token).
+func findClosingQuote(runes []rune, start int) int {
+	// Collect all candidate positions for the closing quote.
+	var candidates []int
+	for j := start; j < len(runes); j++ {
+		if runes[j] == '\\' {
+			j++ // skip escaped character
+			continue
+		}
+		if runes[j] == '"' {
+			candidates = append(candidates, j)
+		}
+	}
+
+	if len(candidates) == 0 {
+		return len(runes)
+	}
+
+	if len(candidates) == 1 {
+		return candidates[0]
+	}
+
+	// Try candidates from FIRST to LAST. The correct closing quote is the
+	// earliest one that produces valid JSON structure after it (verified by
+	// deeper lookahead that checks the next token is a valid JSON start).
+	for _, idx := range candidates {
+		if isValidJSONAfterClose(runes, idx+1) {
+			return idx
+		}
+	}
+
+	// Fallback: return the last candidate
+	return candidates[len(candidates)-1]
+}
+
+// isValidJSONAfterClose checks whether the runes after a candidate closing quote
+// look like valid JSON continuation for a VALUE string. Since we only use this
+// for value positions, ':' is NOT a valid continuation (values are never keys).
+// Checks deeper structure to avoid being fooled by JSON-like content in strings.
+func isValidJSONAfterClose(runes []rune, pos int) bool {
+	j := pos
+	for j < len(runes) && (runes[j] == ' ' || runes[j] == '\t' || runes[j] == '\n' || runes[j] == '\r') {
+		j++
+	}
+
+	if j >= len(runes) {
+		return true
+	}
+
+	next := runes[j]
+	if next == '}' || next == ']' {
+		// Closing a container. Verify what follows the close is also valid:
+		// another structural char, comma, or EOF.
+		return isValidAfterContainerClose(runes, j+1)
+	}
+	if next == ',' {
+		// After comma, must be followed by a valid JSON token
+		j++
+		for j < len(runes) && (runes[j] == ' ' || runes[j] == '\t' || runes[j] == '\n' || runes[j] == '\r') {
+			j++
+		}
+		if j >= len(runes) {
+			return false // trailing comma with nothing after — invalid
+		}
+		return isJSONTokenStart(runes, j)
+	}
+	// ':' is NOT valid here — we're in a value position, not a key.
+	// Any other character is also invalid.
+	return false
+}
+
+// isValidAfterContainerClose checks that after a } or ], the continuation is
+// structurally valid: more closes, comma+token, or EOF.
+func isValidAfterContainerClose(runes []rune, pos int) bool {
+	j := pos
+	for j < len(runes) && (runes[j] == ' ' || runes[j] == '\t' || runes[j] == '\n' || runes[j] == '\r') {
+		j++
+	}
+	if j >= len(runes) {
+		return true
+	}
+	next := runes[j]
+	if next == '}' || next == ']' {
+		return isValidAfterContainerClose(runes, j+1)
+	}
+	if next == ',' {
+		j++
+		for j < len(runes) && (runes[j] == ' ' || runes[j] == '\t' || runes[j] == '\n' || runes[j] == '\r') {
+			j++
+		}
+		if j >= len(runes) {
+			return false
+		}
+		return isJSONTokenStart(runes, j)
+	}
+	return false
+}
+
+// isJSONTokenStart returns true if the rune could begin a JSON value or key.
+// For keywords (true/false/null), verifies the full keyword is present.
+func isJSONTokenStart(runes []rune, pos int) bool {
+	if pos >= len(runes) {
+		return false
+	}
+	r := runes[pos]
+	switch {
+	case r == '"': // string
+		return true
+	case r == '{' || r == '[': // object or array
+		return true
+	case r == 't': // true
+		return pos+4 <= len(runes) && string(runes[pos:pos+4]) == "true"
+	case r == 'f': // false
+		return pos+5 <= len(runes) && string(runes[pos:pos+5]) == "false"
+	case r == 'n': // null
+		return pos+4 <= len(runes) && string(runes[pos:pos+4]) == "null"
+	case r >= '0' && r <= '9': // number
+		return true
+	case r == '-': // negative number
+		return true
+	}
+	return false
+}

review/parser_test.go

@@ -1,6 +1,7 @@
 package review
 
 import (
+	"encoding/json"
 	"testing"
 )
 
@@ -112,3 +113,112 @@ func TestParseResponse_MarkdownFencesNoLang(t *testing.T) {
 		t.Errorf("expected APPROVE, got %q", result.Verdict)
 	}
 }
+
+func TestParseResponse_UnescapedQuotesInStrings(t *testing.T) {
+	// Real failure from CI: Sonnet puts unescaped quotes like (e.g. "28") in findings
+	input := `{"verdict": "APPROVE", "summary": "Clean PR", "findings": [{"severity": "NIT", "file": "ci/Dockerfile", "line": 14, "finding": "The comment says OTP_VERSION is the major version (e.g. \"28\") but it actually contains unescaped quotes like (e.g. "28") which breaks JSON"}], "recommendation": "Ship it"}`
+
+	result, err := ParseResponse(input)
+	if err != nil {
+		t.Fatalf("expected repair to handle unescaped quotes, got error: %v", err)
+	}
+	if result.Verdict != "APPROVE" {
+		t.Errorf("expected APPROVE, got %q", result.Verdict)
+	}
+	if len(result.Findings) != 1 {
+		t.Fatalf("expected 1 finding, got %d", len(result.Findings))
+	}
+}
+
+func TestRepairJSON_NoOpOnValid(t *testing.T) {
+	valid := `{"key": "value", "num": 42}`
+	result := repairJSON(valid)
+	if result != valid {
+		t.Errorf("repairJSON should not modify valid JSON\n  got:  %s\n  want: %s", result, valid)
+	}
+}
+
+func TestRepairJSON_FixesUnescapedQuotes(t *testing.T) {
+	// Interior quote followed by non-structural character
+	input := `{"msg": "use "foo" here"}`
+	result := repairJSON(input)
+
+	// Should be parseable now
+	var m map[string]interface{}
+	if err := json.Unmarshal([]byte(result), &m); err != nil {
+		t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
+	}
+}
+
+func TestRepairJSON_InteriorQuoteBeforeComma(t *testing.T) {
+	// Bug reported by reviewer: interior quoted word immediately before a comma
+	input := `{"msg": "say "yes", and go"}`
+	result := repairJSON(input)
+
+	var m map[string]interface{}
+	if err := json.Unmarshal([]byte(result), &m); err != nil {
+		t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
+	}
+	// The full string content should be preserved
+	msg, ok := m["msg"].(string)
+	if !ok {
+		t.Fatal("msg field missing or not a string")
+	}
+	if msg != `say "yes", and go` {
+		t.Errorf("unexpected msg content: %q", msg)
+	}
+}
+
+func TestRepairJSON_InteriorQuoteBeforeCloseBrace(t *testing.T) {
+	// Bug reported by reviewer: JSON-shaped syntax inside string values
+	input := `{"msg": "input map {"key": "val"} caused error"}`
+	result := repairJSON(input)
+
+	var m map[string]interface{}
+	if err := json.Unmarshal([]byte(result), &m); err != nil {
+		t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
+	}
+}
+
+func TestRepairJSON_MultipleFields(t *testing.T) {
+	// Multiple string fields with unescaped quotes in different positions
+	input := `{"a": "hello "world"", "b": "foo"}`
+	result := repairJSON(input)
+
+	var m map[string]interface{}
+	if err := json.Unmarshal([]byte(result), &m); err != nil {
+		t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
+	}
+	if _, ok := m["b"]; !ok {
+		t.Error("expected 'b' field to be preserved")
+	}
+}
+
+func TestRepairJSON_PreservesEscapedQuotes(t *testing.T) {
+	// Already-escaped quotes should not be double-escaped
+	input := `{"msg": "already \"escaped\" here"}`
+	result := repairJSON(input)
+
+	if result != input {
+		t.Errorf("repairJSON should not modify already-escaped quotes\n  got:  %s\n  want: %s", result, input)
+	}
+
+	var m map[string]interface{}
+	if err := json.Unmarshal([]byte(result), &m); err != nil {
+		t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
+	}
+}
+
+func TestRepairJSON_ComplexNestedContent(t *testing.T) {
+	// Combines both reviewer bugs: quoted words before commas AND JSON-like content
+	input := `{"verdict": "APPROVE", "findings": [{"finding": "The map {"key": "val"} and (e.g. "28") and say "yes", then stop"}]}`
+	result := repairJSON(input)
+
+	var parsed map[string]interface{}
+	if err := json.Unmarshal([]byte(result), &parsed); err != nil {
+		t.Fatalf("repaired JSON should parse, got: %v\nrepaired: %s", err, result)
+	}
+	if parsed["verdict"] != "APPROVE" {
+		t.Errorf("expected verdict APPROVE, got %v", parsed["verdict"])
+	}
+}