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'
-    required: true
+    description: 'OpenAI-compatible LLM API base URL (not required for aicore provider)'
+    required: false
+    default: ''
   llm-api-key:
-    description: 'LLM API key'
-    required: true
+    description: 'LLM API key (not required for aicore provider)'
+    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' 
+    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_API_KEY: ${{ secrets.LLM_API_KEY }}
+          LLM_PROVIDER: aicore
           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-api-key` | Yes | — | LLM API key |
+| `llm-base-url` | No* | `""` | LLM API base URL (required unless using aicore provider) |
+| `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 == "" ||
-		*llmBaseURL == "" || *llmAPIKey == "" || *llmModel == "" {
+	// For aicore provider, llm-base-url and llm-api-key are not required
+	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
-	log.Printf("Sending to LLM (%s)...", *llmModel)
+	// Step 8: Call LLM (with retry on parse failure)
+	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)
-	if err != nil {
-		log.Fatalf("LLM request failed: %v", err)
-	}
-	log.Printf("LLM response received (%d bytes)", len(response))
+	var response string
+	var result *review.ReviewResult
+	for attempt := 1; attempt <= 2; attempt++ {
+		if attempt > 1 {
+			slog.Warn("retrying LLM request after parse failure", "attempt", attempt)
+			time.Sleep(time.Second)
+		}
 
-	// Step 9: Parse response
-	result, err := review.ParseResponse(response)
-	if err != nil {
-		log.Fatalf("Failed to parse LLM response: %v", err)
+		response, err = llmClient.Complete(ctx, messages)
+		if err != nil {
+			slog.Error("LLM request failed", "model", *llmModel, "error", err, "attempt", attempt)
+			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,47 +340,122 @@ func main() {
 
 	sentinel := fmt.Sprintf("<!-- review-bot:%s -->", *reviewerName)
 
-	log.Printf("Posting review (event=%s)...", event)
-	posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody)
+	// 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 {
-		log.Fatalf("Failed to post review: %v", err)
+		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
 	}
-	log.Printf("Review posted (id=%d, user=%s)", posted.ID, posted.User.Login)
 
-	// Delete stale reviews from this bot using sentinel matching
-	if *updateExisting && *reviewerName != "" {
-		reviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
+	// Map findings to inline comments for lines present in the diff
+	diffRanges := gitea.ParseDiffNewLines(diff)
+	var inlineComments []gitea.ReviewComment
+	for _, f := range result.Findings {
+		if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) {
+			inlineComments = append(inlineComments, gitea.ReviewComment{
+				Path:        f.File,
+				NewPosition: int64(f.Line),
+				Body:        fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding),
+			})
+		}
+	}
+	if len(inlineComments) > 0 {
+		slog.Debug("attaching inline comments", "count", len(inlineComments))
+	}
+
+	// --- Review update strategy ---
+	// 1. POST new review first (gets non-stale approval badge on HEAD)
+	// 2. Then supersede old review with link to the new one
+	// Order matters: post first so we have the new review's URL for the supersede message.
+	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 {
-			for _, r := range reviews {
-				if r.ID != posted.ID && r.User.Login == posted.User.Login && strings.Contains(r.Body, sentinel) {
-					if err := giteaClient.DeleteReview(ctx, owner, repoName, prNumber, r.ID); err != nil {
-						log.Printf("Warning: could not delete old review %d: %v", r.ID, err)
-					} else {
-						log.Printf("Deleted stale review %d", r.ID)
-					}
-				}
-			}
-
-			// Worst-wins: if we posted APPROVE but a sibling review from the
-			// same user (same token, different role) has REQUEST_CHANGES,
-			// delete ours and re-post as REQUEST_CHANGES to maintain the block.
-			if event == "APPROVED" && shouldEscalate(reviews, posted.ID, posted.User.Login, sentinel) {
-				log.Printf("Sibling review has REQUEST_CHANGES; escalating")
-				if err := giteaClient.DeleteReview(ctx, owner, repoName, prNumber, posted.ID); err != nil {
-					log.Printf("Warning: could not delete review for escalation: %v", err)
-				} else {
-					_, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, "REQUEST_CHANGES", reviewBody)
-					if err != nil {
-						log.Printf("Warning: could not re-post as REQUEST_CHANGES: %v", err)
-					} else {
-						log.Printf("Review escalated to REQUEST_CHANGES")
-					}
-				}
+			if hasSharedToken(existingReviews, sentinel) {
+				slog.Warn("shared token mode: skipping supersede to avoid clobbering sibling review")
+			} else {
+				oldReviews = findAllOwnReviews(existingReviews, sentinel)
 			}
 		}
 	}
+
+	// Self-request as reviewer (ensures we appear in required-reviewer checks)
+	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 {
+		slog.Error("failed to post review", "pr", prNumber, "event", event, "error", err)
+		os.Exit(1)
+	}
+	slog.Info("review posted", "review_id", posted.ID, "user", posted.User.Login, "pr", prNumber)
+
+	// Supersede all old reviews with link to the new one
+	if len(oldReviews) > 0 {
+		newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*giteaURL, "/"), owner, repoName, prNumber, posted.ID)
+		for _, oldReview := range oldReviews {
+			cid, err := giteaClient.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID)
+			if err != nil {
+				slog.Warn("could not find comment ID for old review", "review_id", oldReview.ID, "error", err)
+				continue
+			}
+			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.
@@ -296,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))
@@ -327,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]
@@ -340,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
 			}
 
@@ -438,14 +612,113 @@ func validateReviewerName(name string) error {
 	return nil
 }
 
-// shouldEscalate checks if the current APPROVED review should be escalated
-// to REQUEST_CHANGES because a sibling bot review (same user, different role)
-// already has REQUEST_CHANGES.
-func shouldEscalate(reviews []gitea.Review, postedID int64, postedLogin, ownSentinel string) bool {
+// buildSupersededBody creates the body for a superseded review: struck-through banner
+// with collapsed original content and the commit it was evaluated against.
+func buildSupersededBody(originalBody, commitSHA, newReviewURL, sentinel string) string {
+	shortSHA := commitSHA
+	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 r.ID != postedID && !r.Stale && r.User.Login == postedLogin && r.State == "REQUEST_CHANGES" && strings.Contains(r.Body, "<!-- review-bot:") && !strings.Contains(r.Body, ownSentinel) {
+		if strings.Contains(r.Body, ownSentinel) {
+			ownLogin = r.User.Login
+			break
+		}
+	}
+	if ownLogin == "" {
+		return false
+	}
+	for _, r := range reviews {
+		if r.User.Login == ownLogin && strings.Contains(r.Body, "<!-- review-bot:") && !strings.Contains(r.Body, ownSentinel) {
+			slog.Warn("shared token detected — another review-bot role is using the same Gitea user",
+				"sibling_role", extractSentinelName(r.Body), "user", ownLogin)
 			return true
 		}
 	}
 	return false
 }
+
+// extractSentinelName pulls the reviewer name from a sentinel comment.
+func extractSentinelName(body string) string {
+	const prefix = "<!-- review-bot:"
+	const suffix = " -->"
+	idx := strings.Index(body, prefix)
+	if idx < 0 {
+		return "unknown"
+	}
+	rest := body[idx+len(prefix):]
+	end := strings.Index(rest, suffix)
+	if end < 0 {
+		return "unknown"
+	}
+	return rest[:end]
+}
+
+// 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) {
+			continue
+		}
+		if strings.Contains(reviews[i].Body, "~~Original review~~") {
+			continue
+		}
+		if best == nil || reviews[i].ID > best.ID {
+			best = &reviews[i]
+		}
+	}
+	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,100 +56,858 @@ func makeReview(id int64, login, state string, stale bool, body string) gitea.Re
 	return r
 }
 
-func TestShouldEscalate(t *testing.T) {
+
+func TestBuildSupersededBody(t *testing.T) {
+	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"
+
+	result := buildSupersededBody(original, "abcdef1234567890", newURL, sentinel)
+
+	// Should contain the struck-through banner
+	if !strings.Contains(result, "~~Original review~~") {
+		t.Error("missing struck-through banner")
+	}
+	// Should contain superseded notice with link
+	if !strings.Contains(result, "**Superseded**") {
+		t.Error("missing superseded notice")
+	}
+	if !strings.Contains(result, "[see current review]("+newURL+")") {
+		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 TestBuildSupersededBodyShortSHA(t *testing.T) {
+	// Short SHA should pass through without panic
+	result := buildSupersededBody("body", "abc", "https://example.com/review", "<!-- review-bot:x -->")
+	if !strings.Contains(result, "abc") {
+		t.Error("short SHA not preserved")
+	}
+}
+
+func TestFindOwnReview(t *testing.T) {
 	tests := []struct {
-		name        string
-		reviews     []gitea.Review
-		postedID    int64
-		postedLogin string
-		ownSentinel string
-		want        bool
+		name     string
+		reviews  []gitea.Review
+		sentinel string
+		wantID   int64
+		wantNil  bool
 	}{
 		{
-			name:        "no reviews",
-			reviews:     nil,
-			postedID:    100,
-			postedLogin: "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
+			name:     "no reviews",
+			reviews:  nil,
+			sentinel: "<!-- review-bot:sonnet -->",
+			wantNil:  true,
 		},
 		{
-			name: "sibling same user has REQUEST_CHANGES",
+			name: "found by sentinel",
 			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", false, "bad\n<!-- review-bot:security -->"),
+				makeReview(42, "bot", "APPROVED", false, "review body\n<!-- review-bot:sonnet -->"),
 			},
-			postedID:    100,
-			postedLogin: "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        true,
+			sentinel: "<!-- review-bot:sonnet -->",
+			wantID:   42,
 		},
 		{
-			name: "sibling different user has REQUEST_CHANGES (should NOT escalate)",
+			name: "wrong sentinel",
 			reviews: []gitea.Review{
-				makeReview(101, "other-bot", "REQUEST_CHANGES", false, "bad\n<!-- review-bot:gpt -->"),
+				makeReview(42, "bot", "APPROVED", false, "body\n<!-- review-bot:gpt -->"),
 			},
-			postedID:    100,
-			postedLogin: "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
+			sentinel: "<!-- review-bot:sonnet -->",
+			wantNil:  true,
 		},
 		{
-			name: "same user REQUEST_CHANGES but stale (should NOT escalate)",
+			name: "multiple reviews, returns first match",
 			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", true, "old\n<!-- review-bot:security -->"),
+				makeReview(10, "bot", "APPROVED", false, "old\n<!-- review-bot:gpt -->"),
+				makeReview(20, "bot", "APPROVED", false, "new\n<!-- review-bot:sonnet -->"),
 			},
-			postedID:    100,
-			postedLogin: "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
+			sentinel: "<!-- review-bot:sonnet -->",
+			wantID:   20,
 		},
 		{
-			name: "same user same sentinel (own stale review, should NOT escalate)",
+			name: "skips superseded review",
 			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", false, "old\n<!-- review-bot:sonnet -->"),
+				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 -->"),
 			},
-			postedID:    100,
-			postedLogin: "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
+			sentinel: "<!-- review-bot:sonnet -->",
+			wantID:   20,
 		},
 		{
-			name: "same user APPROVED sibling (should NOT escalate)",
+			name: "only superseded reviews exist",
 			reviews: []gitea.Review{
-				makeReview(101, "bot", "APPROVED", false, "good\n<!-- review-bot:security -->"),
+				makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n<!-- review-bot:sonnet -->"),
 			},
-			postedID:    100,
-			postedLogin: "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
+			sentinel: "<!-- review-bot:sonnet -->",
+			wantNil:  true,
 		},
 		{
-			name: "human REQUEST_CHANGES no sentinel (should NOT escalate)",
+			name: "picks highest ID among matches",
 			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", false, "please fix this"),
+				makeReview(50, "bot", "APPROVED", false, "v1\n<!-- review-bot:sonnet -->"),
+				makeReview(30, "bot", "APPROVED", false, "v0\n<!-- review-bot:sonnet -->"),
 			},
-			postedID:    100,
-			postedLogin: "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,
-			postedLogin: "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
+			sentinel: "<!-- review-bot:sonnet -->",
+			wantID:   50,
 		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			got := shouldEscalate(tc.reviews, tc.postedID, tc.postedLogin, tc.ownSentinel)
-			if got != tc.want {
-				t.Errorf("shouldEscalate() = %v, want %v", got, tc.want)
+			got := findOwnReview(tc.reviews, tc.sentinel)
+			if tc.wantNil {
+				if got != nil {
+					t.Errorf("findOwnReview() = %v, want nil", got)
+				}
+			} else {
+				if got == nil {
+					t.Fatal("findOwnReview() = nil, want non-nil")
+				}
+				if got.ID != tc.wantID {
+					t.Errorf("findOwnReview().ID = %d, want %d", got.ID, tc.wantID)
+				}
+			}
+		})
+	}
+}
+
+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)
 			}
 		})
 	}

docs/REVIEW_STRATEGY.md

@@ -0,0 +1,97 @@
+# Review Update Strategy
+
+review-bot uses an **edit-in-place** strategy for updating reviews. Reviews are never deleted — this preserves conversation threads on inline comments.
+
+## State Transition Diagram
+
+```mermaid
+stateDiagram-v2
+    [*] --> NoExistingReview: First run
+
+    NoExistingReview --> POST_Review: Generate findings + event
+    POST_Review --> PostEscalationCheck: event == APPROVED?
+
+    PostEscalationCheck --> Done: No sibling blocks
+    PostEscalationCheck --> Supersede_And_Repost: Sibling has REQUEST_CHANGES
+    Supersede_And_Repost --> Done: Posted as REQUEST_CHANGES
+
+    [*] --> ExistingReviewFound: Subsequent run (sentinel match)
+
+    ExistingReviewFound --> CheckEscalation: Determine final event
+    CheckEscalation --> CompareState: Apply worst-wins if needed
+
+    CompareState --> SameState: existing.state == new event
+    CompareState --> StateChange: existing.state != new event
+
+    SameState --> Skip: Body unchanged
+    SameState --> PatchBody: Body changed → PATCH in place
+
+    StateChange --> Escalate: APPROVED → REQUEST_CHANGES
+    StateChange --> Downgrade: REQUEST_CHANGES → APPROVED
+
+    Escalate --> Supersede: PATCH old body → "Superseded"
+    Supersede --> POST_New_RC: POST new REQUEST_CHANGES
+
+    Downgrade --> POST_New_Approve: POST new APPROVED (old stays intact)
+
+    Skip --> Done
+    PatchBody --> Done
+    POST_New_RC --> Done
+    POST_New_Approve --> Done
+```
+
+## Rules
+
+| Scenario | Action | Reason |
+|----------|--------|--------|
+| No existing review | POST new | First run |
+| Same state, same body | Skip | Nothing changed — preserve threads |
+| Same state, body changed | PATCH body | Update findings without losing threads |
+| APPROVED → REQUEST_CHANGES | Supersede old + POST new | Can always escalate; old APPROVED is no longer valid |
+| REQUEST_CHANGES → APPROVED | POST new APPROVED | Can't edit state; old REQUEST_CHANGES stays as historical record |
+| Sibling has REQUEST_CHANGES (worst-wins) | Escalate to REQUEST_CHANGES | PR must stay blocked if ANY reviewer blocks |
+
+## Key Constraints
+
+1. **Review state is immutable after POST** — Gitea has no API to change APPROVED ↔ REQUEST_CHANGES
+2. **Never delete reviews** — Deleting cascades to inline comments and reply threads
+3. **"Last review per user" wins** — Gitea uses the most recent review from a user for merge decisions
+4. **REQUEST_CHANGES reviews are never touched** — Their inline comments and threads are preserved as historical record
+5. **APPROVED reviews can be superseded** — When escalation is needed, mark old as superseded and POST new
+
+## Worst-Wins (Shared Token)
+
+When multiple reviewer roles share a token (e.g., `sonnet` and `security` both use `sonnet-review-bot`):
+
+```
+CI Matrix Run:
+  sonnet  → REQUEST_CHANGES (findings)
+  security → APPROVED (no security issues)
+                ↓
+  security sees sibling REQUEST_CHANGES
+                ↓
+  security escalates → REQUEST_CHANGES
+                ↓
+  PR stays blocked ✓
+```
+
+The **first-run case** (no existing review to read login from) uses a post-posting fallback:
+POST APPROVED → check siblings → if blocked, supersede own APPROVED → re-POST as REQUEST_CHANGES.
+
+## Edit Mechanism
+
+Reviews are edited via `PATCH /repos/{owner}/{repo}/issues/comments/{id}`:
+
+- **Review body**: ID obtained from the timeline API (`/issues/{index}/timeline`, type `"review"`)
+- **Inline comments**: IDs obtained from `/pulls/{index}/reviews/{id}/comments`
+- **Both are editable** by the token that created them
+- **ListReviews always returns the original body** (reads from review table, not comment table) — sentinel matching works regardless of edits
+
+## Inline Comments Lifecycle
+
+| Event | Inline comments behavior |
+|-------|--------------------------|
+| First POST | Created on specific diff lines |
+| PATCH body (same state) | Unchanged — still current findings |
+| Supersede (state change) | Old inline comments stay (readable but on outdated code) |
+| New POST after supersede | Fresh inline comments on current diff |

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 {
@@ -57,9 +80,17 @@ type ChangedFile struct {
 	Status   string `json:"status"`
 }
 
+// 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"`
+}
+
 // GetPullRequest fetches PR metadata.
 func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) {
-	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, owner, repo, number)
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
 	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return nil, fmt.Errorf("fetch PR: %w", err)
@@ -73,7 +104,7 @@ func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number
 
 // GetPullRequestDiff fetches the unified diff for a PR.
 func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
-	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.baseURL, owner, repo, number)
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
 	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return "", fmt.Errorf("fetch diff: %w", err)
@@ -83,7 +114,7 @@ func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, num
 
 // GetPullRequestFiles fetches the list of files changed in a PR.
 func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) {
-	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.baseURL, owner, repo, number)
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
 	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return nil, fmt.Errorf("fetch PR files: %w", err)
@@ -97,7 +128,7 @@ func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, nu
 
 // GetCommitStatuses fetches CI statuses for a commit SHA.
 func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) {
-	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.baseURL, owner, repo, sha)
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(sha))
 	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return nil, fmt.Errorf("fetch commit statuses: %w", err)
@@ -111,7 +142,7 @@ func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string)
 
 // GetFileContent fetches a file from the default branch of a repo.
 func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
-	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, owner, repo, escapePath(filepath))
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(filepath))
 	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return "", fmt.Errorf("fetch file %s: %w", filepath, err)
@@ -121,7 +152,7 @@ func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath strin
 
 // GetFileContentRef fetches a file from a specific ref (branch/tag/sha) in a repo.
 func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
-	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s?ref=%s", c.baseURL, owner, repo, escapePath(filepath), url.QueryEscape(ref))
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s?ref=%s", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(filepath), url.QueryEscape(ref))
 	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err)
@@ -131,15 +162,18 @@ func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, r
 
 // PostReview submits a review to a PR and returns the created review.
 // event should be "APPROVED" or "REQUEST_CHANGES".
-func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string) (*Review, error) {
-	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.baseURL, owner, repo, number)
+// comments are optional inline comments attached to specific lines.
+func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string, comments []ReviewComment) (*Review, error) {
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
 
 	payload := struct {
-		Body  string `json:"body"`
-		Event string `json:"event"`
+		Body     string          `json:"body"`
+		Event    string          `json:"event"`
+		Comments []ReviewComment `json:"comments,omitempty"`
 	}{
-		Body:  body,
-		Event: event,
+		Body:     body,
+		Event:    event,
+		Comments: comments,
 	}
 
 	data, err := json.Marshal(payload)
@@ -191,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)
 }
@@ -220,9 +254,9 @@ type ContentEntry struct {
 func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
 	var reqURL string
 	if path == "" {
-		reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents", c.baseURL, owner, repo)
+		reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents", c.baseURL, url.PathEscape(owner), url.PathEscape(repo))
 	} else {
-		reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents/%s", c.baseURL, owner, repo, escapePath(path))
+		reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents/%s", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(path))
 	}
 	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
@@ -244,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
@@ -258,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 {
@@ -278,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"`
-	Body string `json:"body"`
-	User struct {
+	ID       int64  `json:"id"`
+	Body     string `json:"body"`
+	User     struct {
 		Login string `json:"login"`
 	} `json:"user"`
-	State string `json:"state"`
-	Stale bool   `json:"stale"`
+	State    string `json:"state"`
+	Stale    bool   `json:"stale"`
+	CommitID string `json:"commit_id"`
 }
 
 // ListReviews returns all reviews on a pull request.
@@ -343,3 +383,253 @@ func (c *Client) DeleteReview(ctx context.Context, owner, repo string, number in
 	}
 	return nil
 }
+
+// TimelineEvent represents an entry from the issue timeline API.
+type TimelineEvent struct {
+	ID   int64  `json:"id"`
+	Type string `json:"type"`
+	Body string `json:"body"`
+	User struct {
+		Login string `json:"login"`
+	} `json:"user"`
+}
+
+// GetTimelineReviewCommentID finds the comment ID for a review body by
+// scanning the issue timeline for a review event containing the sentinel.
+func (c *Client) GetTimelineReviewCommentID(ctx context.Context, owner, repo string, number int, sentinel string) (int64, error) {
+	const pageSize = 50
+	for page := 1; ; page++ {
+		reqURL := 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)
+		body, err := c.doGet(ctx, reqURL)
+		if err != nil {
+			return 0, fmt.Errorf("get timeline (page %d): %w", page, err)
+		}
+		var events []TimelineEvent
+		if err := json.Unmarshal(body, &events); err != nil {
+			return 0, fmt.Errorf("parse timeline (page %d): %w", page, err)
+		}
+		for _, ev := range events {
+			if ev.Type == "review" && strings.Contains(ev.Body, sentinel) {
+				return ev.ID, nil
+			}
+		}
+		if len(events) < pageSize {
+			break
+		}
+	}
+	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",
+		c.baseURL,
+		url.PathEscape(owner),
+		url.PathEscape(repo),
+		commentID)
+
+	payload := struct {
+		Body string `json:"body"`
+	}{Body: newBody}
+	data, err := json.Marshal(payload)
+	if err != nil {
+		return fmt.Errorf("marshal edit payload: %w", err)
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodPatch, reqURL, bytes.NewReader(data))
+	if err != nil {
+		return fmt.Errorf("create edit 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("edit comment: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		body, _ := io.ReadAll(resp.Body)
+		return fmt.Errorf("edit comment failed (status %d): %s", resp.StatusCode, body)
+	}
+	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"
 )
 
@@ -128,7 +131,7 @@ func TestPostReview(t *testing.T) {
 	defer server.Close()
 
 	client := NewClient(server.URL, "test-token")
-	review, err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM")
+	review, err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM", nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -175,7 +178,7 @@ func TestPostReview_Non200(t *testing.T) {
 	defer server.Close()
 
 	client := NewClient(server.URL, "test-token")
-	_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test")
+	_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test", nil)
 	if err == nil {
 		t.Fatal("expected error for 403, got nil")
 	}
@@ -426,3 +429,317 @@ func TestDeleteReview_Forbidden(t *testing.T) {
 		t.Fatal("expected error for 403, got nil")
 	}
 }
+
+func TestEditComment(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPatch {
+			t.Errorf("expected PATCH, got %s", r.Method)
+		}
+		if r.URL.Path != "/api/v1/repos/owner/repo/issues/comments/42" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+		}
+
+		var payload struct {
+			Body string `json:"body"`
+		}
+		json.NewDecoder(r.Body).Decode(&payload)
+		if payload.Body != "updated body" {
+			t.Errorf("unexpected body: %s", payload.Body)
+		}
+
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{"id": 42, "body": "updated body"}`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	err := client.EditComment(context.Background(), "owner", "repo", 42, "updated body")
+	if err != nil {
+		t.Fatalf("EditComment() error = %v", err)
+	}
+}
+
+func TestEditComment_Forbidden(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+		w.Write([]byte(`{"message": "not allowed"}`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	err := client.EditComment(context.Background(), "owner", "repo", 42, "new body")
+	if err == nil {
+		t.Fatal("expected error for 403 response")
+	}
+}
+
+func TestGetTimelineReviewCommentID(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/api/v1/repos/owner/repo/issues/5/timeline" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+		}
+		w.Write([]byte(`[
+			{"id": 100, "type": "comment", "body": "random"},
+			{"id": 200, "type": "review", "body": "other review <!-- review-bot:gpt -->"},
+			{"id": 300, "type": "review", "body": "our review <!-- review-bot:sonnet -->"}
+		]`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	id, err := client.GetTimelineReviewCommentID(context.Background(), "owner", "repo", 5, "<!-- review-bot:sonnet -->")
+	if err != nil {
+		t.Fatalf("GetTimelineReviewCommentID() error = %v", err)
+	}
+	if id != 300 {
+		t.Errorf("got id=%d, want 300", id)
+	}
+}
+
+func TestGetTimelineReviewCommentID_NotFound(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte(`[{"id": 100, "type": "review", "body": "no match"}]`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	_, err := client.GetTimelineReviewCommentID(context.Background(), "owner", "repo", 5, "<!-- review-bot:sonnet -->")
+	if err == nil {
+		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")
+	}
+}

gitea/diff.go

@@ -0,0 +1,85 @@
+package gitea
+
+import (
+	"strconv"
+	"strings"
+)
+
+// DiffLineRanges maps filenames to the set of new-file line numbers present in the diff.
+type DiffLineRanges struct {
+	files map[string]map[int]bool
+}
+
+// Contains reports whether the given file+line is within the diff hunks.
+func (d *DiffLineRanges) Contains(file string, line int) bool {
+	if d == nil || d.files == nil {
+		return false
+	}
+	lines, ok := d.files[file]
+	if !ok {
+		return false
+	}
+	return lines[line]
+}
+
+// ParseDiffNewLines parses a unified diff and extracts the new-file line numbers
+// that appear in each hunk (both added and context lines).
+func ParseDiffNewLines(diff string) *DiffLineRanges {
+	result := &DiffLineRanges{files: make(map[string]map[int]bool)}
+
+	var currentFile string
+	var newLine int
+
+	for _, line := range strings.Split(diff, "\n") {
+		// Track current file from +++ header
+		if strings.HasPrefix(line, "+++ b/") {
+			currentFile = strings.TrimPrefix(line, "+++ b/")
+			if result.files[currentFile] == nil {
+				result.files[currentFile] = make(map[int]bool)
+			}
+			continue
+		}
+		if strings.HasPrefix(line, "+++ /dev/null") {
+			currentFile = ""
+			continue
+		}
+
+		// Parse hunk header: @@ -old,count +new,count @@ or @@ -old +new @@
+		if strings.HasPrefix(line, "@@") && currentFile != "" {
+			// Extract the +N part — handle both "+10,8" and "+1" forms
+			parts := strings.Split(line, "+")
+			if len(parts) >= 2 {
+				// Take everything before comma or space
+				numStr := parts[1]
+				if idx := strings.IndexAny(numStr, ", "); idx != -1 {
+					numStr = numStr[:idx]
+				}
+				n, err := strconv.Atoi(numStr)
+				if err == nil {
+					newLine = n
+				}
+			}
+			continue
+		}
+
+		if currentFile == "" {
+			continue
+		}
+
+		// Skip diff metadata lines
+		if strings.HasPrefix(line, "\\") {
+			continue
+		}
+
+		// Count lines in hunk
+		if strings.HasPrefix(line, "+") || strings.HasPrefix(line, " ") {
+			result.files[currentFile][newLine] = true
+			newLine++
+		} else if strings.HasPrefix(line, "-") {
+			// Removed lines don't advance new line counter
+			continue
+		}
+	}
+
+	return result
+}

gitea/diff_test.go

@@ -0,0 +1,115 @@
+package gitea
+
+import (
+	"testing"
+)
+
+func TestParseDiffLineRanges(t *testing.T) {
+	diff := `diff --git a/main.go b/main.go
+index abc1234..def5678 100644
+--- a/main.go
++++ b/main.go
+@@ -10,6 +10,8 @@ func main() {
+ 	fmt.Println("hello")
++	fmt.Println("new line 11")
++	fmt.Println("new line 12")
+ 	fmt.Println("existing")
+ }
+@@ -30,4 +32,5 @@ func other() {
+ 	return nil
++	// added at line 33
+ }
+diff --git a/util.go b/util.go
+new file mode 100644
+--- /dev/null
++++ b/util.go
+@@ -0,0 +1,5 @@
++package main
++
++func helper() string {
++	return "hi"
++}
+`
+
+	ranges := ParseDiffNewLines(diff)
+
+	// main.go should have lines 10-17 (first hunk) and 32-36 (second hunk)
+	if !ranges.Contains("main.go", 11) {
+		t.Error("expected main.go:11 to be in diff")
+	}
+	if !ranges.Contains("main.go", 12) {
+		t.Error("expected main.go:12 to be in diff")
+	}
+	if !ranges.Contains("main.go", 10) {
+		t.Error("expected main.go:10 to be in diff (context line)")
+	}
+	if !ranges.Contains("main.go", 33) {
+		t.Error("expected main.go:33 to be in diff")
+	}
+	if ranges.Contains("main.go", 25) {
+		t.Error("main.go:25 should NOT be in diff")
+	}
+
+	// util.go is entirely new, lines 1-5
+	if !ranges.Contains("util.go", 1) {
+		t.Error("expected util.go:1 to be in diff")
+	}
+	if !ranges.Contains("util.go", 5) {
+		t.Error("expected util.go:5 to be in diff")
+	}
+	if ranges.Contains("util.go", 6) {
+		t.Error("util.go:6 should NOT be in diff")
+	}
+
+	// Unknown file
+	if ranges.Contains("unknown.go", 1) {
+		t.Error("unknown.go should not be in diff")
+	}
+}
+
+func TestParseDiffNewLines_Empty(t *testing.T) {
+	ranges := ParseDiffNewLines("")
+	if ranges.Contains("any.go", 1) {
+		t.Error("empty diff should contain nothing")
+	}
+}
+
+func TestParseDiffNewLines_NoCommaHunk(t *testing.T) {
+	// Single-line hunks omit the comma: @@ -1 +1 @@
+	diff := `diff --git a/single.go b/single.go
+--- a/single.go
++++ b/single.go
+@@ -1 +1 @@
+-old line
++new line
+`
+	ranges := ParseDiffNewLines(diff)
+	if !ranges.Contains("single.go", 1) {
+		t.Error("expected single.go:1 to be in diff (no-comma hunk)")
+	}
+	if ranges.Contains("single.go", 2) {
+		t.Error("single.go:2 should NOT be in diff")
+	}
+}
+
+func TestParseDiffNewLines_NoNewlineMarker(t *testing.T) {
+	// "\ No newline at end of file" should not advance line counter
+	diff := `diff --git a/noeof.go b/noeof.go
+--- a/noeof.go
++++ b/noeof.go
+@@ -1,2 +1,2 @@
++line one
++line two
+\ No newline at end of file
+`
+	ranges := ParseDiffNewLines(diff)
+	if !ranges.Contains("noeof.go", 1) {
+		t.Error("expected noeof.go:1")
+	}
+	if !ranges.Contains("noeof.go", 2) {
+		t.Error("expected noeof.go:2")
+	}
+	if ranges.Contains("noeof.go", 3) {
+		t.Error("noeof.go:3 should NOT be in diff (no-newline marker)")
+	}
+}

gitea/post_review_comments_test.go

@@ -0,0 +1,88 @@
+package gitea
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestPostReview_WithComments(t *testing.T) {
+	var gotPayload struct {
+		Body     string `json:"body"`
+		Event    string `json:"event"`
+		Comments []struct {
+			Path        string `json:"path"`
+			NewPosition int64  `json:"new_position"`
+			Body        string `json:"body"`
+		} `json:"comments"`
+	}
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		json.NewDecoder(r.Body).Decode(&gotPayload)
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(200)
+		json.NewEncoder(w).Encode(map[string]any{
+			"id":   99,
+			"body": gotPayload.Body,
+			"user": map[string]any{"login": "bot"},
+		})
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	comments := []ReviewComment{
+		{Path: "main.go", NewPosition: 42, Body: "[MAJOR] Something bad"},
+		{Path: "util.go", NewPosition: 10, Body: "[MINOR] Style issue"},
+	}
+
+	_, err := client.PostReview(context.Background(), "owner", "repo", 1, "REQUEST_CHANGES", "summary", comments)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if len(gotPayload.Comments) != 2 {
+		t.Fatalf("expected 2 comments, got %d", len(gotPayload.Comments))
+	}
+	if gotPayload.Comments[0].Path != "main.go" {
+		t.Errorf("expected path main.go, got %s", gotPayload.Comments[0].Path)
+	}
+	if gotPayload.Comments[0].NewPosition != 42 {
+		t.Errorf("expected new_position 42, got %d", gotPayload.Comments[0].NewPosition)
+	}
+	if gotPayload.Comments[1].Body != "[MINOR] Style issue" {
+		t.Errorf("unexpected body: %s", gotPayload.Comments[1].Body)
+	}
+}
+
+func TestPostReview_NilComments(t *testing.T) {
+	var gotPayload map[string]any
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		json.NewDecoder(r.Body).Decode(&gotPayload)
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(200)
+		json.NewEncoder(w).Encode(map[string]any{
+			"id":   100,
+			"body": "test",
+			"user": map[string]any{"login": "bot"},
+		})
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "all good", nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// With nil comments, the field should be omitted (omitempty)
+	comments, ok := gotPayload["comments"]
+	if ok && comments != nil {
+		arr, isArr := comments.([]any)
+		if isArr && len(arr) > 0 {
+			t.Error("expected no comments in payload when nil passed")
+		}
+	}
+}

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 {
-	case ProviderAnthropic:
-		return c.completeAnthropic(ctx, messages)
-	default:
-		return c.completeOpenAI(ctx, messages)
+	var result string
+	var err error
+
+	for attempt := 0; attempt < 2; attempt++ {
+		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"`
-	MaxTokens   int               `json:"max_tokens"`
-	System      string            `json:"system,omitempty"`
-	Messages    []anthropicMsg    `json:"messages"`
-	Temperature float64           `json:"temperature,omitempty"`
+	AnthropicVersion string         `json:"anthropic_version,omitempty"`
+	Model       string         `json:"model,omitempty"`
+	MaxTokens   int            `json:"max_tokens"`
+	System      string         `json:"system,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"])
+	}
+}