go-review-lead

name	go-review-lead
description	Orchestrate a comprehensive Go code review by triaging code changes, dispatching vertical review skills (security, concurrency, error, logic, performance, quality, test, observability) as parallel agents, then consolidating results into a unified report. Use for full Go PR review or comprehensive code review. Replaces the monolithic go-code-reviewer with focused parallel analysis.
allowed-tools	Read, Grep, Glob, Bash(go build*), Bash(go vet*), Bash(git diff*), Bash(git log*), Agent

Go Review Lead — Orchestrator

Purpose

Triage Go code changes and dispatch vertical review skills as parallel agents, then consolidate their findings into a unified report. This skill is the entry point for comprehensive Go code review.

Critical rule: The Lead Agent only triages and consolidates. It never reviews code itself. All code review happens in vertical skill agents.

Quick Reference

When you need to…	Jump to
Analyze diff scope + select review depth	§Step 1 + §Step 1.5
Run compile pre-check	§Step 2
Triage → decide which agents to dispatch	§Step 3 (Triage Rules)
Dispatch agents in parallel	§Step 3 (Dispatch) — all calls in one response
Deduplicate and consolidate findings	§Step 4
Format the final report	§Report Format
See a complete formatted report example	Load references/example-output.md
Worker findings JSON contract (dispatch + parse)	Load references/findings-schema.md

When To Use

• Full Go PR review or comprehensive code review requested
• User asks for "Go code review" without specifying a specific dimension
• Need to cover multiple review dimensions in one pass

When NOT To Use

• User explicitly asks for single-dimension review (e.g., "check security only") → dispatch the specific vertical skill directly
• Non-Go code review

Workflow

Step 1: Analyze Scope

Run the following commands to gather triage data. Diff against the merge-base of the main branch so multi-commit branches are reviewed in full — a bare HEAD~1 sees only the last commit and silently skips the rest of the PR:

BASE=$(git merge-base origin/main HEAD 2>/dev/null || git merge-base main HEAD 2>/dev/null || git rev-parse HEAD~1)
git diff --name-only "$BASE"   # changed files
git diff "$BASE"               # full diff

(HEAD~1 is the last-resort fallback for repos with no main branch; state in the report when it was used.) Identify:

• Changed .go files (production) and _test.go files (separate tracking)
• New files added (stronger signals — more domains likely affected)
• Impact radius: exported interface changes → search implementors; exported signature changes → search callers

Raw snippet fallback: If no git context is available (user pasted a code snippet with no file path or diff):

1. Write the snippet to $TMPDIR/review_snippet.go
2. Run all grep-based triage (Phases 1-4) against the temp file
3. Apply the same dispatch validation grep below
4. Clean up the temp file after review

Step 1.5: Select Review Depth (Lite / Standard / Strict)

After analyzing scope, select the review depth before triage. State the selected depth and reasoning in the report's Review Mode section.

Selection Rules

Depth	Trigger Condition	Agent Policy	Finding Soft Cap
Lite	Changed files ≤ 3 AND none of the high-risk signals below	Only always-dispatch: go-quality-reviewer + go-logic-reviewer (2 agents)	5
Standard	Does not qualify for Lite or Strict (default)	Normal 4-phase triage, dispatch as needed	10
Strict	Any high-risk signal present OR > 15 changed Go files OR user explicitly requests comprehensive/strict review	Full 4-phase triage + Dispatch Validation Grep safety net; Residual Risk section required non-empty	15

High-risk signals (any one triggers Strict):

• Security/auth code changed (auth/, middleware/, token/, session/, jwt/, oauth/)
• Concurrency primitives added/modified (go func, sync., chan, errgroup)
• HTTP/API contract changed (exported handler signature, route registration, response schema)
• Persistence/schema changed (sql, migration files, ORM model changes)
• Exported function/interface signature changed
• Broad refactor (> 15 production Go files)

Lite safeguard: Even in Lite mode, if the 2 always-dispatched agents collectively report ≥ 3 High findings, escalate to Standard and re-triage with the full 4-phase process. State the escalation in the report.

Depth-Specific Dispatch Behavior

Lite:

• Skip Steps 1.5b (Compile Pre-check) and 1.6 (Dispatch Validation Grep)
• Skip Step 2 (4-phase triage) — only dispatch go-quality-reviewer + go-logic-reviewer
• Sub-agent prompt appendix: "Lite mode: focus on high-confidence defects only, soft cap 3 findings per agent"

Standard:

• Execute all steps as documented (compile pre-check, 4-phase triage, dispatch validation grep)
• Apply conditional scope narrowing (Step 3) to reduce cross-agent duplication

Strict:

• Execute all steps; do NOT apply scope narrowing — cross-agent duplication serves as cross-validation safety net
• Sub-agent prompt appendix: "Strict mode: full checklist, no item skipped, report all confirmed findings"

Step 1.5b: Compile Pre-check (Standard / Strict Only)

Before triage, run a quick compilation check to catch syntax and type errors that every agent would otherwise redundantly report:

# For file-based review:
cd <repo_root> && go build ./... 2>&1 | head -30

# For raw snippet:
cd $TMPDIR && go build review_snippet.go 2>&1

If compile errors exist:

1. Lead Agent reports compile errors directly as High findings (category: Compile) — these do not need domain expertise
2. Include them in the final report with IDs starting from REV-001
3. Proceed with triage and dispatch for non-compile issues — sub-agents still review logic, concurrency, performance, etc.
4. Add to each sub-agent's dispatch prompt: "Note: compile errors (listed below) are already reported by Lead. Do NOT re-report them. Focus on runtime, logic, and domain-specific issues only." followed by a brief list of the compile error locations

If no compile errors: proceed normally.

Rationale: In the getBatchUser benchmark case, 3 compile errors were independently reported by 4 agents (12 raw findings → 3 merged), consuming ~35% of the redundant token budget. Pre-checking eliminates this entire class of duplication.

Step 1.6: Dispatch Validation Grep (Safety Net — Standard / Strict Only)

Before finalizing which skills to skip, run quick grep for each conditional skill's top trigger patterns against all changed files (or the temp snippet file). If ANY pattern hits, override the skip decision and dispatch that skill.

This catches triage logic gaps where manual Phase 1-4 analysis misses a pattern.

# Run these greps against all changed .go files:
# Performance triggers:
grep -l 'make\(\[\]\|append(' <files>
grep -l 'for.*range.*\.\(Query\|Exec\|Get\|Set\)' <files>
# Concurrency triggers:
grep -l 'go func\|go .*(' <files>
grep -l 'sync\.\|chan ' <files>
# Security triggers:
grep -l '"database/sql"\|"os/exec"\|Sprintf.*SELECT\|Sprintf.*INSERT' <files>
grep -l 'http\.Handle\|http\.ListenAndServe' <files>
# Error triggers:
grep -l '\.Close()\|tx\.\|resp\.Body\|sql\.Rows' <files>
# Test triggers:
grep -l '_test\.go' <file_list>

Skill	Validation Patterns (ANY hit → dispatch)
go-performance-reviewer	make\(\[\], append\(, for.*range.*\.\(Query|Exec\)
go-concurrency-reviewer	go\s+func, sync\., chan\s, make\(chan
go-security-reviewer	"database/sql", "os/exec", http\.Handle, Sprintf.*SELECT
go-error-reviewer	\.Close\(\), tx\., resp\.Body, sql\.Rows, panic\(
go-test-reviewer	_test\.go in changed file list
go-observability-reviewer	"go.uber.org/zap", "log/slog", "go.opentelemetry.io/, prometheus\.New

Override rule: If validation grep hits but Phases 1-4 did not trigger the skill, add it with rationale: "Dispatch-validation grep override: {pattern} found in {file}".

Step 2: Triage — Select Vertical Skills (Multi-Phase)

Execute the 4 triage phases in order. Each phase independently contributes dispatch decisions — run all 4 even if earlier phases already trigger agents.

Two agents are Always dispatched regardless of phases:

• go-quality-reviewer — baseline lint/style for any Go change
• go-logic-reviewer — baseline correctness check for any behavior change

For the remaining 6 (security, concurrency, error, performance, test, observability), use the phases below:

---

Phase 1: Import Analysis (Strongest Signal)

Scan import blocks across ALL changed files (not just diff lines):

grep -n '"database/sql"\|"os/exec"\|"net/http"\|"sync"\|"crypto/\|"math/rand"\|"text/template"\|"html/template"' <changed_files>
grep -n 'errgroup\|singleflight\|semaphore' <changed_files>
grep -n '"go.uber.org/zap"\|"log/slog"\|"go.opentelemetry.io/\|"github.com/prometheus/client_golang/\|"github.com/rs/zerolog"' <changed_files>

Import present	→ Dispatch
"database/sql"	security + error + performance
"os/exec"	security
"net/http" (as server — http.Handler, http.HandleFunc)	security
"net/http" (as client — http.Get, http.NewRequest, http.Client)	security + performance
"crypto/md5", "crypto/sha1", "math/rand"	security
"sync", "sync/atomic"	concurrency
"golang.org/x/sync/...", errgroup, singleflight	concurrency
"html/template", "text/template" with user input context	security
"go.uber.org/zap", "log/slog", "go.opentelemetry.io/..."	observability
"github.com/prometheus/client_golang/", "github.com/rs/zerolog"	observability

---

Phase 2: Diff Pattern Scan (Changed Lines Only)

Scan only lines added/modified in the diff (^+ lines, excluding ^+++):

git diff "$BASE" | grep '^+[^+]'

Pattern in added/modified lines	→ Dispatch
go func, make(chan, <-chan, chan<-	concurrency
sync\.Mutex, sync\.RWMutex, sync\.WaitGroup, select {	concurrency
sql\.Rows, tx\.Begin, tx\.Commit, tx\.Rollback	error
resp\.Body, defer.*\.Close(), panic(	error
make\(\[\][^,)]+,[^,)]+\) (2-arg slice, no capacity: make([]T, n))	performance
append\( co-occurring with make\(\[\] in same function/file	performance
make(map (any map literal, with or without size hint)	performance
strings\.Builder, bytes\.Buffer replacing string concat	performance
loop body containing .Query, .Exec, .Get, .Set (DB/cache)	performance
hardcoded string matching [A-Za-z0-9+/]{20,}= or sk-, pk-, secret	security
filepath\.Join.*r\., os\.Open.*r\., user input flowing into file paths	security
tls\.Config{, InsecureSkipVerify	security
tracer\.Start, span\.End, span\.RecordError, span\.SetStatus	observability
prometheus\.NewCounter, prometheus\.NewGauge, prometheus\.NewHistogram, prometheus\.NewSummary	observability
WithLabelValues(, zap\.L(), slog\.Info(, slog\.Error(	observability

---

Phase 3: File Path Heuristics

Check the paths of changed files — certain packages indicate domain risk regardless of diff patterns:

File path contains	→ Dispatch
auth/, middleware/, token/, session/, jwt/, oauth/, permission/	security
handler/, server/, api/, router/, endpoint/	security
worker/, pool/, queue/, scheduler/, job/, pipeline/	concurrency
repo/, store/, dao/, repository/, db/	error + performance
cache/, redis/, memcache/	performance
observability/, telemetry/, instrumentation/, metrics/, tracing/, logging/	observability

---

Phase 4: Change Scope Assessment

Condition	→ Action
Any _test.go file in diff	dispatch go-test-reviewer
New .go file added (not test)	add error (new code has no established error patterns yet)
> 5 production Go files changed	verify error is already dispatched; add if not
Exported interface definition changed	ensure logic is dispatched (always is, but flag in rationale)
go.mod / go.sum changed (new dependency)	check new import domain → re-run Phase 1 for new package

---

Triage Output — Explicit Reasoning Required

After all 4 phases, write out the dispatch decision with reasoning:

Triage result:
- Always: go-quality-reviewer, go-logic-reviewer
- Phase 1 (imports): go-security-reviewer (database/sql found), go-error-reviewer (database/sql → sql.Rows risk), go-performance-reviewer (database/sql → N+1 risk)
- Phase 2 (diff patterns): go-concurrency-reviewer (go func at worker.go:34)
- Phase 3 (file paths): [none additional]
- Phase 4 (scope): go-test-reviewer (_test.go files present)
- Skipped: [list skipped agents with explicit reason, e.g., "go-test-reviewer would be skipped but triggered by Phase 4"]

No blanket fallback: If an agent is not triggered by any phase, do NOT dispatch it. Absence of trigger signals = absence of domain risk in this change. Record the skip reason explicitly so the review consumer can judge whether the skip was correct.

Step 3: Dispatch Parallel Agents

CRITICAL — TRUE PARALLEL EXECUTION REQUIRED Issue ALL selected Agent tool calls in a single response message — do NOT call one agent, wait for it to finish, then call the next. Every selected agent must appear as a separate Agent tool call block within the same turn. The runtime executes them concurrently only when dispatched together. Serial dispatch defeats the entire architecture.

Launch selected skills as parallel agents using the Agent tool. Each agent receives:

1. The files/diff to review
2. Instruction to follow its SKILL.md and its Grep-Gated Execution Protocol
3. The review scope (files list or diff)
4. Depth-specific appendix (Lite/Standard/Strict — see Step 1.5)
5. Conditional scope narrowing instructions (Standard mode only — see below)

Conditional Scope Narrowing (Standard Mode Only)

In Standard mode, go-logic-reviewer's 10 semantic items overlap with specialized agents' checklists. When a specialized agent is already dispatched, tell go-logic-reviewer to skip the overlapping items to reduce redundant findings.

If this agent is dispatched	go-logic-reviewer skips these items
go-concurrency-reviewer	Goroutine lifecycle / synchronization correctness
go-error-reviewer	Error propagation path completeness
go-performance-reviewer	Resource allocation efficiency

Append to go-logic-reviewer's dispatch prompt:

Scope narrowing (Standard mode): The following dimensions are covered by
dedicated agents in this review — skip checklist items that fall under these
categories to avoid duplicate findings:
- [list dispatched specialized dimensions, e.g., "Concurrency (go-concurrency-reviewer dispatched)", "Error handling (go-error-reviewer dispatched)"]
Focus your analysis on: state machine transitions, boundary conditions,
return value contracts, nil/zero-value assumptions, algorithm correctness,
and any logic items NOT covered by the dispatched specialists.

Important constraints:

• Strict mode: Do NOT apply scope narrowing — all agents run full checklists for cross-validation
• Lite mode: Not applicable (only 2 agents, no specialized agents to overlap with)
• Fallback: If a specialized agent is NOT dispatched (e.g., go-concurrency-reviewer skipped), go-logic-reviewer MUST retain those items — they are its responsibility as the fallback

Example dispatch prompt for each agent:

Review the following Go code changes using the go-{domain}-review skill.
Files: [file list]
Diff context: [diff or "read the files directly"]
Follow your SKILL.md exactly — including the Grep-Gated Execution Protocol.
Run grep pre-scan on all grep-gated checklist items BEFORE semantic analysis.
Return your complete findings report with grep audit line in Execution Status.
End your reply with one fenced json block matching references/findings-schema.md:
{"worker": "<agent-name>", "prefix": "<SEC|CONC|ERR|LOGIC|PERF|QUAL|TEST|OBS>",
 "grep_audit": {"hit": X, "total": Y, "confirmed": Z},
 "findings": [{"id": "<PREFIX>-NN", "severity": "High|Medium|Low", "title": "...",
   "location": "path:line", "evidence": "...", "recommendation": "..."}],
 "suppressed": [{"title": "...", "reason": "..."}]}
[Depth appendix: "Lite mode: ..." or "Strict mode: ..." or omit for Standard]
[Compile pre-check note if applicable: "Compile errors already reported by Lead: ..."]
[Scope narrowing for go-logic-reviewer in Standard mode if applicable]

Sub-Agent Reporting Contract

• Sub-agents report only FOUND items (grep-confirmed + semantic-confirmed, or semantic-only confirmed)
• NOT FOUND items (grep MISS) are silently excluded from reports — do not list them
• Each sub-agent MUST include in Execution Status: Grep pre-scan: X/Y items hit, Z confirmed as findings
• Lead uses this audit line to verify coverage: if 0/Y hit on a dispatched agent, note in Residual Risk

Step 4: Consolidate Results

After all agents return, collect each agent's full response. For each agent response:

• Parse the trailing Findings JSON block (contract: references/findings-schema.md). If a worker omitted or malformed the block, fall back to its prose findings and note the omission in Execution Status — do not fail the review.
• Extract all findings (FOUND items) for deduplication and merging
• Extract the grep audit (grep_audit in the JSON block, or the Grep pre-scan: X/Y hit, Z confirmed line) and record it for the Per-skill grep audit in the final Execution Status section

Then merge their reports:

1. Deduplicate and merge: When multiple agents flag the same location:

   • Different issues at same location → keep both (e.g., Security: "SQL injection at repo.go:67" + Error: "missing error check at repo.go:67" — distinct root causes)
   • Same issue, different domains → merge into the more specific finding (e.g., Error: "resp.Body not closed" + Logic: "resource not closed" → keep Error's finding)
   • Same issue, different severities → merge and promote to the highest severity. For example: Performance flags "N+1 query at repo.go:30" as Medium + Security flags "unbounded query at repo.go:30" as High → merge into High, combine evidence from both agents

2. Sort by severity descending: Final output MUST be ordered High → Medium → Low. Within the same severity level, sort by origin (introduced before pre-existing), then by category alphabetically.

3. Apply volume cap (severity-tiered, depth-dependent):

   • Phase 1: Report ALL High-severity findings (never capped regardless of depth)
   • Phase 2: Fill to soft target with Medium findings (Lite: ~5, Standard: ~10, Strict: ~15)
   • Phase 3: If slots remain, include Low findings
   • Overflow → move to Residual Risk section with one-line summary each

4. Apply change origin classification (when reviewing PR/diff):

   • introduced: finding on code added/modified in this PR
   • pre-existing: finding on unchanged code (discovered via impact radius)

5. Assign unified IDs: Renumber findings sequentially (REV-001, REV-002...) in severity-descending order, preserving the original skill prefix (SEC/CONC/ERR/LOGIC/PERF/QUAL/TEST/OBS) for traceability. REV-001 is always the highest-severity finding.

Output Format

Output as plain text — do NOT use markdown headers (#, ##, ###) or bold (**). Use --- as section dividers. This format renders correctly in terminal (Claude Code CLI) and IDE environments alike.

---

Review Mode

• Review depth: Lite | Standard | Strict (with selection rationale)
• Compile pre-check: PASS | FAIL (N compile errors reported as REV-001..REV-N) | Skipped (Lite mode)
• Dispatched skills: [list]
• Skipped skills: [list with reasons]
• Scope narrowing: [Applied to go-logic-reviewer: skipped {dimensions} | Not applied (Strict/Lite) | N/A]
• Triage rationale: 1-2 lines explaining why conditional skills were included or excluded

---

Findings

Merged from all agents, sorted descending by severity (High → Medium → Low), then by origin (introduced → pre-existing), then by category alphabetically.

[High|Medium|Low] Short Title

• ID: REV-NNN (original: SEC-001 / CONC-001 / etc.)
• Origin: introduced | pre-existing | uncertain
• Baseline: new | regressed | unchanged (compare against base branch; uncertain if diff unavailable)
• Principle: N/A (no constitution.md) | or cite specific clause
• Category: Security | Concurrency | Error | Logic | Performance | Quality | Test
• Location: path:line (or location list for merged findings)
• Impact: user/business/runtime impact
• Evidence: concrete code path or tool output showing the issue
• Recommendation: specific and minimal fix direction
• Action: must-fix | follow-up | needs-clarification

---

Suppressed Items

Merged from all agents. Only include items filtered by anti-example gates.

[Suppressed] Short Title

• Reason: which agent suppressed it + anti-example matched
• Location: path:line
• Residual risk: brief note on remaining exposure

---

Execution Status

• Go version: X.Y (from go.mod) or unknown
• Dispatch validation grep: X overrides applied (list patterns that triggered overrides, or "no overrides needed")
• Skills dispatched: [list]
• Per-skill grep audit:
  • go-security-reviewer: Grep pre-scan X/Y hit, Z confirmed | gosec PASS|FAIL|Not available
  • go-concurrency-reviewer: Grep pre-scan X/Y hit, Z confirmed | go test -race PASS|FAIL|Not run
  • go-error-reviewer: Grep pre-scan X/Y hit, Z confirmed
  • go-performance-reviewer: Grep pre-scan X/Y hit, Z confirmed
  • go-quality-reviewer: Grep pre-scan X/Y hit, Z confirmed | golangci-lint PASS|FAIL|Not available
  • go-test-reviewer: Grep pre-scan X/Y hit, Z confirmed
  • go-observability-reviewer: Grep pre-scan X/12 hit, Z confirmed (2 semantic-only)
  • go-logic-reviewer: Semantic-only (no grep patterns)
  • [only list dispatched agents]
• Excluded (generated): list or None

---

Risk Acceptance / SLA

• SLA recommendation by severity: High = must-fix before merge; Medium = follow-up within 2 weeks; Low = discretionary
• Optional risk acceptance entries for deferred fixes:
  • Finding ID: REV-NNN
  • Owner: [name]
  • Compensating control: [description]
  • Expiry/review date: YYYY-MM-DD

---

Open Questions

Only include blockers that materially affect review confidence. Omit section if none.

---

Residual Risk / Testing Gaps

1. Verification gaps: tools not run and why
2. Volume-cap overflow: findings confirmed but displaced by cap — each as severity | origin | location | short description
3. Pre-existing issues (non-High): Low/Medium pre-existing defects found in impact-radius files (not in diff) — one-line summary each
4. Areas not covered: parts of the change whose risk could not be assessed

---

Summary

X introduced / Y pre-existing / Z uncertain. N High / M Medium / L Low. If capped: "N additional lower-priority issues moved to Residual Risk."

Example Output Reference

When you need to verify report formatting or check what a complete review report looks like: → Load references/example-output.md for a full end-to-end report covering Review Mode, Findings (High/Medium), Suppressed Items, Execution Status, Risk Acceptance, Residual Risk, and Summary sections.

Consolidation Rules

• Lead never reviews code — all findings come from vertical agents
• Never drop High findings — volume cap only affects Medium/Low
• Preserve agent evidence — copy findings verbatim, only renumber IDs
• If an agent returns no findings, note it in Review Mode section, not in Findings
• If an agent fails to run, note in Execution Status with reason

No-Finding Case

If all agents return no findings: state No actionable findings found across all review dimensions. Still output Review Mode, Execution Status, and Residual Risk sections.