From 7375d74b2014b6584f3892d93b31b57513501e27 Mon Sep 17 00:00:00 2001 From: Bruce Arctor <5032356+brucearctor@users.noreply.github.com> Date: Mon, 13 Jul 2026 21:36:12 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20REVIEW.md=20=E2=80=94=20repo-level?= =?UTF-8?q?=20review=20instructions=20with=20highest=20prompt=20priority?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discover REVIEW.md at repo root (walked up from cwd, same as .code-reviewer.yaml). Its contents are injected at the end of the system prompt as the highest-priority instruction block. Prompt priority chain (highest last, due to LLM recency bias): 1. Base prompt (or --custom-prompt file) 2. Focus overlays (--focus bugs,security,...) 3. Extra rules (--extra-rules) 4. REVIEW.md (HIGHEST PRIORITY) This matches the Claude Code Review convention where REVIEW.md shapes what gets flagged, at what severity, and how findings are reported. Changes: - config.go: ReviewMD field, loaded alongside .code-reviewer.yaml - prompt.go: new BuildPromptFull() with reviewMD parameter - reviewer.go: wired to use BuildPromptFull - main.go: log review_md=true/false at startup - prompt_test.go: 5 new tests covering injection, ordering, empty --- cmd/code-reviewer/main.go | 1 + internal/config/config.go | 44 +++++++++++++++++++------ internal/model/prompt.go | 21 +++++++++++- internal/model/prompt_test.go | 62 +++++++++++++++++++++++++++++++++++ internal/reviewer/reviewer.go | 2 +- 5 files changed, 118 insertions(+), 12 deletions(-) diff --git a/cmd/code-reviewer/main.go b/cmd/code-reviewer/main.go index cb50688..c2a7077 100644 --- a/cmd/code-reviewer/main.go +++ b/cmd/code-reviewer/main.go @@ -61,6 +61,7 @@ func run(ctx, initCtx context.Context) (int, error) { "chunk_strategy", cfg.ChunkStrategy, "dry_run", cfg.DryRun, "proxy_enabled", cfg.ProxyURL != "", + "review_md", cfg.ReviewMD != "", ) // Create model provider(s). diff --git a/internal/config/config.go b/internal/config/config.go index dd87651..378cce7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -92,6 +92,7 @@ type Config struct { ExtraRules string CustomPrompt string // Path to custom system prompt file. Incremental bool // Only review files changed in the latest push (CI mode). + ReviewMD string // Contents of REVIEW.md (repo-level review instructions). // Output settings. CommentMode CommentMode @@ -179,25 +180,48 @@ func Load() (*Config, error) { } func (c *Config) loadRepoConfig() error { - // Walk up from cwd to find .code-reviewer.yaml. + // Walk up from cwd to find .code-reviewer.yaml and REVIEW.md. dir, err := os.Getwd() if err != nil { return nil // Non-fatal: skip yaml config. } + var foundYAML bool for { - path := filepath.Join(dir, ".code-reviewer.yaml") - data, err := os.ReadFile(path) - if err == nil { - return c.applyRepoConfig(data) + // Try to load .code-reviewer.yaml/.yml (stop walking after first match). + if !foundYAML { + path := filepath.Join(dir, ".code-reviewer.yaml") + data, err := os.ReadFile(path) + if err == nil { + if err := c.applyRepoConfig(data); err != nil { + return err + } + foundYAML = true + } else { + path = filepath.Join(dir, ".code-reviewer.yml") + data, err = os.ReadFile(path) + if err == nil { + if err := c.applyRepoConfig(data); err != nil { + return err + } + foundYAML = true + } + } } - // Also check .yml extension. - path = filepath.Join(dir, ".code-reviewer.yml") - data, err = os.ReadFile(path) - if err == nil { - return c.applyRepoConfig(data) + + // Try to load REVIEW.md (only if not already found). + if c.ReviewMD == "" { + path := filepath.Join(dir, "REVIEW.md") + data, err := os.ReadFile(path) + if err == nil { + c.ReviewMD = strings.TrimSpace(string(data)) + } } + // Stop if both found or reached root. + if foundYAML && c.ReviewMD != "" { + break + } parent := filepath.Dir(dir) if parent == dir { break // Reached filesystem root. diff --git a/internal/model/prompt.go b/internal/model/prompt.go index 5a73c1f..0d18ff6 100644 --- a/internal/model/prompt.go +++ b/internal/model/prompt.go @@ -119,13 +119,23 @@ Concentrate on documentation quality: // BuildPrompt constructs the full system prompt for a review call. // Uses the built-in basePrompt as the system prompt. func BuildPrompt(focusModes []string, extraRules string) string { - return BuildPromptWithCustom("", focusModes, extraRules) + return BuildPromptFull("", "", focusModes, extraRules) } // BuildPromptWithCustom constructs the system prompt, optionally loading a custom // prompt from disk. If customPromptPath is non-empty, its contents replace the // built-in base prompt. Focus overlays and extra rules are always appended. func BuildPromptWithCustom(customPromptPath string, focusModes []string, extraRules string) string { + return BuildPromptFull(customPromptPath, "", focusModes, extraRules) +} + +// BuildPromptFull constructs the complete system prompt with all layers. +// Priority (highest last, due to LLM recency bias): +// 1. Base prompt (or custom prompt file) +// 2. Focus overlays +// 3. Extra rules +// 4. REVIEW.md instructions (highest priority) +func BuildPromptFull(customPromptPath, reviewMD string, focusModes []string, extraRules string) string { var sb strings.Builder // Base prompt: custom file or built-in. @@ -167,6 +177,15 @@ func BuildPromptWithCustom(customPromptPath string, focusModes []string, extraRu sb.WriteString(extraRules) } + // Append REVIEW.md instructions (highest priority — placed last so LLMs + // attend to it most strongly due to recency bias). + if reviewMD != "" { + sb.WriteString("\n\n## REVIEW INSTRUCTIONS (HIGHEST PRIORITY)\n\n") + sb.WriteString("The following are repository-specific review instructions from REVIEW.md. ") + sb.WriteString("They take precedence over all other guidance.\n\n") + sb.WriteString(reviewMD) + } + return sb.String() } diff --git a/internal/model/prompt_test.go b/internal/model/prompt_test.go index b695d14..191636c 100644 --- a/internal/model/prompt_test.go +++ b/internal/model/prompt_test.go @@ -136,3 +136,65 @@ func TestBuildPromptWithCustom_EmptyPath(t *testing.T) { } } +func TestBuildPromptFull_ReviewMD(t *testing.T) { + reviewMD := "## Always check\n- New API routes have integration tests\n- No PII in logs" + prompt := BuildPromptFull("", reviewMD, []string{"bugs"}, "") + + if !strings.Contains(prompt, "REVIEW INSTRUCTIONS (HIGHEST PRIORITY)") { + t.Error("prompt should contain REVIEW INSTRUCTIONS header") + } + if !strings.Contains(prompt, "New API routes have integration tests") { + t.Error("prompt should contain REVIEW.md content") + } + if !strings.Contains(prompt, "No PII in logs") { + t.Error("prompt should contain all REVIEW.md content") + } + // Verify REVIEW.md comes after base prompt and focus overlays. + baseIdx := strings.Index(prompt, "Principal Software Engineer") + reviewIdx := strings.Index(prompt, "REVIEW INSTRUCTIONS") + if reviewIdx <= baseIdx { + t.Error("REVIEW.md should appear after base prompt (recency = highest priority)") + } +} + +func TestBuildPromptFull_ReviewMD_AfterExtraRules(t *testing.T) { + reviewMD := "Only report CRITICAL severity." + prompt := BuildPromptFull("", reviewMD, []string{"all"}, "Flag raw SQL.") + + rulesIdx := strings.Index(prompt, "ADDITIONAL RULES") + reviewIdx := strings.Index(prompt, "REVIEW INSTRUCTIONS") + if rulesIdx < 0 || reviewIdx < 0 { + t.Fatal("both ADDITIONAL RULES and REVIEW INSTRUCTIONS should be present") + } + if reviewIdx <= rulesIdx { + t.Error("REVIEW.md should appear after extra rules (highest priority = last)") + } +} + +func TestBuildPromptFull_EmptyReviewMD(t *testing.T) { + prompt := BuildPromptFull("", "", []string{"bugs"}, "") + if strings.Contains(prompt, "REVIEW INSTRUCTIONS") { + t.Error("empty reviewMD should not inject REVIEW INSTRUCTIONS section") + } +} + +func TestBuildPromptFull_WithCustomPromptAndReviewMD(t *testing.T) { + dir := t.TempDir() + promptFile := filepath.Join(dir, "custom.md") + if err := os.WriteFile(promptFile, []byte("You are a security auditor."), 0o644); err != nil { + t.Fatal(err) + } + + reviewMD := "Focus on SQL injection only." + prompt := BuildPromptFull(promptFile, reviewMD, []string{"security"}, "") + + if !strings.Contains(prompt, "You are a security auditor.") { + t.Error("custom prompt should be used as base") + } + if !strings.Contains(prompt, "Focus on SQL injection only.") { + t.Error("REVIEW.md should be appended") + } + if strings.Contains(prompt, "Principal Software Engineer") { + t.Error("built-in base should not be present when custom prompt is used") + } +} diff --git a/internal/reviewer/reviewer.go b/internal/reviewer/reviewer.go index aab1fe0..08ecc7a 100644 --- a/internal/reviewer/reviewer.go +++ b/internal/reviewer/reviewer.go @@ -125,7 +125,7 @@ func (r *Reviewer) Run(ctx context.Context) (int, error) { slog.Info(fmt.Sprintf("review split into %d chunk(s)", len(chunks))) // Step 4: Build prompt and call model for each chunk. - systemPrompt := model.BuildPromptWithCustom(r.cfg.CustomPrompt, r.cfg.Focus, r.cfg.ExtraRules) + systemPrompt := model.BuildPromptFull(r.cfg.CustomPrompt, r.cfg.ReviewMD, r.cfg.Focus, r.cfg.ExtraRules) var allFindings []model.Finding var summary string var totalUsage model.TokenUsage From aa243b2214a515f5ae2be9eb09b7fbc5fb3af73c Mon Sep 17 00:00:00 2001 From: Bruce Arctor <5032356+brucearctor@users.noreply.github.com> Date: Fri, 17 Jul 2026 16:54:05 -0700 Subject: [PATCH 2/2] fix: source REVIEW.md from trusted base ref in CI mode In CI mode, contributor-controlled branches could add a REVIEW.md that manipulates review behavior (e.g. 'report no findings'). Now in CI mode with a known base SHA (CI_MERGE_REQUEST_DIFF_BASE_SHA), REVIEW.md is read from the target/base ref via 'git show' instead of the working tree. This ensures only repo maintainers (who control the default branch) can set review instructions. In local --diff mode the filesystem version is still used since the developer controls the checkout. Includes command-injection guard (reject refs starting with '-') consistent with the existing pattern in getLocalDiffs(). --- internal/reviewer/reviewer.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/internal/reviewer/reviewer.go b/internal/reviewer/reviewer.go index 2521ce1..68b773b 100644 --- a/internal/reviewer/reviewer.go +++ b/internal/reviewer/reviewer.go @@ -179,7 +179,13 @@ func (r *Reviewer) Run(ctx context.Context) (int, error) { } // Step 4: Build prompt and call model for each chunk. - systemPrompt := model.BuildPromptFull(r.cfg.CustomPrompt, r.cfg.ReviewMD, r.cfg.Focus, r.cfg.ExtraRules) + // In CI mode, source REVIEW.md from the trusted base/target ref so that + // contributor-controlled branches cannot inject review instructions. + reviewMD := r.cfg.ReviewMD + if r.cfg.CIMode && r.cfg.CIDiffBaseSHA != "" { + reviewMD = readReviewMDFromRef(r.cfg.CIDiffBaseSHA) + } + systemPrompt := model.BuildPromptFull(r.cfg.CustomPrompt, reviewMD, r.cfg.Focus, r.cfg.ExtraRules) var allFindings []model.Finding var summary string var totalUsage model.TokenUsage @@ -469,3 +475,25 @@ func findRepoRoot() string { dir = parent } } + +// readReviewMDFromRef reads REVIEW.md from a specific git ref (e.g. base commit SHA). +// Returns empty string if the file doesn't exist at that ref or git fails. +func readReviewMDFromRef(ref string) string { + // Prevent command injection: reject refs that look like flags. + if strings.HasPrefix(ref, "-") { + slog.Warn("invalid git ref for REVIEW.md lookup, skipping", "ref", ref) + return "" + } + cmd := exec.Command("git", "show", ref+":REVIEW.md") + output, err := cmd.Output() + if err != nil { + // File doesn't exist at this ref — this is normal and expected. + slog.Debug("REVIEW.md not found at base ref", "ref", ref) + return "" + } + content := strings.TrimSpace(string(output)) + if content != "" { + slog.Info("loaded REVIEW.md from trusted base ref", "ref", ref[:min(len(ref), 12)]) + } + return content +}