fix: use bedrock-2023-05-31 for AI Core Anthropic version

SAP AI Core's Anthropic endpoint uses AWS Bedrock API format, not native Anthropic API. Verified via integration testing: - 2023-06-01: Invalid API version - bedrock-2023-05-31: Works ✓
fix: remove anthropic_version from body - AI Core rejects it
2026-05-09 23:48:21 -07:00 · 2026-05-09 23:45:11 -07:00 · 2026-05-09 23:39:22 -07:00 · 2026-05-09 23:36:42 -07:00 · 2026-05-09 23:28:22 -07:00 · 2026-05-09 23:23:42 -07:00
31 changed files with 168 additions and 4411 deletions
@@ -96,14 +96,6 @@ inputs:
    description: 'Local file with additional system prompt instructions (e.g. security review focus)'
    required: false
    default: ''
  persona:
    description: 'Built-in persona name (security, architect, docs)'
    required: false
    default: ''
  persona-file:
    description: 'Path to custom persona JSON file'
    required: false
    default: ''
 runs:
  using: 'composite'
@@ -185,8 +177,6 @@ runs:
        LLM_PROVIDER: ${{ inputs.llm-provider }}
        UPDATE_EXISTING: ${{ inputs.update-existing }}
        SYSTEM_PROMPT_FILE: ${{ inputs.system-prompt-file }}
        PERSONA: ${{ inputs.persona }}
        PERSONA_FILE: ${{ inputs.persona-file }}
        AICORE_CLIENT_ID: ${{ inputs.aicore-client-id }}
        AICORE_CLIENT_SECRET: ${{ inputs.aicore-client-secret }}
        AICORE_AUTH_URL: ${{ inputs.aicore-auth-url }}
@@ -19,9 +19,7 @@ jobs:
      - run: go build -o review-bot ./cmd/review-bot
  # Self-review using native SAP AI Core provider
-  # Models must match SAP AI Core deployments
+  # Models must match SAP AI Core deployments (use 'anthropic--' prefix for Claude)
  # 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'
@@ -38,8 +36,6 @@ jobs:
          - name: security
            token_secret: SECURITY_REVIEW_TOKEN
            model: gpt-5
            patterns_repo: rodin/security-patterns
            patterns_files: "."
            system_prompt_file: SECURITY_REVIEW.md
    steps:
      - uses: actions/checkout@v4
@@ -62,8 +58,8 @@ jobs:
          AICORE_API_URL: ${{ secrets.AICORE_API_URL }}
          AICORE_RESOURCE_GROUP: ${{ secrets.AICORE_RESOURCE_GROUP }}
          CONVENTIONS_FILE: "CONVENTIONS.md"
-          PATTERNS_REPO: ${{ matrix.patterns_repo || 'rodin/go-patterns' }}
+          PATTERNS_REPO: "rodin/go-patterns"
-          PATTERNS_FILES: ${{ matrix.patterns_files || 'README.md,patterns/' }}
+          PATTERNS_FILES: "README.md,patterns/"
          LLM_TIMEOUT: "600"
          SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }}
        run: ./review-bot
@@ -1,38 +0,0 @@
 name: PR Ready Gate
 on:
  pull_request:
    types: [synchronize]
 jobs:
  clear-labels:
    runs-on: ubuntu-24.04
    # Always run - curl commands are safe if labels don't exist
    steps:
      - name: Remove ready and self-reviewed labels, reassign to author
        env:
          GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
        run: |
          PR_NUMBER=${{ github.event.pull_request.number }}
          AUTHOR=${{ github.event.pull_request.user.login }}
          READY_LABEL_ID=38
          SELF_REVIEWED_LABEL_ID=37
          # Remove ready 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/${READY_LABEL_ID}" || true
          # 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 ready/self-reviewed labels and reassigned PR #${PR_NUMBER} to ${AUTHOR}"
@@ -2,26 +2,8 @@
 ## Language & Dependencies
 - Go standard library only — no external dependencies.
 - Target the latest stable Go release.
 - **STRICT ALLOWLIST:** Only packages listed below may be imported. No exceptions.
 ### Approved Third-Party Packages
 | Package | Use Case | Scope |
 |---------|----------|-------|
 | `github.com/goccy/go-yaml` | YAML parsing and AST inspection (subpkgs: `ast`, `parser`) | production |
 | `github.com/google/go-cmp` | Test comparisons (`cmp.Diff`) | test only |
 **Any import not in this table or the Go standard library is forbidden.**
 Transitive dependencies of approved packages are automatically allowed.
 To request a new dependency:
 1. Open a PR that ONLY updates this table
 2. Requires explicit approval from Aaron
 3. After merge, a separate PR may use the package
 *Enforcement: `scripts/check-deps.sh` parses this table — update only here.*
 ## Error Handling
@@ -1,4 +1,4 @@
-.PHONY: build test test-integration lint clean coverage check-deps precommit
+.PHONY: build test test-integration lint clean coverage
 build:
 	go build -o review-bot ./cmd/review-bot/
@@ -12,15 +12,9 @@ test-integration:
 lint:
 	go vet ./...
 check-deps:
 	@./scripts/check-deps.sh
 clean:
 	rm -f review-bot
 coverage:
 	go test -coverprofile=coverage.out ./...
 	go tool cover -func=coverage.out
 # Precommit runs all checks required before pushing
 precommit: check-deps lint test
@@ -9,7 +9,7 @@ AI-powered code review bot for Gitea pull requests. Fetches diff + context, send
 - **Smart budget**: Automatically trims context to fit model token limits
 - **Idempotent reviews**: Posts new review, then cleans up stale ones (one review per bot)
 - **Custom prompts**: Load additional instructions from a file (e.g. security-focused review)
- **Minimal dependencies**: Go stdlib + `gopkg.in/yaml.v3` only
+- **Zero dependencies**: Go stdlib only
 ## Quick Start: Composite Action
@@ -207,8 +207,6 @@ AI Core handles OAuth token management and deployment discovery automatically. M
 | `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 |
 | `system-prompt-file` | No | `""` | Local file with additional system prompt instructions |
 | `persona` | No | `""` | Built-in persona name (security, architect, docs) |
 | `persona-file` | No | `""` | Path to persona file (YAML or JSON) with custom review focus |
 | `temperature` | No | `0` | LLM temperature (0 = server default) |
 | `timeout` | No | `300` | LLM request timeout in seconds |
 | `dry-run` | No | `false` | Print review to stdout instead of posting |
@@ -329,12 +327,11 @@ All flags have environment variable equivalents:
 ### Token Scopes Required
 | Scope | Purpose |
-|-------|--------|
+|-------|---------|
 | `write:issue` | Post and delete reviews |
 | `write:repository` | Read PR diffs, file content, commit statuses |
 | `read:user` | Self-request as reviewer (optional but recommended) |
-Without `read:user`, the bot still works but cannot add itself to the PR's reviewer list.
+No `read:user` scope needed — the bot identifies itself from the review response.
 ## Development
@@ -360,116 +357,3 @@ budget/             Token estimation + context trimming
 ## License
 MIT
 ## Review Personas
 Personas provide role-based review specialization. Instead of generic code review, each persona focuses on a specific domain (security, architecture, documentation) with tailored prompts and severity calibration.
 ### Built-in Personas
 | Persona | Focus |
 |---------|-------|
 | `security` | Vulnerabilities, auth bypass, secrets exposure, injection attacks |
 | `architect` | Design patterns, code organization, API contracts, testability |
 | `docs` | Documentation quality, API clarity, error messages |
 ### Using Built-in Personas
 ```yaml
 - uses: rodin/review-bot/.gitea/actions/review@v1
  with:
    reviewer-name: security
    persona: security
    llm-model: claude-opus-4-20250514  # Security benefits from strong reasoning
    ...
 ```
 ### Multiple Personas in Parallel
 ```yaml
 jobs:
  review:
    strategy:
      matrix:
        include:
          - name: security
            persona: security
          - name: architect
            persona: architect
    steps:
      - uses: rodin/review-bot/.gitea/actions/review@v1
        with:
          reviewer-name: ${{ matrix.name }}
          persona: ${{ matrix.persona }}
          ...
 ```
 Each persona posts independently with its own sentinel, so reviews don't interfere.
 ### Custom Personas
 Create a YAML file with your domain-specific review focus:
 ```yaml
 # .review/personas/trading.yaml
 name: trading
 display_name: Trading Domain Expert
 identity: |
  You are a trading systems expert reviewing code for correctness.
  Your expertise:
  - Order lifecycle and state machines
  - Fill handling and partial fills
  - Position tracking and P&L calculations
  - Event sourcing invariants
 focus:
  - Order state machine correctness
  - Fill handling edge cases (partial, overfill)
  - Position and P&L calculation accuracy
  - Event replay determinism
  - Decimal precision for money
 ignore:
  - Code style
  - General performance
  - Documentation formatting
 severity:
  major: "Bugs that cause incorrect positions, fills, or money calculations"
  minor: "Edge cases that could cause issues under unusual conditions"
  nit: "Clarity improvements for domain logic"
 ```
 Use it in CI:
 ```yaml
 - uses: rodin/review-bot/.gitea/actions/review@v1
  with:
    reviewer-name: trading
    persona-file: .review/personas/trading.yaml
    ...
 ```
 YAML is the recommended format for personas because it supports:
 - Multi-line strings with `|` blocks (cleaner identity definitions)
 - Comments for documentation
 - More readable arrays and nested structures
 JSON is also supported for backwards compatibility—just use `.json` extension.
 ### Persona vs system-prompt-file
 | Feature | `persona` / `persona-file` | `system-prompt-file` |
 |---------|---------------------------|----------------------|
 | Replaces base prompt | Yes | No (appends) |
 | Structured format | Yes (YAML/JSON) | No (freeform) |
 | Focus/ignore lists | Yes | Manual |
 | Severity calibration | Yes | Manual |
 | Header display name | Yes | No |
 | Built-in options | Yes | No |
 Use personas for domain-specialized reviews. Use `system-prompt-file` for minor tweaks to the generic review.
@@ -0,0 +1,19 @@
 ## Self-Review: feat/aicore-provider — 2026-05-09
 ### Verdict: PASS
 No blocking issues found — ready for human review.
 #### Notes (informational, not blocking)
 **[fit]** `staticcheck` reports:
 - `llm/aicore.go:237` and `llm/client.go:231`: struct literal conversion style (S1016) — minor style nit, existing in both old and new code
 - `gitea/diff.go:78`: HasPrefix return ignored (SA4017) — pre-existing, not introduced by this PR
 - `cmd/review-bot/main_test.go:347`: nil Context (SA1012) — pre-existing, not introduced by this PR
 **[fit]** Body length validation: `aicore.go` does not include the Content-Length vs body length validation that `doRequest` has in `client.go`. This is acceptable because:
 1. AI Core uses OAuth tokens which are short-lived, so truncation is less likely
 2. The retry logic still applies via "read response" error pattern
 3. Adding complexity to aicore.go for an edge case that hasn't manifested is premature
 **[completeness]** Tests pass (go test ./...), go vet clean, no uncommitted changes.
@@ -65,13 +65,11 @@ func main() {
 	conventionsFile := flag.String("conventions-file", envOrDefault("CONVENTIONS_FILE", ""), "Conventions file path in repo (e.g. CLAUDE.md)")
 	systemPromptFile := flag.String("system-prompt-file", envOrDefault("SYSTEM_PROMPT_FILE", ""), "Local file with additional system prompt instructions")
 	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", ""), "Comma-separated file paths to fetch from patterns repo (empty = all files)")
+	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")
 	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, anthropic, or aicore")
 	personaName := flag.String("persona", envOrDefault("PERSONA", ""), "Built-in persona name (security, architect, docs)")
 	personaFile := flag.String("persona-file", envOrDefault("PERSONA_FILE", ""), "Path to persona JSON file")
 	// 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)")
@@ -109,14 +107,6 @@ func main() {
 		os.Exit(1)
 	}
 	// Validate persona flags are mutually exclusive
 	if *personaName != "" && *personaFile != "" {
 		slog.Error("--persona and --persona-file are mutually exclusive")
 		os.Exit(1)
 	}
 	// NOTE: Persona loading deferred until after Gitea client init to support repo personas
 	// Validate reviewer-name: only safe characters allowed in sentinel
 	if err := validateReviewerName(*reviewerName); err != nil {
 		slog.Error("invalid reviewer name", "error", err)
@@ -173,43 +163,6 @@ func main() {
 	ctx, cancel := context.WithTimeout(context.Background(), overallTimeout)
 	defer cancel()
 	// Load persona if specified (after Gitea client init to support repo personas)
 	var persona *review.Persona
 	if *personaName != "" {
 		// Try loading from repo first, then fall back to built-in
 		repoPersonas, err := review.LoadRepoPersonas(ctx, newGiteaClientAdapter(giteaClient), owner, repoName)
 		if err != nil {
 			slog.Warn("could not load repo personas", "repo", owner+"/"+repoName, "error", err)
 			// Continue with built-in personas only.
 			// NOTE: repoPersonas is nil here, but map indexing on a nil map is safe in Go
 			// (returns the zero value), so the fallback to built-in below works correctly.
 		}
 		if p, ok := repoPersonas[*personaName]; ok {
 			persona = p
 			slog.Info("loaded repo persona", "persona", persona.Name, "display", persona.DisplayName, "repo", owner+"/"+repoName)
 		} else {
 			// Fall back to built-in
 			persona, err = review.LoadBuiltinPersona(*personaName)
 			if err != nil {
 				slog.Error("failed to load persona", "persona", *personaName, "error", err)
 				os.Exit(1)
 			}
 			slog.Info("loaded built-in persona", "persona", persona.Name, "display", persona.DisplayName)
 		}
 	} else if *personaFile != "" {
 		resolvedPath, err := validateWorkspacePath(*personaFile, "persona-file")
 		if err != nil {
 			slog.Error("invalid persona-file path", "error", err)
 			os.Exit(1)
 		}
 		persona, err = review.LoadPersona(resolvedPath)
 		if err != nil {
 			slog.Error("failed to load persona file", "file", *personaFile, "error", err)
 			os.Exit(1)
 		}
 		slog.Info("loaded persona from file", "file", *personaFile, "persona", persona.Name)
 	}
 	slog.Info("reviewing pull request", "pr", prNumber, "repo", fmt.Sprintf("%s/%s", owner, repoName))
 	// Step 1: Fetch PR metadata
@@ -273,14 +226,34 @@ func main() {
 	// Step 6b: Load additional system prompt if specified
 	additionalPrompt := ""
 	if *systemPromptFile != "" {
-		resolvedPath, err := validateWorkspacePath(*systemPromptFile, "system-prompt-file")
+		workspace := os.Getenv("GITHUB_WORKSPACE")
 		if workspace == "" {
 			workspace, _ = os.Getwd()
 		}
 		absWorkspace, err := filepath.Abs(workspace)
 		if err != nil {
-			slog.Error("invalid system-prompt-file path", "error", 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 {
 			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 {
 			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 {
 			slog.Error("system-prompt-file symlink resolves outside workspace", "resolved", resolvedPath, "workspace", absWorkspace)
 			os.Exit(1)
 		}
 		data, err := os.ReadFile(resolvedPath)
 		if err != nil {
-			slog.Error("failed to read system prompt file", "path", *systemPromptFile, "error", err)
+			slog.Error("failed to read system prompt file", "path", promptPath, "error", err)
 			os.Exit(1)
 		}
 		additionalPrompt = string(data)
@@ -288,13 +261,7 @@ func main() {
 	}
 	// Step 7: Budget-aware prompt assembly
-	var systemBase string
+	systemBase := review.BuildSystemBase()
 	if persona != nil {
 		systemBase = review.BuildPersonaSystemPrompt(persona)
 		slog.Debug("using persona system prompt", "persona", persona.Name)
 	} else {
 		systemBase = review.BuildSystemBase()
 	}
 	if additionalPrompt != "" {
 		systemBase += "\n\n## Additional Review Instructions\n\n" + additionalPrompt
 	}
@@ -351,12 +318,7 @@ func main() {
 	slog.Info("review parsed", "verdict", result.Verdict, "findings", len(result.Findings))
 	// Step 10: Format and post review
-	var reviewBody string
+	reviewBody := review.FormatMarkdown(result, *reviewerName)
 	if persona != nil && persona.DisplayName != "" {
 		reviewBody = review.FormatMarkdownWithDisplay(result, persona.DisplayName, *reviewerName)
 	} else {
 		reviewBody = review.FormatMarkdown(result, *reviewerName)
 	}
 	// Add commit footer so readers know which commit was evaluated
 	if pr.Head.Sha != "" {
@@ -378,24 +340,6 @@ func main() {
 	sentinel := fmt.Sprintf("<!-- review-bot:%s -->", *reviewerName)
 	// Stale check: verify HEAD hasn't moved since we started
 	evaluatedSHA := pr.Head.Sha
 	var currentSHA string
 	currentPR, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber)
 	if err != nil {
 		slog.Warn("could not re-fetch PR for stale check", "pr", prNumber, "error", err)
 		// currentSHA stays empty — shouldSkipStaleReview will return false
 	} else {
 		currentSHA = currentPR.Head.Sha
 	}
 	if shouldSkipStaleReview(evaluatedSHA, currentSHA) {
 		slog.Warn("HEAD moved during review — skipping stale review",
 			"evaluated", evaluatedSHA,
 			"current", currentSHA,
 			"pr", prNumber)
 		return
 	}
 	// Map findings to inline comments for lines present in the diff
 	diffRanges := gitea.ParseDiffNewLines(diff)
 	var inlineComments []gitea.ReviewComment
@@ -523,25 +467,11 @@ func fetchFileContext(ctx context.Context, client *gitea.Client, owner, repo, re
 // patternsRepo is comma-separated list of owner/name repos.
 // patternsFiles is comma-separated list of file paths or directories.
 // If a path ends with / or is a directory, all files within it are fetched recursively.
 // If patternsFiles is empty, all files from the repo root are fetched.
 func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patternsFiles string) string {
 	var sb strings.Builder
 	repos := strings.Split(patternsRepo, ",")
-
+	paths := strings.Split(patternsFiles, ",")
 	// Build the list of paths to fetch
 	var paths []string
 	if patternsFiles == "" {
 		// Empty patternsFiles means "fetch all files from repo root"
 		paths = []string{""}
 	} else {
 		for _, p := range strings.Split(patternsFiles, ",") {
 			p = strings.TrimSpace(p)
 			if p != "" {
 				paths = append(paths, p)
 			}
 		}
 	}
 	for _, repoRef := range repos {
 		if ctx.Err() != nil {
@@ -558,10 +488,12 @@ func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patt
 		}
 		owner, repo := parts[0], parts[1]
 		var repoLoadedFiles []string
 		var repoSkippedFiles []string
 		for _, path := range paths {
 			path = strings.TrimSpace(path)
 			if path == "" {
 				continue
 			}
 			files, err := client.GetAllFilesInPath(ctx, owner, repo, path)
 			if err != nil {
 				slog.Warn("could not fetch patterns", "path", path, "repo", repoRef, "error", err)
@@ -571,22 +503,11 @@ func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patt
 			for filePath, content := range files {
 				// Only include markdown and text files as patterns
 				if !isPatternFile(filePath) {
 					repoSkippedFiles = append(repoSkippedFiles, filePath)
 					continue
 				}
 				repoLoadedFiles = append(repoLoadedFiles, filePath)
 				sb.WriteString(fmt.Sprintf("### %s/%s\n\n%s\n\n", repoRef, filePath, content))
 			}
 		}
 		if len(repoLoadedFiles) > 0 {
 			slog.Info("loaded pattern files", "repo", repoRef, "count", len(repoLoadedFiles), "files", repoLoadedFiles)
 		} else {
 			slog.Warn("no pattern files loaded", "repo", repoRef, "paths", paths)
 		}
 		if len(repoSkippedFiles) > 0 {
 			slog.Debug("skipped non-pattern files", "repo", repoRef, "count", len(repoSkippedFiles), "files", repoSkippedFiles)
 		}
 	}
 	return sb.String()
 }
@@ -673,43 +594,6 @@ func validateReviewerName(name string) error {
 	return nil
 }
 // validateWorkspacePath ensures a file path is within the workspace and resolves
 // symlinks to prevent traversal attacks. Returns the resolved absolute path or
 // an error if the path is outside the workspace.
 func validateWorkspacePath(path, pathName string) (string, error) {
 	workspace := os.Getenv("GITHUB_WORKSPACE")
 	if workspace == "" {
 		workspace, _ = os.Getwd()
 	}
 	absWorkspace, err := filepath.Abs(workspace)
 	if err != nil {
 		return "", fmt.Errorf("failed to resolve workspace path: %w", err)
 	}
 	// Join and clean the path
 	fullPath := filepath.Join(absWorkspace, path)
 	fullPath = filepath.Clean(fullPath)
 	// Check path is within workspace using filepath.Rel (more robust than HasPrefix)
 	rel, err := filepath.Rel(absWorkspace, fullPath)
 	if err != nil || strings.HasPrefix(rel, "..") {
 		return "", fmt.Errorf("%s resolves outside workspace: path=%s workspace=%s", pathName, fullPath, absWorkspace)
 	}
 	// Resolve symlinks and re-validate to prevent symlink traversal
 	resolvedPath, err := filepath.EvalSymlinks(fullPath)
 	if err != nil {
 		return "", fmt.Errorf("failed to resolve %s: %w", pathName, err)
 	}
 	relResolved, err := filepath.Rel(absWorkspace, resolvedPath)
 	if err != nil || strings.HasPrefix(relResolved, "..") {
 		return "", fmt.Errorf("%s symlink resolves outside workspace: resolved=%s workspace=%s", pathName, resolvedPath, absWorkspace)
 	}
 	return resolvedPath, nil
 }
 // 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 {
@@ -807,45 +691,3 @@ func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review {
 	}
 	return result
 }
 // shouldSkipStaleReview reports whether to skip posting because HEAD moved.
 // Returns true (skip) if evaluatedSHA differs from currentSHA.
 // Returns false (don't skip) if:
 //   - SHAs match (no movement)
 //   - currentSHA is empty (re-fetch failed; prefer posting stale over failing)
 func shouldSkipStaleReview(evaluatedSHA, currentSHA string) bool {
 	if currentSHA == "" {
 		// Re-fetch failed; better to post potentially stale than fail
 		return false
 	}
 	return evaluatedSHA != currentSHA
 }
 // giteaClientAdapter adapts gitea.Client to review.GiteaClient interface.
 type giteaClientAdapter struct {
 	client *gitea.Client
 }
 func newGiteaClientAdapter(c *gitea.Client) *giteaClientAdapter {
 	return &giteaClientAdapter{client: c}
 }
 func (a *giteaClientAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) {
 	entries, err := a.client.ListContents(ctx, owner, repo, path)
 	if err != nil {
 		return nil, err
 	}
 	result := make([]review.ContentEntry, len(entries))
 	for i, e := range entries {
 		result[i] = review.ContentEntry{
 			Name: e.Name,
 			Path: e.Path,
 			Type: e.Type,
 		}
 	}
 	return result, nil
 }
 func (a *giteaClientAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
 	return a.client.GetFileContent(ctx, owner, repo, filepath)
 }
@@ -6,7 +6,6 @@ import (
 	"log/slog"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -46,114 +45,6 @@ func TestValidateReviewerName(t *testing.T) {
 	}
 }
 func TestValidateWorkspacePath(t *testing.T) {
 	// Create a temp directory as our workspace
 	tmpDir := t.TempDir()
 	// Create a valid file inside the workspace
 	validFile := filepath.Join(tmpDir, "valid.json")
 	if err := os.WriteFile(validFile, []byte("{}"), 0644); err != nil {
 		t.Fatalf("failed to create test file: %v", err)
 	}
 	// Create a subdirectory with a file
 	subDir := filepath.Join(tmpDir, "subdir")
 	if err := os.MkdirAll(subDir, 0755); err != nil {
 		t.Fatalf("failed to create subdir: %v", err)
 	}
 	nestedFile := filepath.Join(subDir, "nested.json")
 	if err := os.WriteFile(nestedFile, []byte("{}"), 0644); err != nil {
 		t.Fatalf("failed to create nested file: %v", err)
 	}
 	// Create a symlink pointing outside the workspace
 	symlinkPath := filepath.Join(tmpDir, "evil-symlink.json")
 	if err := os.Symlink("/etc/passwd", symlinkPath); err != nil {
 		t.Fatalf("failed to create symlink: %v", err)
 	}
 	// Save and restore GITHUB_WORKSPACE
 	origWorkspace := os.Getenv("GITHUB_WORKSPACE")
 	defer os.Setenv("GITHUB_WORKSPACE", origWorkspace)
 	tests := []struct {
 		name      string
 		workspace string
 		path      string
 		wantErr   bool
 		errMatch  string
 	}{
 		{
 			name:      "valid relative path",
 			workspace: tmpDir,
 			path:      "valid.json",
 			wantErr:   false,
 		},
 		{
 			name:      "valid nested path",
 			workspace: tmpDir,
 			path:      "subdir/nested.json",
 			wantErr:   false,
 		},
 		{
 			name:      "path traversal attempt",
 			workspace: tmpDir,
 			path:      "../../../etc/passwd",
 			wantErr:   true,
 			errMatch:  "resolves outside workspace",
 		},
 		{
 			name:      "absolute path normalized to workspace-relative",
 			workspace: tmpDir,
 			path:      "/etc/passwd",
 			wantErr:   true,
 			// Go 1.21+ filepath.Join normalizes absolute paths: Join("/tmp/x", "/etc/passwd")
 			// becomes "/tmp/x/etc/passwd", which is within workspace but doesn't exist.
 			errMatch: "failed to resolve",
 		},
 		{
 			name:      "nonexistent file",
 			workspace: tmpDir,
 			path:      "nonexistent.json",
 			wantErr:   true,
 			errMatch:  "failed to resolve",
 		},
 		{
 			name:      "symlink escaping workspace",
 			workspace: tmpDir,
 			path:      "evil-symlink.json",
 			wantErr:   true,
 			errMatch:  "symlink resolves outside workspace",
 		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			os.Setenv("GITHUB_WORKSPACE", tc.workspace)
 			resolved, err := validateWorkspacePath(tc.path, "test-file")
 			if tc.wantErr {
 				if err == nil {
 					t.Errorf("expected error for %q, got nil", tc.path)
 				} else if tc.errMatch != "" && !strings.Contains(err.Error(), tc.errMatch) {
 					t.Errorf("error %q should contain %q", err.Error(), tc.errMatch)
 				}
 			} else {
 				if err != nil {
 					t.Errorf("expected no error for %q, got %v", tc.path, err)
 				}
 				if resolved == "" {
 					t.Error("expected non-empty resolved path")
 				}
 				// Verify resolved path is within workspace
 				if !strings.HasPrefix(resolved, tc.workspace) {
 					t.Errorf("resolved path %q not within workspace %q", resolved, tc.workspace)
 				}
 			}
 		})
 	}
 }
 func makeReview(id int64, login, state string, stale bool, body string) gitea.Review {
 	r := gitea.Review{
 		ID:    id,
@@ -165,6 +56,7 @@ func makeReview(id int64, login, state string, stale bool, body string) gitea.Re
 	return r
 }
 func TestBuildSupersededBody(t *testing.T) {
 	original := "# Review\n\nLooks good.\n\n<!-- review-bot:sonnet -->"
 	sentinel := "<!-- review-bot:sonnet -->"
@@ -504,52 +396,6 @@ func TestIsPatternFile(t *testing.T) {
 	}
 }
 // TestBuildPatternPaths verifies the path-building logic for fetchPatterns.
 // Empty patternsFiles means "fetch all from root" (represented as [""]).
 func TestBuildPatternPaths(t *testing.T) {
 	buildPaths := func(patternsFiles string) []string {
 		if patternsFiles == "" {
 			return []string{""}
 		}
 		var paths []string
 		for _, p := range strings.Split(patternsFiles, ",") {
 			p = strings.TrimSpace(p)
 			if p != "" {
 				paths = append(paths, p)
 			}
 		}
 		return paths
 	}
 	tests := []struct {
 		name  string
 		input string
 		want  []string
 	}{
 		{"empty fetches root", "", []string{""}},
 		{"single file", "README.md", []string{"README.md"}},
 		{"multiple files", "README.md,PATTERNS.md", []string{"README.md", "PATTERNS.md"}},
 		{"trims whitespace", " foo.md , bar.md ", []string{"foo.md", "bar.md"}},
 		{"skips empty between commas", "foo.md,,bar.md", []string{"foo.md", "bar.md"}},
 		{"directory path", "patterns/", []string{"patterns/"}},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			got := buildPaths(tc.input)
 			if len(got) != len(tc.want) {
 				t.Errorf("buildPaths(%q) = %v, want %v", tc.input, got, tc.want)
 				return
 			}
 			for i := range got {
 				if got[i] != tc.want[i] {
 					t.Errorf("buildPaths(%q)[%d] = %q, want %q", tc.input, i, got[i], tc.want[i])
 				}
 			}
 		})
 	}
 }
 func TestEvaluateCIStatus(t *testing.T) {
 	tests := []struct {
 		name       string
@@ -780,8 +626,8 @@ func TestExtractSentinelName_EdgeCases(t *testing.T) {
 		{"<!-- review-bot:sonnet --> rest", "sonnet"},
 		{"<!-- review-bot:gpt-review --> rest", "gpt-review"},
 		{"no sentinel here", "unknown"},
-		{"<!-- review-bot:", "unknown"},                   // prefix but no suffix
+		{"<!-- review-bot:", "unknown"},       // prefix but no suffix
-		{"prefix <!-- review-bot:abc --> end", "abc"},     // embedded in text
+		{"prefix <!-- review-bot:abc --> end", "abc"}, // embedded in text
 	}
 	for _, tc := range tests {
@@ -1016,53 +862,3 @@ func TestFindAllOwnReviews(t *testing.T) {
 		}
 	}
 }
 func TestShouldSkipStaleReview(t *testing.T) {
 	tests := []struct {
 		name         string
 		evaluatedSHA string
 		currentSHA   string
 		wantSkip     bool
 	}{
 		{
 			name:         "matching SHAs",
 			evaluatedSHA: "abc123def456",
 			currentSHA:   "abc123def456",
 			wantSkip:     false,
 		},
 		{
 			name:         "different SHAs",
 			evaluatedSHA: "abc123def456",
 			currentSHA:   "xyz789abc123",
 			wantSkip:     true,
 		},
 		{
 			name:         "empty current SHA (re-fetch failed)",
 			evaluatedSHA: "abc123def456",
 			currentSHA:   "",
 			wantSkip:     false,
 		},
 		{
 			name:         "both empty (edge case)",
 			evaluatedSHA: "",
 			currentSHA:   "",
 			wantSkip:     false,
 		},
 		{
 			name:         "only current empty",
 			evaluatedSHA: "abc123",
 			currentSHA:   "",
 			wantSkip:     false,
 		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			got := shouldSkipStaleReview(tc.evaluatedSHA, tc.currentSHA)
 			if got != tc.wantSkip {
 				t.Errorf("shouldSkipStaleReview(%q, %q) = %v, want %v",
 					tc.evaluatedSHA, tc.currentSHA, got, tc.wantSkip)
 			}
 		})
 	}
 }
@@ -1,334 +0,0 @@
 # Design: Role-based Review Personas (Issue #51)
 > **Note:** This design was revised during implementation to use JSON instead of YAML
 > to maintain the repository's zero-external-dependencies convention. All persona
 > files use JSON format. See "Design Revision" section at the end for details.
 ## Problem
 Current review-bot performs generic code review. Every reviewer (regardless of `reviewer-name`) uses the same base prompt and evaluates the same concerns. This leads to:
 1. **Redundancy** — Two reviewers (e.g., GPT + Claude twins) often flag identical issues
 2. **Gaps** — Generic reviewers miss specialized concerns (security, domain logic, architecture)
 3. **Noise** — NITs about style mixed with critical security findings
 4. **No ownership** — Findings lack clear domain attribution
 ## Constraints
 - Must work with existing CLI flags and CI workflow patterns
 - Must not break backwards compatibility (existing configs still work)
 - Must integrate cleanly with the budget system (personas add to context)
 - Multiple personas running in parallel must not interfere with each other
 - Each persona must have clear scope boundaries (no duplication)
 ## Proposed Approach
 ### 1. Persona Definition
 A persona is a named review role with:
 - **Identity** — Who am I? What's my expertise?
 - **Focus** — What do I look for?
 - **Scope boundaries** — What do I explicitly NOT comment on?
 - **Severity calibration** — What counts as MAJOR/MINOR/NIT for MY domain?
 Personas are defined in JSON files that can live:
 1. In the pattern repos (shared across projects)
 2. In the target repo (project-specific personas)
 3. Inline via a new `--persona-file` flag (JSON format)
 ### 2. Persona File Format
 ```json
 # .review/personas/security.yaml
 name: security
 display_name: Security Specialist
 model_preference: opus  # optional hint for expensive analysis
 identity: |
  You are a security specialist reviewing code for vulnerabilities.
  Your expertise: OWASP Top 10, injection attacks, auth/authz, secrets management,
  event sourcing security (replay attacks, event injection).
 focus:
  - Injection attacks (SQL, command, path traversal, template)
  - Authentication and authorization gaps
  - Secrets exposure (hardcoded credentials, tokens in logs)
  - Input validation (unsanitized input, unsafe deserialization)
  - Race conditions with security implications
  - Event sourcing attack vectors
 ignore:
  - Code style and naming conventions
  - Performance (unless security-related)
  - Documentation
  - General code quality
  - Test coverage
 severity:
  critical: "Remote code execution, auth bypass, data exfiltration"
  major: "Privilege escalation, information disclosure, DoS"
  minor: "Missing rate limiting, verbose errors"
  nit: "Theoretical risk with low exploitability"
 output_format: |
  For each finding:
  - Severity: [CRITICAL|MAJOR|MINOR|NIT]
  - Attack vector: How could this be exploited?
  - Evidence: Code snippet showing the vulnerability
  - Recommendation: Specific fix
 ```
 ### 3. New CLI Flags
 ```
 --persona-file PATH      Path to persona JSON file (local or in repo)
 --persona NAME           Built-in persona name (security, architect, domain)
 ```
 Either flag sets the persona. If neither is provided, behavior is unchanged (generic review).
 ### 4. Prompt Assembly
 Current flow:
 ```
 SystemBase → Patterns → Conventions → [LLM]
 ```
 New flow with persona:
 ```
 PersonaPrompt (from YAML) → Patterns (filtered?) → Conventions → [LLM]
 ```
 The persona's identity/focus/ignore/severity sections become the system prompt, replacing the generic "You are an expert code reviewer" base.
 ### 5. Built-in Personas
 Ship with these built-in personas (loadable via `--persona NAME`):
 | Name | Focus |
 |------|-------|
 | `security` | Vulnerabilities, auth, secrets |
 | `architect` | Patterns, consistency, design |
 | `domain` | Business logic (requires repo-specific config) |
 | `docs` | Documentation, API clarity |
 Built-in personas live in `review/personas/` as embedded Go assets or YAML shipped with the binary.
 ### 6. CI Workflow Integration
 Single persona:
 ```yaml
 - uses: rodin/review-bot/.gitea/actions/review@v1
  with:
    reviewer-name: security
    persona: security
    ...
 ```
 Multiple personas (parallel jobs):
 ```yaml
 jobs:
  review:
    strategy:
      matrix:
        include:
          - name: security
            persona: security
          - name: architect
            persona: architect
    steps:
      - uses: rodin/review-bot/.gitea/actions/review@v1
        with:
          reviewer-name: ${{ matrix.name }}
          persona: ${{ matrix.persona }}
 ```
 Custom persona from repo:
 ```yaml
 - uses: rodin/review-bot/.gitea/actions/review@v1
  with:
    reviewer-name: trading
    persona-file: .review/personas/trading.yaml
 ```
 ### 7. Persona + Patterns Interaction
 Some personas benefit from filtered patterns:
 - Security → only security-related patterns
 - Architect → all patterns (structural focus)
 - Domain → domain docs, not language patterns
 For v1, keep it simple: all patterns are included regardless of persona. Future enhancement could add `patterns_filter` to persona YAML.
 ### 8. Output Format Changes
 Persona name appears in the review header:
 ```markdown
 # Security Review
 ## Summary
 No critical vulnerabilities found in this change.
 ## Findings
 | # | Severity | File | Line | Finding |
 ...
 ## Recommendation
 **APPROVE** — No security-relevant issues detected.
 ---
 *Review by security*
 <!-- review-bot:security -->
 ```
 ## State/Data Model
 ### Persona struct
 ```go
 // review/persona.go
 type Persona struct {
    Name          string   `yaml:"name"`
    DisplayName   string   `yaml:"display_name"`
    ModelPref     string   `yaml:"model_preference,omitempty"`
    Identity      string   `yaml:"identity"`
    Focus         []string `yaml:"focus"`
    Ignore        []string `yaml:"ignore"`
    Severity      Severity `yaml:"severity"`
    OutputFormat  string   `yaml:"output_format,omitempty"`
 }
 type Severity struct {
    Critical string `yaml:"critical"`
    Major    string `yaml:"major"`
    Minor    string `yaml:"minor"`
    Nit      string `yaml:"nit"`
 }
 ```
 ### Loading precedence
 1. `--persona-file PATH` → load from local file system
 2. `--persona NAME` → load from embedded built-ins
 3. Neither → use generic system prompt (current behavior)
 ## Error Cases
 | Error | Handling |
 |-------|----------|
 | Persona file not found | Fatal exit with clear message |
 | Invalid YAML in persona file | Fatal exit with parse error |
 | Both `--persona` and `--persona-file` specified | Fatal exit: mutually exclusive |
 | Unknown built-in persona name | Fatal exit with list of valid names |
 | Empty identity in persona | Warning, fall back to generic prompt |
 ## Edge Cases
 - **Empty focus list**: Valid — persona relies on identity alone
 - **Empty ignore list**: Valid — no explicit scope exclusions
 - **No severity section**: Use default MAJOR/MINOR/NIT definitions
 - **Model preference set but budget insufficient**: Ignore preference, log warning
 - **Persona file in pattern repo**: Fetch like other pattern files
 ## Testing Strategy
 ### Unit tests
 - `persona_test.go`: Parse valid/invalid YAML, validate required fields
 - `prompt_test.go`: Verify persona prompt assembly
 - Integration with budget: persona prompts count toward token limit
 ### Integration tests
 - End-to-end with `--persona security` (built-in)
 - End-to-end with `--persona-file custom.yaml`
 - Backwards compatibility: no flags = generic behavior
 ### Manual verification
 - Run security persona on a PR with obvious vulnerability
 - Verify security persona ignores style issues
 - Verify non-security persona doesn't flag security issues
 ## Implementation Phases
 ### Phase 1: Persona types and loading
 - [ ] `review/persona.go`: Persona struct + YAML parsing
 - [ ] `review/persona_test.go`: Unit tests
 - [ ] Embed built-in personas in binary
 - [ ] Compiles clean, tests pass
 ### Phase 2: Prompt generation
 - [ ] `review/prompt.go`: `BuildPersonaPrompt(p Persona) string`
 - [ ] Modify `BuildSystemBase()` to accept optional persona
 - [ ] Integrate persona prompt with budget system
 - [ ] Tests for prompt assembly
 ### Phase 3: CLI integration
 - [ ] Add `--persona` and `--persona-file` flags
 - [ ] Flag validation (mutually exclusive, valid names)
 - [ ] Load persona based on flags
 - [ ] Pass persona to prompt builder
 ### Phase 4: Action integration
 - [ ] Add `persona` and `persona-file` inputs to action.yml
 - [ ] Update README with persona examples
 - [ ] End-to-end CI test
 ### Phase 5: Built-in personas
 - [ ] `security.yaml` built-in
 - [ ] `architect.yaml` built-in
 - [ ] `docs.yaml` built-in
 - [ ] Document each persona's focus
 ## Open Questions
 1. **Persona file location in repo**: Should we support `--persona-file .review/security.yaml` where the file is fetched from the PR's repo (like conventions)? This adds complexity but enables project-specific personas without action changes.
 2. **Model preference enforcement**: If persona specifies `model_preference: opus` but the action uses a different model, should we warn? Override? Ignore? Current thinking: log warning, use the specified model (user controls model via action input).
 3. **Severity override output**: If persona defines custom severity levels (CRITICAL), should the JSON output include them, or map back to standard MAJOR/MINOR/NIT? Current thinking: keep standard output format, use severity calibration only for prompt guidance.
 ## Completion Checklist
 1. Persona struct matches YAML schema exactly?
 2. Built-in personas embedded in binary (not external files)?
 3. `--persona` and `--persona-file` are mutually exclusive?
 4. Unknown persona name produces clear error with valid options?
 5. Empty persona file fields have sensible defaults?
 6. Persona prompt integrates with budget system (token counting)?
 7. Backwards compatibility: no flags = current behavior?
 8. Review header shows persona display name?
 9. Sentinel still uses reviewer-name (not persona name)?
 10. Unit tests cover parse errors, missing fields, valid YAML?
 ## Design Review Findings (Self-Review)
 ### Finding 1: Severity Mapping
 The persona YAML allows `critical` severity, but the LLM output parser (`review/parser.go`) only accepts MAJOR/MINOR/NIT. 
 **Resolution:** Keep standard output format. Persona severity section is ONLY for calibrating the LLM's judgment (prompt guidance). Output must still use MAJOR/MINOR/NIT. Document this clearly in persona format docs.
 ### Finding 2: Embedding Built-in Personas
 Go doesn't natively embed YAML. Must use `//go:embed` directive (Go 1.16+).
 **Resolution:** Create `review/personas/` directory with YAML files and use:
 ```go
 //go:embed personas/*.yaml
 var embeddedPersonas embed.FS
 ```
 ### Finding 3: display_name vs reviewer-name
 Design says header shows "persona display name" but sentinel uses "reviewer-name". This is correct - they serve different purposes:
 - `display_name` → human-readable header ("Security Specialist Review")
 - `reviewer-name` → machine sentinel for cleanup (`<!-- review-bot:security -->`)
 When persona is used, `display_name` takes precedence for the header title, but `reviewer-name` (CLI flag) is still used for the sentinel.
 ## Design Revision: YAML with gopkg.in/yaml.v3
 **Decision:** Add `gopkg.in/yaml.v3` as a dependency.
 YAML is preferred over JSON for persona files because:
 - Multi-line strings are cleaner (no escaping quotes in identity/focus text)
 - Comments are supported for documentation
 - More human-readable for complex persona definitions
 The implementation supports both YAML (`.yaml`, `.yml`) and JSON (`.json`) for backwards compatibility, with YAML as the default for built-in personas.
@@ -1,87 +0,0 @@
 # Design: YAML Support for Persona Files (#57)
 ## Problem
 JSON is awkward for persona files that contain multi-line text (identity, severity descriptions). YAML supports cleaner multi-line strings and comments, improving readability and maintainability.
 ## Constraints
 - Backwards compatibility: existing JSON personas must continue to work
 - Security: protect against DoS via deeply nested YAML (AIKIDO-2024-10486)
 - Consistency: use `.yaml` extension (not `.yml`)
 - Library: use `github.com/goccy/go-yaml` v1.16.0+ (approved in CONVENTIONS.md); we implement custom AST-based depth/node-count checks for precise alias-aware validation
 ## Proposed Approach
 1. **Update `parsePersona`** to detect format from file extension
 2. **Add YAML parsing** with explicit depth limit (defense in depth)
 3. **Keep JSON as fallback** for files without `.yaml`/`.yml` extension
 4. **Convert built-in personas** to YAML format
 5. **Update embed directive** to include both formats
 ### File Extension Detection
 ```go
 func parsePersona(data []byte, source string) (*Persona, error) {
    isYAML := strings.HasSuffix(source, ".yaml") || strings.HasSuffix(source, ".yml")
    if isYAML {
        return parseYAML(data, source)
    }
    return parseJSON(data, source)
 }
 ```
 ### YAML Parsing with Depth Protection
 We implement a custom AST-based depth/node-count walk (`checkYAMLDepth` in
 `review/persona.go`) rather than relying on library decoder options. Key design
 decisions:
 - **Library:** `github.com/goccy/go-yaml` with `ast.Node`-based traversal
 - **Dual-map tracking:** `validated` (depth-aware short-circuit) + `visiting` (cycle detection)
 - **Node-count limit:** Conservative overcounting bounds total validation work
 - **Alias-aware depth:** Aliases increment depth and are re-checked when encountered at greater depths
 See `review/persona.go:checkYAMLDepth` for the authoritative implementation.
 ## State/Data Model
 No new state. Same `Persona` struct, just different parsing.
 ## Error Cases
 | Error | Handling |
 |-------|----------|
 | Invalid YAML syntax | Return parse error with source file |
 | Deeply nested YAML | Custom AST walk (`checkYAMLDepth`) rejects before decode |
 | Unknown extension | Fall back to JSON parsing |
 | Missing required fields | Validation rejects after parse |
 ## Edge Cases
 - File with `.json` extension but YAML content → JSON parse fails, user sees error
 - File with no extension → defaults to JSON
 - Embedded persona reference like `builtin:security` → detect by embed path (`personas/X.yaml`)
 ## Testing Strategy
 1. Unit tests for YAML parsing (valid, invalid, deeply nested)
 2. Unit tests for extension detection
 3. Integration test for built-in personas (now YAML)
 4. Backwards compat test: verify JSON still works for external files
 ## Completion Checklist
 1. [ ] `go-yaml` dependency added at v1.16.0+
 2. [ ] Extension detection uses case-insensitive comparison
 3. [ ] YAML parse errors include source file name
 4. [ ] JSON parsing still works for `.json` files
 5. [ ] Built-in personas converted to YAML with readable multi-line strings
 6. [ ] Embed directive updated to include `*.yaml`
 7. [ ] Test for deeply nested YAML rejection
 8. [ ] All existing tests pass
 ## Open Questions
 - Should we support both `.yaml` AND `.yml`? Issue says `.yaml` only for consistency, but some users expect `.yml`. **Decision:** Support both for reading, recommend `.yaml` in docs.
 - Should we add a "format" field to detect mismatched extension/content? **Decision:** No, keep it simple. Extension determines format.
@@ -11,12 +11,9 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
 	"math"
 	"net"
 	"net/http"
 	"net/url"
 	"strings"
 	"syscall"
 	"time"
 )
@@ -42,40 +39,12 @@ func IsNotFound(err error) bool {
 	return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound
 }
 // IsServerError reports whether an error is an API 5xx response.
 func IsServerError(err error) bool {
 	var apiErr *APIError
 	return errors.As(err, &apiErr) && apiErr.StatusCode >= 500 && apiErr.StatusCode < 600
 }
 // DefaultMaxDiffSize is the default maximum diff size in bytes (10 MB).
 const DefaultMaxDiffSize = 10 * 1024 * 1024
 // ErrDiffTooLarge is returned when a PR diff exceeds the configured MaxDiffSize.
 var ErrDiffTooLarge = errors.New("diff size exceeds maximum allowed size")
 // Client interacts with the Gitea API.
 // A Client is safe for concurrent use by multiple goroutines.
 type Client struct {
 	baseURL string
 	token   string
 	http    *http.Client
 	// RetryBackoff defines the delays between retry attempts.
 	// RetryBackoff[i] is the delay before attempt i+1 (after attempt i fails).
 	// If nil, defaults to {1s, 2s}. Set to shorter durations in tests.
 	//
 	// This field must be configured before the first request is made.
 	// Modifying it while requests are in flight is not safe.
 	RetryBackoff []time.Duration
 	// MaxDiffSize is the maximum number of bytes allowed when fetching a PR diff.
 	// If zero, defaults to DefaultMaxDiffSize (10 MB). Set to any negative value
 	// (or math.MaxInt64) to disable the limit.
 	//
 	// This field must be configured before the first request is made.
 	// Modifying it while requests are in flight is not safe.
 	MaxDiffSize int64
 }
 // NewClient creates a new Gitea API client.
@@ -87,12 +56,6 @@ func NewClient(baseURL, token string) *Client {
 	}
 }
 // SetHTTPClient sets the underlying HTTP client used for requests.
 // This is intended for testing to inject mock transports.
 func (c *Client) SetHTTPClient(hc *http.Client) {
 	c.http = hc
 }
 // PullRequest holds relevant PR metadata.
 type PullRequest struct {
 	Title string `json:"title"`
@@ -140,28 +103,9 @@ func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number
 }
 // GetPullRequestDiff fetches the unified diff for a PR.
 // It enforces MaxDiffSize to prevent unbounded memory allocation.
 // Returns ErrDiffTooLarge if the diff exceeds the configured limit.
 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, url.PathEscape(owner), url.PathEscape(repo), number)
-
+	body, err := c.doGet(ctx, reqURL)
 	maxSize := c.MaxDiffSize
 	if maxSize == 0 {
 		maxSize = DefaultMaxDiffSize
 	}
 	// When the limit is disabled (negative) or set to math.MaxInt64 (which
 	// would overflow the +1 detection and silently disable enforcement),
 	// use the standard unlimited doGet path.
 	if maxSize < 0 || maxSize == math.MaxInt64 {
 		body, err := c.doGet(ctx, reqURL)
 		if err != nil {
 			return "", fmt.Errorf("fetch diff: %w", err)
 		}
 		return string(body), nil
 	}
 	body, err := c.doGetLimited(ctx, reqURL, maxSize)
 	if err != nil {
 		return "", fmt.Errorf("fetch diff: %w", err)
 	}
@@ -266,218 +210,24 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
 	return &review, nil
 }
 // isTemporaryNetError reports whether err is a temporary network error worth retrying.
 // This includes connection refused, network unreachable, connection reset, and DNS
 // timeouts. It explicitly excludes permanent errors like permission denied or
 // "no such host" DNS failures.
 func isTemporaryNetError(err error) bool {
 	if err == nil {
 		return false
 	}
 	// Check for OpError and inspect the underlying syscall error.
 	// Not all OpErrors are transient — permission denied, for example, is permanent.
 	var opErr *net.OpError
 	if errors.As(err, &opErr) {
 		return isRetriableSyscallError(opErr.Err)
 	}
 	// DNS errors: only retry on timeout, not on "no such host" which is permanent.
 	var dnsErr *net.DNSError
 	if errors.As(err, &dnsErr) {
 		return dnsErr.IsTimeout
 	}
 	// Check for net.Error with Timeout() (Temporary is deprecated)
 	var netErr net.Error
 	if errors.As(err, &netErr) {
 		return netErr.Timeout()
 	}
 	return false
 }
 // isRetriableSyscallError reports whether the underlying error from a net.OpError
 // is a transient syscall error worth retrying.
 func isRetriableSyscallError(err error) bool {
 	if err == nil {
 		return false
 	}
 	// Check for syscall.Errno directly or wrapped
 	var errno syscall.Errno
 	if errors.As(err, &errno) {
 		switch errno {
 		case syscall.ECONNREFUSED, // connection refused — server not listening
 			syscall.ECONNRESET,   // connection reset by peer
 			syscall.ENETUNREACH,  // network unreachable
 			syscall.EHOSTUNREACH, // host unreachable
 			syscall.ETIMEDOUT:    // connection timed out
 			return true
 		default:
 			// EACCES, EPERM, etc. are permanent — don't retry
 			return false
 		}
 	}
 	// If we can't identify the specific syscall error, be conservative and retry.
 	// This handles wrapped errors or platform-specific error types.
 	// The retry count is limited, so erring on the side of retrying is safe.
 	return true
 }
 // redactURL strips query parameters and userinfo credentials from a URL for
 // safe logging. This prevents accidental exposure of sensitive data (tokens in
 // query strings, or user:pass in the authority) in log output.
 func redactURL(rawURL string) string {
 	parsed, err := url.Parse(rawURL)
 	if err != nil {
 		// If we cannot parse it, return a safe placeholder rather than
 		// potentially logging something sensitive.
 		return "[invalid URL]"
 	}
 	if parsed.User != nil {
 		parsed.User = url.User("REDACTED")
 	}
 	if parsed.RawQuery != "" {
 		parsed.RawQuery = "[redacted]"
 	}
 	return parsed.String()
 }
 // sanitizeErrorForLog returns a loggable version of an error that omits
 // potentially sensitive content like response bodies. For APIError, only
 // the status code is included; for other errors, the type is preserved.
 func sanitizeErrorForLog(err error) string {
 	if err == nil {
 		return "<nil>"
 	}
 	var apiErr *APIError
 	if errors.As(err, &apiErr) {
 		return fmt.Sprintf("HTTP %d", apiErr.StatusCode)
 	}
 	return err.Error()
 }
 // doGetWithReader performs an HTTP GET request with retry on 5xx errors and
 // temporary network errors. Retries up to 3 times with exponential backoff
 // (1s, 2s delays by default; configurable via Client.RetryBackoff for testing).
 // The readBody function is called with the response body on success (2xx) and
 // is responsible for reading and closing it.
 func (c *Client) doGetWithReader(ctx context.Context, reqURL string, readBody func(io.ReadCloser) ([]byte, error)) ([]byte, error) {
 	const maxAttempts = 3
 	// backoff[i] is the delay before attempt i+1 (i.e., after attempt i fails).
 	// First attempt (i=0) has no delay; retries wait 1s then 2s by default.
 	backoff := c.RetryBackoff
 	if backoff == nil {
 		backoff = []time.Duration{1 * time.Second, 2 * time.Second}
 	}
 	// maxErrorBodyBytes limits how much of an error response body we read
 	// to protect against malicious servers sending unbounded data.
 	const maxErrorBodyBytes = 64 * 1024 // 64 KB
 	var lastErr error
 	for attempt := 0; attempt < maxAttempts; attempt++ {
 		if attempt > 0 {
 			// Determine delay: use backoff slice if available, otherwise retry immediately.
 			// An empty RetryBackoff slice means "retry without delay" — this is intentional
 			// as the caller explicitly configured no delays.
 			var delay time.Duration
 			if attempt-1 < len(backoff) {
 				delay = backoff[attempt-1]
 			}
 			if delay > 0 {
 				slog.Warn("retrying request after error",
 					"attempt", attempt+1,
 					"url", redactURL(reqURL),
 					"delay", delay.String(),
 					"lastError", sanitizeErrorForLog(lastErr))
 				timer := time.NewTimer(delay)
 				select {
 				case <-timer.C:
 				case <-ctx.Done():
 					timer.Stop()
 					return nil, ctx.Err()
 				}
 			}
 		}
 		req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
 		if err != nil {
 			return nil, err
 		}
 		req.Header.Set("Authorization", "token "+c.token)
 		resp, err := c.http.Do(req)
 		if err != nil {
 			// Always capture the error for consistent return at loop end.
 			// This ensures both network errors and HTTP 5xx return lastErr.
 			lastErr = err
 			// Only retry temporary network errors when attempts remain.
 			if attempt < maxAttempts-1 && isTemporaryNetError(err) {
 				slog.Warn("temporary network error, will retry",
 					"attempt", attempt+1,
 					"url", redactURL(reqURL),
 					"error", err)
 				continue
 			}
 			// Non-retryable network error or final attempt exhausted.
 			return nil, lastErr
 		}
 		if resp.StatusCode >= 200 && resp.StatusCode < 300 {
 			return readBody(resp.Body)
 		}
 		// Error path: limit how much we read from potentially malicious server
 		errBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes))
 		resp.Body.Close()
 		lastErr = &APIError{StatusCode: resp.StatusCode, Body: string(errBody)}
 		// Only retry on 5xx server errors
 		if resp.StatusCode < 500 || resp.StatusCode >= 600 {
 			return nil, lastErr
 		}
 	}
 	return nil, lastErr
 }
 // doGet performs an HTTP GET request with retry, reading the full response body.
 func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
-	return c.doGetWithReader(ctx, reqURL, func(body io.ReadCloser) ([]byte, error) {
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
-		defer body.Close()
+	if err != nil {
-		return io.ReadAll(body)
+		return nil, err
-	})
+	}
-}
+	req.Header.Set("Authorization", "token "+c.token)
-// doGetLimited performs an HTTP GET request with retry but enforces a maximum
+	resp, err := c.http.Do(req)
-// response body size. Returns ErrDiffTooLarge if the response exceeds maxBytes.
+	if err != nil {
-// It reads maxBytes+1 (clamped to avoid overflow) to detect truncation without
+		return nil, err
-// buffering the entire body.
+	}
-func (c *Client) doGetLimited(ctx context.Context, reqURL string, maxBytes int64) ([]byte, error) {
+	defer resp.Body.Close()
-	return c.doGetWithReader(ctx, reqURL, func(body io.ReadCloser) ([]byte, error) {
+
-		defer body.Close()
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		// Read up to maxBytes+1 to detect overflow.
+		body, _ := io.ReadAll(resp.Body)
-		// Clamp to prevent integer overflow when maxBytes == math.MaxInt64.
+		return nil, &APIError{StatusCode: resp.StatusCode, Body: string(body)}
-		limitBytes := maxBytes + 1
+	}
-		if limitBytes <= 0 {
+	return io.ReadAll(resp.Body)
 			limitBytes = math.MaxInt64
 		}
 		limited := io.LimitReader(body, limitBytes)
 		data, err := io.ReadAll(limited)
 		if err != nil {
 			return nil, err
 		}
 		if int64(len(data)) > maxBytes {
 			return nil, fmt.Errorf("%w: response exceeds %d bytes", ErrDiffTooLarge, maxBytes)
 		}
 		return data, nil
 	})
 }
 // escapePath escapes each segment of a relative file path for use in URLs.
@@ -501,13 +251,7 @@ type ContentEntry struct {
 // ListContents lists files and directories at a given path in a repo.
 // Pass an empty path to list the repository root.
 // If the path points to a file (not a directory), Gitea returns a single
 // object instead of an array; this method normalizes both cases to a slice.
 func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
 	// Normalize "." to empty string — Gitea API rejects "." with 500
 	if path == "." {
 		path = ""
 	}
 	var reqURL string
 	if path == "" {
 		reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents", c.baseURL, url.PathEscape(owner), url.PathEscape(repo))
@@ -520,16 +264,7 @@ func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]
 	}
 	var entries []ContentEntry
 	if err := json.Unmarshal(body, &entries); err != nil {
-		// Gitea returns a single object (not an array) when path is a file
+		return nil, fmt.Errorf("parse contents JSON: %w", err)
 		var single ContentEntry
 		if err2 := json.Unmarshal(body, &single); err2 != nil {
 			return nil, fmt.Errorf("parse contents JSON: %w", err)
 		}
 		// Guard against empty/malformed responses
 		if single.Name == "" && single.Path == "" {
 			return nil, fmt.Errorf("parse contents JSON: empty response for path %q", path)
 		}
 		entries = []ContentEntry{single}
 	}
 	return entries, nil
 }
@@ -582,9 +317,9 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
 // Review represents a pull request review from the Gitea API.
 type Review struct {
-	ID   int64  `json:"id"`
+	ID       int64  `json:"id"`
-	Body string `json:"body"`
+	Body     string `json:"body"`
-	User struct {
+	User     struct {
 		Login string `json:"login"`
 	} `json:"user"`
 	State    string `json:"state"`
@@ -6,14 +6,10 @@ import (
 	"errors"
 	"fmt"
 	"io"
 	"net"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"sync/atomic"
 	"syscall"
 	"testing"
 	"time"
 )
 func TestGetPullRequest(t *testing.T) {
@@ -280,64 +276,11 @@ func TestListContents(t *testing.T) {
 	}
 }
 func TestListContents_DotPath(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// "." should be normalized to empty path, which hits the root contents endpoint
 		if r.URL.Path != "/api/v1/repos/owner/repo/contents" {
 			t.Errorf("expected root contents path, got: %s", r.URL.Path)
 		}
 		w.Header().Set("Content-Type", "application/json")
 		fmt.Fprintf(w, `[{"name":"README.md","path":"README.md","type":"file"}]`)
 	}))
 	defer server.Close()
 	client := NewClient(server.URL, "test-token")
 	entries, err := client.ListContents(context.Background(), "owner", "repo", ".")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 	if len(entries) != 1 {
 		t.Fatalf("expected 1 entry, got %d", len(entries))
 	}
 	if entries[0].Name != "README.md" {
 		t.Errorf("expected README.md, got %s", entries[0].Name)
 	}
 }
 func TestListContents_FilePath(t *testing.T) {
 	// Gitea returns a single object (not an array) when path is a file
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.URL.Path != "/api/v1/repos/owner/repo/contents/README.md" {
 			t.Errorf("unexpected path: %s", r.URL.Path)
 		}
 		w.Header().Set("Content-Type", "application/json")
 		// Single object, not an array
 		fmt.Fprintf(w, `{"name":"README.md","path":"README.md","type":"file"}`)
 	}))
 	defer server.Close()
 	client := NewClient(server.URL, "test-token")
 	entries, err := client.ListContents(context.Background(), "owner", "repo", "README.md")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 	if len(entries) != 1 {
 		t.Fatalf("expected 1 entry, got %d", len(entries))
 	}
 	if entries[0].Name != "README.md" {
 		t.Errorf("expected README.md, got %s", entries[0].Name)
 	}
 	if entries[0].Type != "file" {
 		t.Errorf("expected type file, got %s", entries[0].Type)
 	}
 }
 func TestGetAllFilesInPath_File(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.URL.Path == "/api/v1/repos/owner/repo/contents/README.md" {
-			// Gitea returns a single object (not array) when path is a file
+			// Gitea returns 404 for contents API on files (it's not a dir)
-			w.Header().Set("Content-Type", "application/json")
+			http.NotFound(w, r)
 			fmt.Fprintf(w, `{"name":"README.md","path":"README.md","type":"file"}`)
 			return
 		}
 		if r.URL.Path == "/api/v1/repos/owner/repo/raw/README.md" {
@@ -641,9 +584,9 @@ func TestGetAllFilesInPath_403Propagates(t *testing.T) {
 func TestIsNotFound(t *testing.T) {
 	tests := []struct {
-		name string
+		name   string
-		err  error
+		err    error
-		want bool
+		want   bool
 	}{
 		{"nil error", nil, false},
 		{"non-API error", fmt.Errorf("network timeout"), false},
@@ -800,362 +743,3 @@ func TestResolveComment_Error(t *testing.T) {
 		t.Fatal("expected error for 404 response")
 	}
 }
 func TestIsServerError(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"}, false},
 		{"500 APIError", &APIError{StatusCode: 500, Body: "server error"}, true},
 		{"502 APIError", &APIError{StatusCode: 502, Body: "bad gateway"}, true},
 		{"503 APIError", &APIError{StatusCode: 503, Body: "unavailable"}, true},
 		{"599 APIError", &APIError{StatusCode: 599, Body: "edge case"}, true},
 		{"600 not server error", &APIError{StatusCode: 600, Body: "edge"}, false},
 		{"400 not server error", &APIError{StatusCode: 400, Body: "bad request"}, false},
 		{"wrapped 500", fmt.Errorf("fetch: %w", &APIError{StatusCode: 500, Body: "err"}), true},
 		{"wrapped 404", fmt.Errorf("fetch: %w", &APIError{StatusCode: 404, Body: "err"}), false},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := IsServerError(tt.err)
 			if got != tt.want {
 				t.Errorf("IsServerError(%v) = %v, want %v", tt.err, got, tt.want)
 			}
 		})
 	}
 }
 func TestDoGet_RetriesOn500(t *testing.T) {
 	attempts := 0
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		attempts++
 		if attempts < 3 {
 			w.WriteHeader(http.StatusInternalServerError)
 			w.Write([]byte(`{"message":"transient error"}`))
 			return
 		}
 		w.WriteHeader(http.StatusOK)
 		w.Write([]byte(`{"data":"success"}`))
 	}))
 	defer server.Close()
 	client := NewClient(server.URL, "test-token")
 	// Use short backoff for fast tests
 	client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}
 	body, err := client.doGet(context.Background(), server.URL+"/test")
 	if err != nil {
 		t.Fatalf("expected success after retry, got error: %v", err)
 	}
 	if string(body) != `{"data":"success"}` {
 		t.Errorf("body = %q, want %q", string(body), `{"data":"success"}`)
 	}
 	if attempts != 3 {
 		t.Errorf("attempts = %d, want 3", attempts)
 	}
 }
 func TestDoGet_FailsAfterMaxRetries(t *testing.T) {
 	attempts := 0
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		attempts++
 		w.WriteHeader(http.StatusInternalServerError)
 		w.Write([]byte(`{"message":"persistent error"}`))
 	}))
 	defer server.Close()
 	client := NewClient(server.URL, "test-token")
 	// Use short backoff for fast tests
 	client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}
 	_, err := client.doGet(context.Background(), server.URL+"/test")
 	if err == nil {
 		t.Fatal("expected error after max retries")
 	}
 	var apiErr *APIError
 	if !errors.As(err, &apiErr) {
 		t.Fatalf("expected APIError, got: %v", err)
 	}
 	if apiErr.StatusCode != http.StatusInternalServerError {
 		t.Errorf("status = %d, want 500", apiErr.StatusCode)
 	}
 	if attempts != 3 {
 		t.Errorf("attempts = %d, want 3 (max retries)", attempts)
 	}
 }
 func TestDoGet_NoRetryOn4xx(t *testing.T) {
 	attempts := 0
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		attempts++
 		w.WriteHeader(http.StatusForbidden)
 		w.Write([]byte(`{"message":"forbidden"}`))
 	}))
 	defer server.Close()
 	client := NewClient(server.URL, "test-token")
 	_, err := client.doGet(context.Background(), server.URL+"/test")
 	if err == nil {
 		t.Fatal("expected error for 403")
 	}
 	var apiErr *APIError
 	if !errors.As(err, &apiErr) {
 		t.Fatalf("expected APIError, got: %v", err)
 	}
 	if apiErr.StatusCode != http.StatusForbidden {
 		t.Errorf("status = %d, want 403", apiErr.StatusCode)
 	}
 	if attempts != 1 {
 		t.Errorf("attempts = %d, want 1 (no retry on 4xx)", attempts)
 	}
 }
 func TestDoGet_RespectsContextCancellation(t *testing.T) {
 	attempts := 0
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		attempts++
 		w.WriteHeader(http.StatusInternalServerError)
 		w.Write([]byte(`{"message":"error"}`))
 	}))
 	defer server.Close()
 	ctx, cancel := context.WithCancel(context.Background())
 	client := NewClient(server.URL, "test-token")
 	// Use longer backoff to give us time to cancel during the wait
 	client.RetryBackoff = []time.Duration{100 * time.Millisecond, 100 * time.Millisecond}
 	// Cancel after first attempt returns and retry begins
 	go func() {
 		time.Sleep(20 * time.Millisecond)
 		cancel()
 	}()
 	_, err := client.doGet(ctx, server.URL+"/test")
 	if err == nil {
 		t.Fatal("expected error on context cancellation")
 	}
 	// Should have made 1 attempt, then context cancelled during backoff
 	if attempts != 1 {
 		t.Errorf("attempts = %d, expected 1 before context cancel during backoff", attempts)
 	}
 }
 // mockTransport is a test helper that returns errors for the first N calls,
 // then delegates to a real server.
 type mockTransport struct {
 	failCount    int32 // number of failures remaining (atomic)
 	failErr      error // error to return on failure
 	realServer   *httptest.Server
 	attemptsMade atomic.Int32 // tracks total attempts
 }
 func (m *mockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	m.attemptsMade.Add(1)
 	remaining := atomic.AddInt32(&m.failCount, -1)
 	if remaining >= 0 {
 		// Still have failures to return
 		return nil, m.failErr
 	}
 	// Redirect to real server
 	req.URL.Host = m.realServer.Listener.Addr().String()
 	req.URL.Scheme = "http"
 	return http.DefaultTransport.RoundTrip(req)
 }
 func TestDoGet_RetriesOnTemporaryNetError(t *testing.T) {
 	// Real server that will handle successful requests
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		w.Write([]byte(`{"status":"ok"}`))
 	}))
 	defer server.Close()
 	// Mock transport: fail twice with ECONNREFUSED, then succeed
 	mt := &mockTransport{
 		failCount:  2,
 		failErr:    &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED},
 		realServer: server,
 	}
 	client := NewClient("http://fake-host/", "test-token")
 	client.SetHTTPClient(&http.Client{Transport: mt})
 	client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}
 	body, err := client.doGet(context.Background(), "http://fake-host/test")
 	if err != nil {
 		t.Fatalf("expected success after retries, got error: %v", err)
 	}
 	if string(body) != `{"status":"ok"}` {
 		t.Errorf("body = %q, want %q", string(body), `{"status":"ok"}`)
 	}
 	// Should have made exactly 3 attempts: 2 failures + 1 success
 	if got := mt.attemptsMade.Load(); got != 3 {
 		t.Errorf("attempts = %d, want 3 (2 failures + 1 success)", got)
 	}
 }
 func TestIsTemporaryNetError(t *testing.T) {
 	tests := []struct {
 		name string
 		err  error
 		want bool
 	}{
 		{"nil error", nil, false},
 		{"plain error", fmt.Errorf("some error"), false},
 		// OpError with retriable syscall errors
 		{"OpError ECONNREFUSED", &net.OpError{Op: "dial", Err: syscall.ECONNREFUSED}, true},
 		{"OpError ECONNRESET", &net.OpError{Op: "read", Err: syscall.ECONNRESET}, true},
 		{"OpError ENETUNREACH", &net.OpError{Op: "dial", Err: syscall.ENETUNREACH}, true},
 		{"OpError EHOSTUNREACH", &net.OpError{Op: "dial", Err: syscall.EHOSTUNREACH}, true},
 		{"OpError ETIMEDOUT", &net.OpError{Op: "dial", Err: syscall.ETIMEDOUT}, true},
 		// OpError with permanent syscall errors — should NOT retry
 		{"OpError EACCES", &net.OpError{Op: "dial", Err: syscall.EACCES}, false},
 		{"OpError EPERM", &net.OpError{Op: "dial", Err: syscall.EPERM}, false},
 		// OpError with unknown inner error — conservative retry
 		{"OpError unknown inner", &net.OpError{Op: "dial", Err: fmt.Errorf("unknown")}, true},
 		// DNS errors
 		{"DNS timeout", &net.DNSError{IsTimeout: true}, true},
 		{"DNS no such host", &net.DNSError{IsTimeout: false, Name: "bad.host"}, false},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := isTemporaryNetError(tt.err)
 			if got != tt.want {
 				t.Errorf("isTemporaryNetError(%v) = %v, want %v", tt.err, got, tt.want)
 			}
 		})
 	}
 }
 func TestIsRetriableSyscallError(t *testing.T) {
 	tests := []struct {
 		name string
 		err  error
 		want bool
 	}{
 		{"nil", nil, false},
 		{"ECONNREFUSED", syscall.ECONNREFUSED, true},
 		{"ECONNRESET", syscall.ECONNRESET, true},
 		{"ENETUNREACH", syscall.ENETUNREACH, true},
 		{"EHOSTUNREACH", syscall.EHOSTUNREACH, true},
 		{"ETIMEDOUT", syscall.ETIMEDOUT, true},
 		{"EACCES (permanent)", syscall.EACCES, false},
 		{"EPERM (permanent)", syscall.EPERM, false},
 		{"ENOENT (permanent)", syscall.ENOENT, false},
 		{"unknown error", fmt.Errorf("something"), true}, // conservative retry
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := isRetriableSyscallError(tt.err)
 			if got != tt.want {
 				t.Errorf("isRetriableSyscallError(%v) = %v, want %v", tt.err, got, tt.want)
 			}
 		})
 	}
 }
 func TestRedactURL(t *testing.T) {
 	tests := []struct {
 		name  string
 		input string
 		want  string
 	}{
 		{
 			name:  "no query params",
 			input: "https://gitea.example.com/api/v1/repos/owner/repo/pulls/1",
 			want:  "https://gitea.example.com/api/v1/repos/owner/repo/pulls/1",
 		},
 		{
 			name:  "with query params - redacts",
 			input: "https://gitea.example.com/api/v1/repos/owner/repo/raw/file?ref=main",
 			want:  "https://gitea.example.com/api/v1/repos/owner/repo/raw/file?[redacted]",
 		},
 		{
 			name:  "multiple query params",
 			input: "https://example.com/path?token=secret&page=1",
 			want:  "https://example.com/path?[redacted]",
 		},
 		{
 			name:  "invalid URL",
 			input: "://invalid",
 			want:  "[invalid URL]",
 		},
 		{
 			name:  "empty string",
 			input: "",
 			want:  "",
 		},
 		{
 			name:  "with userinfo - redacts credentials",
 			input: "https://admin:secret@gitea.example.com/api/v1/repos",
 			want:  "https://REDACTED@gitea.example.com/api/v1/repos",
 		},
 		{
 			name:  "with userinfo and query params",
 			input: "https://user:pass@example.com/path?token=abc",
 			want:  "https://REDACTED@example.com/path?[redacted]",
 		},
 		{
 			name:  "username only - no password",
 			input: "https://user@example.com/path",
 			want:  "https://REDACTED@example.com/path",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := redactURL(tt.input)
 			if got != tt.want {
 				t.Errorf("redactURL(%q) = %q, want %q", tt.input, got, tt.want)
 			}
 		})
 	}
 }
 func TestSanitizeErrorForLog(t *testing.T) {
 	tests := []struct {
 		name string
 		err  error
 		want string
 	}{
 		{
 			name: "nil error",
 			err:  nil,
 			want: "<nil>",
 		},
 		{
 			name: "APIError omits body",
 			err:  &APIError{StatusCode: 500, Body: "internal error: database connection failed"},
 			want: "HTTP 500",
 		},
 		{
 			name: "APIError with large body still only shows status",
 			err:  &APIError{StatusCode: 502, Body: strings.Repeat("x", 1000)},
 			want: "HTTP 502",
 		},
 		{
 			name: "non-API error preserved",
 			err:  fmt.Errorf("connection refused"),
 			want: "connection refused",
 		},
 		{
 			name: "wrapped APIError",
 			err:  fmt.Errorf("request failed: %w", &APIError{StatusCode: 503, Body: "service unavailable"}),
 			want: "HTTP 503",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := sanitizeErrorForLog(tt.err)
 			if got != tt.want {
 				t.Errorf("sanitizeErrorForLog() = %q, want %q", got, tt.want)
 			}
 		})
 	}
 }
@@ -1,97 +0,0 @@
 package gitea
 import (
 	"context"
 	"errors"
 	"math"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 	"time"
 )
 func TestGetPullRequestDiff_SizeLimits(t *testing.T) {
 	tests := []struct {
 		name        string
 		diff        string
 		maxDiffSize int64
 		wantErr     error
 		wantDiff    string
 	}{
 		{
 			name:        "exceeds max size",
 			diff:        strings.Repeat("+ added line\n", 1000), // ~13 KB
 			maxDiffSize: 100,
 			wantErr:     ErrDiffTooLarge,
 		},
 		{
 			name:        "within max size",
 			diff:        "diff --git a/f.go b/f.go\n--- a/f.go\n+++ b/f.go\n@@ -1 +1 @@\n-old\n+new\n",
 			maxDiffSize: 1024,
 			wantDiff:    "diff --git a/f.go b/f.go\n--- a/f.go\n+++ b/f.go\n@@ -1 +1 @@\n-old\n+new\n",
 		},
 		{
 			name:        "exactly at limit",
 			diff:        strings.Repeat("x", 50),
 			maxDiffSize: 50,
 			wantDiff:    strings.Repeat("x", 50),
 		},
 		{
 			name:        "one byte over limit",
 			diff:        strings.Repeat("x", 51),
 			maxDiffSize: 50,
 			wantErr:     ErrDiffTooLarge,
 		},
 		{
 			name:        "disabled limit",
 			diff:        strings.Repeat("x", 10000),
 			maxDiffSize: -1,
 			wantDiff:    strings.Repeat("x", 10000),
 		},
 		{
 			name:        "math.MaxInt64 treated as disabled",
 			diff:        strings.Repeat("x", 10000),
 			maxDiffSize: math.MaxInt64,
 			wantDiff:    strings.Repeat("x", 10000),
 		},
 		{
 			name:        "default limit",
 			diff:        "diff content",
 			maxDiffSize: 0, // zero means use DefaultMaxDiffSize
 			wantDiff:    "diff content",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				w.Write([]byte(tt.diff)) //nolint:errcheck // test handler
 			}))
 			defer server.Close()
 			client := NewClient(server.URL, "test-token")
 			client.MaxDiffSize = tt.maxDiffSize
 			client.RetryBackoff = []time.Duration{}
 			got, err := client.GetPullRequestDiff(context.Background(), "owner", "repo", 1)
 			if tt.wantErr != nil {
 				if err == nil {
 					t.Fatal("expected error, got nil")
 				}
 				if !errors.Is(err, tt.wantErr) {
 					t.Errorf("expected %v, got: %v", tt.wantErr, err)
 				}
 				return
 			}
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
 			if got != tt.wantDiff {
 				t.Errorf("diff mismatch: got length %d, want length %d", len(got), len(tt.wantDiff))
 			}
 		})
 	}
 }
@@ -1,5 +1,3 @@
 module gitea.weiker.me/rodin/review-bot
 go 1.26.2
 require github.com/goccy/go-yaml v1.19.2
@@ -1,2 +0,0 @@
 github.com/goccy/go-yaml v1.19.2 h1:PmFC1S6h8ljIz6gMRBopkjP1TVT7xuwrButHID66PoM=
 github.com/goccy/go-yaml v1.19.2/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA=
@@ -17,10 +17,6 @@ import (
 // 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
@@ -32,10 +28,6 @@ type AICoreConfig struct {
 // 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
@@ -43,7 +35,12 @@ type AICoreClient struct {
 	mu          sync.RWMutex
 	token       string
 	tokenExpiry time.Time
-	deployments map[string]string // model name -> deployment URL
+	deployments map[string]deployment // model name -> deployment info
 }
 type deployment struct {
 	ID  string
 	URL string
 }
 // NewAICoreClient creates a new AI Core client with the given configuration.
@@ -52,7 +49,7 @@ func NewAICoreClient(cfg AICoreConfig) *AICoreClient {
 	return &AICoreClient{
 		config:      cfg,
 		http:        &http.Client{Timeout: 5 * time.Minute},
-		deployments: make(map[string]string),
+		deployments: make(map[string]deployment),
 	}
 }
@@ -63,15 +60,6 @@ func (c *AICoreClient) WithTimeout(d time.Duration) *AICoreClient {
 	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()
@@ -125,7 +113,7 @@ func (c *AICoreClient) fetchToken(ctx context.Context) (string, time.Time, error
 	}
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		return "", time.Time{}, fmt.Errorf("token request failed (status %d): %s", resp.StatusCode, truncateBody(body))
+		return "", time.Time{}, fmt.Errorf("token request failed (status %d): %s", resp.StatusCode, string(body))
 	}
 	var tokenResp struct {
@@ -145,48 +133,36 @@ func (c *AICoreClient) fetchToken(ctx context.Context) (string, time.Time, error
 }
 // getDeploymentURL returns the deployment URL for a model, fetching deployments if needed.
-// getDeploymentURL returns the deployment URL for a model, fetching deployments if needed.
+func (c *AICoreClient) getDeploymentURL(ctx context.Context, model string) (string, error) {
 // 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 {
+	if d, ok := c.deployments[model]; ok {
 		c.mu.RUnlock()
-		// Still need a token for the caller
+		return d.URL, nil
 		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)
+	// Fetch token first (before acquiring write lock to avoid deadlock)
-	token, err = c.getToken(ctx)
+	token, err := c.getToken(ctx)
 	if err != nil {
-		return "", "", fmt.Errorf("get token for deployments: %w", err)
+		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 {
+	if d, ok := c.deployments[model]; ok {
-		return u, token, nil
+		return d.URL, nil
 	}
 	if err := c.fetchDeployments(ctx, token); err != nil {
-		return "", "", err
+		return "", err
 	}
-	if u, ok := c.deployments[model]; ok {
+	if d, ok := c.deployments[model]; ok {
-		return u, token, nil
+		return d.URL, nil
 	}
-	return "", "", fmt.Errorf("no deployment found for model %q", model)
+	return "", fmt.Errorf("no deployment found for model %q", model)
 }
 func (c *AICoreClient) fetchDeployments(ctx context.Context, token string) error {
@@ -210,11 +186,12 @@ func (c *AICoreClient) fetchDeployments(ctx context.Context, token string) error
 	}
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		return fmt.Errorf("deployments request failed (status %d): %s", resp.StatusCode, truncateBody(body))
+		return fmt.Errorf("deployments request failed (status %d): %s", resp.StatusCode, string(body))
 	}
 	var deployResp struct {
 		Resources []struct {
 			ID            string `json:"id"`
 			DeploymentURL string `json:"deploymentUrl"`
 			Status        string `json:"status"`
 			Details       struct {
@@ -240,7 +217,10 @@ func (c *AICoreClient) fetchDeployments(ctx context.Context, token string) error
 		if modelName == "" {
 			continue
 		}
-		c.deployments[modelName] = r.DeploymentURL
+		c.deployments[modelName] = deployment{
 			ID:  r.ID,
 			URL: r.DeploymentURL,
 		}
 	}
 	return nil
@@ -248,7 +228,12 @@ func (c *AICoreClient) fetchDeployments(ctx context.Context, token string) error
 // 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)
+	deployURL, err := c.getDeploymentURL(ctx, model)
 	if err != nil {
 		return "", err
 	}
 	token, err := c.getToken(ctx)
 	if err != nil {
 		return "", err
 	}
@@ -305,7 +290,7 @@ func (c *AICoreClient) CompleteAnthropic(ctx context.Context, model string, mess
 	}
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		return "", fmt.Errorf("AI Core API error (status %d): %s", resp.StatusCode, truncateBody(body))
+		return "", fmt.Errorf("AI Core API error (status %d): %s", resp.StatusCode, string(body))
 	}
 	var anthropicResp anthropicResponse
@@ -332,7 +317,12 @@ func (c *AICoreClient) CompleteAnthropic(ctx context.Context, model string, mess
 // 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)
+	deployURL, err := c.getDeploymentURL(ctx, model)
 	if err != nil {
 		return "", err
 	}
 	token, err := c.getToken(ctx)
 	if err != nil {
 		return "", err
 	}
@@ -370,7 +360,7 @@ func (c *AICoreClient) CompleteOpenAI(ctx context.Context, model string, message
 	}
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		return "", fmt.Errorf("AI Core API error (status %d): %s", resp.StatusCode, truncateBody(body))
+		return "", fmt.Errorf("AI Core API error (status %d): %s", resp.StatusCode, string(body))
 	}
 	var openaiResp ChatResponse
@@ -142,7 +142,7 @@ func TestAICoreClient_DeploymentFetch(t *testing.T) {
 	})
 	// Should find running deployment
-	url, _, err := client.getDeploymentURL(context.Background(), "anthropic--claude-4.6-sonnet")
+	url, err := client.getDeploymentURL(context.Background(), "anthropic--claude-4.6-sonnet")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -151,7 +151,7 @@ func TestAICoreClient_DeploymentFetch(t *testing.T) {
 	}
 	// Should find running gpt-5, not stopped one
-	url, _, err = client.getDeploymentURL(context.Background(), "gpt-5")
+	url, err = client.getDeploymentURL(context.Background(), "gpt-5")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -160,14 +160,14 @@ func TestAICoreClient_DeploymentFetch(t *testing.T) {
 	}
 	// Should error on unknown model
-	_, _, err = client.getDeploymentURL(context.Background(), "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
+	// Use a pointer to capture the server URL for use in the handler
 	var baseURL string
 	mux := http.NewServeMux()
 	mux.HandleFunc("/oauth/token", func(w http.ResponseWriter, r *http.Request) {
@@ -7,37 +7,10 @@ import (
 // FormatMarkdown formats a ReviewResult into the markdown body for a Gitea review.
 func FormatMarkdown(result *ReviewResult, reviewerName string) string {
 	return FormatMarkdownWithDisplay(result, reviewerName, reviewerName)
 }
 // GiteaEvent converts the verdict to the Gitea API event string.
 func GiteaEvent(verdict string) string {
 	switch verdict {
 	case "APPROVE":
 		return "APPROVED"
 	case "REQUEST_CHANGES":
 		return "REQUEST_CHANGES"
 	default:
 		return "COMMENT"
 	}
 }
 // FormatMarkdownWithDisplay formats a ReviewResult with separate display name and sentinel name.
 // Note: displayName is not HTML-escaped as Gitea sanitizes rendered Markdown.
 // Persona display names are controlled by repo owners (trusted input).
 // displayName is used for the header title, sentinelName is used for the cleanup sentinel.
 // If displayName is empty, sentinelName is used for both.
 func FormatMarkdownWithDisplay(result *ReviewResult, displayName, sentinelName string) string {
 	var sb strings.Builder
-	// Use display name for header, or fall back to sentinel name
+	if reviewerName != "" {
-	headerName := displayName
+		title := strings.ToUpper(reviewerName[:1]) + reviewerName[1:]
 	if headerName == "" {
 		headerName = sentinelName
 	}
 	if headerName != "" {
 		title := CapitalizeFirst(headerName)
 		sb.WriteString(fmt.Sprintf("# %s Review\n\n", title))
 	}
@@ -60,11 +33,23 @@ func FormatMarkdownWithDisplay(result *ReviewResult, displayName, sentinelName s
 	sb.WriteString("## Recommendation\n\n")
 	sb.WriteString(fmt.Sprintf("**%s** — %s\n", result.Verdict, result.Recommendation))
-	if sentinelName != "" {
+	if reviewerName != "" {
-		sb.WriteString(fmt.Sprintf("\n---\n*Review by %s*\n", headerName))
+		sb.WriteString(fmt.Sprintf("\n---\n*Review by %s*\n", reviewerName))
 		// Hidden sentinel for identifying this bot's reviews during cleanup
-		sb.WriteString(fmt.Sprintf("\n<!-- review-bot:%s -->\n", sentinelName))
+		sb.WriteString(fmt.Sprintf("\n<!-- review-bot:%s -->\n", reviewerName))
 	}
 	return sb.String()
 }
 // GiteaEvent converts the verdict to the Gitea API event string.
 func GiteaEvent(verdict string) string {
 	switch verdict {
 	case "APPROVE":
 		return "APPROVED"
 	case "REQUEST_CHANGES":
 		return "REQUEST_CHANGES"
 	default:
 		return "COMMENT"
 	}
 }
@@ -159,58 +159,3 @@ func TestFormatMarkdown_RoleTitle(t *testing.T) {
 		t.Error("should not contain role title header when reviewer name is empty")
 	}
 }
 func TestFormatMarkdownWithDisplay(t *testing.T) {
 	result := &ReviewResult{
 		Verdict:        "APPROVE",
 		Summary:        "Test summary",
 		Findings:       nil,
 		Recommendation: "Test recommendation",
 	}
 	t.Run("with display name", func(t *testing.T) {
 		body := FormatMarkdownWithDisplay(result, "Security Specialist", "security")
 		// Header should use display name
 		if !strings.Contains(body, "# Security Specialist Review") {
 			t.Error("header should use display name")
 		}
 		// Sentinel should use sentinel name
 		if !strings.Contains(body, "<!-- review-bot:security -->") {
 			t.Error("sentinel should use sentinel name")
 		}
 		// Footer "Review by" should use display name
 		if !strings.Contains(body, "*Review by Security Specialist*") {
 			t.Error("footer should use display name")
 		}
 	})
 	t.Run("without display name", func(t *testing.T) {
 		body := FormatMarkdownWithDisplay(result, "", "reviewer")
 		// Should fall back to sentinel name for header
 		if !strings.Contains(body, "# Reviewer Review") {
 			t.Error("header should fall back to sentinel name")
 		}
 		if !strings.Contains(body, "<!-- review-bot:reviewer -->") {
 			t.Error("sentinel should use sentinel name")
 		}
 	})
 	t.Run("empty both names", func(t *testing.T) {
 		body := FormatMarkdownWithDisplay(result, "", "")
 		// Should not have header
 		if strings.Contains(body, "# ") && strings.Contains(body, " Review") {
 			t.Error("should not have header when both names empty")
 		}
 		// Should not have sentinel
 		if strings.Contains(body, "<!-- review-bot:") {
 			t.Error("should not have sentinel when sentinel name empty")
 		}
 	})
 }
@@ -1,367 +0,0 @@
 package review
 import (
 	"bytes"
 	"embed"
 	"encoding/json"
 	"fmt"
 	"io"
 	"os"
 	"sort"
 	"strings"
 	"unicode/utf8"
 	"github.com/goccy/go-yaml"
 	"github.com/goccy/go-yaml/ast"
 	"github.com/goccy/go-yaml/parser"
 )
 //go:embed personas/*.yaml
 var embeddedPersonas embed.FS
 // MaxPersonaFileSize is the maximum size for persona files (64 KB).
 // This prevents denial-of-service via excessively large files.
 const MaxPersonaFileSize = 64 * 1024
 // MaxYAMLDepth is the maximum nesting depth allowed in YAML persona files.
 // This prevents stack exhaustion from deeply nested structures.
 const MaxYAMLDepth = 20
 // MaxYAMLNodes is the maximum number of YAML nodes allowed in persona files.
 // This prevents DoS via wide-but-shallow structures that bypass depth limits.
 const MaxYAMLNodes = 1000
 // Persona defines a specialized review role with focused expertise.
 type Persona struct {
 	Name         string   `json:"name" yaml:"name"`
 	DisplayName  string   `json:"display_name" yaml:"display_name"`
 	ModelPref    string   `json:"model_preference,omitempty" yaml:"model_preference,omitempty"`
 	Identity     string   `json:"identity" yaml:"identity"`
 	Focus        []string `json:"focus" yaml:"focus"`
 	Ignore       []string `json:"ignore" yaml:"ignore"`
 	Severity     Severity `json:"severity" yaml:"severity"`
 	OutputFormat string   `json:"output_format,omitempty" yaml:"output_format,omitempty"`
 }
 // Severity defines what constitutes each severity level for this persona.
 // These are prompt guidance for the LLM, not output format changes.
 type Severity struct {
 	Major string `json:"major" yaml:"major"`
 	Minor string `json:"minor" yaml:"minor"`
 	Nit   string `json:"nit" yaml:"nit"`
 }
 // LoadPersona loads a persona from a JSON or YAML file path.
 // Format is detected by file extension: .yaml/.yml for YAML, .json or other for JSON.
 // Files larger than MaxPersonaFileSize are rejected.
 //
 // Symlinks are supported: os.Stat follows symlinks, so a symlink pointing to
 // a regular file will pass the IsRegular() check. Symlinks to non-regular files
 // (directories, FIFOs, devices) are still rejected.
 func LoadPersona(path string) (*Persona, error) {
 	// os.Stat follows symlinks, so symlinks to regular files are supported.
 	// The IsRegular() check operates on the target, not the symlink itself.
 	info, err := os.Stat(path)
 	if err != nil {
 		return nil, fmt.Errorf("read persona file %s: %w", path, err)
 	}
 	if !info.Mode().IsRegular() {
 		return nil, fmt.Errorf("persona file %s is not a regular file", path)
 	}
 	if info.Size() > MaxPersonaFileSize {
 		return nil, fmt.Errorf("persona file %s exceeds maximum size (%d bytes)", path, MaxPersonaFileSize)
 	}
 	data, err := os.ReadFile(path)
 	if err != nil {
 		return nil, fmt.Errorf("read persona file %s: %w", path, err)
 	}
 	// Re-check size after read to defend against TOCTOU races where file
 	// grows between stat and read (e.g., appending process, replaced file).
 	if len(data) > MaxPersonaFileSize {
 		return nil, fmt.Errorf("persona file %s exceeds maximum size (%d bytes)", path, MaxPersonaFileSize)
 	}
 	return parsePersona(data, path)
 }
 // LoadBuiltinPersona loads a built-in persona by name.
 // Returns an error if the persona doesn't exist.
 // Built-in personas are stored in YAML format only (see embed directive).
 func LoadBuiltinPersona(name string) (*Persona, error) {
 	yamlFile := name + ".yaml"
 	data, err := embeddedPersonas.ReadFile("personas/" + yamlFile)
 	if err != nil {
 		available := ListBuiltinPersonas()
 		return nil, fmt.Errorf("unknown built-in persona %q (available: %s)", name, strings.Join(available, ", "))
 	}
 	return parsePersona(data, "builtin:"+yamlFile)
 }
 // ListBuiltinPersonas returns the names of all built-in personas in sorted order.
 // Returns an empty slice if the embedded directory cannot be read.
 func ListBuiltinPersonas() []string {
 	entries, err := embeddedPersonas.ReadDir("personas")
 	if err != nil {
 		return []string{}
 	}
 	seen := make(map[string]bool)
 	for _, e := range entries {
 		if e.IsDir() {
 			continue
 		}
 		name := e.Name()
 		// Strip extension to get persona name
 		var personaName string
 		switch {
 		case strings.HasSuffix(name, ".yaml"):
 			personaName = strings.TrimSuffix(name, ".yaml")
 		case strings.HasSuffix(name, ".yml"):
 			personaName = strings.TrimSuffix(name, ".yml")
 		case strings.HasSuffix(name, ".json"):
 			personaName = strings.TrimSuffix(name, ".json")
 		default:
 			continue
 		}
 		seen[personaName] = true
 	}
 	names := make([]string, 0, len(seen))
 	for name := range seen {
 		names = append(names, name)
 	}
 	sort.Strings(names)
 	return names
 }
 // parsePersona parses persona data from JSON or YAML format.
 // Format is detected by the source file extension.
 func parsePersona(data []byte, source string) (*Persona, error) {
 	lowerSource := strings.ToLower(source)
 	isYAML := strings.HasSuffix(lowerSource, ".yaml") || strings.HasSuffix(lowerSource, ".yml")
 	var p Persona
 	var err error
 	if isYAML {
 		err = unmarshalYAMLWithDepthLimit(data, &p, MaxYAMLDepth)
 	} else {
 		// Use json.Decoder with DisallowUnknownFields for consistency with
 		// YAML's Strict() - both reject unknown fields to catch typos.
 		dec := json.NewDecoder(bytes.NewReader(data))
 		dec.DisallowUnknownFields()
 		err = dec.Decode(&p)
 		if err == nil {
 			// Reject trailing content after the first valid JSON object.
 			// Without this check, input like `{"name":"x"}garbage` would
 			// silently succeed because Decoder stops after one object.
 			var dummy json.RawMessage
 			if err2 := dec.Decode(&dummy); err2 != io.EOF {
 				err = fmt.Errorf("unexpected trailing content after JSON object")
 			}
 		}
 	}
 	if err != nil {
 		return nil, fmt.Errorf("parse persona %s: %w", source, err)
 	}
 	if err := validatePersona(&p, source); err != nil {
 		return nil, err
 	}
 	return &p, nil
 }
 // unmarshalYAMLWithDepthLimit unmarshals YAML data with three safety checks:
 //   - Depth limiting: rejects AST trees exceeding maxDepth to prevent stack exhaustion.
 //   - Multi-document rejection: prevents silent data loss from ignored extra documents.
 //   - Strict field checking: rejects unknown YAML keys to catch typos early.
 func unmarshalYAMLWithDepthLimit(data []byte, out any, maxDepth int) error {
 	// First pass: parse into AST to check depth limits, node counts, and
 	// multi-document rejection. This prevents stack exhaustion before we
 	// attempt to decode into structs.
 	file, err := parser.ParseBytes(data, 0)
 	if err != nil {
 		return err
 	}
 	// Reject empty YAML input (whitespace-only, comment-only, or truly empty files).
 	// The parser returns a single doc with nil body for these cases.
 	if len(file.Docs) == 0 || file.Docs[0].Body == nil {
 		return fmt.Errorf("empty YAML document")
 	}
 	// Reject multi-document YAML files - silently ignoring additional documents
 	// could lead to confusing behavior where users think their changes take effect.
 	if len(file.Docs) > 1 {
 		return fmt.Errorf("multi-document YAML is not supported; only single-document files are allowed")
 	}
 	nodeCount := 0
 	if err := checkYAMLDepth(file.Docs[0].Body, 0, maxDepth, MaxYAMLNodes, make(map[ast.Node]int), make(map[ast.Node]bool), &nodeCount); err != nil {
 		return err
 	}
 	// Second pass: decode with strict field checking enabled.
 	// Strict() rejects unknown keys, catching typos like "focuss" or "identiy".
 	//
 	// Safety note: goccy/go-yaml's decoder does not expand YAML aliases
 	// recursively — it resolves them via the pre-built AST, which our first
 	// pass already depth-checked. Alias chains that would exceed depth limits
 	// are caught above; the decoder merely reads the resolved scalar values.
 	dec := yaml.NewDecoder(bytes.NewReader(data), yaml.Strict())
 	return dec.Decode(out)
 }
 // checkYAMLDepth recursively checks that YAML AST nodes don't exceed the depth
 // limit or the total node count limit. It uses two tracking maps:
 //   - validated: maps each node to the maximum depth at which it was previously
 //     checked. If a node is revisited at a deeper depth (e.g., via an alias),
 //     we re-check it to ensure the combined effective depth doesn't exceed limits.
 //   - visiting: per-path recursion stack for true cycle detection. A node on the
 //     current path is a cycle (alias loop); we return nil to avoid infinite recursion.
 //
 // This design prevents the alias depth bypass where an anchored subtree validated
 // at a shallow depth could be referenced via alias at a greater depth, effectively
 // exceeding MaxYAMLDepth.
 func checkYAMLDepth(node ast.Node, depth, maxDepth, maxNodes int, validated map[ast.Node]int, visiting map[ast.Node]bool, nodeCount *int) error {
 	if node == nil {
 		return nil
 	}
 	if depth > maxDepth {
 		return fmt.Errorf("YAML nesting depth exceeds maximum (%d)", maxDepth)
 	}
 	// Cycle detection: if we're currently visiting this node on the current
 	// recursion path, it's a cycle (e.g., alias pointing to an ancestor).
 	// Return nil to break the cycle without error — cycles are a structural
 	// property, not a depth violation.
 	if visiting[node] {
 		return nil
 	}
 	// Track total nodes visited as defense-in-depth against wide-but-shallow attacks.
 	// Placed after cycle detection but before the depth-aware short-circuit. This means
 	// nodes revisited at shallower depths (via aliases) are counted each time they are
 	// encountered — intentional conservative overcounting. This bounds the total work
 	// performed during validation rather than tracking unique nodes, which is the safer
 	// security posture for untrusted YAML input.
 	*nodeCount++
 	if *nodeCount > maxNodes {
 		return fmt.Errorf("YAML node count exceeds maximum (%d)", maxNodes)
 	}
 	// Depth-aware short-circuit: skip re-validation only when the current visit
 	// depth is the same or shallower than the depth at which this node was
 	// previously validated. A shallower (or equal) current depth means the
 	// prior, deeper validation already covered any subtree depth violations.
 	// If the current depth exceeds the previous validation depth (e.g., an alias
 	// references this node deeper in the tree), we must re-traverse to ensure
 	// the combined effective depth doesn't exceed maxDepth.
 	//
 	// Note: using ast.Node (interface) as map key relies on pointer identity,
 	// which is correct because all goccy/go-yaml AST node types are pointer
 	// receivers (*MappingNode, *SequenceNode, etc.), never value types.
 	if prevDepth, ok := validated[node]; ok && depth <= prevDepth {
 		return nil
 	}
 	validated[node] = depth
 	// Mark as visiting (on the current recursion path) for cycle detection.
 	visiting[node] = true
 	defer func() { visiting[node] = false }()
 	// Walk children based on node type.
 	switch n := node.(type) {
 	case *ast.MappingNode:
 		for _, value := range n.Values {
 			if err := checkYAMLDepth(value, depth+1, maxDepth, maxNodes, validated, visiting, nodeCount); err != nil {
 				return err
 			}
 		}
 	case *ast.MappingValueNode:
 		// Both Key and Value are visited at depth+1 relative to this
 		// MappingValueNode. Since MappingNode visits its MappingValueNode
 		// children at depth+1 as well, keys and values end up at depth+2
 		// from the parent MappingNode. This is intentional: it mirrors the
 		// actual nesting structure (mapping → key-value pair → key/value).
 		if err := checkYAMLDepth(n.Key, depth+1, maxDepth, maxNodes, validated, visiting, nodeCount); err != nil {
 			return err
 		}
 		if err := checkYAMLDepth(n.Value, depth+1, maxDepth, maxNodes, validated, visiting, nodeCount); err != nil {
 			return err
 		}
 	case *ast.SequenceNode:
 		for _, value := range n.Values {
 			if err := checkYAMLDepth(value, depth+1, maxDepth, maxNodes, validated, visiting, nodeCount); err != nil {
 				return err
 			}
 		}
 	case *ast.AliasNode:
 		// Follow alias to its target, incrementing depth since aliases expand
 		// the effective structure.
 		if err := checkYAMLDepth(n.Value, depth+1, maxDepth, maxNodes, validated, visiting, nodeCount); err != nil {
 			return err
 		}
 	case *ast.AnchorNode:
 		// Increment depth for anchor values as a conservative measure: the
 		// anchor definition itself is structural, and treating it as a depth
 		// level ensures that deeply nested anchors are caught at definition
 		// time rather than only when referenced via alias. This +1 is
 		// asymmetric with alias (which also increments) — by design, the
 		// effective depth budget for anchored-then-aliased content is reduced
 		// because both the definition site and the reference site each consume
 		// a level, making deeply nested anchor/alias pairs hit the limit sooner.
 		if err := checkYAMLDepth(n.Value, depth+1, maxDepth, maxNodes, validated, visiting, nodeCount); err != nil {
 			return err
 		}
 	case *ast.TagNode:
 		if err := checkYAMLDepth(n.Value, depth+1, maxDepth, maxNodes, validated, visiting, nodeCount); err != nil {
 			return err
 		}
 	case *ast.MergeKeyNode:
 		// MergeKeyNode represents the literal "<<" merge key token. It has no
 		// child nodes — the value side of a merge (e.g., *alias) lives in the
 		// parent MappingValueNode.Value, which is already recursed into above.
 		// Explicitly listed here (rather than in the default case) to prevent
 		// future library changes from silently bypassing depth checks.
 	default:
 		// Scalar leaf nodes (StringNode, IntegerNode, FloatNode, BoolNode,
 		// NullNode, InfinityNode, NanNode, LiteralNode) have no children to
 		// recurse into.
 	}
 	return nil
 }
 // ParsePersonaBytes parses persona data from bytes with a source label for errors.
 // This is useful for parsing personas fetched from external sources (e.g., Gitea API)
 // without requiring filesystem access. Format is detected by source extension.
 // Input is bounded by MaxPersonaFileSize to prevent resource exhaustion.
 func ParsePersonaBytes(data []byte, source string) (*Persona, error) {
 	if len(data) > MaxPersonaFileSize {
 		return nil, fmt.Errorf("persona data from %s exceeds maximum size (%d bytes, limit %d)", source, len(data), MaxPersonaFileSize)
 	}
 	return parsePersona(data, source)
 }
 func validatePersona(p *Persona, source string) error {
 	if p.Name == "" {
 		return fmt.Errorf("persona %s: name is required", source)
 	}
 	if p.Identity == "" {
 		return fmt.Errorf("persona %s: identity is required", source)
 	}
 	// DisplayName defaults to Name if not set
 	if p.DisplayName == "" {
 		p.DisplayName = p.Name
 	}
 	return nil
 }
 // CapitalizeFirst capitalizes the first rune of a string in a Unicode-safe way.
 // Returns the original string if it's empty.
 func CapitalizeFirst(s string) string {
 	if s == "" {
 		return s
 	}
 	r, size := utf8.DecodeRuneInString(s)
 	if r == utf8.RuneError {
 		return s
 	}
 	return strings.ToUpper(string(r)) + s[size:]
 }
@@ -1,104 +0,0 @@
 package review
 import (
 	"fmt"
 	"strings"
 )
 // BuildPersonaSystemPrompt constructs a system prompt from a persona definition.
 // This replaces BuildSystemBase when a persona is provided.
 func BuildPersonaSystemPrompt(p *Persona) string {
 	var sb strings.Builder
 	// Identity section
 	sb.WriteString(p.Identity)
 	sb.WriteString("\n\n")
 	// Focus section
 	if len(p.Focus) > 0 {
 		sb.WriteString("## Focus Areas\n\n")
 		sb.WriteString("Concentrate your review on:\n")
 		for _, f := range p.Focus {
 			sb.WriteString(fmt.Sprintf("- %s\n", f))
 		}
 		sb.WriteString("\n")
 	}
 	// Ignore section
 	if len(p.Ignore) > 0 {
 		sb.WriteString("## Explicitly Out of Scope\n\n")
 		sb.WriteString("Do NOT comment on:\n")
 		for _, i := range p.Ignore {
 			sb.WriteString(fmt.Sprintf("- %s\n", i))
 		}
 		sb.WriteString("\n")
 	}
 	// Severity calibration
 	if p.Severity.Major != "" || p.Severity.Minor != "" || p.Severity.Nit != "" {
 		sb.WriteString("## Severity Calibration\n\n")
 		sb.WriteString("Use these severity definitions for YOUR domain:\n")
 		if p.Severity.Major != "" {
 			sb.WriteString(fmt.Sprintf("- **MAJOR**: %s\n", p.Severity.Major))
 		}
 		if p.Severity.Minor != "" {
 			sb.WriteString(fmt.Sprintf("- **MINOR**: %s\n", p.Severity.Minor))
 		}
 		if p.Severity.Nit != "" {
 			sb.WriteString(fmt.Sprintf("- **NIT**: %s\n", p.Severity.Nit))
 		}
 		sb.WriteString("\n")
 	}
 	// Output format instructions (shared schema from prompt.go)
 	sb.WriteString("## Review Instructions\n\n")
 	sb.WriteString("CONTEXT:\n")
 	sb.WriteString("- You will receive the full content of modified files for reference, followed by the diff showing what changed.\n")
 	sb.WriteString("- The diff shows ONLY what was added/removed. The full file content provides complete context.\n")
 	sb.WriteString("- Focus your review on the CHANGES (the diff), using the full files for context.\n\n")
 	sb.WriteString("Your task:\n")
 	sb.WriteString("1. Review the diff for issues within YOUR focus areas only.\n")
 	sb.WriteString("2. Consider the CI status — if CI has failed, that is an automatic REQUEST_CHANGES regardless of code quality.\n")
 	sb.WriteString("3. Output your review as structured JSON (and ONLY JSON, no markdown fences or other text).\n\n")
 	sb.WriteString("Output format:\n")
 	sb.WriteString(outputSchemaJSON)
 	sb.WriteString("\n\n")
 	sb.WriteString(verdictRules)
 	sb.WriteString("\n- Only report findings within your focus areas. Ignore everything else.\n")
 	sb.WriteString("- Line numbers should reference the new file line numbers from the diff headers.\n")
 	sb.WriteString("- If the diff has no changes relevant to your focus areas, APPROVE with no findings.\n")
 	// Custom output format if provided
 	if p.OutputFormat != "" {
 		sb.WriteString("\n\n## Additional Output Guidelines\n\n")
 		sb.WriteString(p.OutputFormat)
 	}
 	return sb.String()
 }
 // BuildSystemPromptWithPersona constructs the full system prompt, using either
 // a persona or the default generic prompt. This is a convenience wrapper that
 // combines BuildPersonaSystemPrompt (or BuildSystemBase) with patterns and conventions.
 // It is exported for use by callers who want one-shot prompt assembly.
 func BuildSystemPromptWithPersona(persona *Persona, conventions, patterns string) string {
 	var base string
 	if persona != nil {
 		base = BuildPersonaSystemPrompt(persona)
 	} else {
 		base = BuildSystemBase()
 	}
 	var sb strings.Builder
 	sb.WriteString(base)
 	if patterns != "" {
 		sb.WriteString(fmt.Sprintf("\n\n## Language Patterns & Idioms\n\nUse the following patterns as review criteria. Code that violates these established patterns is a finding:\n\n%s\n", patterns))
 	}
 	if conventions != "" {
 		sb.WriteString(fmt.Sprintf("\n\n## Repository Conventions\n\nThe repository has the following coding conventions that must be respected:\n\n%s\n", conventions))
 	}
 	return sb.String()
 }
@@ -1,157 +0,0 @@
 package review
 import (
 	"strings"
 	"testing"
 )
 func TestBuildPersonaSystemPrompt(t *testing.T) {
 	p := &Persona{
 		Name:        "security",
 		DisplayName: "Security Specialist",
 		Identity:    "You are a security specialist.",
 		Focus:       []string{"injection attacks", "auth bypass"},
 		Ignore:      []string{"code style", "performance"},
 		Severity: Severity{
 			Major: "exploitable vulnerabilities",
 			Minor: "defense in depth",
 			Nit:   "theoretical risks",
 		},
 	}
 	prompt := BuildPersonaSystemPrompt(p)
 	// Check identity is included
 	if !strings.Contains(prompt, "You are a security specialist.") {
 		t.Error("prompt should contain identity")
 	}
 	// Check focus areas
 	if !strings.Contains(prompt, "Focus Areas") {
 		t.Error("prompt should contain Focus Areas section")
 	}
 	if !strings.Contains(prompt, "injection attacks") {
 		t.Error("prompt should contain focus item")
 	}
 	// Check ignore section
 	if !strings.Contains(prompt, "Out of Scope") {
 		t.Error("prompt should contain Out of Scope section")
 	}
 	if !strings.Contains(prompt, "code style") {
 		t.Error("prompt should contain ignore item")
 	}
 	// Check severity calibration
 	if !strings.Contains(prompt, "Severity Calibration") {
 		t.Error("prompt should contain Severity Calibration section")
 	}
 	if !strings.Contains(prompt, "exploitable vulnerabilities") {
 		t.Error("prompt should contain major severity definition")
 	}
 	// Check JSON output format is included
 	if !strings.Contains(prompt, `"verdict"`) {
 		t.Error("prompt should contain JSON output format")
 	}
 	if !strings.Contains(prompt, "APPROVE") {
 		t.Error("prompt should mention APPROVE verdict")
 	}
 }
 func TestBuildPersonaSystemPromptMinimal(t *testing.T) {
 	// Minimal persona with only required fields
 	p := &Persona{
 		Name:     "minimal",
 		Identity: "You are a minimal reviewer.",
 	}
 	prompt := BuildPersonaSystemPrompt(p)
 	// Should still work without optional fields
 	if !strings.Contains(prompt, "You are a minimal reviewer.") {
 		t.Error("prompt should contain identity")
 	}
 	// Should not have empty sections
 	if strings.Contains(prompt, "Focus Areas") && !strings.Contains(prompt, "Concentrate your review on:") {
 		t.Error("should not have Focus Areas header without content")
 	}
 }
 func TestBuildSystemPromptWithPersona(t *testing.T) {
 	t.Run("with persona", func(t *testing.T) {
 		p := &Persona{
 			Name:     "test",
 			Identity: "Test persona identity.",
 			Focus:    []string{"testing"},
 		}
 		prompt := BuildSystemPromptWithPersona(p, "test conventions", "test patterns")
 		if !strings.Contains(prompt, "Test persona identity.") {
 			t.Error("should contain persona identity")
 		}
 		if !strings.Contains(prompt, "test conventions") {
 			t.Error("should contain conventions")
 		}
 		if !strings.Contains(prompt, "test patterns") {
 			t.Error("should contain patterns")
 		}
 	})
 	t.Run("without persona", func(t *testing.T) {
 		prompt := BuildSystemPromptWithPersona(nil, "test conventions", "test patterns")
 		// Should use default system base
 		if !strings.Contains(prompt, "expert code reviewer") {
 			t.Error("should contain default system base when no persona")
 		}
 		if !strings.Contains(prompt, "test conventions") {
 			t.Error("should contain conventions")
 		}
 	})
 	t.Run("empty conventions and patterns", func(t *testing.T) {
 		p := &Persona{
 			Name:     "test",
 			Identity: "Test identity.",
 		}
 		prompt := BuildSystemPromptWithPersona(p, "", "")
 		if strings.Contains(prompt, "Language Patterns") {
 			t.Error("should not contain patterns section when empty")
 		}
 		if strings.Contains(prompt, "Repository Conventions") {
 			t.Error("should not contain conventions section when empty")
 		}
 	})
 }
 func TestPersonaPromptContainsOutputRules(t *testing.T) {
 	p := &Persona{
 		Name:     "test",
 		Identity: "Test.",
 	}
 	prompt := BuildPersonaSystemPrompt(p)
 	// Must contain the critical output rules
 	requiredStrings := []string{
 		"APPROVE",
 		"REQUEST_CHANGES",
 		"MAJOR",
 		"MINOR",
 		"NIT",
 		"verdict",
 		"findings",
 		"CI",
 	}
 	for _, s := range requiredStrings {
 		if !strings.Contains(prompt, s) {
 			t.Errorf("prompt should contain %q", s)
 		}
 	}
 }
@@ -1,959 +0,0 @@
 package review
 import (
 	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 	"github.com/goccy/go-yaml/ast"
 )
 func TestLoadBuiltinPersona(t *testing.T) {
 	tests := []struct {
 		name        string
 		personaName string
 		wantErr     bool
 		wantDisplay string
 	}{
 		{
 			name:        "security persona",
 			personaName: "security",
 			wantErr:     false,
 			wantDisplay: "Security Specialist",
 		},
 		{
 			name:        "architect persona",
 			personaName: "architect",
 			wantErr:     false,
 			wantDisplay: "Software Architect",
 		},
 		{
 			name:        "docs persona",
 			personaName: "docs",
 			wantErr:     false,
 			wantDisplay: "Documentation Reviewer",
 		},
 		{
 			name:        "unknown persona",
 			personaName: "nonexistent",
 			wantErr:     true,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			p, err := LoadBuiltinPersona(tt.personaName)
 			if tt.wantErr {
 				if err == nil {
 					t.Error("expected error, got nil")
 				}
 				return
 			}
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
 			if p.Name != tt.personaName {
 				t.Errorf("Name = %q, want %q", p.Name, tt.personaName)
 			}
 			if p.DisplayName != tt.wantDisplay {
 				t.Errorf("DisplayName = %q, want %q", p.DisplayName, tt.wantDisplay)
 			}
 			if p.Identity == "" {
 				t.Error("Identity should not be empty")
 			}
 			if len(p.Focus) == 0 {
 				t.Error("Focus should not be empty")
 			}
 		})
 	}
 }
 func TestListBuiltinPersonas(t *testing.T) {
 	names := ListBuiltinPersonas()
 	if len(names) == 0 {
 		t.Fatal("expected at least one built-in persona")
 	}
 	// Check for expected personas
 	expected := map[string]bool{"security": false, "architect": false, "docs": false}
 	for _, name := range names {
 		if _, ok := expected[name]; ok {
 			expected[name] = true
 		}
 	}
 	for name, found := range expected {
 		if !found {
 			t.Errorf("expected built-in persona %q not found", name)
 		}
 	}
 }
 func TestLoadPersonaFromYAMLFile(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "test.yaml")
 	content := `# Test persona
 name: test
 display_name: Test Persona
 identity: |
  You are a test persona.
  Multi-line identity works.
 focus:
  - testing
  - validation
 ignore:
  - nothing
 severity:
  major: Big problems
  minor: Small problems
  nit: Tiny problems
 `
 	if err := os.WriteFile(path, []byte(content), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	p, err := LoadPersona(path)
 	if err != nil {
 		t.Fatalf("LoadPersona failed: %v", err)
 	}
 	if p.Name != "test" {
 		t.Errorf("Name = %q, want %q", p.Name, "test")
 	}
 	if p.DisplayName != "Test Persona" {
 		t.Errorf("DisplayName = %q, want %q", p.DisplayName, "Test Persona")
 	}
 	if len(p.Focus) != 2 {
 		t.Errorf("Focus len = %d, want 2", len(p.Focus))
 	}
 	if !strings.Contains(p.Identity, "Multi-line") {
 		t.Error("Identity should contain multi-line content")
 	}
 }
 func TestLoadPersonaFromYMLFile(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "test.yml")
 	content := `name: test
 display_name: Test YML
 identity: Test identity
 focus:
  - testing
 ignore: []
 severity:
  major: Big
  minor: Small
  nit: Tiny
 `
 	if err := os.WriteFile(path, []byte(content), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	p, err := LoadPersona(path)
 	if err != nil {
 		t.Fatalf("LoadPersona failed: %v", err)
 	}
 	if p.Name != "test" {
 		t.Errorf("Name = %q, want %q", p.Name, "test")
 	}
 	if p.DisplayName != "Test YML" {
 		t.Errorf("DisplayName = %q, want %q", p.DisplayName, "Test YML")
 	}
 }
 func TestLoadPersonaFromJSONFile(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "test.json")
 	content := `{
 		"name": "test",
 		"display_name": "Test Persona",
 		"identity": "You are a test persona.\nMulti-line identity works.",
 		"focus": ["testing", "validation"],
 		"ignore": ["nothing"],
 		"severity": {
 			"major": "Big problems",
 			"minor": "Small problems",
 			"nit": "Tiny problems"
 		}
 	}`
 	if err := os.WriteFile(path, []byte(content), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	p, err := LoadPersona(path)
 	if err != nil {
 		t.Fatalf("LoadPersona failed: %v", err)
 	}
 	if p.Name != "test" {
 		t.Errorf("Name = %q, want %q", p.Name, "test")
 	}
 	if p.DisplayName != "Test Persona" {
 		t.Errorf("DisplayName = %q, want %q", p.DisplayName, "Test Persona")
 	}
 	if len(p.Focus) != 2 {
 		t.Errorf("Focus len = %d, want 2", len(p.Focus))
 	}
 	if !strings.Contains(p.Identity, "Multi-line") {
 		t.Error("Identity should contain multi-line content")
 	}
 }
 func TestLoadPersonaValidation(t *testing.T) {
 	tests := []struct {
 		name    string
 		content string
 		ext     string
 		wantErr string
 	}{
 		{
 			name:    "missing name yaml",
 			content: "identity: test\n",
 			ext:     ".yaml",
 			wantErr: "name is required",
 		},
 		{
 			name:    "missing identity yaml",
 			content: "name: test\n",
 			ext:     ".yaml",
 			wantErr: "identity is required",
 		},
 		{
 			name:    "missing name json",
 			content: `{"identity": "test"}`,
 			ext:     ".json",
 			wantErr: "name is required",
 		},
 		{
 			name:    "missing identity json",
 			content: `{"name": "test"}`,
 			ext:     ".json",
 			wantErr: "identity is required",
 		},
 		{
 			name:    "display_name defaults to name",
 			content: "name: test\nidentity: test identity\n",
 			ext:     ".yaml",
 			// No error expected - should succeed
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			dir := t.TempDir()
 			path := filepath.Join(dir, "test"+tt.ext)
 			if err := os.WriteFile(path, []byte(tt.content), 0644); err != nil {
 				t.Fatalf("failed to write test file: %v", err)
 			}
 			p, err := LoadPersona(path)
 			if tt.wantErr != "" {
 				if err == nil {
 					t.Errorf("expected error containing %q, got nil", tt.wantErr)
 					return
 				}
 				if !strings.Contains(err.Error(), tt.wantErr) {
 					t.Errorf("error = %q, want containing %q", err.Error(), tt.wantErr)
 				}
 				return
 			}
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
 			// Check display_name defaulting
 			if p.DisplayName == "" {
 				t.Error("DisplayName should default to Name")
 			}
 			if p.DisplayName != p.Name {
 				t.Errorf("DisplayName should default to Name, got %q", p.DisplayName)
 			}
 		})
 	}
 }
 func TestLoadPersonaFileNotFound(t *testing.T) {
 	_, err := LoadPersona("/nonexistent/path/persona.yaml")
 	if err == nil {
 		t.Error("expected error for nonexistent file")
 	}
 }
 func TestLoadPersonaInvalidYAML(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "invalid.yaml")
 	if err := os.WriteFile(path, []byte("not valid yaml:\n  - [broken"), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	_, err := LoadPersona(path)
 	if err == nil {
 		t.Error("expected error for invalid YAML")
 	}
 }
 func TestLoadPersonaInvalidJSON(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "invalid.json")
 	if err := os.WriteFile(path, []byte("not valid json {"), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	_, err := LoadPersona(path)
 	if err == nil {
 		t.Error("expected error for invalid JSON")
 	}
 }
 func TestLoadPersonaCaseInsensitiveExtension(t *testing.T) {
 	tests := []struct {
 		name string
 		ext  string
 	}{
 		{"lowercase yaml", ".yaml"},
 		{"uppercase YAML", ".YAML"},
 		{"mixed case Yaml", ".Yaml"},
 		{"lowercase yml", ".yml"},
 		{"uppercase YML", ".YML"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			dir := t.TempDir()
 			path := filepath.Join(dir, "test"+tt.ext)
 			content := "name: test\nidentity: test identity\n"
 			if err := os.WriteFile(path, []byte(content), 0644); err != nil {
 				t.Fatalf("failed to write test file: %v", err)
 			}
 			p, err := LoadPersona(path)
 			if err != nil {
 				t.Fatalf("LoadPersona failed for extension %s: %v", tt.ext, err)
 			}
 			if p.Name != "test" {
 				t.Errorf("Name = %q, want %q", p.Name, "test")
 			}
 		})
 	}
 }
 func TestCapitalizeFirst(t *testing.T) {
 	tests := []struct {
 		input string
 		want  string
 	}{
 		{"hello", "Hello"},
 		{"Hello", "Hello"},
 		{"HELLO", "HELLO"},
 		{"a", "A"},
 		{"", ""},
 		{"日本語", "日本語"}, // Non-ASCII: Japanese doesn't have case
 		{"über", "Über"},   // German umlaut
 		{"élève", "Élève"}, // French accent
 	}
 	for _, tt := range tests {
 		t.Run(tt.input, func(t *testing.T) {
 			got := CapitalizeFirst(tt.input)
 			if got != tt.want {
 				t.Errorf("CapitalizeFirst(%q) = %q, want %q", tt.input, got, tt.want)
 			}
 		})
 	}
 }
 func TestListBuiltinPersonasReturnsEmptySlice(t *testing.T) {
 	// ListBuiltinPersonas should return an empty slice (not nil) on error.
 	// We can't easily test the error case, but we can verify the success case
 	// returns a proper slice.
 	names := ListBuiltinPersonas()
 	if names == nil {
 		t.Error("ListBuiltinPersonas should return empty slice, not nil")
 	}
 }
 func TestYAMLMultilineStrings(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "multiline.yaml")
 	// Test literal block scalar (|) which preserves newlines
 	content := `name: multiline
 display_name: Multiline Test
 identity: |
  First line.
  Second line.
  Third line.
 focus:
  - item one
 ignore: []
 severity:
  major: Major issue
  minor: Minor issue
  nit: Nit
 `
 	if err := os.WriteFile(path, []byte(content), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	p, err := LoadPersona(path)
 	if err != nil {
 		t.Fatalf("LoadPersona failed: %v", err)
 	}
 	// Literal block scalar preserves newlines
 	if !strings.Contains(p.Identity, "\n") {
 		t.Error("Identity should contain newlines from literal block scalar")
 	}
 	if !strings.Contains(p.Identity, "Second line") {
 		t.Error("Identity should contain 'Second line'")
 	}
 }
 func TestYAMLComments(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "comments.yaml")
 	content := `# This is a comment
 name: commented  # inline comment
 display_name: Commented Persona
 # Another comment
 identity: Test identity
 focus:
  - item  # comment after item
 ignore: []
 severity:
  major: Major
  minor: Minor
  nit: Nit
 `
 	if err := os.WriteFile(path, []byte(content), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	p, err := LoadPersona(path)
 	if err != nil {
 		t.Fatalf("LoadPersona failed: %v", err)
 	}
 	// Comments should be ignored
 	if p.Name != "commented" {
 		t.Errorf("Name = %q, want %q", p.Name, "commented")
 	}
 	if p.Focus[0] != "item" {
 		t.Errorf("Focus[0] = %q, want %q", p.Focus[0], "item")
 	}
 }
 func TestYAMLDeeplyNestedRejection(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "deeply-nested.yaml")
 	// Build a deeply nested YAML structure that exceeds MaxYAMLDepth (20).
 	// Depth accumulation trace for "nested: \n  level0: \n    level1: ...":
 	//   - Document root parsed at depth 0
 	//   - Root MappingNode children (MappingValueNodes) visited at depth 1
 	//   - "nested" MappingValueNode: key at depth 2, value at depth 2
 	//   - Each levelN adds depth via MappingValueNode traversal (key + value)
 	//   - Exact depth per level depends on AST structure (MappingNode wrapping),
 	//     but 25 levels reliably exceeds MaxYAMLDepth (20) with comfortable margin.
 	// The test uses 25 levels rather than exactly 21 to avoid brittleness.
 	var sb strings.Builder
 	sb.WriteString("name: test\nidentity: test\nnested:\n")
 	indent := "  "
 	for i := 0; i < 25; i++ {
 		sb.WriteString(strings.Repeat(indent, i+1))
 		sb.WriteString(fmt.Sprintf("level%d:\n", i))
 	}
 	sb.WriteString(strings.Repeat(indent, 26))
 	sb.WriteString("value: too-deep\n")
 	if err := os.WriteFile(path, []byte(sb.String()), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	_, err := LoadPersona(path)
 	if err == nil {
 		t.Error("expected error for deeply nested YAML, got nil")
 	}
 	if !strings.Contains(err.Error(), "nesting depth exceeds") {
 		t.Errorf("error = %q, want containing 'nesting depth exceeds'", err.Error())
 	}
 }
 func TestYAMLEmptyFileRejection(t *testing.T) {
 	tests := []struct {
 		name    string
 		content string
 	}{
 		{"completely_empty", ""},
 		{"whitespace_only", "   \n\n  "},
 		{"comment_only", "# just a comment\n"},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			dir := t.TempDir()
 			path := filepath.Join(dir, tc.name+".yaml")
 			if err := os.WriteFile(path, []byte(tc.content), 0644); err != nil {
 				t.Fatalf("failed to write test file: %v", err)
 			}
 			_, err := LoadPersona(path)
 			if err == nil {
 				t.Fatal("expected error for empty YAML input, got nil")
 			}
 			if !strings.Contains(err.Error(), "empty YAML document") {
 				t.Errorf("expected error containing %q, got: %v", "empty YAML document", err)
 			}
 		})
 	}
 }
 func TestYAMLFileSizeLimit(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "huge.yaml")
 	// Create a file larger than MaxPersonaFileSize (64 KB)
 	content := "name: test\nidentity: " + strings.Repeat("x", MaxPersonaFileSize+1) + "\n"
 	if err := os.WriteFile(path, []byte(content), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	_, err := LoadPersona(path)
 	if err == nil {
 		t.Error("expected error for oversized file, got nil")
 	}
 	if !strings.Contains(err.Error(), "exceeds maximum size") {
 		t.Errorf("error = %q, want containing 'exceeds maximum size'", err.Error())
 	}
 }
 func TestYAMLAliasCycleDetection(t *testing.T) {
 	// Test that our checkYAMLDepth function handles alias cycles gracefully
 	// by using the visiting map to prevent infinite recursion.
 	// Create a node structure where an alias points to a parent node,
 	// simulating what could happen with crafted input.
 	parent := &ast.MappingNode{
 		Values: []*ast.MappingValueNode{
 			{
 				Key:   &ast.StringNode{Value: "name"},
 				Value: &ast.StringNode{Value: "test"},
 			},
 		},
 	}
 	// Create a child that aliases back to the parent (artificial cycle)
 	aliasToParent := &ast.AliasNode{
 		Value: parent,
 	}
 	parent.Values = append(parent.Values, &ast.MappingValueNode{
 		Key:   &ast.StringNode{Value: "nested"},
 		Value: aliasToParent,
 	})
 	nodeCount := 0
 	validated := make(map[ast.Node]int)
 	visiting := make(map[ast.Node]bool)
 	// This should NOT hang or stack overflow - cycle detection prevents infinite recursion
 	err := checkYAMLDepth(parent, 0, MaxYAMLDepth, MaxYAMLNodes, validated, visiting, &nodeCount)
 	if err != nil {
 		t.Errorf("unexpected error traversing cyclic structure: %v", err)
 	}
 	// Verify we tracked the parent in the validated map
 	if _, ok := validated[parent]; !ok {
 		t.Error("parent node not tracked in validated map")
 	}
 }
 func TestYAMLMultiDocumentRejection(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "multi.yaml")
 	// Multi-document YAML (documents separated by ---)
 	content := `name: first
 identity: first document
 ---
 name: second
 identity: second document
 `
 	if err := os.WriteFile(path, []byte(content), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	_, err := LoadPersona(path)
 	if err == nil {
 		t.Error("expected error for multi-document YAML, got nil")
 	}
 	if !strings.Contains(err.Error(), "multi-document") {
 		t.Errorf("error = %q, want containing 'multi-document'", err.Error())
 	}
 }
 func TestYAMLNodeCountLimit(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "wide.yaml")
 	// Build a YAML structure that's shallow but wide - many keys at the same level
 	// to test the node count limit (should exceed MaxYAMLNodes = 1000)
 	var sb strings.Builder
 	sb.WriteString("name: test\nidentity: test\n")
 	for i := 0; i < 600; i++ {
 		sb.WriteString(fmt.Sprintf("key%d: value%d\n", i, i))
 	}
 	if err := os.WriteFile(path, []byte(sb.String()), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	_, err := LoadPersona(path)
 	if err == nil {
 		t.Error("expected error for wide YAML exceeding node count, got nil")
 	}
 	if !strings.Contains(err.Error(), "node count exceeds") {
 		t.Errorf("error = %q, want containing 'node count exceeds'", err.Error())
 	}
 }
 func TestCheckYAMLDepthCycleDetectionDirect(t *testing.T) {
 	// Direct test of cycle detection in checkYAMLDepth by creating
 	// a node structure with an artificial cycle.
 	node := &ast.MappingNode{
 		Values: []*ast.MappingValueNode{
 			{
 				Key:   &ast.StringNode{Value: "key"},
 				Value: &ast.StringNode{Value: "value"},
 			},
 		},
 	}
 	// Create a cycle by making a child reference the parent
 	cycleChild := &ast.AliasNode{
 		Value: node, // Points back to the parent
 	}
 	node.Values = append(node.Values, &ast.MappingValueNode{
 		Key:   &ast.StringNode{Value: "cyclic"},
 		Value: cycleChild,
 	})
 	nodeCount := 0
 	validated := make(map[ast.Node]int)
 	visiting := make(map[ast.Node]bool)
 	err := checkYAMLDepth(node, 0, MaxYAMLDepth, MaxYAMLNodes, validated, visiting, &nodeCount)
 	// Should complete without infinite recursion due to cycle detection
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
 	// The validated map should contain multiple entries
 	if len(validated) < 2 {
 		t.Errorf("validated map has %d entries, expected at least 2", len(validated))
 	}
 }
 func TestYAMLAliasDepthBypass(t *testing.T) {
 	// Test that an anchored subtree first validated at a shallow depth is
 	// re-checked when referenced via alias at a deeper position. Without the
 	// depth-aware validated map, the alias reference would skip re-checking
 	// and allow the effective nesting to exceed MaxYAMLDepth.
 	dir := t.TempDir()
 	path := filepath.Join(dir, "alias-depth-bypass.yaml")
 	// Build YAML with an anchor at shallow depth containing a subtree near the limit,
 	// then reference it via alias deep enough that effective depth exceeds MaxYAMLDepth.
 	var sb strings.Builder
 	sb.WriteString("name: test\nidentity: test\n")
 	// Create the anchored subtree at depth 1 (key level) that nests 15 levels deep.
 	sb.WriteString("anchor_key: &deep_anchor\n")
 	for i := 0; i < 15; i++ {
 		sb.WriteString(strings.Repeat("  ", i+1))
 		sb.WriteString(fmt.Sprintf("level%d:\n", i))
 	}
 	sb.WriteString(strings.Repeat("  ", 16))
 	sb.WriteString("leaf: value\n")
 	// Create a wrapper that nests 6 levels deep, then references the anchor.
 	// Effective depth at alias target = 6 (wrapper nesting) + 1 (alias) + 15 (subtree) = 22 > 20
 	sb.WriteString("wrapper:\n")
 	for i := 0; i < 6; i++ {
 		sb.WriteString(strings.Repeat("  ", i+1))
 		sb.WriteString(fmt.Sprintf("n%d:\n", i))
 	}
 	sb.WriteString(strings.Repeat("  ", 7))
 	sb.WriteString("alias_ref: *deep_anchor\n")
 	if err := os.WriteFile(path, []byte(sb.String()), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	_, err := LoadPersona(path)
 	if err == nil {
 		t.Fatal("expected error for alias depth bypass, got nil")
 	}
 	if !strings.Contains(err.Error(), "nesting depth exceeds") {
 		t.Errorf("error = %q, want containing 'nesting depth exceeds'", err.Error())
 	}
 }
 func TestListBuiltinPersonasSortedOrder(t *testing.T) {
 	names := ListBuiltinPersonas()
 	if len(names) < 2 {
 		t.Skip("need at least 2 personas to test ordering")
 	}
 	// Verify the list is sorted
 	for i := 1; i < len(names); i++ {
 		if names[i-1] > names[i] {
 			t.Errorf("ListBuiltinPersonas not sorted: %q > %q", names[i-1], names[i])
 		}
 	}
 }
 func TestYAMLUnknownFieldsRejected(t *testing.T) {
 	tests := []struct {
 		name    string
 		content string
 		wantErr string
 	}{
 		{
 			name: "unknown top-level field",
 			content: `name: test
 identity: test identity
 unknown_field: should fail
 `,
 			wantErr: "unknown_field",
 		},
 		{
 			name: "typo in field name",
 			content: `name: test
 identiy: typo should fail
 `,
 			wantErr: "identiy",
 		},
 		{
 			name: "unknown field in severity",
 			content: `name: test
 identity: test
 severity:
  major: Major
  minro: typo
 `,
 			wantErr: "minro",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			dir := t.TempDir()
 			path := filepath.Join(dir, "unknown.yaml")
 			if err := os.WriteFile(path, []byte(tt.content), 0644); err != nil {
 				t.Fatalf("failed to write test file: %v", err)
 			}
 			_, err := LoadPersona(path)
 			if err == nil {
 				t.Errorf("expected error for unknown field %q, got nil", tt.wantErr)
 				return
 			}
 			if !strings.Contains(err.Error(), tt.wantErr) {
 				t.Errorf("error = %q, want containing %q", err.Error(), tt.wantErr)
 			}
 		})
 	}
 }
 func TestJSONUnknownFieldsRejected(t *testing.T) {
 	tests := []struct {
 		name    string
 		content string
 		wantErr string
 	}{
 		{
 			name: "unknown top-level field",
 			content: `{
 				"name": "test",
 				"identity": "test identity",
 				"unknown_field": "should fail"
 			}`,
 			wantErr: "unknown_field",
 		},
 		{
 			name: "typo in field name",
 			content: `{
 				"name": "test",
 				"identiy": "typo should fail"
 			}`,
 			wantErr: "identiy",
 		},
 		{
 			name: "unknown field in severity",
 			content: `{
 				"name": "test",
 				"identity": "test",
 				"severity": {
 					"major": "ok",
 					"miner": "typo"
 				}
 			}`,
 			wantErr: "miner",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			dir := t.TempDir()
 			path := filepath.Join(dir, "test.json")
 			if err := os.WriteFile(path, []byte(tt.content), 0644); err != nil {
 				t.Fatalf("failed to write test file: %v", err)
 			}
 			_, err := LoadPersona(path)
 			if err == nil {
 				t.Fatal("expected error for unknown field, got nil")
 			}
 			if !strings.Contains(err.Error(), tt.wantErr) {
 				t.Errorf("error = %q, want to contain %q", err.Error(), tt.wantErr)
 			}
 		})
 	}
 }
 func TestLoadPersonaSymlink(t *testing.T) {
 	// Create a regular persona file
 	dir := t.TempDir()
 	realFile := filepath.Join(dir, "real.yaml")
 	content := `name: test
 identity: test identity
 `
 	if err := os.WriteFile(realFile, []byte(content), 0644); err != nil {
 		t.Fatalf("failed to write test file: %v", err)
 	}
 	// Create a symlink to it
 	symlink := filepath.Join(dir, "link.yaml")
 	if err := os.Symlink(realFile, symlink); err != nil {
 		t.Fatalf("failed to create symlink: %v", err)
 	}
 	// LoadPersona should work via symlink
 	p, err := LoadPersona(symlink)
 	if err != nil {
 		t.Fatalf("LoadPersona via symlink failed: %v", err)
 	}
 	if p.Name != "test" {
 		t.Errorf("Name = %q, want %q", p.Name, "test")
 	}
 }
 func TestJSONTrailingContentRejected(t *testing.T) {
 	tests := []struct {
 		name    string
 		content string
 	}{
 		{
 			name:    "trailing garbage after object",
 			content: `{"name":"test","identity":"test identity"}garbage`,
 		},
 		{
 			name:    "two JSON objects",
 			content: `{"name":"test","identity":"test identity"}{"name":"other"}`,
 		},
 		{
 			name:    "trailing array",
 			content: `{"name":"test","identity":"test identity"}[]`,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			dir := t.TempDir()
 			path := filepath.Join(dir, "test.json")
 			if err := os.WriteFile(path, []byte(tt.content), 0644); err != nil {
 				t.Fatalf("failed to write test file: %v", err)
 			}
 			_, err := LoadPersona(path)
 			if err == nil {
 				t.Fatal("expected error for trailing content, got nil")
 			}
 			if !strings.Contains(err.Error(), "trailing content") {
 				t.Errorf("error = %q, want to contain 'trailing content'", err.Error())
 			}
 		})
 	}
 }
 func TestParsePersonaBytesSizeLimit(t *testing.T) {
 	// ParsePersonaBytes should reject input exceeding MaxPersonaFileSize
 	oversized := make([]byte, MaxPersonaFileSize+1)
 	for i := range oversized {
 		oversized[i] = 'x'
 	}
 	_, err := ParsePersonaBytes(oversized, "oversized.yaml")
 	if err == nil {
 		t.Fatal("expected error for oversized input, got nil")
 	}
 	if !strings.Contains(err.Error(), "exceeds maximum size") {
 		t.Errorf("error = %q, want to contain 'exceeds maximum size'", err.Error())
 	}
 	// Just under the limit should not trigger size error (may fail parse, but not size)
 	underLimit := []byte("name: test\nidentity: test persona\n")
 	p, err := ParsePersonaBytes(underLimit, "valid.yaml")
 	if err != nil {
 		t.Fatalf("unexpected error for valid input: %v", err)
 	}
 	if p.Name != "test" {
 		t.Errorf("Name = %q, want %q", p.Name, "test")
 	}
 }
 func TestYAMLMergeKeyDepthCheck(t *testing.T) {
 	// Verify that YAML merge keys (<<: *alias) are properly handled by the
 	// depth checker. The merge key content is in the MappingValueNode.Value
 	// (an AliasNode), not in the MergeKeyNode itself.
 	p, err := ParsePersonaBytes([]byte("name: merge-test\nidentity: test\n"), "merge.yaml")
 	if err != nil {
 		t.Fatalf("basic parse failed: %v", err)
 	}
 	if p.Name != "merge-test" {
 		t.Errorf("Name = %q, want %q", p.Name, "merge-test")
 	}
 	// Test that deeply nested merge keys still hit depth limit.
 	// Build YAML with merge key content nested beyond MaxYAMLDepth.
 	var sb strings.Builder
 	sb.WriteString("name: deep-merge\nidentity: deep merge persona\n")
 	sb.WriteString("anchor: &deep\n")
 	indent := "  "
 	for i := 0; i < MaxYAMLDepth+5; i++ {
 		sb.WriteString(indent)
 		sb.WriteString(fmt.Sprintf("level%d:\n", i))
 		indent += "  "
 	}
 	sb.WriteString(indent + "leaf: value\n")
 	sb.WriteString("target:\n  <<: *deep\n")
 	_, err = ParsePersonaBytes([]byte(sb.String()), "deep-merge.yaml")
 	if err == nil {
 		t.Fatal("expected error for deeply nested merge key content, got nil")
 	}
 	if !strings.Contains(err.Error(), "depth") {
 		t.Errorf("error = %q, want to contain 'depth'", err.Error())
 	}
 }
@@ -1,37 +0,0 @@
 # Software Architect Persona
 # Focuses on design quality, patterns, and code organization
 name: architect
 display_name: Software Architect
 identity: |
  You are a software architect reviewing code for design quality.
  Your expertise:
  - Design patterns and anti-patterns
  - Code organization and module boundaries
  - API design and contracts
  - Testability and dependency injection
  - Consistency with existing architecture
  - Technical debt identification
 focus:
  - Design pattern violations or misuse
  - Module boundary violations (inappropriate coupling)
  - API design issues (unclear contracts, leaky abstractions)
  - Testability problems (hidden dependencies, god objects)
  - Inconsistency with existing codebase patterns
  - Unnecessary complexity or over-engineering
  - Missing abstractions or premature abstraction
 ignore:
  - Security vulnerabilities (security persona handles these)
  - Performance micro-optimizations
  - Code style and formatting
  - Documentation typos
  - Test implementation details
 severity:
  major: "Architectural violations that will cause maintenance problems or make the codebase harder to evolve"
  minor: "Design issues that reduce clarity or testability but don't block progress"
  nit: "Minor pattern deviations or style preferences"
@@ -1,36 +0,0 @@
 # Documentation Reviewer Persona
 # Focuses on clarity, documentation quality, and self-documenting code
 name: docs
 display_name: Documentation Reviewer
 identity: |
  You are a documentation specialist reviewing code for clarity and documentation quality.
  Your expertise:
  - API documentation and examples
  - Code comments and their accuracy
  - Error message clarity
  - README and guide quality
  - Naming clarity and self-documenting code
 focus:
  - Missing or outdated documentation
  - Unclear or misleading comments
  - Poor error messages (cryptic, unhelpful, missing context)
  - Confusing naming (functions, variables, types)
  - Missing examples for complex APIs
  - Inconsistent terminology
  - Documentation that contradicts the code
 ignore:
  - Security vulnerabilities
  - Performance issues
  - Design patterns
  - Test coverage
  - Code style (unless it affects readability)
 severity:
  major: "Documentation that actively misleads or missing docs for critical functionality"
  minor: "Unclear documentation or poor error messages that will confuse users"
  nit: "Minor clarity improvements or typo fixes"
@@ -1,37 +0,0 @@
 # Security Specialist Persona
 # Focuses on vulnerabilities, auth issues, and security best practices
 name: security
 display_name: Security Specialist
 identity: |
  You are a security specialist reviewing code for vulnerabilities.
  Your expertise:
  - OWASP Top 10 vulnerabilities
  - Injection attacks (SQL, command, path traversal, template)
  - Authentication and authorization patterns
  - Secrets management and exposure risks
  - Race conditions with security implications
  - Event sourcing attack vectors (replay attacks, event injection)
 focus:
  - Injection attacks (SQL, command, path traversal, template injection)
  - Authentication and authorization gaps or bypasses
  - Secrets exposure (hardcoded credentials, tokens in logs, config leaks)
  - Input validation failures (unsanitized input, unsafe deserialization)
  - Race conditions that could be exploited
  - Cryptographic weaknesses (weak algorithms, improper key handling)
  - Information disclosure through error messages or logs
 ignore:
  - Code style and naming conventions
  - Performance optimizations (unless security-related)
  - Documentation quality
  - General code quality or readability
  - Test coverage
 severity:
  major: "Exploitable vulnerabilities: auth bypass, injection, data exfiltration, privilege escalation, RCE"
  minor: "Defense-in-depth issues: missing rate limiting, verbose errors, weak input validation"
  nit: "Theoretical risks with low exploitability or impact"
@@ -7,28 +7,6 @@ import (
 	"strings"
 )
 // outputSchemaJSON is the shared JSON output format specification used by both
 // the generic reviewer and persona-based reviewers.
 const outputSchemaJSON = `{
  "verdict": "APPROVE" or "REQUEST_CHANGES",
  "summary": "Brief overall assessment (1-3 sentences)",
  "findings": [
    {
      "severity": "MAJOR" or "MINOR" or "NIT",
      "file": "path/to/file",
      "line": <line number from the diff>,
      "finding": "Description of the issue"
    }
  ],
  "recommendation": "Full recommendation text explaining your verdict"
 }`
 // verdictRules is the shared verdict determination rules.
 const verdictRules = `Rules:
 - If there are any MAJOR findings → verdict must be REQUEST_CHANGES
 - If there are no MAJOR findings → verdict should be APPROVE
 - If CI has failed → verdict must be REQUEST_CHANGES with a finding noting the CI failure`
 // BuildSystemBase returns the core system prompt instructions without
 // patterns or conventions. Used by the budget package to separate
 // trimmable from non-trimmable content.
@@ -45,10 +23,24 @@ func BuildSystemBase() string {
 	sb.WriteString("2. Consider the CI status — if CI has failed, that is an automatic REQUEST_CHANGES regardless of code quality.\n")
 	sb.WriteString("3. Output your review as structured JSON (and ONLY JSON, no markdown fences or other text).\n\n")
 	sb.WriteString("Output format:\n")
-	sb.WriteString(outputSchemaJSON)
+	sb.WriteString("{\n")
-	sb.WriteString("\n\n")
+	sb.WriteString("  \"verdict\": \"APPROVE\" or \"REQUEST_CHANGES\",\n")
-	sb.WriteString(verdictRules)
+	sb.WriteString("  \"summary\": \"Brief overall assessment (1-3 sentences)\",\n")
-	sb.WriteString("\n- Be thorough but fair. Don't nitpick style unless it impacts readability significantly.\n")
+	sb.WriteString("  \"findings\": [\n")
 	sb.WriteString("    {\n")
 	sb.WriteString("      \"severity\": \"MAJOR\" or \"MINOR\" or \"NIT\",\n")
 	sb.WriteString("      \"file\": \"path/to/file\",\n")
 	sb.WriteString("      \"line\": <line number from the diff>,\n")
 	sb.WriteString("      \"finding\": \"Description of the issue\"\n")
 	sb.WriteString("    }\n")
 	sb.WriteString("  ],\n")
 	sb.WriteString("  \"recommendation\": \"Full recommendation text explaining your verdict\"\n")
 	sb.WriteString("}\n\n")
 	sb.WriteString("Rules:\n")
 	sb.WriteString("- If there are any MAJOR findings → verdict must be REQUEST_CHANGES\n")
 	sb.WriteString("- If there are no MAJOR findings → verdict should be APPROVE\n")
 	sb.WriteString("- If CI has failed → verdict must be REQUEST_CHANGES with a finding noting the CI failure\n")
 	sb.WriteString("- Be thorough but fair. Don't nitpick style unless it impacts readability significantly.\n")
 	sb.WriteString("- Line numbers should reference the new file line numbers from the diff headers.\n")
 	sb.WriteString("- If the diff is empty or trivial (only formatting/whitespace), APPROVE with no findings.\n")
@@ -1,150 +0,0 @@
 package review
 import (
 	"context"
 	"log/slog"
 	"strings"
 )
 // RepoPersonaPath is the directory path where repo-specific personas are stored.
 const RepoPersonaPath = ".review-bot/personas"
 // GiteaClient defines the subset of gitea.Client methods needed for loading repo personas.
 // This interface allows for easier testing and decouples the review package from gitea.
 type GiteaClient interface {
 	ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error)
 	GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error)
 }
 // ContentEntry represents a file or directory entry from the contents API.
 // This mirrors gitea.ContentEntry to avoid import cycles.
 type ContentEntry struct {
 	Name string `json:"name"`
 	Path string `json:"path"`
 	Type string `json:"type"` // "file" or "dir"
 }
 // LoadRepoPersonas fetches personas from a repository's .review-bot/personas/ directory.
 // Returns an empty map (not nil) if the directory doesn't exist or is empty.
 // Individual parse failures are logged and skipped; the remaining personas are still returned.
 // Auth errors and other non-404 errors are propagated.
 // Files exceeding MaxPersonaFileSize are rejected to prevent resource exhaustion.
 func LoadRepoPersonas(ctx context.Context, client GiteaClient, owner, repo string) (map[string]*Persona, error) {
 	result := make(map[string]*Persona)
 	entries, err := client.ListContents(ctx, owner, repo, RepoPersonaPath)
 	if err != nil {
 		// Check if this is a 404 (directory doesn't exist) - expected case
 		if isNotFoundError(err) {
 			slog.Debug("no repo personas directory found", "repo", owner+"/"+repo)
 			return result, nil
 		}
 		// Other errors (auth, server) should propagate
 		return nil, err
 	}
 	if len(entries) == 0 {
 		slog.Debug("repo personas directory is empty", "repo", owner+"/"+repo)
 		return result, nil
 	}
 	for _, entry := range entries {
 		if entry.Type != "file" {
 			continue
 		}
 		// Only process YAML files
 		if !isYAMLFile(entry.Name) {
 			continue
 		}
 		content, err := client.GetFileContent(ctx, owner, repo, entry.Path)
 		if err != nil {
 			slog.Warn("could not fetch repo persona file",
 				"file", entry.Path,
 				"repo", owner+"/"+repo,
 				"error", err)
 			continue
 		}
 		// Enforce size limit before parsing to prevent resource exhaustion
 		if len(content) > MaxPersonaFileSize {
 			slog.Warn("repo persona file exceeds maximum size",
 				"file", entry.Path,
 				"repo", owner+"/"+repo,
 				"size", len(content),
 				"max", MaxPersonaFileSize)
 			continue
 		}
 		persona, err := ParsePersonaBytes([]byte(content), entry.Path)
 		if err != nil {
 			slog.Warn("could not parse repo persona file",
 				"file", entry.Path,
 				"repo", owner+"/"+repo,
 				"error", err)
 			continue
 		}
 		result[persona.Name] = persona
 		slog.Debug("loaded repo persona",
 			"name", persona.Name,
 			"file", entry.Path,
 			"repo", owner+"/"+repo)
 	}
 	return result, nil
 }
 // MergePersonas combines built-in personas with repo personas.
 // Repo personas take precedence on name collision.
 // Returns a new map; inputs are not modified.
 func MergePersonas(builtin, repo map[string]*Persona) map[string]*Persona {
 	result := make(map[string]*Persona, len(builtin)+len(repo))
 	// Copy built-in personas first
 	for name, p := range builtin {
 		result[name] = p
 	}
 	// Overlay repo personas (override on collision)
 	for name, p := range repo {
 		if _, exists := result[name]; exists {
 			slog.Debug("repo persona overrides built-in", "name", name)
 		}
 		result[name] = p
 	}
 	return result
 }
 // GetBuiltinPersonasMap returns all built-in personas as a map keyed by name.
 // Returns an empty map (not nil) if loading fails.
 func GetBuiltinPersonasMap() map[string]*Persona {
 	result := make(map[string]*Persona)
 	for _, name := range ListBuiltinPersonas() {
 		p, err := LoadBuiltinPersona(name)
 		if err != nil {
 			slog.Warn("could not load built-in persona", "name", name, "error", err)
 			continue
 		}
 		result[name] = p
 	}
 	return result
 }
 // isYAMLFile checks if a filename has a YAML extension.
 func isYAMLFile(name string) bool {
 	lower := strings.ToLower(name)
 	return strings.HasSuffix(lower, ".yaml") || strings.HasSuffix(lower, ".yml")
 }
 // isNotFoundError checks if an error represents a 404 response.
 // This uses a specific "HTTP 404" substring match rather than a generic "not found"
 // match to avoid masking authentication failures or transport errors that might
 // contain "not found" in their message.
 func isNotFoundError(err error) bool {
 	if err == nil {
 		return false
 	}
 	return strings.Contains(err.Error(), "HTTP 404")
 }
@@ -1,443 +0,0 @@
 package review
 import (
 	"context"
 	"errors"
 	"strings"
 	"testing"
 )
 func TestParsePersonaBytes(t *testing.T) {
 	tests := []struct {
 		name       string
 		data       string
 		source     string
 		wantName   string
 		wantErr    string
 	}{
 		{
 			name: "valid yaml",
 			data: `name: test
 identity: test identity
 focus:
  - testing
 `,
 			source:   "test.yaml",
 			wantName: "test",
 		},
 		{
 			name:    "missing name",
 			data:    "identity: test\n",
 			source:  "test.yaml",
 			wantErr: "name is required",
 		},
 		{
 			name:    "invalid yaml",
 			data:    "not: valid:\n  yaml: [broken",
 			source:  "test.yaml",
 			wantErr: "parse",
 		},
 		{
 			name: "json format by extension",
 			data: `{"name": "jsontest", "identity": "json identity"}`,
 			source:   "test.json",
 			wantName: "jsontest",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			p, err := ParsePersonaBytes([]byte(tt.data), tt.source)
 			if tt.wantErr != "" {
 				if err == nil {
 					t.Fatalf("expected error containing %q, got nil", tt.wantErr)
 				}
 				if !strings.Contains(err.Error(), tt.wantErr) {
 					t.Errorf("error = %q, want containing %q", err.Error(), tt.wantErr)
 				}
 				return
 			}
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
 			if p.Name != tt.wantName {
 				t.Errorf("Name = %q, want %q", p.Name, tt.wantName)
 			}
 		})
 	}
 }
 // mockGiteaClient implements GiteaClient for testing.
 type mockGiteaClient struct {
 	contents map[string][]ContentEntry // path -> entries
 	files    map[string]string         // path -> content
 	listErr  error
 	fileErr  map[string]error // path -> error
 }
 func (m *mockGiteaClient) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
 	if m.listErr != nil {
 		return nil, m.listErr
 	}
 	entries, ok := m.contents[path]
 	if !ok {
 		return nil, errors.New("list contents .review-bot/personas: HTTP 404: not found")
 	}
 	return entries, nil
 }
 func (m *mockGiteaClient) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
 	if m.fileErr != nil {
 		if err, ok := m.fileErr[filepath]; ok {
 			return "", err
 		}
 	}
 	content, ok := m.files[filepath]
 	if !ok {
 		return "", errors.New("HTTP 404: file not found")
 	}
 	return content, nil
 }
 func TestLoadRepoPersonas(t *testing.T) {
 	ctx := context.Background()
 	t.Run("directory not found returns empty map", func(t *testing.T) {
 		client := &mockGiteaClient{} // No contents configured -> 404
 		personas, err := LoadRepoPersonas(ctx, client, "owner", "repo")
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		if personas == nil {
 			t.Error("expected empty map, got nil")
 		}
 		if len(personas) != 0 {
 			t.Errorf("expected 0 personas, got %d", len(personas))
 		}
 	})
 	t.Run("empty directory returns empty map", func(t *testing.T) {
 		client := &mockGiteaClient{
 			contents: map[string][]ContentEntry{
 				RepoPersonaPath: {},
 			},
 		}
 		personas, err := LoadRepoPersonas(ctx, client, "owner", "repo")
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		if len(personas) != 0 {
 			t.Errorf("expected 0 personas, got %d", len(personas))
 		}
 	})
 	t.Run("loads valid personas", func(t *testing.T) {
 		client := &mockGiteaClient{
 			contents: map[string][]ContentEntry{
 				RepoPersonaPath: {
 					{Name: "trading.yaml", Path: ".review-bot/personas/trading.yaml", Type: "file"},
 					{Name: "crypto.yaml", Path: ".review-bot/personas/crypto.yaml", Type: "file"},
 				},
 			},
 			files: map[string]string{
 				".review-bot/personas/trading.yaml": `name: trading
 display_name: Trading Expert
 identity: You are a trading expert.
 focus:
  - order handling
  - risk management
 `,
 				".review-bot/personas/crypto.yaml": `name: crypto
 display_name: Crypto Expert
 identity: You are a cryptography expert.
 focus:
  - key management
  - encryption
 `,
 			},
 		}
 		personas, err := LoadRepoPersonas(ctx, client, "owner", "repo")
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		if len(personas) != 2 {
 			t.Fatalf("expected 2 personas, got %d", len(personas))
 		}
 		if personas["trading"] == nil {
 			t.Error("expected trading persona")
 		}
 		if personas["crypto"] == nil {
 			t.Error("expected crypto persona")
 		}
 		if personas["trading"].DisplayName != "Trading Expert" {
 			t.Errorf("trading display name = %q, want %q", personas["trading"].DisplayName, "Trading Expert")
 		}
 	})
 	t.Run("skips invalid persona files", func(t *testing.T) {
 		client := &mockGiteaClient{
 			contents: map[string][]ContentEntry{
 				RepoPersonaPath: {
 					{Name: "valid.yaml", Path: ".review-bot/personas/valid.yaml", Type: "file"},
 					{Name: "invalid.yaml", Path: ".review-bot/personas/invalid.yaml", Type: "file"},
 				},
 			},
 			files: map[string]string{
 				".review-bot/personas/valid.yaml": `name: valid
 identity: Valid persona
 `,
 				".review-bot/personas/invalid.yaml": "not valid yaml: [broken",
 			},
 		}
 		personas, err := LoadRepoPersonas(ctx, client, "owner", "repo")
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		// Should have the valid one, skip the invalid
 		if len(personas) != 1 {
 			t.Fatalf("expected 1 persona (skipped invalid), got %d", len(personas))
 		}
 		if personas["valid"] == nil {
 			t.Error("expected valid persona")
 		}
 	})
 	t.Run("skips non-yaml files", func(t *testing.T) {
 		client := &mockGiteaClient{
 			contents: map[string][]ContentEntry{
 				RepoPersonaPath: {
 					{Name: "persona.yaml", Path: ".review-bot/personas/persona.yaml", Type: "file"},
 					{Name: "README.md", Path: ".review-bot/personas/README.md", Type: "file"},
 					{Name: "notes.txt", Path: ".review-bot/personas/notes.txt", Type: "file"},
 				},
 			},
 			files: map[string]string{
 				".review-bot/personas/persona.yaml": `name: test
 identity: Test persona
 `,
 				".review-bot/personas/README.md": "# Personas\n\nPut your personas here.",
 			},
 		}
 		personas, err := LoadRepoPersonas(ctx, client, "owner", "repo")
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		if len(personas) != 1 {
 			t.Fatalf("expected 1 persona (yaml only), got %d", len(personas))
 		}
 	})
 	t.Run("skips subdirectories", func(t *testing.T) {
 		client := &mockGiteaClient{
 			contents: map[string][]ContentEntry{
 				RepoPersonaPath: {
 					{Name: "persona.yaml", Path: ".review-bot/personas/persona.yaml", Type: "file"},
 					{Name: "subdir", Path: ".review-bot/personas/subdir", Type: "dir"},
 				},
 			},
 			files: map[string]string{
 				".review-bot/personas/persona.yaml": `name: test
 identity: Test persona
 `,
 			},
 		}
 		personas, err := LoadRepoPersonas(ctx, client, "owner", "repo")
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		if len(personas) != 1 {
 			t.Fatalf("expected 1 persona (files only), got %d", len(personas))
 		}
 	})
 	t.Run("propagates auth errors", func(t *testing.T) {
 		client := &mockGiteaClient{
 			listErr: errors.New("HTTP 401: unauthorized"),
 		}
 		_, err := LoadRepoPersonas(ctx, client, "owner", "repo")
 		if err == nil {
 			t.Fatal("expected error for auth failure")
 		}
 		if !strings.Contains(err.Error(), "401") {
 			t.Errorf("error = %q, want containing '401'", err.Error())
 		}
 	})
 	t.Run("skips files that fail to fetch", func(t *testing.T) {
 		client := &mockGiteaClient{
 			contents: map[string][]ContentEntry{
 				RepoPersonaPath: {
 					{Name: "good.yaml", Path: ".review-bot/personas/good.yaml", Type: "file"},
 					{Name: "bad.yaml", Path: ".review-bot/personas/bad.yaml", Type: "file"},
 				},
 			},
 			files: map[string]string{
 				".review-bot/personas/good.yaml": `name: good
 identity: Good persona
 `,
 			},
 			fileErr: map[string]error{
 				".review-bot/personas/bad.yaml": errors.New("HTTP 500: internal server error"),
 			},
 		}
 		personas, err := LoadRepoPersonas(ctx, client, "owner", "repo")
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		if len(personas) != 1 {
 			t.Fatalf("expected 1 persona (skipped failed fetch), got %d", len(personas))
 		}
 	})
 	t.Run("skips oversized files", func(t *testing.T) {
 		// Create a content string that exceeds MaxPersonaFileSize (64KB)
 		oversizedContent := strings.Repeat("a", MaxPersonaFileSize+1)
 		client := &mockGiteaClient{
 			contents: map[string][]ContentEntry{
 				RepoPersonaPath: {
 					{Name: "normal.yaml", Path: ".review-bot/personas/normal.yaml", Type: "file"},
 					{Name: "huge.yaml", Path: ".review-bot/personas/huge.yaml", Type: "file"},
 				},
 			},
 			files: map[string]string{
 				".review-bot/personas/normal.yaml": `name: normal
 identity: Normal sized persona
 `,
 				".review-bot/personas/huge.yaml": oversizedContent,
 			},
 		}
 		personas, err := LoadRepoPersonas(ctx, client, "owner", "repo")
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		// Should have the normal one, skip the oversized
 		if len(personas) != 1 {
 			t.Fatalf("expected 1 persona (skipped oversized), got %d", len(personas))
 		}
 		if personas["normal"] == nil {
 			t.Error("expected normal persona")
 		}
 	})
 }
 func TestMergePersonas(t *testing.T) {
 	builtin := map[string]*Persona{
 		"security": {Name: "security", Identity: "Built-in security"},
 		"docs":     {Name: "docs", Identity: "Built-in docs"},
 	}
 	repo := map[string]*Persona{
 		"security": {Name: "security", Identity: "Repo security override"},
 		"trading":  {Name: "trading", Identity: "Repo trading"},
 	}
 	merged := MergePersonas(builtin, repo)
 	t.Run("repo overrides builtin on collision", func(t *testing.T) {
 		if merged["security"].Identity != "Repo security override" {
 			t.Errorf("security identity = %q, want repo override", merged["security"].Identity)
 		}
 	})
 	t.Run("builtin preserved when no collision", func(t *testing.T) {
 		if merged["docs"].Identity != "Built-in docs" {
 			t.Errorf("docs identity = %q, want built-in", merged["docs"].Identity)
 		}
 	})
 	t.Run("repo-only persona added", func(t *testing.T) {
 		if merged["trading"] == nil {
 			t.Error("expected trading persona from repo")
 		}
 		if merged["trading"].Identity != "Repo trading" {
 			t.Errorf("trading identity = %q, want repo", merged["trading"].Identity)
 		}
 	})
 	t.Run("original maps not modified", func(t *testing.T) {
 		if builtin["trading"] != nil {
 			t.Error("builtin map was modified")
 		}
 		if len(repo) != 2 {
 			t.Error("repo map was modified")
 		}
 	})
 }
 func TestGetBuiltinPersonasMap(t *testing.T) {
 	personas := GetBuiltinPersonasMap()
 	if len(personas) == 0 {
 		t.Fatal("expected at least one built-in persona")
 	}
 	// Verify expected personas exist
 	expected := []string{"security", "architect", "docs"}
 	for _, name := range expected {
 		if personas[name] == nil {
 			t.Errorf("expected built-in persona %q", name)
 		}
 	}
 	// Verify personas are valid
 	for name, p := range personas {
 		if p.Name != name {
 			t.Errorf("persona %q has mismatched name %q", name, p.Name)
 		}
 		if p.Identity == "" {
 			t.Errorf("persona %q has empty identity", name)
 		}
 	}
 }
 func TestIsYAMLFile(t *testing.T) {
 	tests := []struct {
 		name string
 		want bool
 	}{
 		{"test.yaml", true},
 		{"test.yml", true},
 		{"test.YAML", true},
 		{"test.YML", true},
 		{"test.json", false},
 		{"test.md", false},
 		{"test.txt", false},
 		{"yaml", false},
 		{"yaml.md", false},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			if got := isYAMLFile(tt.name); got != tt.want {
 				t.Errorf("isYAMLFile(%q) = %v, want %v", tt.name, got, tt.want)
 			}
 		})
 	}
 }
 func TestIsNotFoundError(t *testing.T) {
 	tests := []struct {
 		err  error
 		want bool
 	}{
 		{nil, false},
 		{errors.New("HTTP 404: not found"), true},
 		{errors.New("HTTP 404"), true},
 		// Intentionally false: generic "not found" could mask auth/transport errors.
 		// Only explicit HTTP 404 responses should be treated as "directory doesn't exist".
 		{errors.New("something not found"), false},
 		{errors.New("HTTP 401: unauthorized"), false},
 		{errors.New("connection refused"), false},
 	}
 	for _, tt := range tests {
 		name := "nil"
 		if tt.err != nil {
 			name = tt.err.Error()
 		}
 		t.Run(name, func(t *testing.T) {
 			if got := isNotFoundError(tt.err); got != tt.want {
 				t.Errorf("isNotFoundError(%v) = %v, want %v", tt.err, got, tt.want)
 			}
 		})
 	}
 }
@@ -1,127 +0,0 @@
 #!/usr/bin/env bash
 # check-deps.sh - Enforces the strict dependency allowlist from CONVENTIONS.md
 # Exit 1 if any unapproved import is found.
 #
 # Requires: Bash 4+ (for associative arrays), Go toolchain
 # 
 # The allowlist is parsed from CONVENTIONS.md to maintain a single source of truth.
 # Enforces Scope column: "test only" packages cannot appear in non-test code.
 set -euo pipefail
 # Check bash version
 if ((BASH_VERSINFO[0] < 4)); then
    echo "❌ Bash 4+ required (found ${BASH_VERSION})"
    echo "   On macOS: brew install bash"
    exit 1
 fi
 CONVENTIONS_FILE="${1:-CONVENTIONS.md}"
 if [ ! -f "$CONVENTIONS_FILE" ]; then
    echo "❌ CONVENTIONS.md not found"
    exit 1
 fi
 # Parse approved packages from CONVENTIONS.md table using awk (POSIX-compatible)
 # Format: | `package` | use case | scope |
 declare -A ALLOWED_PROD=()
 declare -A ALLOWED_TEST=()
 while IFS= read -r line; do
    # Use awk to extract package and scope from table row
    pkg=$(echo "$line" | awk -F'|' '{gsub(/^[[:space:]]*`|`[[:space:]]*$/, "", $2); print $2}')
    scope=$(echo "$line" | awk -F'|' '{gsub(/^[[:space:]]+|[[:space:]]+$/, "", $4); print tolower($4)}')
    if [ -n "$pkg" ] && [ "$pkg" != "Package" ] && [[ "$pkg" =~ ^[a-zA-Z] ]]; then
        if [[ "$scope" == *"test"* ]]; then
            ALLOWED_TEST["$pkg"]=1
        else
            ALLOWED_PROD["$pkg"]=1
        fi
    fi
 done < <(grep '| `' "$CONVENTIONS_FILE" 2>/dev/null || true)
 ALL_ALLOWED=("${!ALLOWED_PROD[@]}" "${!ALLOWED_TEST[@]}")
 if [ ${#ALL_ALLOWED[@]} -eq 0 ]; then
    echo "⚠️  No approved packages found in $CONVENTIONS_FILE"
    echo "   (This is fine if you want stdlib-only)"
 fi
 # Helper: check if import matches any package in an associative array (literal prefix, no glob)
 matches_allowlist() {
    local import="$1"
    shift
    local -n allowlist=$1
    for allowed in "${!allowlist[@]}"; do
        # Exact match
        if [ "$import" = "$allowed" ]; then
            return 0
        fi
        # Literal prefix match for subpackages: must match "pkg/" exactly
        if [ "${import#"$allowed/"}" != "$import" ]; then
            return 0
        fi
    done
    return 1
 }
 # Get direct module dependencies from go.mod
 DIRECT_IMPORTS=$(go list -m -f '{{if and (not .Indirect) (not .Main)}}{{.Path}}{{end}}' all 2>&1) || {
    echo "❌ Failed to list dependencies: $DIRECT_IMPORTS"
    exit 1
 }
 DIRECT_IMPORTS=$(echo "$DIRECT_IMPORTS" | grep -v '^$' || true)
 if [ -z "$DIRECT_IMPORTS" ]; then
    echo "✅ No external dependencies"
    exit 0
 fi
 # Check ALL direct dependencies are in some allowlist
 VIOLATIONS=""
 while IFS= read -r import; do
    [ -z "$import" ] && continue
    if ! matches_allowlist "$import" ALLOWED_PROD && ! matches_allowlist "$import" ALLOWED_TEST; then
        VIOLATIONS="${VIOLATIONS}  - ${import} (not in allowlist)"$'\n'
    fi
 done <<< "$DIRECT_IMPORTS"
 if [ -n "$VIOLATIONS" ]; then
    echo "❌ UNAPPROVED DEPENDENCIES DETECTED"
    echo ""
    echo "The following imports are not in the allowlist:"
    printf "%s" "$VIOLATIONS"
    echo ""
    echo "To add a dependency, update CONVENTIONS.md (requires Aaron's approval)"
    exit 1
 fi
 # Enforce Scope: test-only packages must not appear in non-test code
 # Get imports used by non-test code only (go list -deps without -test excludes test deps)
 PROD_IMPORTS=$(go list -deps -f '{{if not .Standard}}{{.ImportPath}}{{end}}' ./... 2>/dev/null || true)
 TEST_ONLY_IN_PROD=""
 for test_pkg in "${!ALLOWED_TEST[@]}"; do
    # Use word-boundary matching: exact match or followed by /
    if echo "$PROD_IMPORTS" | grep -qE "^${test_pkg}(/|\$|$)"; then
        TEST_ONLY_IN_PROD="${TEST_ONLY_IN_PROD}  - ${test_pkg} (marked 'test only' but used in production code)"$'\n'
    fi
 done
 if [ -n "$TEST_ONLY_IN_PROD" ]; then
    echo "❌ TEST-ONLY DEPENDENCIES IN PRODUCTION CODE"
    echo ""
    printf "%s" "$TEST_ONLY_IN_PROD"
    echo ""
    echo "These packages are marked 'test only' in CONVENTIONS.md"
    echo "and must only be imported from *_test.go files."
    exit 1
 fi
 echo "✅ All dependencies are approved"
 echo "   Direct module deps: $(echo "$DIRECT_IMPORTS" | wc -l | tr -d ' ')"
 echo "   Production allowlist: ${#ALLOWED_PROD[@]}, Test-only allowlist: ${#ALLOWED_TEST[@]}"
Author	SHA1	Message	Date
Rodin	44c80c36cf	fix: use bedrock-2023-05-31 for AI Core Anthropic version CI / test (pull_request) Successful in 15s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 38s Details CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m11s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m52s Details SAP AI Core's Anthropic endpoint uses AWS Bedrock API format, not native Anthropic API. Verified via integration testing: - 2023-06-01: Invalid API version - bedrock-2023-05-31: Works ✓	2026-05-09 23:48:21 -07:00
Rodin	f71f26fcff	fix: remove anthropic_version from body - AI Core rejects it CI / test (pull_request) Successful in 15s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Failing after 17s Details CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m37s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m2s Details	2026-05-09 23:45:11 -07:00
Rodin	8da8fca19d	fix: add omitempty to model field so it's not sent when empty CI / test (pull_request) Successful in 15s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Failing after 16s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m25s Details CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 2m39s Details	2026-05-09 23:39:22 -07:00
Rodin	b12df1a636	test: update Anthropic test to check anthropic_version instead of model CI / test (pull_request) Successful in 16s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Failing after 16s Details CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m33s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m50s Details	2026-05-09 23:36:42 -07:00
Rodin	d13e062866	fix: omit model field from AI Core Anthropic request CI / test (pull_request) Failing after 10s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Has been skipped Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Has been skipped Details CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Has been skipped Details AI Core deployment URL already specifies the model. Sending model in the body causes 'Extra inputs are not permitted' error.	2026-05-09 23:28:22 -07:00
Rodin	b76270c21b	fix: put anthropic_version in request body, not header CI / test (pull_request) Successful in 12s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Failing after 17s Details CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m50s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m43s Details SAP AI Core expects anthropic_version in the JSON body, not as a header.	2026-05-09 23:23:42 -07:00
Rodin	b92a968d93	fix: add anthropic-version header for AI Core Anthropic endpoint CI / test (pull_request) Successful in 14s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Failing after 17s Details CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m4s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m9s Details	2026-05-09 23:19:00 -07:00
Rodin	d02c75486e	ci: switch to native aicore provider, remove HAI proxy dependency CI / test (pull_request) Successful in 14s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Failing after 18s Details CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m10s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m44s Details	2026-05-09 23:14:46 -07:00
Rodin	34507dd9ff	fix: propagate LLM timeout to AI Core client CI / test (pull_request) Successful in 15s Details CI / review (/anthropic/v1, anthropic--claude-4.6-sonnet, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Successful in 32s Details CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m58s Details CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m16s Details Address review feedback: MAJOR: - AICoreClient now defaults to 5min timeout (matching Client) - Add AICoreClient.WithTimeout() for explicit timeout control - Client.WithAICore() inherits parent client's timeout - Client.WithTimeout() propagates to aicore client if set MINOR: - Extract AICoreOpenAIAPIVersion constant for the hardcoded api-version - Tighten IsAnthropicModel to only match 'anthropic--' prefix (SAP AI Core always uses this prefix for Anthropic models) NIT: - Use fmt.Sprintf for token generation in tests (robust for >9 calls) - Add error checking in TestAICoreClient_TokenExpiry - Add tests for WithTimeout propagation	2026-05-09 22:29:19 -07:00
Rodin	a62b791b9e	ci: use SAP AI Core model names and remove unavailable models CI / test (pull_request) Successful in 15s Details CI / review (/anthropic/v1, anthropic--claude-4.6-sonnet, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Successful in 34s Details CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 2m10s Details CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m21s Details - Use 'anthropic--claude-4.6-sonnet' instead of 'claude-sonnet-4-6' (hai-aicore proxy requires the 'anthropic--' prefix) - Remove gpt-4.1, gpt-4.1-mini, gpt-5-mini reviewers since those models are not deployed in SAP AI Core - Keep sonnet, gpt-5, and security reviews	2026-05-09 21:58:31 -07:00
Rodin	c3ec44a87b	chore: retrigger CI after LLM_BASE_URL fix CI / test (pull_request) Successful in 14s Details CI / review (/openai/v1, gpt-4.1-mini, gpt41-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 16s Details CI / review (/anthropic/v1, claude-sonnet-4-6, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Failing after 17s Details CI / review (/openai/v1, gpt-4.1, gpt41, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 16s Details CI / review (/openai/v1, gpt-5-mini, gpt5-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 13s Details CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m17s Details CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m2s Details	2026-05-09 21:53:51 -07:00
claw	cf453504cb	feat: add native SAP AI Core support CI / test (pull_request) Successful in 13s Details CI / review (/anthropic/v1, claude-sonnet-4-6, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Failing after 12s Details CI / review (/openai/v1, gpt-4.1, gpt41, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 12s Details CI / review (/openai/v1, gpt-4.1-mini, gpt41-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 14s Details CI / review (/openai/v1, gpt-5-mini, gpt5-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 11s Details CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m12s Details CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m28s Details Adds a new 'aicore' LLM provider that authenticates directly with SAP AI Core using OAuth client credentials. This eliminates the need for an external proxy (hai-aicore or hai) when running review-bot in SAP environments. Key changes: - New llm/aicore.go with AICoreClient for OAuth token management and deployment discovery - Thread-safe token caching with automatic refresh before expiry - Automatic routing to /invoke (Anthropic) or /chat/completions (OpenAI) based on model name - New CLI flags: --aicore-client-id, --aicore-client-secret, --aicore-auth-url, --aicore-api-url, --aicore-resource-group - Updated action.yml with corresponding inputs - Comprehensive test coverage for AI Core client Example usage in CI: llm-provider: aicore llm-model: anthropic--claude-4.6-sonnet 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 }} Closes #49	2026-05-08 17:49:26 -07:00
		`@@ -1,2 +0,0 @@`
			`github.com/goccy/go-yaml v1.19.2 h1:PmFC1S6h8ljIz6gMRBopkjP1TVT7xuwrButHID66PoM=`
			`github.com/goccy/go-yaml v1.19.2/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA=`