jr-review

star 0

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.

julienroussel By julienroussel schedule Updated 6/9/2026

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 via gh, forces nofix mode (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 via gh pr list --head (linked PR), else origin/<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 on CLAUDE_EFFORT (see protocols/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=N overrides the effort-adaptive default. Convergence passes auto-approve certain and likely findings in interactive mode; in headless/CI mode (--auto-approve), only certain findings are auto-approved by default — see --auto-approve for 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-approve certain findings 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/speculative deferred to Phase 7. With --converge: likely auto-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 --converge or run a follow-up /jr-review after.
  • --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-claims skips the fetch (offline / speed); unverifiable external-authority claims are then capped to speculative (Tier 1 fallback) and thus never auto-approved (auto-approve covers only certain/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):

  1. Match ^[0-9]+$ — non-digit input warns Convergence limit must be a number. Using 3. and uses 3.
  2. Parse base-10 integer (handles leading zeros like 007).
  3. Range check [2, 10]. If N == 1, warn Convergence with 1 iteration provides no re-review or fresh-eyes pass. Use --auto-approve without --converge, or set N >= 2. and use 2. If N == 0 or N > 10, warn Convergence 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 + quickquick wins (it's the explicit override for speed). Ignore full.
  • --refresh-baseline + (quick | nofix | --pr) — Track D is skipped in these modes, so --refresh-baseline is silently ignored.
  • --pr implies nofix — do not warn, just apply.
  • --converge + nofix — Conflict. nofix disables implementation, so there is nothing to converge on. Warn: "Cannot converge without fixing — nofix disables implementation. Remove one flag." and abort.
  • --converge + --pr — Conflict. --pr implies nofix. Same as above.
  • --branch + --pr — Conflict. Both flags define alternate diff scopes (local feature branch vs. remote PR). Abort with: "Cannot combine --branch and --pr — they define different diff scopes. Use --branch to 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 imply nofix (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 same quick constraints (max 2 reviewers, compact report).
  • --converge + full — Allowed. First pass uses full swarm. 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-approve activates headless mode, but Phase 8's headless skip has a --pr exception: 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-approve without --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=2 and log to the Phase 7 report. Whenever --auto-approve AND --converge are both active (explicit or auto-added), set freshEyesMandatory=true so 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-claims skips 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 to speculative and 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 _..foo that 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 --scope and use --only=<dimensions> to limit the reviewer dimensions instead. Always double-quote the path in Bash commands. Note: --scope is 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 --scope as 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 into git merge-base "<base>" HEAD and git 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 --scope defense-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-audit supports 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 --model override 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 — use speculative when 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 --model override 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 --model override (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-ship Phase 1 reads .claude/secret-warnings*.json and refuses to proceed if entries match the working tree — NOT IMPLEMENTED. Until implemented, secret-warnings*.json is 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.comgh, gitlab.comglab, 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_EFFORT is xhigh or max. 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.
  1. Attempt to call mcp__codebase-memory-mcp__list_projects (via ToolSearch if the schema isn't loaded). On any failure (tool unavailable, load error), set GRAPH_AVAILABLE=false and GRAPH_INDEXED=false — do NOT block the review on graph absence.
  2. If the tool loads, check whether the current repo (git rev-parse --show-toplevel) is indexed. Set GRAPH_AVAILABLE=true and GRAPH_INDEXED=<true|false> accordingly.
  3. If GRAPH_AVAILABLE=true and GRAPH_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, call mcp__codebase-memory-mcp__index_repository and wait for completion; set GRAPH_INDEXED=true. On Proceed, continue with GRAPH_INDEXED=false.
  4. If neither trigger fires (diff < 20 files AND effort is not xhigh/max), or when isHeadless=true, skip the probe entirely — set both flags to false. 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.
  5. Pass GRAPH_AVAILABLE and GRAPH_INDEXED to 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.md to glab mr diff <iid> and glab mr view <iid> -F json with 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. Force nofix mode (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-review and /jr-audit and 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-array runId reachability, 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):
    1. Reviewer false-positive rates: take the last 5 reviewerStats entries per dimension (any skill, both /jr-review and /jr-audit count) and compute the per-dimension running average of rejectionRate. 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.
    2. lastPromptedAt map for the global-preference promotion suppression check (see Phase 4.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 repo CLAUDE.md "shared/ — single source of truth" section. Pass reviewer-boundaries.md content verbatim to every reviewer prompt; pass untrusted-input-defense.md content verbatim into every reviewer and implementer prompt; pass claim-verification.md content 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 via grep -F, AND-joined within a row). Self-reference escape hatch (hardcoded): before parsing the canonical's table, verify ../shared/phase1-track-a-protocol.md itself contains the literal string Canonical 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.md if any file is absent / empty or fails its smoke-parse anchor. Smoke-parse anchors (case-sensitive grep -F):
    • ${CLAUDE_SKILL_DIR}/protocols/phase2-reviewers.mdEffort-adaptive breadth AND codeExcerpt
    • ${CLAUDE_SKILL_DIR}/protocols/finding-sanity-check.mdcontent-excerpt match
    • ${CLAUDE_SKILL_DIR}/protocols/secret-warnings-lifecycle.mdLifecycle of \unverified` entries`
    • ${CLAUDE_SKILL_DIR}/protocols/base-anchor.mdCombined revert sequence
    • ${CLAUDE_SKILL_DIR}/protocols/pre-commit-hook-offer.mdInstall procedure
    • ${CLAUDE_SKILL_DIR}/protocols/phase7-cleanup-report.mdRun-scoped flags initialization AND Per-session filename banner
    • ${CLAUDE_SKILL_DIR}/protocols/phase8-followups.mdPublic repository check AND Dedup decision logging
    • ${CLAUDE_SKILL_DIR}/convergence-protocol.md (only when --converge is set) — priorFindings AND freshEyesMandatory AND Proceed 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 --converge was not passed; if --converge is 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 $PWD with -). Read "$memoryDir/MEMORY.md" first; the file is an index of - [Title](file.md) pointers. Then fan out in parallel to read every referenced feedback_*.md, project_*.md, reference_*.md, and user_*.md file in $memoryDir. These entries are explicit user decisions from prior sessions in this project — treat them with the same precedence as CLAUDE.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 to user_*.md files only — skip feedback_*.md, project_*.md, and reference_*.md here 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_*.md entries 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 --scope is set, append -- <path>.
  • git diff --no-color (unstaged changes). If --scope is set, append -- <path> to scope at the git level.
  • git diff --cached --no-color (staged changes). If --scope is 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:

  1. 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 (when isHeadless is true — 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.

  2. Merge conflict check: If the diff contains <<<<<<< or >>>>>>> markers, abort with: "Merge conflicts detected. Resolve conflicts before running /jr-review."

  3. Empty diff check: If the combined diff (staged + unstaged + untracked) is empty, stop with: "Nothing to review — no changes detected."

  4. 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).

  5. Read any untracked files in full.

  6. 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 isHeadless is true — see ../shared/secret-scan-protocols.md → "Headless/CI detection"), do not use AskUserQuestion. For diffs exceeding 3,000 lines, automatically apply quick mode. For diffs exceeding 8,000 lines, abort with an error message recommending the diff be split. Log the decision in the Phase 7 report.

  7. 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 -Ei invocation, 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.

  1. Read .claude/review-profile.json (if it exists) and run stat -f %m package.json tsconfig.json Makefile 2>/dev/null — both in parallel.

  2. If the profile exists AND --refresh-stack was NOT passed: Compare current modification timestamps against cached sourceTimestamps. 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 both review-profile.json and review-baseline.json). On any failure, force full re-detection. Otherwise use cached packageManager, lockFile, validationCommands. Output: Stack: cached (${packageManager}, ${Object.keys(validationCommands).join('+')}). Skip to Track D.

  3. If the profile is missing, timestamps differ, or --refresh-stack was passed — run full detection:

    • Read package.json in parallel with checking for lock files (bun.lockb, pnpm-lock.yaml, yarn.lock) and reading tsconfig.json, Makefile, docker-compose.yml.
    • Detect validation commands from package.json scripts: look for lint, typecheck/type-check/tsc, test, validate. Build a list using the detected package manager. If a Makefile has 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 set validationCommands to all-null values to disable validation."

    • Output: Stack: detected (${packageManager}, ${Object.keys(validationCommands).join('+')}) — cached for next run

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.

  1. Read .claude/review-baseline.json (if it exists).
  2. If the cache exists, is within TTL (default 10 minutes), AND --refresh-baseline was 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.
  3. 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:
    {
      "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>" }
      }
    }
    
    Output: Baseline: fresh (lint+typecheck+test) — cached for 10m Security 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:

  1. Sanity-check findings (reject hallucinations): Apply the canonical procedure in protocols/finding-sanity-check.md — for each finding, verify file exists, line is valid and in range, and codeExcerpt matches the file content (working-tree/branch reads local; --pr fetches via gh api to 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 7 ACTION REQUIRED note. 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 in package.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 single WebFetch summary — cross-check it). confirmed → keep and tag [verified: <source> <date>]; refuted → reject under [REJECTED — CLAIM REFUTED BY SOURCE] with the cited file: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 to speculative and 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 only certain/likely). Verify each unique claim-source at most once per run (in-memory dedup). Track a per-reviewer refuted rate; ≥ 25% triggers a Phase 7 ACTION REQUIRED note (mirrors step 0). The external-doc fetch is independent of --pr/--branch (it fetches docs, not the changed files) and is forge-independent: this gh api …/contents/<path> verifies an external authority (framework/Anthropic docs), so it STAYS gh even 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 --pr codeExcerpt fetch in finding-sanity-check.md) switches with the detected forge.
  2. 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.
  3. Drop low-value items: Delete tasks with severity low unless 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 a high-severity unverified deprecation stays visible tagged [unverified — needs confirmation].
  4. Prioritize: Reorder by severity first, then by confidence within the same severity level — certain before likely before speculative.
  5. Assign file ownership: Group tasks by file. Each file should be owned by at most one implementer agent to prevent conflicts.
  6. 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% of totalFindings. (Skip the check if totalFindings < 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 to speculative by 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-reviewer dimension. 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+category has accumulated 2+ rejections in 2+ separate runs by scanning .claude/audit-history.json (canonical, shared with /jr-audit — see ../shared/audit-history-schema.md below). If 2+ separate-run rejections are confirmed AND dimension is NOT security AND isHeadless=false AND no lastPromptedAt entry 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 a feedback-type memory file to ~/.claude/projects/-Users-jroussel--claude-skills/memory/feedback_<dimension>_<category>.md with body structure rule + **Why:** + **How to apply:** per the auto-memory system's feedback format, then append a one-line pointer to that memory dir's MEMORY.md. On either Yes or No, record lastPromptedAt: ISO8601 keyed 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 AFTER lastPromptedAt. Rationale: 2 separate-run rejections of the same exact dimension+category is 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-review and /jr-audit MUST 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 lines

  • Append under a ## Auto-learned suppressions section, 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 speculative findings — 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 git commands. 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):

  1. Re-read the cited file over the range [line - 5, line + 5] (clamped to file bounds).
  2. 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.
  3. 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 a verified=false flag and surface in the Phase 7 report under ACTION 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 7 ACTION REQUIRED note: High unverified rate in <dimension> (<N>/<M>). Review implementer output manually. Do NOT auto-revert — unverified is a soft flag that informs the user, not a halt signal. The user decides whether to run /jr-review again or revert.
  • If any critical-severity finding is unverified, 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]. On Abort, apply the full revert sequence (see Phase 5 "Base commit anchor"), set abortMode=true and abortReason="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 the secret-warnings.json audit trail persists). Phase 7 will exit the review with a non-zero status per Phase 7 exit-code rules. On Continue, 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 set abortReason="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.
  • Max retries exhausted: Before moving to Phase 7, run the stuck-loop advisor check (single-fire). Initialize phase6AdvisorFired=false at Phase 6 entry; on the first time this branch is reached AND phase6AdvisorFired=false, set phase6AdvisorFired=true and call advisor() (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 survived maxRetries rounds 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 edited auth.ts but the failing test imports a stale symbol from auth-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"), set abortMode=true and abortReason="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.

Install via CLI
npx skills add https://github.com/julienroussel/skills --skill jr-review
Repository Details
star Stars 0
call_split Forks 0
navigation Branch main
article Path SKILL.md
More from Creator
julienroussel
julienroussel Explore all skills →