name: jr-review
description: Multi-agent PR review swarm (local). Spawns specialized reviewers, deduplicates findings, gets approval, auto-fixes, and validates. Scales dynamically based on diff size. For cloud-based parallel review of very large diffs, see /ultrareview (Claude Code v2.1.111+). Prioritizes thoroughness over speed; use quick or nofix for fast paths, or --auto-approve for unattended/CI runs.
argument-hint: "[nofix|full|quick|--converge[=N]|--auto-approve|--no-verify-claims|--refresh-stack|--refresh-baseline|--only=dims|--scope=path|--pr=N|--branch[= ]] [max-retries]"
effort: high
model: opus
disable-model-invocation: true
user-invocable: true
allowed-tools: Read Glob Grep WebFetch AskUserQuestion Agent advisor TaskCreate TaskList TeamCreate TeamDelete SendMessage Write(.claude/) Edit(.claude/) Write(.gitignore) Edit(.gitignore) Bash(git diff *) Bash(git status *) Bash(git log *) Bash(git ls-files *) Bash(git rev-parse *) Bash(git symbolic-ref *) Bash(git rev-list *) Bash(git merge-base *) Bash(git show *) Bash(git diff-tree *) Bash(git cat-file *) Bash(git config --get *) Bash(gh pr view *) Bash(gh pr diff *) Bash(gh pr list *) Bash(gh issue list *) Bash(gh repo view *) Bash(gh api *) Bash(gh auth status *) Bash(glab mr view *) Bash(glab mr diff *) Bash(glab mr list *) Bash(glab issue list *) Bash(glab repo view *) Bash(glab api *) Bash(glab auth status *) Bash(grep *) Bash(wc *) Bash(test *) Bash([ *) Bash(stat *) Bash(find . *) Bash(jq *) Bash(perl *) Bash(printf *) Bash(date *) Bash(mktemp *) Bash(comm *) Bash(sort *) Bash(awk *) Bash(cut *) Bash(head *) Bash(tail *) Bash(xargs *) Bash(command -v *) Bash(shasum *) Bash(echo *) Bash(mv *) Bash(mkdir -p *)
Review the current changes as if you were doing a PR review.
Arguments: $ARGUMENTS
Parse arguments as space-separated tokens. Recognized flags:
nofix— Findings-only mode. Skip Phase 5 (implementation) and Phase 6 (validation). Just report findings and stop.full— Force the full swarm even for small diffs.quick— Force lightweight mode (max 2 reviewers) even for large diffs. Also skips Track D baseline and outputs compact report.--refresh-stack— Force re-detection of package manager, validation commands, and stack info. Use after changing package.json scripts or switching package managers.--refresh-baseline— Force re-run of validation baseline even if a cached baseline exists within TTL.--only=<dimensions>— Run only the specified reviewer dimensions (comma-separated). All others are skipped. Example:--only=security,--only=typescript,testing,--only=simplicity.--scope=<path>— Limit review to files under this path prefix. Example:--scope=src/api/.--pr=<number>— Review a remote PR instead of working-tree changes. Fetches the PR diff viagh, forcesnofixmode (can't edit remote files). Phase 8 comments on the PR instead of creating standalone issues.--branch[=<base>]— Review the full feature-branch diff (committed-on-branch + working tree). Default<base>resolves viagh pr list --head(linked PR), elseorigin/<default-branch>. Aborts if local HEAD is behind upstream. Mutually exclusive with--pr. See Phase 1 Branch mode for resolution and safety details.--converge[=N]— After the first review-fix pass, loop back and re-review files modified by implementers. Continues until zero new findings, no files modified, or N iterations reached. Default N depends onCLAUDE_EFFORT(seeprotocols/phase2-reviewers.md"Effort-adaptive breadth"):low/medium→ 2,high→ 3,xhigh/max→ 5. Bounded to the 2–10 validation range below. An explicit--converge=Noverrides the effort-adaptive default. Convergence passes auto-approvecertainandlikelyfindings in interactive mode; in headless/CI mode (--auto-approve), onlycertainfindings are auto-approved by default — see--auto-approvefor details. Speculative findings are deferred to the final report. Skip Phase 4/4.5. Produces a single consolidated report at the end.--auto-approve— Skip Phase 4 approval; auto-approvecertainfindings on pass 1. Also activates headless/CI mode (per../shared/secret-scan-protocols.md"Headless/CI detection" — changes secret-halt behavior, skips Phase 8 issue creation, applies auto-quick for large diffs). Without--converge:likely/speculativedeferred to Phase 7. With--converge:likelyauto-approved on pass 2+ (interactive) or deferred (headless). Implies no Phase 4.5. See Flag conflicts for the auto-add-converge behavior in headless mode and the interactive warning. Safety note: without--converge, no post-fix security verification beyond Phase 5.6 secret re-scan; the fresh-eyes pass only covers the security dimension. For safety-critical code, combine with--convergeor run a follow-up/jr-reviewafter.--no-verify-claims— Opt OUT of claim verification (anti-hallucination), which is on by default. By default, for findings the lead classifies in Phase 3 as external-authority claims (API deprecation, version behavior, framework rules, CVEs, WCAG/OWASP — anything not provable from the cited diff), the lead fetches an authoritative source to confirm or refute before the finding is surfaced or auto-approved (confirmed→ tagged[verified: …];refuted→ rejected). Passing--no-verify-claimsskips the fetch (offline / speed); unverifiable external-authority claims are then capped tospeculative(Tier 1 fallback) and thus never auto-approved (auto-approve covers onlycertain/likely). Honors the source-fetch discipline in../shared/claim-verification.md(raw.md/gh api, never a lone WebFetch summary). Composes with all modes.--model=<tier>— Override the model for every subagent spawned this run (sonnet|opus|haiku|fable); nested spawns inherit it. Does NOT change the lead (frontmatter applies before argument parsing — run/model <tier>first for a uniform run). Compatible with all other flags. Canonical semantics:../shared/model-override.md.
Parameter validation for --converge=N (apply in order, reject on first failure):
- Match
^[0-9]+$— non-digit input warnsConvergence limit must be a number. Using 3.and uses 3. - Parse base-10 integer (handles leading zeros like
007). - Range check
[2, 10]. IfN == 1, warnConvergence with 1 iteration provides no re-review or fresh-eyes pass. Use --auto-approve without --converge, or set N >= 2.and use 2. IfN == 0orN > 10, warnConvergence limit must be between 2 and 10. Using 3.and use 3. The cap bounds the maximum number of fully automated code modification cycles per invocation.
- Any number (e.g.,
3) — Max validation retries (default: 3). If the value exceeds 10, warn: 'Max retries capped at 10 to limit automated modification cycles.' and use 10. If 0, negative, or non-numeric, warn and use default (3).
Examples: /jr-review, /jr-review nofix, /jr-review quick, /jr-review full 5, /jr-review nofix quick, /jr-review --refresh-stack, /jr-review --only=security, /jr-review --scope=src/api/, /jr-review --pr=123, /jr-review --pr=456 --only=security, /jr-review --scope=src/api/ --only=typescript,node, /jr-review --converge, /jr-review --converge=5, /jr-review --converge --auto-approve, /jr-review --converge quick, /jr-review --branch, /jr-review --branch=develop, /jr-review --branch --converge, /jr-review --branch --only=security
Flag conflicts
full+quick—quickwins (it's the explicit override for speed). Ignorefull.--refresh-baseline+ (quick|nofix|--pr) — Track D is skipped in these modes, so--refresh-baselineis silently ignored.--primpliesnofix— do not warn, just apply.--converge+nofix— Conflict.nofixdisables implementation, so there is nothing to converge on. Warn: "Cannot converge without fixing —nofixdisables implementation. Remove one flag." and abort.--converge+--pr— Conflict.--primpliesnofix. Same as above.--branch+--pr— Conflict. Both flags define alternate diff scopes (local feature branch vs. remote PR). Abort with: "Cannot combine--branchand--pr— they define different diff scopes. Use--branchto review your own in-flight feature branch (committed-on-branch + working tree) or--pr=<N>to read-only-review a remote PR."--branch+nofix— Allowed. Branch mode does NOT implynofix(unlike--pr); the user may opt in if they want findings-only output without local fix-application.--branch+--converge/--auto-approve/quick/full/--scope/--only/--refresh-stack/--refresh-baseline— Allowed (compose normally; convergence and auto-approve keep their normal-mode semantics).--converge+quick— Allowed. Convergence passes use the samequickconstraints (max 2 reviewers, compact report).--converge+full— Allowed. First pass usesfullswarm. Convergence passes scale dynamically based on modified file count (usually smaller, so they naturally scale down).--auto-approve+nofix— Allowed but pointless. Findings are listed with no approval gate, but nothing is fixed. Silently allow.--auto-approve+--pr— Allowed.--auto-approveactivates headless mode, but Phase 8's headless skip has a--prexception: PR comments are still posted because the user explicitly provided a PR number (implicit consent to comment on that specific PR). Issue creation is still skipped.--auto-approvewithout--converge— In interactive mode (per../shared/secret-scan-protocols.md"Headless/CI detection"), warn via AskUserQuestion:[Add --converge (Recommended)] / [Continue] / [Abort].[Add --converge]is the safer default and listed first;[Continue]proceeds without post-fix security verification. In headless mode, auto-add--converge=2and log to the Phase 7 report. Whenever--auto-approveAND--convergeare both active (explicit or auto-added), setfreshEyesMandatory=trueso the fresh-eyes security pass runs regardless of other skip conditions.--no-verify-claims+ any mode — Allowed; composes with all flags including--pr,--branch,--auto-approve,--converge. Classification + local grounding always run in Phase 3; verification (the Tier 2 fetch) runs by default, and--no-verify-claimsskips it (Tier 1 cap fallback). In--auto-approve/headless mode a confirmed claim is auto-applied like any approved finding; only an unverifiable one (offline, uncorroborable, or--no-verify-claims) is capped tospeculativeand deferred to the report, never auto-applied.
Parameter sanitization
--scope=<path>: Apply the following checks in sequence; reject on first failure: (1) Reject paths containing control characters including null bytes (\0), newlines (\n), and carriage returns (\r). (2) Validate against allowlist regex^[a-zA-Z0-9][a-zA-Z0-9_./-]*$(must start with an alphanumeric character — NOT underscore; remainder allows alphanumerics, underscores, dots, forward slashes, and hyphens). Rationale: allowing a leading underscore admits inputs like_..foothat pass check 2 before check 4's..guard runs, so the leading underscore is removed to fail-closed earlier. (3) Reject absolute paths (beginning with/). (4) Reject paths containing any\.{2,}substring (two or more consecutive dots, anywhere — covers..,..., etc.). (5) Reject paths where any segment starts with a dot (matches(^|/)\.) to block hidden directories such as.git/or.env.d/and bare.segments. (6) Reject paths where any segment starts with a hyphen (matches(^|/)-) to prevent argument injection in shell commands when paths are passed to CLI tools. (Checks 3-6 serve as defense-in-depth against future relaxation of check 2.) Note: Check 5 intentionally blocks all dot-prefixed path segments, including legitimate dotfiles (e.g.,.eslintrc.js,.prettierrc). To review files under dot-prefixed directories, omit--scopeand use--only=<dimensions>to limit the reviewer dimensions instead. Always double-quote the path in Bash commands. Note:--scopeis a performance/focus tool, not a security boundary. Any component that needs access control (secret redaction, reviewer isolation, cache validation, etc.) must validate independently — don't treat--scopeas a trust gate.--pr=<number>: Validate that the number consists only of digits. Reject non-numeric values with: 'Invalid PR number: must be a positive integer.'--branch=<base>: The<base>value is interpolated intogit merge-base "<base>" HEADandgit diff "<base>"..HEAD— it is the security boundary. Apply the following checks in sequence; reject on first failure: (1) Treat empty--branch=(no value after=) as bare--branch(auto-resolve), NOT as a literal empty string. (2) Reject values containing control characters including null bytes (\0), newlines (\n), and carriage returns (\r). (3) Validate against allowlist regex^[a-zA-Z0-9][a-zA-Z0-9._/-]*$(must start with an alphanumeric character; remainder allows alphanumerics, dots, underscores, forward slashes, and hyphens). (4) Reject any\.{2,}substring (two or more consecutive dots, anywhere — covers..,..., etc.). (5) Reject paths where any segment starts with a dot (matches(^|/)\.). (6) Reject values where any segment starts with a hyphen (matches(^|/)-) to prevent argument injection in shell commands. The regex is tighter than git's actual ref-name rules but matches the--scopedefense-in-depth posture; legitimate branch names that fail this check (e.g., a branch literally named..main) are extraordinarily rare and the user can work around them by checking out a renamed branch. Always double-quote the<base>value in Bash commands.--only=<dimensions>: Trim leading and trailing whitespace from each comma-separated value before validation. Ignore empty entries resulting from consecutive commas. Validate that each remaining value matches one of the recognized built-in dimension names:security,typescript,react,node,database,performance,testing,accessibility,infra,error-handling,simplicity, OR is defined as a custom reviewer dimension in.claude/review-config.md(if loaded in Track A). Reject values that match neither a built-in nor a custom dimension with: 'Unrecognized dimension: "". Valid built-in dimensions: security, typescript, react, node, database, performance, testing, accessibility, infra, error-handling, simplicity. Custom dimensions can be defined in .claude/review-config.md.' Note: /jr-auditsupports additional dimensions (css,dependency,architecture,comment) beyond those listed here. These are specific to audit's broader scope and are not available in/jr-review.--model=<tier>: Allowlist regex^(sonnet|opus|haiku|fable)$. Reject any other value with:Invalid --model value '<value>'. Valid values: sonnet, opus, haiku, fable.(per../shared/model-override.md).
Model requirements
- Reviewer agents (Phase 2) and implementer agents (Phase 5): Spawn with
model: "opus"(or the--modeloverride when set —../shared/model-override.md). Include in each agent's prompt: "Before reporting each finding: (1) re-read the cited code and confirm the issue is real, not speculative; (2) check whether another dimension owns it per the boundary rules and defer if so; (3) calibrate confidence honestly — usespeculativewhen you cannot verify without context you don't have. Do not report findings you would not defend in a PR review." - Simplification agent (Phase 5.5): Spawn with
model: "opus"(or the--modeloverride when set) — it modifies code and needs the same quality bar as implementers. Include the full content of../shared/code-edit-discipline.md(read into lead context at Phase 1 Track A) verbatim in the agent's prompt, prefixed with a Phase-5.5-specific lead-in: "Your assignment is to simplify only when the change measurably improves clarity or removes duplication, while preserving all behavior. The following discipline applies to every line you change:" then the verbatim canonical body. - All other phases (context gathering, dedup, validation, cleanup): Default model is fine — these are mechanical steps. Any agent spawned in these phases also honors a
--modeloverride (the override is total, not premium-sites-only).
Shared secret-scan protocols
The full set of protocols — isHeadless predicate (with AUTO_APPROVE export + post-export self-check), CI/headless secret-halt protocol (combined revert sequence + per-site abortReason), User-continue path after post-implementation secret detection (six mandatory behaviors with register-now-vs-execute-later split), and Advisory-tier classification for re-scans — lives in ../shared/secret-scan-protocols.md (read into lead context at Phase 1 Track A; hard-fail guard ensures it was non-empty and structurally valid). Apply those rules verbatim at every site they're referenced in this skill. The pattern-specific demotion criteria for SK/sk-/dapi are documented inline in Phase 1 Track B step 7 (they're scope-specific to /jr-review's diff-mode reviewing).
Cross-skill contract status
This section tracks cross-skill contracts declared by /jr-review and their implementation status in the consuming skills. When a contract is marked NOT IMPLEMENTED, the artifact produced by /jr-review is an audit trail only — it does not provide an automated safety barrier, and users bear full responsibility for acting on it.
-
/jr-shipPhase 1 reads.claude/secret-warnings*.jsonand refuses to proceed if entries match the working tree — NOT IMPLEMENTED. Until implemented,secret-warnings*.jsonis an audit trail only; users must manually verify that all listed secrets have been removed before committing or opening a PR.
secret-warnings.json schema: The full schema (top-level structure, per-entry fields, patternType enum, validation rules, atomic-write requirements) lives in ../shared/secret-warnings-schema.md (read at Phase 1 Track A). The consumerEnforcement value is "not-implemented" until /jr-ship (or another consumer) implements the block-on-match contract; at that point it flips to "enforced" and this checklist item flips to checked.
Display protocol
Common rules — phase headers, running progress timeline, silent-reviewers/noisy-lead, compact reviewer progress table (Phase 2), Phase 5/6 compact display, console-output redaction (interactive + headless variants) — are in ../shared/display-protocol.md (read into lead context at Phase 1 Track A; hard-fail guard ensures it was non-empty and structurally valid). Apply those verbatim. The two subsections below are /jr-review-specific and stay inline.
Findings approval display (Phase 4)
Always show all findings with full details before asking for approval. For each finding, display:
- Severity and confidence (e.g.,
critical / certain) - File path and line numbers
- What's wrong and what the fix should look like
- Reviewer dimension (e.g.,
typescript,security)
Group findings by confidence tier (certain → likely → speculative), and within each tier sort by severity.
Then present AskUserQuestion as a menu (never a free-text prompt) with these options:
- Approve all — approve every finding
- Approve critical/high, review rest — auto-approve findings that are both certain-confidence AND critical-or-high-severity, then present all remaining findings individually
- Review individually — present each finding one by one via AskUserQuestion menus (approve / reject per finding)
- Abort — cancel the review
Convergence loop display (if --converge is set)
Iteration headers use the ━━━ separator (per ../shared/display-protocol.md). Per-iteration one-line summary: Pass N: <files> reviewed → <findings> findings → <fixed> fixed → validation <state> (<dur>). End-of-loop summary lists total passes, cumulative findings, and the converged-at pass; the fresh-eyes line (if triggered) reads Fresh-eyes: <findings> findings (full diff, single reviewer) — <dur>. Timeline format compresses Phase 2-6 within a pass to Pass N [P2-6]. Example:
Phase 1 ✓ (3s) → Pass 1 [P2-6] ✓ (45s) → Pass 2 [P2-6] ✓ (22s) → Pass 3 [P2-3] ✓ (8s) → Phase 7 ✓ (2s) Total: 80s
Phase 1 — Gather context and detect stack
Pre-checks
Verify git --version succeeds. Detect the forge (algorithm in ../shared/forge-detection.md §a, whose canonical copy is read in Track A): parse the host from git remote get-url origin and set FORGE (github|gitlab) and CLI (gh|glab) — github.com→gh, gitlab.com→glab, unknown→gh + a one-line warning; CLAUDE_FORGE overrides. Translation directive: every gh/PR/checks/github.com command and user-facing string in this skill is the GitHub reference form; when FORGE=gitlab translate each to its glab/MR/pipeline equivalent per forge-detection.md's command-equivalence and terminology tables. External-authority carve-out: the Phase 3 step 0.5 gh api …/contents/<path> doc fetch verifies an external authority (framework/Anthropic docs) and STAYS gh regardless of forge — only commands about the user's own repo switch. Experimental advisory: when FORGE=gitlab AND a user-repo glab call will run this session (--pr, or --branch auto-resolution, or Phase 8 issue/MR ops), print one line per ../shared/forge-detection.md §a "Experimental advisory" — e.g. Note: GitLab support is experimental (pre-Milestone-2): glab field mappings are unverified — PR metadata may be incomplete.. If --pr is set, OR if --branch is set without an explicit <base> value (auto-resolution needs the forge's PR/MR list + repo lookup), also verify forge auth (gh auth status on GitHub; glab auth status on GitLab). If either fails, warn the user and abort. Note: --branch=<explicit-base> does NOT require the forge CLI — only the auto-resolution path does.
AUTO_APPROVE export (when --auto-approve is set): If --auto-approve was parsed, apply the mandatory export AUTO_APPROVE=1 + post-export self-check procedure from ../shared/secret-scan-protocols.md → "AUTO_APPROVE export (mandatory)" — canonical, it owns the self-check and the rationale (the shell predicate keys on the env var, not the parsed flag; failing to export silently downgrades headless to interactive). Restated here only for visibility in the Phase 1 linear flow.
Codebase-memory graph probe: After Track B's diff is collected and the changed-file count is known, probe for codebase-memory-mcp if either of the following triggers fires AND isHeadless is false:
- Diff-size trigger: changed-file count is 20 or more.
- Effort trigger (NEW):
CLAUDE_EFFORTisxhighormax. Read at runtime:effort="$CLAUDE_EFFORT"; case "$effort" in xhigh|max) eagerProbe=true ;; *) eagerProbe=false ;; esac. At higher effort settings, the user explicitly opted into deeper analysis and the index cost is worth paying even on small diffs — the cross-file impact data improves reviewer accuracy on borderline cases.
- Attempt to call
mcp__codebase-memory-mcp__list_projects(via ToolSearch if the schema isn't loaded). On any failure (tool unavailable, load error), setGRAPH_AVAILABLE=falseandGRAPH_INDEXED=false— do NOT block the review on graph absence. - If the tool loads, check whether the current repo (
git rev-parse --show-toplevel) is indexed. SetGRAPH_AVAILABLE=trueandGRAPH_INDEXED=<true|false>accordingly. - If
GRAPH_AVAILABLE=trueandGRAPH_INDEXED=false, offer via AskUserQuestion:Codebase graph is available but not indexed. Indexing improves cross-file impact analysis on this diff. Options: [Index now, then proceed (Recommended)] / [Proceed without indexing]. On Index now, callmcp__codebase-memory-mcp__index_repositoryand wait for completion; setGRAPH_INDEXED=true. On Proceed, continue withGRAPH_INDEXED=false. - If neither trigger fires (diff < 20 files AND effort is not
xhigh/max), or whenisHeadless=true, skip the probe entirely — set both flags tofalse. Rationale: small diffs at default effort don't benefit enough to justify the index cost, and headless sessions shouldn't block on a user prompt. - Pass
GRAPH_AVAILABLEandGRAPH_INDEXEDto Phase 2 so reviewers know whether to call graph tools or fall back to Grep.
PR mode (if --pr=<number> is set)
When reviewing a remote PR, replace Tracks B and D:
- Track B (PR): Run in parallel (GitHub form shown; on GitLab translate per
../shared/forge-detection.mdtoglab mr diff <iid>andglab mr view <iid> -F jsonwith remapped fields):gh pr diff <number> --no-color(get the diff),gh pr view <number> --json title,body,commits,files(PR metadata and context). Use the PR diff as the change set. Forcenofixmode (can't edit remote files). Skip Track D entirely. - Still apply safety checks to the PR diff: diff size guard (step 6), secret pre-scan (step 7), and scope filter (step 4) from normal Track B apply to the PR diff too.
- Cross-file impact caveat: Reviewers Grep the local codebase for consumers of changed exports, but the local checkout may differ from the PR's target branch. Note this limitation in the report if the local branch doesn't match the PR base.
- Tracks A and C proceed as normal (project standards and stack info still apply for reviewer selection).
Branch mode (if --branch is set)
When --branch is set, FIRST read ${CLAUDE_SKILL_DIR}/protocols/branch-mode.md into lead context (hard-fail + non-empty + smoke-parse anchors Detached HEAD check AND Compute merge base; abort [ABORT — SHARED FILE MISSING] per ../shared/abort-markers.md if absent/empty/invalid — skip this read entirely when --branch is not set, mirroring the conditional convergence-protocol.md read under --converge), then run its four sequential pre-checks (detached-HEAD, base resolution, behind-upstream guard, merge-base) BEFORE any tracks fire — all four are hard-fail aborts. On pass it sets branchMode=true, caches mergeBase for Track B, and logs Detected mode: branch (base=<base>, mergeBase=<short-sha>); Tracks A/C/D run unchanged and Track B adds a fifth git diff in branch mode (see Track B).
Normal mode (no --pr)
Maximize parallelism — run all tracks simultaneously using parallel tool calls.
Track A — Read configuration (parallel with Tracks B and C)
Read all of the following files in parallel using multiple Read tool calls in a single message:
CLAUDE.md,AGENTS.md,.claude/CLAUDE.md(project standards, if they exist).claude/review-config.md(suppressions, tuning, overrides from past reviews).claude/audit-history.json(cross-run shared state — schema in../shared/audit-history-schema.md; the file is shared between/jr-reviewand/jr-auditand may be created by either). At Phase 1 Track A, before reading any derivations, apply the canonical Read-side integrity check (mandatory) from../shared/audit-history-schema.md(all three steps: quarantine sentinel, cross-arrayrunIdreachability, timestamp sanity, plus the quarantine protocol). On quarantine, treat audit-history as absent for this run — skip both derivations below AND the Phase 4.5 cross-run promotion check; the Phase 7 step 5 append still recreates the file with this run's entries. Then read two derivations from this file (skip silently if absent — fresh repo, OR quarantined this run):- Reviewer false-positive rates: take the last 5
reviewerStatsentries per dimension (anyskill, both/jr-reviewand/jr-auditcount) and compute the per-dimension running average ofrejectionRate. If a dimension's running average ≥ 0.25, mark it for a calibration note prepended to that reviewer's Phase 2 prompt:Calibration: Your last 5 runs in this project rejected an average of N% of findings — be more conservative on borderline cases. Prefer "speculative" confidence and skip findings you can't cite with a verbatim 3-line excerpt.The calibration note is operational, not informational — reviewers must apply it. lastPromptedAtmap for the global-preference promotion suppression check (see Phase 4.5).
- Reviewer false-positive rates: take the last 5
- Shared protocol files — single source of truth (resolve paths relative to this SKILL.md — one directory up then into
shared/):../shared/reviewer-boundaries.md,../shared/untrusted-input-defense.md,../shared/gitignore-enforcement.md,../shared/abort-markers.md,../shared/secret-warnings-schema.md,../shared/display-protocol.md,../shared/audit-history-schema.md,../shared/secret-scan-protocols.md,../shared/secret-patterns.md,../shared/cache-schema-validation.md,../shared/phase1-track-a-protocol.md,../shared/code-edit-discipline.md,../shared/claim-verification.md,../shared/forge-detection.md,../shared/model-override.md. These files are load-bearing — their content is referenced by later phases via call-sites (apply … per ../shared/<file>) rather than duplicated inline; the per-file usage map lives in the repoCLAUDE.md"shared/ — single source of truth" section. Passreviewer-boundaries.mdcontent verbatim to every reviewer prompt; passuntrusted-input-defense.mdcontent verbatim into every reviewer and implementer prompt; passclaim-verification.mdcontent verbatim to every reviewer prompt (the lead performs the authoritative claim classification at Phase 3 step 0.5). Hard-fail guard: if any shared file fails to Read, or returns empty content (zero bytes or whitespace-only), abort Phase 1 immediately with: "Phase 1 aborted:<path>is missing or empty. /jr-review requires shared protocol files to enforce reviewer boundaries, untrusted-input safety, cache-write .gitignore checks, abort-marker rendering, secret-warnings schema validation, display-protocol consistency, audit-history schema, secret-scan protocols, secret-pattern catalog, cache-schema validation, Phase 1 Track A smoke-parse, code-edit discipline, claim verification, forge detection, and model-override semantics. Restore the file from git or the repo's canonical copy before re-running." Do NOT fall back to inline text — the inline duplicates were intentionally removed to eliminate drift; a missing shared file means the skill's guarantees cannot be enforced. Record the file byte-size and first-line hash in memory at Phase 1 so later call-sites can re-verify if they suspect tampering between Read and use. Structural smoke-parse (mandatory): After the read succeeds and content is non-empty, apply the smoke-parse algorithm and Canonical Anchor Table from../shared/phase1-track-a-protocol.md— each row of the canonical's table lists the required substrings for one shared file (case-sensitive, fixed-string match viagrep -F, AND-joined within a row). Self-reference escape hatch (hardcoded): before parsing the canonical's table, verify../shared/phase1-track-a-protocol.mditself contains the literal stringCanonical Anchor Table— a stub corruption of the canonical that preserves only its self-row anchor would otherwise pass the table-driven check. If any file fails the smoke-parse (self check OR any row check), abort Phase 1 with: "Phase 1 aborted:<path>is structurally invalid (smoke-parse:<missing-substring>). Restore the file from git or the repo's canonical copy before re-running." - Skill-local script and template presence (mandatory): Verify each skill-local file under
${CLAUDE_SKILL_DIR}/{scripts,templates}/exists (and, for scripts, is executable) BEFORE Phase 5 dispatches anything that needs it. Mirror the shared-file hard-fail discipline — silent absence at the call-site causes a mysterious Phase 5 failure instead of a clean Phase 1 abort. Run all checks in parallel via[ -x <script> ]/[ -f <template> ]Bash tests; if any check fails, abort Phase 1 with: "Phase 1 aborted:<path>is missing or not executable. /jr-review requires skill-local scripts and templates to enforce Phase 5 base-commit-anchor symlink validation and Phase 5.6 pre-commit hook installation. Reinstall the skill or restore the file from git before re-running." Required files (must each be present; scripts must also have the executable bit set):${CLAUDE_SKILL_DIR}/scripts/establish-base-anchor.sh— Phase 5 base-commit anchor + symlink-escape validation (executable).${CLAUDE_SKILL_DIR}/scripts/install-pre-commit-secret-guard.sh— Phase 5.6 pre-commit hook installer; performs SHA-256 verification of the template before writing to.git/hooks/pre-commit(executable).${CLAUDE_SKILL_DIR}/templates/pre-commit-secret-guard.sh.tmpl— canonical pre-commit hook body (read-only file; the install script reads + hash-verifies before appending).
- Skill-local protocol files (mandatory): Read the following protocol files into lead context (parallel Reads with the shared/* files above). Apply the same hard-fail + non-empty + smoke-parse discipline: abort Phase 1 with
[ABORT — SHARED FILE MISSING]per../shared/abort-markers.mdif any file is absent / empty or fails its smoke-parse anchor. Smoke-parse anchors (case-sensitivegrep -F):${CLAUDE_SKILL_DIR}/protocols/phase2-reviewers.md—Effort-adaptive breadthANDcodeExcerpt${CLAUDE_SKILL_DIR}/protocols/finding-sanity-check.md—content-excerpt match${CLAUDE_SKILL_DIR}/protocols/secret-warnings-lifecycle.md—Lifecycle of \unverified` entries`${CLAUDE_SKILL_DIR}/protocols/base-anchor.md—Combined revert sequence${CLAUDE_SKILL_DIR}/protocols/pre-commit-hook-offer.md—Install procedure${CLAUDE_SKILL_DIR}/protocols/phase7-cleanup-report.md—Run-scoped flags initializationANDPer-session filename banner${CLAUDE_SKILL_DIR}/protocols/phase8-followups.md—Public repository checkANDDedup decision logging${CLAUDE_SKILL_DIR}/convergence-protocol.md(only when--convergeis set) —priorFindingsANDfreshEyesMandatoryANDProceed to Phase 7 (cleanup and report). Three anchors at top/mid/bottom — a mid-file truncation that drops the convergence loop body would otherwise pass two top-half-only anchors. Skip read+smoke-parse entirely if--convergewas not passed; if--convergeis set and the file is invalid, hard-fail.
- Project memory (auto-memory system, silent no-op if absent — new project): compute the memory dir via
memoryDir=~/.claude/projects/"${PWD//[.\/]/-}"/memory(the encoding replaces/and.in$PWDwith-). Read"$memoryDir/MEMORY.md"first; the file is an index of- [Title](file.md)pointers. Then fan out in parallel to read every referencedfeedback_*.md,project_*.md,reference_*.md, anduser_*.mdfile in$memoryDir. These entries are explicit user decisions from prior sessions in this project — treat them with the same precedence asCLAUDE.md. Pass the concatenated content to reviewers in Phase 2 as an additional Project memory block alongside the existing project-standards context. - User-global memory —
user-type entries only (silent no-op if absent): also read~/.claude/projects/-Users-jroussel--claude-skills/memory/MEMORY.md(the user's auto-memory dir; the path is fixed and independent of$PWD). From the index, fan out in parallel touser_*.mdfiles only — skipfeedback_*.md,project_*.md, andreference_*.mdhere because those are project-specific and must NOT leak across repos (e.g., a React/Tailwind-flavored feedback entry must not apply when reviewing a Python service).user_*.mdentries describe the user's role, expertise, and communication preferences — those apply globally. If the current CWD is itself the skills repo, this file is the same as the project-memory read above; read it once and de-duplicate. Pass the user-global block to reviewers in Phase 2 under a User-global context header, separate from Project memory.
These rules override generic best practices. If a pattern is suppressed, no reviewer should flag it.
Track B — Collect diff and git context (parallel with Tracks A and C)
Run all of the following git commands in parallel using multiple Bash tool calls in a single message:
- Branch mode only (when
branchMode=true, set in Phase 1 Branch mode pre-checks):git diff --no-color "$mergeBase"..HEAD(committed-on-branch changes — every commit since the branch diverged from the resolved base). If--scopeis set, append-- <path>. git diff --no-color(unstaged changes). If--scopeis set, append-- <path>to scope at the git level.git diff --cached --no-color(staged changes). If--scopeis set, append-- <path>.git status(untracked files)git log --format="%h %s" -10(recent commit context — subject lines only)
In branch mode, all five outputs are concatenated and fed to reviewers as the diff context. Each git diff segment carries its own diff --git a/... header, so the boundary between committed/staged/unstaged segments is unambiguous to both reviewers and the Phase 3 codeExcerpt match. Untracked files are still read in full at step 5 below. Do not collapse the four-piece form into a single git diff "$mergeBase" — the separation preserves index-vs-working-tree-vs-committed semantics that reviewers depend on (e.g., a finding citing a staged-only change should be distinguishable from a committed change in the report).
After the above complete:
Staged secret file check: If any staged/untracked files match
.env*,*.pem,*.key,credentials*,*secret*, take action: In interactive mode, use AskUserQuestion: 'Sensitive files are staged: [list]. Options: [Continue — files will be read by reviewers] / [Abort and unstage first]'. In headless/CI mode (whenisHeadlessistrue— see../shared/secret-scan-protocols.md→ "Headless/CI detection"), abort immediately with an error listing the sensitive filenames: 'Sensitive files staged in headless mode — aborting. Unstage these files before running /jr-review: [list]'. Rationale: in CI, there is no user to make an informed decision about whether sensitive file contents should be passed to reviewer agents.Merge conflict check: If the diff contains
<<<<<<<or>>>>>>>markers, abort with: "Merge conflicts detected. Resolve conflicts before running /jr-review."Empty diff check: If the combined diff (staged + unstaged + untracked) is empty, stop with: "Nothing to review — no changes detected."
Scope filter: If
--scope=<path>is set, filter untracked files to only include those under the path prefix (the diff was already scoped at the git level above).Read any untracked files in full.
Diff size guard: Count total changed lines. If exceeding 3,000 lines, warn via AskUserQuestion: "This diff is large (N lines). Options: [Continue] / [Use quick mode] / [Scope to path] / [Abort]". If exceeding 8,000 lines, strongly recommend splitting. In headless/CI mode (when
isHeadlessistrue— see../shared/secret-scan-protocols.md→ "Headless/CI detection"), do not use AskUserQuestion. For diffs exceeding 3,000 lines, automatically applyquickmode. For diffs exceeding 8,000 lines, abort with an error message recommending the diff be split. Log the decision in the Phase 7 report.Secret pre-scan: Apply the canonical pattern catalog and safeguards from
../shared/secret-patterns.md(POSIX ERE smoke probe, per-line 10000-byte cap,grep -Eiinvocation, full token-prefix + connection-string + quoted/unquoted-assignment regex union). At this Phase 1 site, treat all matches as strict tier — no advisory-tier demotion (the demotion criteria for SK/sk-/dapi apply only to post-implementation re-scans per../shared/secret-scan-protocols.md).On match: in interactive mode, AskUserQuestion
Potential secrets detected in diff: [pattern types]. Options: [Continue anyway] / [Abort and unstage]. In headless mode (per../shared/secret-scan-protocols.md"Headless/CI detection"), abort immediately listing pattern types only (NOT matched values). Never auto-continue past a secret detection in headless mode.
User-continue path applies to Phase 1 too. When the user chooses "Continue anyway" at Phase 1's interactive secret pre-scan, apply ALL SIX behaviors of the User-continue path protocol defined in ../shared/secret-scan-protocols.md ("User-continue path after post-implementation secret detection") — no subset permitted; same execution-order requirements as documented in the shared file.
Track C — Detect stack and validation (parallel with Tracks A and B)
Stack profile cache: Check .claude/review-profile.json to avoid re-detecting unchanged stack info.
Read
.claude/review-profile.json(if it exists) and runstat -f %m package.json tsconfig.json Makefile 2>/dev/null— both in parallel.If the profile exists AND
--refresh-stackwas NOT passed: Compare current modification timestamps against cachedsourceTimestamps. If all match (and absent files are still absent), apply the schema validation + binary availability probe + same-session shortcut rules from../shared/cache-schema-validation.md(canonical for bothreview-profile.jsonandreview-baseline.json). On any failure, force full re-detection. Otherwise use cachedpackageManager,lockFile,validationCommands. Output:Stack: cached (${packageManager}, ${Object.keys(validationCommands).join('+')}). Skip to Track D.If the profile is missing, timestamps differ, or
--refresh-stackwas passed — run full detection:- Read
package.jsonin parallel with checking for lock files (bun.lockb,pnpm-lock.yaml,yarn.lock) and readingtsconfig.json,Makefile,docker-compose.yml. - Detect validation commands from
package.jsonscripts: look forlint,typecheck/type-check/tsc,test,validate. Build a list using the detected package manager. If aMakefilehas matching targets, use those. If no commands found, skip validation phases. - Write cache: Save results to
.claude/review-profile.json:{ "version": 1, "generatedAt": "<ISO timestamp>", "sourceTimestamps": { "package.json": <mtime or null>, "tsconfig.json": <mtime or null>, "Makefile": <mtime or null> }, "packageManager": "<bun|pnpm|yarn|npm>", "lockFile": "<filename or null>", "validationCommands": { "lint": "<command or null>", "typecheck": "<command or null>", "test": "<command or null>", "format": "<command or null>" }, "frameworks": ["<detected from dependencies, e.g. next, react, tailwindcss, drizzle-orm>"] }
Security check (enforced): Apply the
.gitignore-enforcement protocol (see../shared/gitignore-enforcement.md, read at Phase 1 Track A) for path.claude/review-profile.json. Core command:git ls-files --error-unmatch .claude/review-profile.json 2>/dev/null— then apply the shared file's warn/append-and-inform protocol (headless/CI logs to the Phase 7 report). Per-site reason if tracked: "A committed cache file could be manipulated — a malicious commit could setvalidationCommandsto all-null values to disable validation."- Output:
Stack: detected (${packageManager}, ${Object.keys(validationCommands).join('+')}) — cached for next run
- Read
Track D — Establish validation baseline (skip if nofix, quick, or PR mode)
Baseline cache: Check .claude/review-baseline.json to avoid re-running slow validation commands on rapid back-to-back reviews.
- Read
.claude/review-baseline.json(if it exists). - If the cache exists, is within TTL (default 10 minutes), AND
--refresh-baselinewas NOT passed: apply the schema validation rules from../shared/cache-schema-validation.md(review-baseline.json section). On any failure, fall through to step 3. Otherwise use cached results. Output:Baseline: cached (${age}m old, TTL ${ttl}m). Skip validation runs. - Otherwise: Run all detected validation commands in parallel (lint, typecheck, test as separate simultaneous Bash calls in a single message). Store outputs as the baseline. Write results to
.claude/review-baseline.json:
Output:{ "generatedAt": "<ISO timestamp>", "ttlMinutes": 10, "results": { "lint": { "exitCode": 0, "issueCount": 3 }, "typecheck": { "exitCode": 0, "errorCount": 0 }, "test": { "exitCode": 0, "passCount": 42, "failCount": 0, "summary": "<first 5 lines of output>" } } }Baseline: fresh (lint+typecheck+test) — cached for 10mSecurity check (enforced): Apply the.gitignore-enforcement protocol (see../shared/gitignore-enforcement.md, read at Phase 1 Track A) for path.claude/review-baseline.json. Core command:git ls-files --error-unmatch .claude/review-baseline.json 2>/dev/null— then apply the shared file's warn/append-and-inform protocol (headless/CI logs to the Phase 7 report). Per-site reason if tracked: "A committed baseline with inflated failure counts could make real regressions appear pre-existing, silently passing Phase 6 validation."
Invalidate the baseline cache automatically whenever .claude/review-profile.json is invalidated (stack change implies baseline change). Pre-existing failures are NOT the review's responsibility — only new regressions introduced by fixes count.
After all tracks complete
Store the list of changed files, project standards, review config rules, commit context, stack info, and validation commands.
Build file→dimension mapping: Classify each changed file into one or more reviewer dimensions (typescript, react, node, database, performance, testing, accessibility, infra, error-handling). This mapping drives: (1) which reviewers to spawn, (2) which files each reviewer receives. If --only is set AND --scope is set, verify the intersection is non-empty — if no files match the requested dimensions within the scope, stop with: "No files match --only=${dims} within --scope=${path}. Nothing to review."
Phase 2 — Spawn reviewers (dynamically scaled)
The full Phase 2 protocol — effort-adaptive breadth (the CLAUDE_EFFORT overlay), diff-size classification, dynamic reviewer selection, swarm scaling + team-creation thresholds, the reviewer-instructions block (scope rule, finding budget, per-reviewer calibration note, untrusted-input defense), cross-file impact analysis, and the finding format (severity / confidence / codeExcerpt) — lives in protocols/phase2-reviewers.md (read into lead context at Phase 1 Track A under the hard-fail + smoke-parse guard). Apply that protocol verbatim.
Phase 3 — Deduplicate and prioritize
After all reviewers complete (or hit their turn budget), read the full task list (TaskList). As the lead:
- Sanity-check findings (reject hallucinations): Apply the canonical procedure in
protocols/finding-sanity-check.md— for each finding, verifyfileexists,lineis valid and in range, andcodeExcerptmatches the file content (working-tree/branch reads local;--prfetches viagh apito a snapshot tmpdir). Run all checks in parallel via batched Bash + Read. Reject under[REJECTED — INVALID CITATION]with reason. Track per-reviewer rejection rate; ≥ 25% triggers a Phase 7ACTION REQUIREDnote. 0.5. Verify claims (reject + cap hallucinated external facts): Apply../shared/claim-verification.md. For every finding that survived step 0, the lead (not the reviewer) classifies it by scanning its title/description for external-authority language (deprecated,removed in,as of version,violates the rules of,OWASP,CVE-,WCAG,best practice, version numbers tied to a behavior claim), defaulting to external-authority when in doubt; a finding is code-internal only when its correctness is provable from the cited code plus other local files (already covered by step 0). For each external-authority claim, first try to ground it in the project's own internal official documentation (pinned dependency version inpackage.json/lockfile, local.d.ts/type defs, config) — if grounded, keep it and tag the source. Otherwise, by default fetch an authoritative source (skip only under--no-verify-claims) per the doctrine's source-fetch discipline (gh api …/contents/<path>raw markdown, or the doc's raw.md; never rest a load-bearing claim on a singleWebFetchsummary — cross-check it).confirmed→ keep and tag[verified: <source> <date>];refuted→ reject under[REJECTED — CLAIM REFUTED BY SOURCE]with the citedfile:line, the claim, and the contradicting source;unreachable/ambiguous/--no-verify-claims→ fall through to the cap. In the fall-through (verification skipped, unreachable, or uncorroborable), cap the finding's confidence tospeculativeand tag it[unverified external claim](original severity preserved for display — see step 2). This runs BEFORE Phase 4, so a capped claim is never auto-approved (auto-approve and convergence cover onlycertain/likely). Verify each unique claim-source at most once per run (in-memory dedup). Track a per-reviewerrefutedrate; ≥ 25% triggers a Phase 7ACTION REQUIREDnote (mirrors step 0). The external-doc fetch is independent of--pr/--branch(it fetches docs, not the changed files) and is forge-independent: thisgh api …/contents/<path>verifies an external authority (framework/Anthropic docs), so it STAYSgheven when the user's repo is on GitLab — per the external-authority carve-out in../shared/forge-detection.md§e. Only the user's own repo (e.g. the--prcodeExcerpt fetch infinding-sanity-check.md) switches with the detected forge. - Deduplicate: If multiple reviewers flagged the same code location, merge into one task and keep the most actionable description. Use the highest confidence level among the merged findings. Exception: if merged findings have contradictory suggested fixes (e.g., "add type assertion" vs "remove type assertion"), keep both as separate findings and flag the conflict for the user in Phase 4.
- Drop low-value items: Delete tasks with severity
lowunless they are trivially fixable. Exception: never drop a finding whose lowered confidence is solely a step-0.5 unverified-external cap — its original severity governs visibility, so ahigh-severity unverified deprecation stays visible tagged[unverified — needs confirmation]. - Prioritize: Reorder by severity first, then by confidence within the same severity level —
certainbeforelikelybeforespeculative. - Assign file ownership: Group tasks by file. Each file should be owned by at most one implementer agent to prevent conflicts.
- Verify reviewer coverage: For each reviewer, check that findings or explicit "no issues" notes exist for all files in their scope. If a reviewer missed files (no findings AND no explicit clearance), note the gap in the Phase 7 report.
Display: Output a compact dedup summary: 18 raw → 12 deduplicated (4 merged, 2 dropped). Update the running progress timeline.
Zero findings: If no findings remain after dedup, skip Phases 4-6 and 8. Output: "Clean review — no findings. All changed code looks good." Proceed directly to Phase 7 for cleanup and a compact report.
Within a convergence pass (iteration 2+): Zero findings after dedup means convergence is achieved. Set converged = true, log the iteration, and exit the convergence loop. Proceed to the fresh-eyes verification check (see "Convergence loop" section).
Phase 4 — User approval gate
If --auto-approve is set: Skip the approval menu entirely. Auto-approve findings with confidence certain on the first pass. If --converge is also set, defer likely-confidence findings to convergence passes where prior fixes provide additional context and the iterative verification provides a safety net — likely findings are auto-approved starting from convergence pass 2+ per the convergence auto-approval policy. If --converge is NOT set, include likely-confidence findings in the Phase 7 report as remaining findings requiring human review. Defer speculative findings to Phase 7 as remaining in all cases (they require human judgment). Because Phase 3 step 0.5 verifies external-authority claims by default, a confirmed one is auto-approved like any finding at its confidence, a refuted one is dropped, and only an unverifiable one (offline, uncorroborable, or --no-verify-claims) is capped to speculative and deferred here — never auto-applied. Output: Phase 4 — N findings auto-approved (certain), M likely deferred to convergence, J speculative deferred (or M likely deferred to report if --converge is not set). Skip Phase 4.5. Proceed directly to Phase 5.
Display: Show the finding count header first: Phase 4 — N findings for approval. Then show all findings with full details (severity, confidence, file, line numbers, description, suggested fix, reviewer dimension) grouped by confidence tier and sorted by severity.
Pre-approval advisor check (conditional): Before presenting the approval menu (and before the --converge consent prompt below), evaluate two signals over the deduplicated findings:
- High volume:
totalFindings >= 20. - Dimension skew: any single reviewer dimension contributes
>= 60%oftotalFindings. (Skip the check iftotalFindings < 5— too few to compute a meaningful skew.)
If EITHER signal trips, call advisor() (no parameters — the full transcript is auto-forwarded) for a second opinion on the finding set BEFORE the user is asked to approve. The advisor sees the run's evidence (reviewer outputs, Phase 3 step 0 hallucination-rejection rates, codeExcerpt verifications) and can spot reviewer drift the lead missed. If the advisor concurs, proceed silently to the menu. If the advisor raises a concrete concern (e.g., "12 of 18 findings are from react-reviewer flagging the same component shape — likely a calibration issue, not 12 separate problems"), surface it via AskUserQuestion BEFORE the approval menu: Advisor flagged a concern about the finding set: <one-line summary>. Options: [Continue to approval menu] / [Drop the flagged dimension's findings and proceed] / [Abort the review and re-run with --only=...]. On Drop the flagged dimension, treat all findings from that dimension as Phase 3 step 0 hallucination rejections (record them in audit-history.json under runs[] with rejected: true for FP-rate persistence). The advisor runs at most once per /jr-review run at this site — do NOT re-call between iterations or convergence passes (the convergence pre-iteration advisor at iter ≥ 3 covers later checkpoints). Skip the pre-approval check entirely if --auto-approve is set (no menu to gate), if quick is set (advisor latency outweighs benefit on lightweight reviews), or if totalFindings == 0 (already short-circuited to Phase 7 by Phase 3).
If --converge is set (without --auto-approve): Before presenting the findings approval menu, use AskUserQuestion to confirm the convergence auto-approval policy: "Convergence passes 2+ will auto-approve findings without prompting. likely findings are ones where the reviewer was 'fairly confident but the issue depends on context they can't fully verify' — incorrect auto-approved fixes could introduce logic bugs not caught by the fresh-eyes security pass. To review every iteration interactively, run /jr-review repeatedly instead of using --converge. Options: [Auto-approve certain AND likely in convergence (default)] / [Only auto-approve certain in convergence — defer likely to report] / [Abort]". If the user chooses "Only auto-approve certain", convergence passes auto-approve only certain-confidence findings and defer likely alongside speculative to the Phase 7 report. Store the choice for the convergence loop. (Unverified external-authority claims are capped to speculative at Phase 3 step 0.5, so they are never auto-approved under either choice.)
Then show all findings and use AskUserQuestion as a menu (always provide selectable options, never ask the user to type free text) with:
- Approve all — approve every finding
- Approve critical/high, review rest — auto-approve findings that are both certain-confidence AND critical-or-high-severity, then present all remaining findings individually
- Review individually — present each finding one by one via AskUserQuestion menus (approve / reject per finding)
- Abort — cancel the review without making changes
If the user aborts, set abortMode=true and abortReason="user-abort" unconditionally — whether or not $baseCommit is established. The run was cancelled; the audit trail must persist. Then skip to Phase 7 (cleanup). When aborting, if $baseCommit has been established (i.e., implementers have previously run in an earlier convergence pass), apply the full revert sequence (see Phase 5 "Base commit anchor") before proceeding to Phase 7 to ensure no potentially tainted changes remain in the working tree. Untracked files are cleaned first because they may contain secrets introduced by implementers and would not be removed by a subsequent git checkout if the process is interrupted. If $baseCommit has not been established (first pass, no implementers have run yet), no revert is needed — simply skip to Phase 7. Otherwise, delete any tasks the user rejected and proceed. On either abort path (revert or no-revert), if a team was created in Phase 2, call TeamDelete to clean up agents (do not wait for shutdown confirmations) and exit the review with a non-zero status, consistent with the CI/headless secret-halt protocol.
If nofix flag is set: After approval/review, skip directly to Phase 7 — do not implement or validate. The approved findings serve as the review output.
Phase 4.5 — Auto-learn from rejections
If the user rejected any tasks, analyze the rejected findings for patterns and automatically append learned suppressions to .claude/review-config.md. Create the file if it doesn't exist.
Rules for auto-learning:
Only add a suppression if 2 or more rejected findings share the same pattern (same reviewer dimension + same type of issue) within this review OR across recent reviews. Check existing entries in
## Auto-learned suppressions— if a pattern was rejected once before and once now, that counts as 2. A single rejection might be situational — a repeated rejection is a preference. Findings auto-capped tospeculativeby Phase 3 step 0.5 (unverified external claims) that the user did NOT explicitly reject do NOT count toward this threshold — a cap is not a rejection.Never auto-learn suppressions for the
security-reviewerdimension. Security rejections should always be treated as situational rather than patterns to learn from. If a user wants to suppress specific security patterns, they must add them manually to.claude/review-config.md(requiring explicit intent). This prevents progressive disabling of security checks through accumulated auto-learned suppressions.Cross-run memory promotion (global preference check): After the repo-local suppression is appended, check whether this
dimension+categoryhas accumulated 2+ rejections in 2+ separate runs by scanning.claude/audit-history.json(canonical, shared with/jr-audit— see../shared/audit-history-schema.mdbelow). If 2+ separate-run rejections are confirmed ANDdimensionis NOTsecurityANDisHeadless=falseAND nolastPromptedAtentry blocks re-prompting (see below), offer via AskUserQuestion:You've rejected this pattern across N separate runs. Save as a global preference in your user memory? [Yes — save globally] / [No — keep repo-scoped]. On Yes, write afeedback-type memory file to~/.claude/projects/-Users-jroussel--claude-skills/memory/feedback_<dimension>_<category>.mdwith body structurerule + **Why:** + **How to apply:**per the auto-memory system's feedback format, then append a one-line pointer to that memory dir'sMEMORY.md. On either Yes or No, recordlastPromptedAt: ISO8601keyed by<dimension>:<category>in.claude/audit-history.json(see../shared/audit-history-schema.md— single canonical map shared with/jr-audit). Re-prompting on the same pair is suppressed until 2 additional rejections accumulate AFTERlastPromptedAt. Rationale: 2 separate-run rejections of the same exactdimension+categoryis already a strong preference signal — categories are granular enough that 2-of-2 is conservative; explicit consent still required so a misclassified rejection cannot silence findings without the user's say-so.Cross-run shared state (
.claude/audit-history.json): schema, append-only invariants, schema-mismatch handling, and.gitignore-enforcement rules live in../shared/audit-history-schema.md(read at Phase 1 Track A). Both/jr-reviewand/jr-auditMUST read and write the same schema; the shared file is the single source of truth.Write each suppression as a clear, scoped rule. Include the reviewer dimension and the specific pattern. Example:
- [react-reviewer] Do not suggest extracting sub-components for components under 100 linesAppend under a
## Auto-learned suppressionssection, with a date stamp for each entry.Never overwrite or remove existing rules in the file — only append.
If no patterns are detected in the rejections (all were one-offs), skip this step.
Note: If an auto-learned suppression is wrong, the user can manually edit .claude/review-config.md to remove it. The skill will never auto-remove suppressions — only append.
Security check (enforced): Apply the .gitignore-enforcement protocol (see ../shared/gitignore-enforcement.md, read at Phase 1 Track A) for path .claude/review-config.md. Core command: git ls-files --error-unmatch .claude/review-config.md 2>/dev/null — then apply the shared file's warn/append-and-inform protocol (headless/CI logs to the Phase 7 report). Per-site reason if tracked: "A committed config with crafted suppression rules could silence security findings across all future reviews. If the team intentionally commits shared review configuration, scope auto-learned suppressions to a separate section that reviewers can distinguish from manually authored rules."
Additionally, when loading review-config.md in Track A, check whether the file is tracked by git (git ls-files --error-unmatch .claude/review-config.md 2>/dev/null). If it is tracked and contains any [security-reviewer] suppression rules, emit a warning: 'review-config.md is tracked by git and contains security-reviewer suppressions. Committed security suppressions can silence security findings for all users. Verify the file intentionally contains these rules.' In headless/CI mode, log this warning to the Phase 7 report rather than to console output.
Write-failure handling: If the append to .claude/review-config.md fails (disk full, permission denied, etc.), log the failure to the Phase 7 report under an [AUTO-LEARN SKIPPED] marker with the filesystem error message and the pattern that was not learned. Do not halt — auto-learn is a non-critical ergonomics feature. Subsequent runs will retry naturally when the user rejects the pattern again.
Phase 5 — Spawn implementer swarm
Skip this phase entirely if nofix flag is set.
Pre-dispatch advisor check (mandatory): Before recording the base commit anchor below, call advisor() (no parameters — the full transcript is auto-forwarded). /jr-review Phase 5 is the highest-blast-radius operation in the skill — multiple implementers will modify multiple files in parallel based on auto-approved findings. The advisor sees the approved finding set, the file→implementer allocation plan from Phase 3, and the project context, and can flag risky combinations (e.g., "implementer A is rewriting auth.ts while implementer B is rewriting middleware.ts that imports from auth.ts — these should be sequenced, not parallel"). If the advisor concurs, proceed silently. If the advisor raises a concrete concern, surface via AskUserQuestion BEFORE spawning: Advisor flagged a Phase 5 dispatch concern: <one-line summary>. Options: [Proceed as planned] / [Re-allocate files (give all related files to one implementer)] / [Abort and re-run with --only=...]. On Re-allocate files, redo the file-ownership assignment to merge the implicated files into a single implementer's task list, then proceed. On Abort, skip Phase 5 and Phase 6, set abortMode=true and abortReason="user-abort", and proceed to Phase 7 (no revert needed — $baseCommit is not yet established at this site). The advisor runs at most once per /jr-review run at this site — do NOT re-fire on convergence-iteration dispatches (the pre-iteration advisor at iter ≥ 3 covers convergence checkpoints). Phase 5 is the substantive-edit boundary regardless of mode and regardless of finding count: even a small dispatch mutates user code in parallel. The only skips are (a) nofix (no dispatch happens) and (b) a single-implementer/single-file dispatch where blast radius is genuinely contained.
Base commit anchor: Before spawning implementers, record the current commit hash and capture pre-Phase-5 baselines for untracked files and symlinks. Apply the canonical procedure in protocols/base-anchor.md (anchor capture via ${CLAUDE_SKILL_DIR}/scripts/establish-base-anchor.sh, NUL-delimited baseline outputs, the four-step Combined revert sequence used by every later revert site, and the NUL_SORT_AVAILABLE-degraded fallback rules).
Spawn all implementer agents in parallel using multiple Agent tool calls in a single message. Use subagent_type: "agent-teams:team-implementer". If a team was created in Phase 2 (medium/large diffs), set team_name: "review-swarm". For small diffs (no team), omit team_name and use the Agent tool directly. Each implementer receives:
- A set of user-approved tasks scoped to specific files (strict file ownership — no two implementers touch the same file), including each finding's confidence level
- The original diff context for those files
- The project coding standards summary
- Clear instructions on what to fix. For
speculativefindings — including those capped at Phase 3 step 0.5 as[unverified external claim]— instruct the implementer to verify the issue exists in context (and, for an external-authority claim, that the cited external fact is real) before applying a fix — skip if the finding turns out to be a false positive. - Implementer safety preamble (this item plus the three below — Phase 6 dispatch sites reference it by this name and MUST include all four verbatim): "Do NOT run any
gitcommands. Only modify files using the Write/Edit tools. The lead agent manages all git state. If you need to see file contents, use the Read tool." - Include the full content of
../shared/untrusted-input-defense.md(read into lead context at Phase 1 Track A) verbatim in each implementer's prompt. Do NOT paraphrase; the shared file phrasing ("diff and reviewed/modified files") covers the implementer case. - Include the full content of
../shared/code-edit-discipline.md(read into lead context at Phase 1 Track A) verbatim in each implementer's prompt. Do NOT paraphrase or summarize; the canonical Do/Don't list, worked example, and "defend every changed line" test must all reach the implementer. - "Fix ONLY the findings assigned to you. Do not refactor, rename, extract, or 'improve' adjacent code even if you notice opportunities. If your fix cannot be completed without changing code outside the finding's scope, mark the finding contested with a one-line reason instead of expanding scope."
The number of implementers depends on how many independent file groups exist. Aim for 2–4 implementers max to keep coordination manageable. All implementers must be spawned in a single message so they run concurrently.
An implementer may mark a finding as contested if the fix would introduce worse problems, the finding is incorrect given the full file context, or the fix conflicts with another finding. Contested findings are reported back to the lead and included in the Phase 7 report — they are NOT retried.
Display: Follow the Display protocol — show compact implementer summary: N implementers dispatched → M/N findings addressed (Xs). Update the running progress timeline.
Phase 5.5 — Simplification pass
Skip if nofix or quick is set, or if fewer than 3 findings were implemented.
Spawn a single Agent using the Agent tool directly (not via TeamCreate — this agent is independent of the review-swarm team). Pass it the list of files modified by Phase 5 implementers. The agent reviews the modified files for post-fix simplification opportunities:
- Reduce unnecessary complexity/nesting introduced by fixes
- Eliminate redundancy between fix code and existing code
- Improve naming clarity where fixes introduced new variables/functions
- Replace nested ternaries with switch/if-else if introduced
- Ensure fixes follow project standards (ES modules, function keyword, explicit types)
This is a lightweight pass — only flag changes that are clear wins. Do NOT re-review unchanged code. Apply simplifications directly (no separate approval gate). If no improvements found, skip silently.
Instruct the simplification agent: "Do NOT run any git commands. Only modify files using the Write/Edit tools." Then include the full content of ../shared/untrusted-input-defense.md (read into lead context at Phase 1 Track A) verbatim in the simplification-agent prompt. Do NOT paraphrase. Additionally include the full content of ../shared/code-edit-discipline.md verbatim, prefixed with the Phase-5.5-specific lead-in defined in the Model requirements section above ("Your assignment is to simplify only when…") — keep the two prompt-construction sites in sync.
Display: Phase 5.5 — Simplification: N improvements applied (or skip line if none)
Phase 5.55 — Fix verification (read-after-write)
Skip if nofix flag is set (no fixes were applied). Skip findings marked contested by their implementer (the implementer explicitly declined to fix; there is nothing to verify).
For each finding that Phase 5 marked as "addressed", the lead agent performs a targeted re-read to confirm the fix actually resolved the cited issue. This catches the failure mode where an implementer technically modified the file but did not address the finding (e.g., added // @ts-expect-error instead of handling the null, renamed a variable but left the bug, or fixed the wrong line). The check is lightweight and strictly additive — it does NOT re-review the rest of the file.
For each addressed finding, in parallel (batch Read calls into a single message across findings):
- Re-read the cited
fileover the range[line - 5, line + 5](clamped to file bounds). - Evaluate against the finding's description and suggested fix: is the issue described as "wrong" still present at (or near) the cited line? Consider small line-number drift (±5) from the implementer's edit.
- Classify the finding into one of:
- verified: the fix is visible and plausibly resolves the cited issue. No action.
- unverified: the fix is not visible, the cited line is unchanged, or the fix looks like a suppression (
@ts-expect-error,eslint-disable, swallowing catch) rather than a resolution. Mark the finding with averified=falseflag and surface in the Phase 7 report underACTION REQUIRED: Fix did not resolve cited issue: <dimension>/<category> at <file>:<line>. - moved: the cited line no longer contains the problem but the fix is at a different nearby line (common with formatter adjustments). Treat as verified and note the line shift in the Phase 7 report.
Thresholds:
- If more than 30% of findings in a single dimension are
unverified, surface a Phase 7ACTION REQUIREDnote:High unverified rate in <dimension> (<N>/<M>). Review implementer output manually.Do NOT auto-revert —unverifiedis a soft flag that informs the user, not a halt signal. The user decides whether to run/jr-reviewagain or revert. - If any
critical-severity finding isunverified, escalate to a single targeted AskUserQuestion in interactive mode:Critical finding "<description>" may not have been resolved. Options: [Accept as-is — I'll verify manually] / [Revert all Phase 5 changes and re-run]. In headless/CI mode, skip the prompt but keep the ACTION REQUIRED entry and exit non-zero.
Display: Phase 5.55 — Verification: M/N fixes verified (K unverified, L moved) (or skip line if zero fixes).
Phase 5.6 — Secret re-scan
Skip if nofix flag is set (Phase 5 and 5.5 are skipped entirely, so no files were modified). Otherwise, run unconditionally whenever Phase 5 implementers or Phase 5.5 simplification modified any files. Rationale: Phase 4 user approval covers the findings, not the implementer or simplification agent output — those agents modify code autonomously after approval, and their changes are never directly reviewed by the user.
After the simplification pass completes (Phase 5.5), before scanning, verify the base commit anchor is still valid: if [ "$(git rev-parse HEAD)" != "$baseCommit" ]; then warn: 'HEAD has moved unexpectedly — an implementer may have run git commands. Aborting for safety.' First reset HEAD to the base commit (git reset "$baseCommit"), then apply the full revert sequence (see Phase 5 "Base commit anchor"), set abortMode=true and abortReason="head-moved-phase-5.6", and proceed to Phase 7 in abort mode (see Phase 7: "Abort-mode execution" — run steps 1, 2, 4, and 6 only; skip step 3 so the secret-warnings.json audit trail persists), and exit the review with a non-zero status.
Re-run the secret pre-scan from Phase 1 (Track B, step 7) against all files modified by implementers and the simplification agent. Additionally, check for new untracked files created by implementers (git ls-files --others --exclude-standard -z, NUL-delimited) and compare against $untrackedBaseline (the NUL-delimited temp file captured in Phase 5); include any new entries in the scan. Newly created files are not captured by git diff --name-only -z "$baseCommit" and would otherwise evade the re-scan. Additionally, run git ls-files --others -z (without --exclude-standard, NUL-delimited) and compare against $untrackedBaselineAll (the NUL-delimited temp file) to detect files written to gitignored paths. Include any new gitignored files in the secret re-scan. If strict-tier secrets are found in gitignored files, include them in the halt and cleanup using rm -f -- (since git clean skips them). Apply the advisory-tier classification for re-scans (see ../shared/secret-scan-protocols.md). If strict-tier secrets are detected, halt immediately. In interactive mode, present matches to the user via AskUserQuestion — secrets override all auto-approval settings for findings. In headless/CI mode, apply the CI/headless secret-halt protocol (see ../shared/secret-scan-protocols.md) — the protocol sets abortMode=true; the caller MUST also set abortReason="secret-halt-phase-5.6" before invoking the protocol. If the user chooses 'Continue' (interactive path only), apply the User-continue path protocol defined in ../shared/secret-scan-protocols.md (all six behaviors: ACTION REQUIRED logging, audit-trail write, pre-commit hook offer, final ⚠ SECRET STILL PRESENT warning, non-zero exit via the latched userContinueWithSecret=true, and suppression-list snapshot into $postImplAcceptedTuples), then proceed to Phase 6. Behavior 5's latch ensures the non-zero exit survives any downstream clean validation or retry; behavior 6's snapshot prevents re-prompting the user on the same accepted match in subsequent Phase 6 retries, Convergence Phase 5.6 re-scans, or Fresh-eyes re-scans, and prevents the retroactive-revert penalty in headless mode where a later halt would otherwise wipe fixes the user already accepted here.
Additionally, write the detected secret locations to .claude/secret-warnings.json per the schema and atomic-write rules in ../shared/secret-warnings-schema.md (top-level consumerEnforcement + warnings[]; per-entry required + optional fields; flock-wrapped atomic-rename + per-session fallback). Preserve any pre-existing consumerEnforcement value when appending. This file is the /jr-review ↔ /jr-ship contract artifact; the enforcement side is NOT YET IMPLEMENTED IN /jr-ship — see the Cross-skill contract status section above.
Schema validation, atomic write, and per-session filename fallback: Apply the canonical rules in ../shared/secret-warnings-schema.md (read into lead context at Phase 1 Track A; hard-fail guard ensures it was non-empty and structurally valid). The shared file specifies: top-level schema (consumerEnforcement + warnings[]), per-entry required/optional fields (file, line, patternType, detectedAt, status, missingRunCount), the path-allowlist regex and shared path validation block (mirrors --scope checks 4-6 in "Parameter sanitization"), the patternType enum, the validation-failure halt protocol (emits [AUDIT TRAIL REJECTED — PATH VALIDATION]), the schema-failure backup protocol (emits [SECRET-WARNINGS BACKUP FAILED] if the backup itself fails), the flock(1) availability probe, and the per-session filename fallback (secret-warnings-${baseCommit:0:8}-${CLAUDE_SESSION_ID:-$(date +%s)}.json) when FLOCK_AVAILABLE=false. The protocol applies at BOTH read sites in /jr-review: Phase 5.6 append (this site) AND Phase 7 step 3 prune. Both sites MUST consult the shared file — do NOT duplicate the rules inline.
Until /jr-ship enforcement is implemented, secret-warnings.json is an audit trail only — it does NOT block commits or PRs automatically. To add automated enforcement locally, the skill offers to install a scoped git pre-commit hook. Apply the canonical procedure in protocols/pre-commit-hook-offer.md (offer flow with headless/FLOCK_AVAILABLE=false carve-outs, install-script invocation with the 0/2/4/* error-code matrix, hook design invariants, disarming rules). The patterns file .claude/secret-hook-patterns.txt is subject to the .gitignore-enforcement protocol from ../shared/gitignore-enforcement.md before write.
Security check (enforced): Before writing .claude/secret-warnings.json (or any per-session variant), check if the path is tracked by git (git ls-files --error-unmatch .claude/secret-warnings.json 2>/dev/null). If it is tracked, emit a warning: '.claude/secret-warnings.json is tracked by git. A committed cache file could be manipulated. Add it to .gitignore or verify this is intentional.' If the path is not in .gitignore, append it automatically and inform the user: 'Added .claude/secret-warnings.json to .gitignore.' In headless/CI mode, log the action in the Phase 7 report. WHY: This file records file paths, line numbers, and pattern types where secret patterns were detected — committing it could reveal the locations of current or historical secrets to anyone with repository access.
If the user aborts (interactive path only), apply the full revert sequence (see Phase 5 "Base commit anchor"), set abortMode=true and abortReason="secret-halt-phase-5.6-user-abort", and proceed to Phase 7 in abort mode (run steps 1, 2, 4, and 6 only; skip step 3 so the secret-warnings.json audit trail persists), and exit the review with a non-zero status — this ensures no secret-containing changes remain in the working tree and prevents step 3(c) from rescanning reverted files and silently dropping audit-trail entries.
Phase 6 — Validate-fix loop
Skip this phase entirely if nofix flag is set.
Post-implementation formatting
If the project has a formatter configured (detected from review-profile: format command), run it on modified files first before validation to prevent lint failures caused by formatting drift.
Validation
Run all detected validation commands in parallel (lint, typecheck, test as separate simultaneous Bash calls in a single message). In quick mode (no baseline was collected), just check pass/fail — no baseline comparison. Otherwise, compare against the Phase 1 baseline — only new failures count as regressions.
- No new failures: Move to Phase 7.
- New failures found: Analyze and fix regressions (dispatch multiple implementer agents in parallel if fixes span multiple files — pass them the project coding standards so fixes don't introduce new violations, and include the implementer safety preamble from Phase 5 in full), then re-validate with all commands in parallel again. Repeat up to the max retry count (default 3). Do NOT re-review — only fix what the validation tools report as new regressions.
- Secret re-scan after regression fixes: Run the secret pre-scan (Track B, step 7) against files modified by regression-fix implementers after each fix attempt. This set may overlap with files already scanned in Phase 5.6 — re-scan them anyway, as regression-fix implementers may have altered their content. Apply the advisory-tier classification for re-scans (see
../shared/secret-scan-protocols.md). If strict-tier secrets are detected, halt and present to user via AskUserQuestion regardless of any auto-approval settings. Options: [Continue — log ACTION REQUIRED] / [Abort and revert all changes since $baseCommit]. OnAbort, apply the full revert sequence (see Phase 5 "Base commit anchor"), setabortMode=trueandabortReason="user-abort", if a team was created in Phase 2, call TeamDelete to clean up agents (do not wait for shutdown confirmations), and proceed to Phase 7 in abort mode (run steps 1, 2, 4, and 6 only; skip step 3 so thesecret-warnings.jsonaudit trail persists). Phase 7 will exit the review with a non-zero status per Phase 7 exit-code rules. OnContinue, apply the User-continue path protocol defined in../shared/secret-scan-protocols.md(all six behaviors — see that file's "The six behaviors" section). This includes behavior 6's suppression-list snapshot — subsequent Phase 6 retry attempts will not re-prompt on the same accepted match. Rationale: same as Phase 5.6 — regression-fix implementers modify code autonomously, and their output is never directly reviewed by the user. In headless/CI mode, apply the CI/headless secret-halt protocol (see../shared/secret-scan-protocols.md); the caller MUST also setabortReason="secret-halt-phase-6-regression"before invoking the protocol. Note: in Phase 6's retry loop, the CI revert removes ALL uncommitted changes since$baseCommit(including Phase 5 implementation, Phase 5.5 simplification, and all prior retry attempts), not just the current fix attempt. This is the intended safe default — if a secret was introduced, the safest action is to revert all automated modifications.
- Secret re-scan after regression fixes: Run the secret pre-scan (Track B, step 7) against files modified by regression-fix implementers after each fix attempt. This set may overlap with files already scanned in Phase 5.6 — re-scan them anyway, as regression-fix implementers may have altered their content. Apply the advisory-tier classification for re-scans (see
- Max retries exhausted: Before moving to Phase 7, run the stuck-loop advisor check (single-fire). Initialize
phase6AdvisorFired=falseat Phase 6 entry; on the first time this branch is reached ANDphase6AdvisorFired=false, setphase6AdvisorFired=trueand calladvisor()(no parameters — the full transcript is auto-forwarded). Per the cross-cutting habit "call advisor when stuck — errors recurring, approach not converging," validation regressions that have survivedmaxRetriesrounds of fix attempts are the canonical stuck signal. The advisor sees every fix attempt the regression-fix implementers made and the validation output that kept re-failing, and can spot non-obvious causes (e.g., "all three fix attempts editedauth.tsbut the failing test imports a stale symbol fromauth-utils.ts— fix the import, not auth.ts"). If the advisor concurs with stopping (no actionable insight), proceed silently to Phase 7 and report the remaining failures. If the advisor offers a concrete actionable insight, surface via AskUserQuestion:Advisor flagged a fix-loop concern: <one-line summary>. Options: [Apply suggested fix and retry once more] / [Stop here — proceed to Phase 7 with remaining failures] / [Abort and revert all changes since $baseCommit]. On Apply suggested fix and retry once more, dispatch ONE additional implementer agent with the advisor's suggestion as targeted instruction (include the implementer safety preamble from Phase 5 in full); if that retry also fails, do NOT re-fire the advisor — proceed to Phase 7. On Abort, apply the full revert sequence (see Phase 5 "Base commit anchor"), setabortMode=trueandabortReason="user-abort", and proceed to Phase 7 in abort mode. The single-fire guard (phase6AdvisorFired) prevents budget burn — the advisor sees the full retry history once, not on every retry. After the advisor branch resolves (or the post-advisor retry fails), move to Phase 7 and report the remaining failures.
After a successful validation (no new regressions), update .claude/review-baseline.json with the post-fix results so the next review has an accurate baseline.
Display: Show compact validation summary as defined in the Display protocol. Show each validation command result separately. Update the running progress timeline.
Convergence loop (if --converge is set)
Skip if --converge is not set. Skip if nofix or --pr is set (these conflict — should have been caught in flag validation).
The full convergence-loop protocol — state tracking (iteration, maxIterations, modifiedFiles, allModifiedFiles, iterationLog, priorFindings sanitization, converged, convergenceStartTime), file tracking mechanism (NUL-safe baselines + delta computation + allModifiedFiles storage + post-build sanity), convergence pass loop (Phases 2/3/4/5/5.5/5.6/6 with the convergence-specific reviewer/auto-approve/secret-rescan rules), fresh-eyes verification pass (single security-reviewer over the cumulative diff with the recursion bound), and terminal-failure handling — lives in convergence-protocol.md (read into lead context at Phase 1 Track A; hard-fail guard ensures it was non-empty and structurally valid). Apply those rules verbatim. Convergence-pass prompts MUST include priorFindings sanitization rules and the auto-approval policy stored from first-pass Phase 4 consent. The pre-iteration advisor() call fires at iteration ≥ 3 (per the convergence-protocol spec); the threshold is hard-coded there, not here.
After the convergence loop terminates (converged, max iterations reached, timeout, abort), proceed to Phase 7 (cleanup and report) and Phase 8 (follow-up issues). These run exactly once, covering all iterations.
Phase 7 — Cleanup and report
Declare-done advisor check (gated): Before applying the Phase 7 protocol below, call advisor() once (no parameters — the full transcript is auto-forwarded) if the run is non-trivial: findingCount >= 5 OR dimensionCount >= 3 OR rejectionCount >= 1 OR any fix was applied in Phase 5/6 OR abortMode=true. Per ../shared/advisor-criteria.md, declaring done on multi-phase work warrants a second opinion; the gate skips trivially-clean small runs (the "Unconditional advisor on every run" anti-pattern). Fixes are already committed to the working tree by this point, so an interruption mid-call loses nothing. If the advisor concurs, proceed silently. If it raises a concrete concern (e.g., a fix that doesn't match its finding, a missed regression, a contested finding that should have blocked), surface it in the report under an ADVISOR NOTES line. The Phase 5 pre-dispatch advisor covers the substantive-edit boundary; this one covers the run as a whole, and fires once after the --converge loop (if any) terminates.
The full Phase 7 protocol — run-scoped flag initialization, exit-code rules, the per-session-filename banner, the six steps (team shutdown / git diff --stat / secret-warnings prune / report redaction / audit-history append / base-anchor temp cleanup), abort-mode execution, the timeline display, the compact-vs-full report split, and the 16-item full-report enumeration — lives in protocols/phase7-cleanup-report.md (read into lead context at Phase 1 Track A under the hard-fail + smoke-parse guard). Apply that protocol verbatim.
Phase 8 — Follow-up issue tracking
The full Phase 8 protocol — skip rules, headless/CI carve-outs, the five steps (fetch / dedup with structural-first matching policy + decision logging / candidate selection / user prompt / cross-repo + public-repo visibility checks + PR-comment vs new-issue dispatch), and the compact display — lives in protocols/phase8-followups.md (read into lead context at Phase 1 Track A under the hard-fail + smoke-parse guard). Apply that protocol verbatim.