review

name

review

description

Runs an independent multi-dimension review of completed work and adversarially verifies every finding before reporting it. Use after every native wave in the current root session is complete, after refactoring verifies, or before merging to main. Triggers when independent validation is needed that code meets requirements, has no security gaps, passes quality standards, and has no performance regressions. User phrases like "review this", "is this ready to merge", "validate the implementation".

Codex Backend

This skill is assembled for Codex. Before following the workflow, read references/codex-backend.md completely. Its operation mappings are binding: SessionPlanRead reads the root session's native wave plan, SessionPlanWrite mutates it only through update_plan, and SessionContextRead reads the same root transcript. One native plan step is one Gambit wave; parallel workers are subagent threads inside that single step. These are backend operations, not literal shell commands.

Review

Freedom: LOW — dispatch all four reviewers for the initial audit, then freeze scope and the finding ledger.

Overview

Dispatch four specialized reviewer agents to independently audit completed work once, then dispatch a dedicated verifier sub-agent to kill-or-keep each finding and freeze the survivors into a review ledger. Remediation closes that ledger; it does not restart an open-ended audit.

The verification work is delegated to a dedicated verifier sub-agent, not done in the main context. Main context's job is dispatch + assembly; the verifier's job is ruthless kill-or-keep classification. This split follows Anthropic's CitationAgent pattern and avoids the synthesizer becoming context-starved from juggling four simultaneous roles (dispatch, verify, dedup, implement).

Works in epic or standalone workflow context; either context has an initial-audit phase and a bounded closure phase:

• Epic review: When this root transcript contains an approved epic contract, conformance checks against its requirements and success criteria
• Workflow review: For standalone work, conformance checks against the complete workflow brief and checkpoint in this root transcript

Core principle: Review is adversarial and broad once; closure is adversarial and narrow until the frozen findings are resolved.

Announce at start: "I'm using gambit:review to validate this implementation before finishing."

Quick Reference

Step	Action	STOP If
1. Detect Context	Approved contract/brief + any open review ledger in this root transcript	Same-session context is absent
2. Load Context	Complete contract/brief + changed files list	Context is incomplete
3. Freeze Boundary	Requirements + base revision + review snapshot + changed hunks	Boundary incomplete
4. Dispatch Reviewers	Initial audit only: 4 agents in parallel, each reading instructions by path	Any agent fails to run
5. Scope + Dedupe	Reject out-of-boundary candidates; byte-identical dedupe only	—
6. Dispatch Verifier	1 verifier sub-agent with the deduped candidate list	Verifier fails to run
7. Freeze Ledger	Confirmed findings become the complete, immutable blocker set	—
8. Remediate / Close	Fix ledger items; re-verify only open IDs + original gates	Evidence fails
9. Gate	APPROVED or OPEN LEDGER with verification counts	Ledger remains open → fix briefs and a later wave, STOP

When to Use

• Every native wave step is completed (called automatically by gambit:executing-plans Step 5)
• After gambit:refactoring completes changes (mandatory)
• Before gambit:finishing-branch
• Any time you want independent review of completed work

Don't use when:

• A native wave is pending or in progress → use gambit:executing-plans
• Mid-implementation, per-task quality check → that's the executing-plans checkpoint quality gate's job (it reuses this skill's quality reviewer, scoped to one diff, when it escalates). This skill is the multi-dimension end-of-epic backstop, not the per-task gate.

The Process

Step 1: Detect Context

First detect an open Review Closure Ledger in the current workflow state. A ledger is open when a prior review recorded confirmed finding IDs and has not recorded terminal closure for all of them.

• Open ledger found → closure mode. Skip Steps 3–5 and all four finders. Load the frozen boundary and only the still-open ledger entries, then continue at Step 6 with mode: closure.
• No open ledger → initial mode. Determine what you're reviewing against below and run the full audit.

An old APPROVED report is not an open ledger. A ledger is invalidated only when the user explicitly changes requirements or authorizes implementation outside the recorded remediation boundary; then record why and begin a new initial review. Incidental scope creep is not a reason to restart review — remove it or return it to the worker.

Use SessionContextRead in this root session to detect context:

Epic context (default when a complete approved epic contract exists):

SessionContextRead → approved epic contract (requirements, success criteria, anti-patterns)
SessionPlanRead → concise wave steps; verify every wave is completed
SessionContextRead → latest checkpoint and native subagent results for individual worker completion

Workflow context (refactoring or standalone work with no epic contract):

SessionContextRead → complete workflow brief and checkpoint result (goal, implementation steps, success criteria)

The review brief adapts based on which same-session context exists. If both exist, prefer the epic context. Detect an open ledger from the prior review checkpoint in this root transcript; plan steps never carry it. Never search plan steps for an epic selector or individual worker record.

Step 2: Load Context

For epic context: use SessionContextRead for the full approved contract and latest checkpoint, and SessionPlanRead only to verify every concise wave step is completed.

For workflow context: use SessionContextRead for the complete workflow brief and checkpoint result.

Initial mode, both contexts: freeze exact revisions before dispatch:

git merge-base main HEAD            # review_base
git rev-parse HEAD                  # review_snapshot
git diff <review_base>..<review_snapshot> --name-only
git diff --unified=0 <review_base>..<review_snapshot>  # frozen changed hunks

If tracked or untracked implementation changes are absent from review_snapshot, STOP. The audited snapshot must contain everything intended for merge.

Step 3: Freeze Boundary and Prepare Brief

Build a brief that each reviewer agent will receive. Include:

For epic context:

1. Epic requirements — full text from the approved contract returned by SessionContextRead (requirements, success criteria, anti-patterns)
2. Changed files — the --name-only output
3. Base branch — what the diff is against

For task context:

1. Workflow goal and success criteria — full text from the workflow brief returned by SessionContextRead
2. Changed files — the --name-only output
3. Base branch — what the diff is against
4. Context type indicator — "This is a task-level review (debugging/refactoring), not an epic review. Evaluate against the task's stated goal and success criteria."

Both contexts also include a frozen Review Boundary:

• review_base and review_snapshot
• exact changed files and zero-context changed hunks between those revisions
• explicit requirements/success criteria from the approved contract or workflow brief
• this rule: every finding must anchor to a line changed in that frozen diff, including missing-test/docs/config findings via the changed line that creates the obligation

Commit history, checkpoint formatting, transcript/process compliance, unchanged code, and "while I was reading" observations are outside the gate unless an explicit approved requirement names them. Report such observations separately; they cannot become candidates, ledger entries, fix work, or reasons to restart review.

Do NOT include your opinions, implementation notes, or rationale. The reviewers should form their own conclusions from the code.

Step 4: Dispatch Four Reviewers

Resolve the absolute path to this skill's reviewers/ directory once (Glob **/skills/review/reviewers/conformance.md if you don't already know it). You pass this path to the agents — do NOT read the reviewer files into this context. The four reviewer files are ~8k tokens; reading them here and re-emitting them as prompts wastes ~18k tokens every review. Each agent reads its own instruction file in its own fresh context.

Native Codex finder dispatch

In ONE message, emit exactly four finder SpawnAgent calls. Use an installed finder profile when available, otherwise use default with the finder contract. Each prompt is just: (1) a directive to read and follow that agent's instruction file by path, then (2) the review brief.

SpawnAgent agent_type="finder" task_name="conformance_review" message="Read <abs>/reviewers/conformance.md — that file is your complete instructions; your FIRST action must be to Read it, then follow it exactly.\n\n## Review Brief\n\n[brief]" fork_turns="none"  # Profile-aware: requires hide_spawn_agent_metadata = false and a non-reserved tool_namespace.
SpawnAgent agent_type="finder" task_name="security_review"    message="Read <abs>/reviewers/security.md — that file is your complete instructions; your FIRST action must be to Read it, then follow it exactly.\n\n## Review Brief\n\n[brief]" fork_turns="none"  # Profile-aware: requires hide_spawn_agent_metadata = false and a non-reserved tool_namespace.
SpawnAgent agent_type="finder" task_name="quality_review"     message="Read <abs>/reviewers/quality.md — that file is your complete instructions; your FIRST action must be to Read it, then follow it exactly.\n\n## Review Brief\n\n[brief]" fork_turns="none"  # Profile-aware: requires hide_spawn_agent_metadata = false and a non-reserved tool_namespace.
SpawnAgent agent_type="finder" task_name="performance_review" message="Read <abs>/reviewers/performance.md — that file is your complete instructions; your FIRST action must be to Read it, then follow it exactly.\n\n## Review Brief\n\n[brief]" fork_turns="none"  # Profile-aware: requires hide_spawn_agent_metadata = false and a non-reserved tool_namespace.

Parallelism is structural, not a reminder. That single message contains four native SpawnAgent calls and nothing else: no file reads, no prose between them. Reading one reviewer file before each dispatch is exactly what forces the agents sequential; passing paths removes the read step, so there's nothing left to interleave. If you catch yourself reading a reviewer file here, you've reverted to the old serializing pattern — stop and dispatch by path.

Each reviewer will:

• Read the changed files independently
• Evaluate their dimensions with evidence
• Fetch documentation or references from the web when local knowledge is insufficient or the code is sensitive/complex
• Attach a **Verify by:** line to every Gap and Improvement (required — see each reviewer file's "Verification Requirement" section)
• Return findings as APPROVED or GAPS FOUND

Critical: Reviewers are strictly advisory. They must NOT run tests or edit files. Local inspection is limited to the bounded commands and locations in each reviewer contract. Use live web search to validate edge cases, check API documentation, verify security patterns, or confirm language-specific behavior when local knowledge is insufficient.

Step 5: Scope-Filter and Dedupe Candidate Findings

Collect the four reviewer reports into one candidate list. Each finding carries a **Verify by:** line; assign each finding an opaque id (any stable string — reviewer name + sequence works).

Apply the frozen boundary mechanically before verification. A candidate is eligible only when its cited anchor intersects a changed hunk in review_base..review_snapshot, or an explicit requirement directly names the non-code artifact it cites. Put rejected items in a non-blocking Out of scope audit trail with the failed boundary check. Do not send them to the verifier and do not create work from them.

Dedupe on byte-identical (path, line_range, Verify by:) tuples only. Do NOT dedupe on semantic similarity.

Semantic dedup ("these two findings sound alike, collapse them") silently drops true positives — different reviewers flagging the same line with different verify_by steps have different investigation paths, and losing one loses coverage. Only collapse when all three fields match byte-for-byte. The verifier handles near-duplicates downstream.

Before dispatching to the verifier, build a side-table keyed by id recording each finding's category (gap or improvement), verify_by (original reviewer text), and reviewer (which of the four emitted it). The verifier never sees this side-table. Retain it to route verdicts, build complete ledger entries, and author any fix briefs; losing it breaks closure.

The deduped list and frozen boundary go to the verifier in Step 6.

Step 6: Dispatch Verifier Sub-Agent

Dispatch ONE verifier agent. Use an installed verifier profile when available, otherwise use default with the verifier contract. Pass the path rather than reading verifier.md into this context. The candidate list IS passed inline (it's dynamic):

SpawnAgent agent_type="verifier" task_name="verify_candidates" message="Read <abs>/reviewers/verifier.md — that file is your complete instructions; your FIRST action must be to Read it, then follow it exactly.\n\nmode: initial\nreview_base: [revision]\nreview_snapshot: [revision]\n\n## Candidate Findings\n\n[deduped list with ids]" fork_turns="none"  # Profile-aware: requires hide_spawn_agent_metadata = false and a non-reserved tool_namespace.

Do NOT include reviewer severity, category (Gap vs. Improvement), or reasoning chain in the candidate list. The verifier receives the mode, frozen revisions, and only id, path, line_range, body, verify_by per candidate. Fresh context prevents anchoring. Retain stripped fields in the Step 5 side-table.

Do NOT verify findings in the main context. Main context's job is dispatch + assembly. The verifier is the single source of truth for classification.

Skip the verifier dispatch only if the candidate list is empty. Continue to Step 9 with an empty ledger; original criteria and the full project gate still require fresh evidence before APPROVED.

Step 7: Assemble Findings From Verifier Output

The verifier returns one classification per candidate, each with verdict, quoted_evidence, evidence_location, tool_calls_made, confidence, and (for gaps) gap_reason.

Route by verdict, using the Step 5 side-table to recover each finding's original category:

• confirmed → keep in the final report as a finding. Preserve the reviewer's original body text and the verifier's quoted_evidence / evidence_location. Place the finding in the "Gaps" section if the side-table's category is gap, or the "Improvements to Implement" section if improvement.
• gap → surface in a "🔍 Couldn't verify" section of the final report. NOT a confirmed finding — a coverage boundary. Include the verifier's gap_reason verbatim.
• refuted → drop from the findings and the gate. But list each one tersely in the "Refuted (dropped)" audit trail (file:line + the reviewer's one-line claim + the verifier's quoted counter-evidence). This is the only window into the verifier's one documented failure mode — aggressive refutation suppressing a real bug. Do not act on refuted findings, author worker briefs for them, or let them block; the audit trail exists so you (and the user) can spot a bad refutation, not to re-litigate verdicts.

After the initial verdicts, create a Review Closure Ledger containing every confirmed finding and no others:

review_base: <revision>
review_snapshot: <revision>
requirements: <approved contract/brief identity>
open:
  - id: <stable id>
    category: <gap|improvement>
    path: <path>
    line_range: <range in review_snapshot>
    body: <original claim>
    verify_by: <original check>
    evidence: <verifier quote + location>

The ledger is immutable: closure may change only an entry's status from open to resolved. Refuted, gap-classified, and boundary-rejected candidates stay in non-blocking audit trails and never enter later work. Preserve the complete ledger in the review checkpoint; fix work must reference its IDs.

Step 8: Remediate and Close the Ledger

In initial mode, implement every confirmed improvement. Confirmed gaps become fix work through the owning workflow. A suggestion may be skipped only with code evidence that the reviewer misunderstood it; record that as a verifier-calibration issue, not a new finding.

After any remediation, enter closure mode. Do not dispatch the four finders again. Dispatch the verifier with only open ledger entries.

SpawnAgent agent_type="verifier" task_name="close_review_ledger" message="Read <abs>/reviewers/verifier.md and follow it exactly.\n\nmode: closure\nreview_base: [revision]\nreview_snapshot: [original reviewed revision]\ncurrent_revision: [current HEAD]\n\n## Open Ledger Findings\n\n[original candidate fields for open IDs only]" fork_turns="none"  # Profile-aware: requires hide_spawn_agent_metadata = false and a non-reserved tool_namespace.

Interpret closure verdicts against the original claim:

• refuted → the original claim no longer holds; mark that ID resolved.
• confirmed → the defect remains; keep that same ID open.
• gap → resolution lacks evidence; keep that same ID open with the literal wall.

Then invoke gambit:verification to run each ledger item's targeted check, the original success criteria, and the full project gate. Verification may fail these declared claims but may not invent new ones. Check review_snapshot..current_revision for remediation scope: unrelated edits return to the worker for removal; they do not expand the ledger.

Newly noticed issues from the frozen snapshot, process/history concerns, and unrelated observations are non-blocking Outside frozen review boundary notes. They cannot reopen an ID or create one. Only explicit user-approved requirement or implementation-scope expansion invalidates the ledger and authorizes a new initial audit.

Step 9: Gate Decision

APPROVED requires either zero confirmed findings in initial mode, or every ledger ID resolved in closure mode, plus green original criteria and full project gate. This is the terminal condition; proceed directly to gambit:finishing-branch and pass the fresh test evidence.

If entries remain open, report only those IDs with their evidence and complete fix briefs. Preserve the same ledger:

Write complete fix worker briefs for the open IDs only in the root checkpoint. Update the existing native wave plan with concise fix-wave summaries, preserving completed waves, then STOP and return to gambit:executing-plans (or the owning standalone workflow). The ledger remains in the transcript, never in plan text.

Never create work from refuted, gap-classified-in-initial-mode, boundary-rejected, or newly noticed closure observations. Never replace closure with another full review merely because the verifier or tests found an open ledger item.

Integration

Called by:

• gambit:executing-plans (Step 5, when every native wave is completed)
• gambit:refactoring (mandatory, after final verification passes)
• User via $gambit:review

Calls:

• gambit:finishing-branch (if approved)

Dispatches default agents (parallel, read-only) at the finder tier (codex-contracts/models.md). Each agent reads its own instruction file by path — main context never loads them:

• reviewers/conformance.md — completeness, architecture, dead code
• reviewers/security.md — OWASP audit, secrets, auth, data exposure
• reviewers/quality.md — language idioms, linter circumvention, test quality
• reviewers/performance.md — scaling, N+1, resource management

Dispatches one verifier through the configured or native verifier executor, also by path:

• reviewers/verifier.md — initial kill-or-keep classification; later bounded closure of the frozen ledger

Call chain (epic context):

executing-plans (all waves completed) → review → finishing-branch
                                      ↓
                           (if ledger open: STOP → fix → close ledger)

Call chain (task context):

refactoring (changes verified) → review → finishing-branch
                                    ↓
                         (if ledger open: STOP → fix → close ledger)