Quality Review

Quality Reviewing

Deep review with research to verify a work-product — code, docs, specs, plans, decisions — against current, authoritative sources.

Stakes set depth. Review as if your verdict is the last gate before this ships — no one re-checks behind you. That standard, not "the hook already looked," sets how hard you research. Before searching, write your review plan: which angles (§2–3) this work-product actually needs and the specific question each must answer, then work the list — don't stop at the first finding.

When to use (vs. the automatic hook): the hook does a fast, code-only check from existing knowledge after an edit; this skill adds research (~2-3 min) and works on any work-product. Reach for it on explicit verification ("double check against latest docs", "verify versions", "check security"), deep dives (performance, architecture, trade-offs, an argument's soundness), or pre-change review.

Invocation log

Required before marking done a ticket with two or more RGR loops. The line below logs a current-run entry to skill-invocations.log under the project namespace root so the done-gate hook can verify $safeword:quality-review actually ran; Claude Code expands the ! line automatically. On Cursor and Codex the pre-shell hook (beforeShellExecution / PreToolUse) bridges the session id, so the fallback runs on all three runtimes without hand-picking one. Hand-writing review notes cannot produce this gate proof.

!PROJECT_DIR="${CLAUDE_PROJECT_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}" && bun "${CODEX_HOME:-$HOME/.codex}/plugins/cache/safeword/safeword/1.0.0-rc.3/runtime/cli.js" project record-skill-invocation --cwd "$PROJECT_DIR" quality-review "${CLAUDE_SESSION_ID:-}" || echo "[skill-invocation-log] FAILED - no current-run proof logged"

If no [skill-invocation-log] quality-review ✓ line appears above, run this fallback before continuing:

PROJECT_DIR="${CLAUDE_PROJECT_DIR:-$(git rev-parse --show-toplevel 2> /dev/null || pwd)}"
bun "${CODEX_HOME:-$HOME/.codex}/plugins/cache/safeword/safeword/1.0.0-rc.3/runtime/cli.js" project record-skill-invocation --cwd "$PROJECT_DIR" quality-review "${CLAUDE_SESSION_ID:-}"

If the automatic line or fallback prints [skill-invocation-log] FAILED, prints no run identity, or still does not print quality-review ✓: a ticket with 2+ RGR loops can't be marked done without this proof — don't substitute hand-written notes for it. Report the failure to the user (usual causes: inline shell execution was denied, the runtime exposed no usable run identity, or Bun could not run the installed helper) and ask them to resolve it before re-invoking $safeword:quality-review.

Single-loop tickets, patches, and no-ticket reviews may continue the same way — note it's missing and carry on.

1. Detect phase (code / BDD tickets)

Pull-request boundary

When the work-product is a pull request or this review will be used to promote one, resume the same $safeword:pr-readiness run after the review loop; do not start a second readiness run. This review supplies the AI review gate only: every finding must be applied or answered, and the other six current-head gates still decide whether the PR remains Draft. Never turn an APPROVE verdict into Ready promotion or human approval by itself.

If in a BDD workflow, read the current ticket from <namespace-root>/tickets/ and apply phase-appropriate research:

Project-principle challenge

For a BDD ticket, run bun "${CODEX_HOME:-$HOME/.codex}/plugins/cache/safeword/safeword/1.0.0-rc.3/runtime/cli.js" project review-knowledge --json at the start of each pass and read the current principles, personas, and surfaces paths and content it returns (including overrides such as paths.principles). Do not substitute labels or intake-era content. With impl-plan.md, read those sources alongside the plan and work-product. Treat the plan's principle → concrete consequence → proof entries as claims to refute, not a compliance checklist:

• Challenge applicability, including a principle the plan may have omitted; report only omissions that would materially change behavior, design, proof, or a deliberate deviation.
• Check that each consequence actually follows from the principle and appears in the shipped work; check that the named proof demonstrates that consequence rather than adjacent mechanics.
• An experiential principle is not proven by tests alone. Require the user-facing signal the plan named—such as a persona walkthrough, usability observation, or Rave Moment check—and state any evidence limitation.
• For sourcing or architecture principles, independently check current options, extension boundaries, and compatibility claims against primary sources; do not accept the plan's research summary as its own proof.
• Treat an intentional conflict as valid only when Known deviations names it and explains the trade-off.

This is the judgment gate. $safeword:audit later checks trace integrity as observable facts only; it does not decide whether a principle was applicable or wise.

Persona and surface challenge

For a BDD ticket, read spec.md plus the configured persona and surface inventories (paths.personas and paths.surfaces). Challenge whether the shipped behavior fulfills each persona's JTBD and Rules, rather than merely resolving a persona code. Then reconcile every affected surface against the plan, scenarios, and verification output:

• Require one concrete proof result per affected surface, or a named skip: with its limitation; an @surface.* tag alone is coverage intent, not surface evidence.
• Check the surface evidence used the real surface boundary or names why that boundary could not run. A generic unit test does not prove runtime, client, protocol, or deployment parity.
• Challenge omitted personas or surfaces only when the source artifacts and ticket scope make the omission material; do not turn either inventory into a universal checklist.

Persona fulfillment and proof fidelity are review judgments. $safeword:audit owns only unknown references, stale tags, and dead evidence links.

2. Research Angles

Run each angle that applies — angle diversity is the lever, not search volume: source-currency + risk/security (this section), supersession + primary-source docs (§3). If the user gave a focus or scope restriction, apply it to every angle — don't use it only for the first search.

Source-currency & risk

This is your main differentiator from the automatic hook.

Read the live Current time: line from the prompt timestamp hook and use that date as the current prompt timestamp. Then check the work-product's dependencies and load-bearing claims against the current state of their sources:

• Code: "[library name] latest stable version as of " and "[library name] security vulnerabilities".
• Docs / specs / decisions: are the facts, guidance, or standards it relies on still current as of that date — or superseded, retracted, or overtaken?

Flag if outdated or unsupported:

• A generation behind (major version, or guidance overtaken by newer practice) -> WARN (e.g., React 17 when 19 is stable)
• A small drift behind (minor version, minor staleness) -> NOTE
• A security vulnerability, or a load-bearing claim with no current source -> CRITICAL
• Current and well-sourced -> Confirm

3. Verify against primary sources — supersession + authority

Fetch the authoritative source for each dependency or load-bearing claim.

Look for:

• Deprecated or superseded — APIs, facts, guidance, or standards no longer current?
• A more established pattern or more authoritative source available?
• Recommendation changes since the work-product was written?

Output Format

Shared adversarial-review severity foundation

An error requires a concrete, release-relevant failure within the accepted scope: a violated requirement, regression, established invariant, or credible security or trust-boundary failure. State the triggering conditions and the observable consequence. A missing requirement may be an error when the omission permits materially different shipped behavior and at least one outcome would violate the work's goal or an established invariant.

Speculative future-proofing, optional resilience, theoretical completeness, and protection against an actor already inside a trusted boundary are warnings unless the accepted scope makes that condition hostile. Do not expand the accepted scope through review. A concrete path that can report success while the accepted user-facing claim is false remains an error.

Use request_changes only when an error requires action. Approve when no errors remain; warnings and information are non-blocking. Never invent a finding.

Apply these regression boundaries:

• Error: an omitted contract permits two reasonable implementations and one can falsely report the accepted user-facing claim as satisfied.
• Error: supplied proof is non-discriminating, so the claimed behavior can be broken while every named check still passes.
• Warning: a future unsupported host or version might add a new behavior.
• Warning: an actor inside an explicitly trusted boundary could defeat a diagnostic that is not claimed as protection from that actor.

## Quality Review

**Currency:** [✓/⚠️/❌] [sources/versions current as of the prompt date]
**Sources:** [✓/⚠️/❌] [each load-bearing claim traced to a primary source]
**Correct:** [✓/⚠️/❌] [solves the actual problem, edge cases included]
**Elegant:** [✓/⚠️/❌] [minimal, readable, no incidental complexity]
**No-bloat:** [✓/⚠️/❌] [smallest thing that works, or name the cut]
**Wiring (code only):** [✓/⚠️/❌] [each new entry-point has a real-collaborator test; mocks only the boundary — name it or justify absence]

**Verdict:** [APPROVE / REQUEST CHANGES / NEEDS DISCUSSION]

**Critical issues:** [List or "None"]
**Suggested improvements:** [List or "None"]
**Provenance:** For every version, API, or factual claim:

- (verified: [source URL or doc title]) — fetched this session
- (training data: may be outdated) — not verified
- (uncertain) — could not verify

**Next:** [concrete action — upgrade X from a.b.c to x.y.z, revise {file}:{line}, ask team about Z, or proceed if APPROVE]

The **Next:** line is required. On APPROVE, name what to do now (proceed, commit, run $safeword:verify). On REQUEST CHANGES, name the specific edit and re-review trigger. On NEEDS DISCUSSION, name the question to ask. A verdict that doesn't tell the reader what to do next is incomplete.

Wiring gate (code changes, required)

For each new entry point or command in a code change, confirm a test built from real collaborators that mocks only the process boundary (network / fs / clock / subprocess) — and name it, or justify its absence. A fully-mocked suite can be green while the real config→module wiring is broken (see testing/SKILL.md → Wiring Tests). Internal-seam mocks and provider: none-style short circuits do not count as wiring coverage.

Provenance gate (required)

Severity is bounded by evidence: a CRITICAL or REQUEST CHANGES verdict based on an external factual claim must cite a verified source fetched this session. An error demonstrated directly by the work-product or repository evidence may block when that evidence is cited; it does not need an unrelated external source. An external claim tagged (training data) or (uncertain) caps at NOTE / a non-blocking suggestion — it can inform, never block. Tag every issue with its provenance inline, and surface an unverifiable external concern as a NOTE with the gap named ("couldn't verify X"), never silently drop it. Abstention discipline: LLM judges over-state confidence by default, so an unverified external blocker is false certainty.

Loop: review → fix → re-review

Run the review in passes. Two rules and an objective gate govern the loop, because "until it comes back clean" is not a condition an adversarial reviewer reliably produces — expect it to keep finding something, and let severity rather than patience end the loop.

Continue while any finding is an error. Judge that against the shared severity foundation above, not against the label the reviewer attached. Require a concrete triggering condition and observable consequence; a named failing input is one sufficient form, not the only form. In the output below, rubric errors belong under Critical issues and require REQUEST CHANGES; rubric warnings and information belong under Suggested improvements and do not hold a pass open.

At the third error-level finding in one defect class across the review's passes, fix the class, not the instance. A defect class shares one root mechanism such that one repair removes all its instances; different inputs or call sites do not make separate classes when they reach that same mechanism. Patching the third instance buys one pass; replacing the mechanism ends the class. If you cannot see the class, that itself is the finding worth reporting. If the class-level repair is outside the accepted scope, do not expand scope silently: keep it under Critical issues, use REQUEST CHANGES, and put the question about widening scope in Next. If the user chooses to stop instead, disclose the outstanding error.

Each pass:

1. Run the shared independent-review coordinator. After gathering any current-source evidence needed by §1–3, keep each work-product under review as a target. If the target alone cannot validate a finding, add only the minimum directly relevant supporting evidence: a source, test, contract, or plan. Prefer stable evidence that will not change while the reviewer works. Pass each evidence file with --context; context is not additional work under review. Do not dump the repository or add merely related files. Resolve a review-capable Safeword CLI first; source checkouts do not guarantee a bare safeword on PATH:

   SAFEWORD_REVIEW_PROGRESS=1 bun "${CODEX_HOME:-$HOME/.codex}/plugins/cache/safeword/safeword/1.0.0-rc.3/runtime/cli.js" review run quality-review [--context path/to/evidence] --agent-handoff --json -- changed-file [more-changed-files...]
   

   The dispatch is authorized; skipping it is not your call. The coordinator runs through a local CLI the user installed and signed in to (claude, codex, or opencode), under their own credentials, on this machine; that local process may send the packet to a remote model provider, and the packet is bounded to the targets and --context you pass. This review route ships enabled by default; a user who does not want it sets crossAgentReview: off in .safeword/config.json. The coordinator enforces that setting before provider dispatch, so do not duplicate its policy check in chat. The enabled route authorizes ordinary dispatch, and a cross-vendor second opinion is the point of the gate. So treat the command as ordinary tool use: do not invent a disclosure-approval requirement. Do not stop and ask the user for consent in chat before running this command, even when the packet contains private repository files or crosses provider boundaries. Never pass credentials, customer data, or secret-bearing files as targets or --context; redact them or report the bounded packet as blocked. Invoke the coordinator first. If the host blocks the command, use its native tool-approval request: request the approval it needs, or report that block as the blocker; never replace that request with a chat question. A retry, status check, or permitted fallback for the same bounded packet proceeds without asking again. A review you never dispatched is not coverage — say so unprompted, before any finding, name what ran in its place, and never let your own pass stand in for the review.

   A healthy deep review may return REVIEW_PENDING after its foreground courtesy wait. That is a handoff, not a failed route: keep the returned review_id, continue other useful work, then run the typed nextActions status command until it reaches a terminal result. Never start a second review for the same sources merely because the first is still running. Apply the normal verdict rules only to the collected terminal result; if it is REVIEW_STALE, rerun against the current sources.

   Claude-authored work prefers headless Codex; Codex-authored work prefers headless Claude. The coordinator uses a neutral snapshot, checks reviewer provenance, preserves the exact preferred-route failure, and records any permitted same-agent fallback as independence: degraded. Treat its typed result as the review verdict. If the typed result is REVIEW_AUTHENTICATION_REQUIRED, execute its exact recovery command; the user's browser or device flow may need to complete. After successful authentication, rerun the same coordinator command once. Do not invoke $safeword:finish-review, accept degraded coverage, or loop on another auth denial; report an unsuccessful reauthentication as the blocker. Only when the typed result is REVIEW_ROUTES_EXHAUSTED, invoke $safeword:finish-review immediately with the original result and the same accepted targets. For every other result, return it unchanged. The canonical fallback may use one host-native subagent. Never substitute another surface-private reviewer or hand-written independent evidence.

   Say when a review was not independent. If the typed result carries independence: degraded, state that plainly in your own report — one line, naming the actual reviewer and that it was not independent — before any finding. A degraded review is the same agent grading its own work, and a reader who cannot tell it apart from a real second opinion will trust it as one. Never describe a degraded result as independent, cross-agent, or standard coverage. Say nothing extra when independence is intact.

   The quiet-by-default rule below governs setup advice — recovery commands and install hints. It never licenses withholding the independence of the review itself.

   Keep optional setup advice quiet by default. When the user asks Show review coverage details., report the typed result's achieved coverage, raw independence, and actual reviewer when present. Derive at most one optional upgrade from typed assigned_reviewer and preferred_failure. Preserve a blocked or require-unsatisfied result, and never invent provenance, completed coverage, or a recovery command.

2. Triage. Fix every rubric error under Critical issues this pass. Apply the Suggested improvements worth the change; list the rest — don't chase them.

3. Decide. Run the objective check — for code that's $safeword:verify (tests, lint, typecheck); for other work-products it's the relevant measurable acceptance. Stop only when it passes and no finding is an error; remaining warnings and suggestions are optional. An error is either fixed and re-reviewed or explicitly disclosed when stopping — it is never silently carried. Re-review only if you changed the work-product this pass. Stopping while errors remain is a choice to ship a known defect — say so in your report and in the ticket's evidence, rather than letting a stopped loop read as a clean one.

Voice: plainspoken and concise — write to be scanned. Avoid bloat.