ad-review

Mechanical shape:

THIS SESSION:
  1. Scope the review (which range / PR / commit?).
  2. Read Standards sources (AGENTS.md, ARCHITECTURE.md, GUIDELINES.md, CONTEXT.md, accepted ADRs).
  3. Read Spec source (task Acceptance Criteria → spec → PRD → issue body).
  4. Write the assembled context to .agentic/reviews/<ISO>-<scope>.md (audit trail).
  5. Perform the review in this session. Output findings under two headings:
        ## Standards Findings   (bugs, coupling, edge cases, doc violations)
        ## Spec Findings        (missing requirements, scope creep, wrong impl vs quoted spec line)
  6. End with a one-line aggregate (counts per axis + worst finding).

The two-axis split is structural rigor — same reviewer, but findings must be classified before mixing. A change that passes Spec can still fail Standards (and vice versa); reporting axes separately prevents one from masking the other.

<background_information> Implements WORKFLOW §10 (Reviewer With Adversarial Discipline). On Claude Code, §10 is delivered via two parallel Task subagent calls with fresh context. Codex skills cannot spawn agents, so the Codex variant ships the next-best discipline: structural axis separation inside a single review pass. The reviewer cannot rationalize a Spec pass as covering Standards (or vice versa) because the output schema forces both lists to be produced separately.

The two-axis dichotomy is borrowed from mattpocock/skills/review and bound to this kit's six-layer artifact stack (Constitution → Domain → Product → Spec → Plan/Decisions → Code).

For Codex users who want true fresh-context review (the §10 ideal), spawn a subagent manually after the skill writes the audit-trail file — see the "Optional escalation" block at the bottom of the instructions. </background_information>

Running ad-review (Codex single-pass two-axis). I will read the diff and binding context, write an audit trail to .agentic/reviews/, then report findings under ## Standards Findings and ## Spec Findings in this session.

Step 1 — scope the review. Confirm what to review. Default scopes, in priority order:

1. User-named ref or PR (ad-review main..HEAD, ad-review <commit-sha>).
2. Current branch vs main (git diff main...HEAD).
3. Working-tree changes (git diff plus git diff --staged).

If no diff exists, stop and tell the user — there's nothing to review.

Capture the diff command once: git diff <range> (use ... three-dot for ref-vs-ref so the comparison is against the merge-base). Note the commit list with git log <range> --format=%B.

Size guard: if git diff <range> --stat reports >50 files, ask the user to narrow scope before continuing — reviewing a giant diff in one pass loses signal.

Step 2 — read Standards sources. Read what exists; do not fabricate references.

• AGENTS.md at the repo root.
• ARCHITECTURE.md at the repo root.
• GUIDELINES.md at the repo root.
• CONTEXT.md at the repo root, or CONTEXT-MAP.md plus per-context CONTEXT.mds.
• Every ADR under doc/adr/ with Status: accepted whose subject is touched by the diff. When in doubt, include rather than skip.
• CONTRIBUTING.md if present.
• Machine-enforced configs (.editorconfig, eslint.config.*, biome.json, prettier.config.*, tsconfig.json) — note their presence and skip re-checking what tooling already enforces.

Step 3 — read Spec source. In this order, take the first that resolves:

1. Task references in the diff or recent commit messages (Task NNNN, 0NNN-, Closes task-0042) → read the file's Acceptance Criteria and Plan sections.
2. An originating spec under doc/specs/ whose filename matches the branch name or the dominant feature touched by the diff.
3. A parent PRD under doc/product/ referenced by the spec.
4. Issue references in commit messages (#123, Closes #45) — fetch via gh issue view if available.

If nothing resolves, mark Spec as no spec source provided. The Spec axis output will say so explicitly and report no findings.

Step 4 — write the audit-trail handoff. Persist the assembled context at .agentic/reviews/<ISO-timestamp>-<scope-slug>.md (single file, not two). <scope-slug> encodes the review target (branch-vs-main, pr-42, commit-abc1234, working-tree). Create the directory if missing.

File body:

=== AGENTIC-REVIEW HANDOFF (single-session, two-axis) ===

Range:   <git diff <range>>
Commits: <git log <range> --format=%h\ %s>

--- DIFF ---
<git diff output>

--- STANDARDS SOURCES ---
<AGENTS.md, ARCHITECTURE.md, GUIDELINES.md, CONTEXT.md / CONTEXT-MAP.md,
applicable accepted ADRs, CONTRIBUTING.md>

--- SPEC SOURCES ---
<task file Acceptance Criteria + Plan, originating spec, parent PRD,
recent commit messages, originating issue body if fetched — or
"no spec source provided">

--- TOOLING NOTE ---
<list of machine-enforced configs found — skip what they already check>

=== END HANDOFF ===

Advise the user to add .agentic/reviews/ to .gitignore if it isn't already — handoffs are ephemeral audit artifacts, not committed history.

Step 5 — review. Apply the two axes with discipline. Read the diff once, classify each finding into exactly one axis before adding it to the report.

Standards axis — report only:

• Bugs (null/undefined paths, off-by-one, race conditions, broken invariants, wrong types, unhandled errors).
• Coupling (modules that shouldn't know about each other, leaked abstractions, hidden globals).
• Edge cases (empty inputs, large inputs, concurrent access, unicode, paths with spaces, missing files, permission errors).
• Diff violations of any standards source from Step 2 (AGENTS.md / ARCHITECTURE.md / GUIDELINES.md / CONTEXT.md / accepted ADRs / CONTRIBUTING.md).
• Vocabulary drift (paraphrasing canonical CONTEXT.md nouns).
• Skip what tooling already enforces (lint, format, type-check) — listed under TOOLING NOTE.

Spec axis — report only:

• (a) Requirements the spec asked for that are missing or partial.
• (b) Behaviour in the diff that wasn't asked for (scope creep).
• (c) Requirements that look implemented but where the implementation looks wrong against the spec line.
• Quote the spec line for each finding.
• If Spec was marked no spec source provided, report exactly that and report no Spec findings.

Classification rule when a finding could belong to either axis: route it to the axis whose source defines the constraint.

• "function returns undefined on empty input" → Standards (a code-quality bug).
• "function returns undefined on empty input, but the spec required []" → Spec (the spec line is what's broken).

When in genuine doubt, place it in Standards.

Step 6 — output. Print findings under two headings, in this exact order:

## Standards Findings

<one finding per line: `file:line: <severity>: <problem>. <fix>.`>
<severity is the literal word `Blocker`, `Concern`, or `Note` — no emoji>
<if no findings, write exactly: "no real issues found in this axis">

End axis line: `Standards: ship as-is` / `Standards: ship with the Concerns logged` / `Standards: don't ship until Blockers resolved`.

## Spec Findings

<one finding per line: `file:line: <severity>: <problem> (spec: <quoted-line>). <fix>.`>
<if Spec was skipped, write exactly: "Spec: skipped — no spec source provided" and produce no findings>

End axis line: `Spec: ship as-is` / `Spec: ship with the Concerns logged` / `Spec: don't ship until Blockers resolved` / `Spec: skipped — no spec source provided`.

End the reply with a one-line aggregate summary:

Aggregate: <N Standards Blockers, M Standards Concerns> / <P Spec Blockers, Q Spec Concerns>. Worst: <one-line quote of the highest-severity finding from either axis, or "none">. Audit trail: <path>.

Do NOT synthesize an overall "approve" verdict. §10 frames the review as adversarial; the senior engineer weighs the findings.

Optional escalation — true fresh-context review via subagent (user-initiated only). If the user wants the §10 ideal (a reviewer with no inherited bias), tell them after Step 6:

For a fresh-context review, you can spawn a Codex subagent manually:
  > spawn an agent to act as a senior code reviewer. Read <audit-path> and report findings under ## Standards Findings and ## Spec Findings only.
The subagent loads only the handoff file, so it has no inherited context from this session. Requires Codex 2025+ with the `[agents]` block configured in ~/.codex/config.toml.

Do not spawn the agent yourself — Codex skills cannot. Only the user's natural-language command can.

<output_contract>

• One audit-trail file at .agentic/reviews/<ISO-timestamp>-<scope-slug>.md carrying the diff plus assembled Standards + Spec context.
• One review reply in the current session with findings under ## Standards Findings and ## Spec Findings, each axis with its own end-line verdict (ship as-is / ship with the Concerns logged / don't ship until Blockers resolved / skipped — no spec source provided).
• One aggregate summary line at the end with axis counts, worst finding, and audit-trail path.
• No "approve" verdict. No /clear choreography. No skill-initiated subagent spawn (only documented as a user-initiated escalation). </output_contract>

Next

• Address every Standards Blocker before merge — that's the code-quality hard gate. Re-run ad-review on the fix to confirm it cleared.
• Address Spec Blockers next — implementation-vs-spec drift is the second hard gate.
• Each Concern (from either axis) becomes a follow-up ad-task; do not let them silently accumulate.
• Notes are informational; close them out in the original task's Notes log if relevant.
• If the Spec axis was skipped, decide whether an ad-spec is overdue — work without a spec means future reviews are Standards-only.
• For maximum §10 fidelity on Codex, escalate to a user-spawned reviewer subagent against the persisted audit-trail file.
• Once both axes are clear: merge per project conventions.