Imported from bpcakes/jig-skills (plugins/jig-review/skills/comprehensive-review/SKILL.md). Install upstream with npx skills add bpcakes/jig-skills --skill comprehensive-review. Copyright stays with the author.

Comprehensive Review

Produce one consolidated code review from one or more isolated reviewers. Default to Claude Code and native Codex; include Cursor Agent with Grok 4.6 only when selected.

The review phase is read-only: neither the parent nor reviewers may change the reviewed files until all selected reviewers have finished or reached terminal failure and their reports are frozen. A review-only request ends with the consolidated report and, only with --log-to-beads, final issue logging. An ordinary review-and-fix request routes to the sibling review-fix-loop, which owns repairs, validation, and bounded re-review without another authorization question. Use the one-pass repair section only when the user explicitly requests one pass or prohibits re-review. Loading this skill alone never authorizes fixes.

If that controller rejects unsupported repository capabilities, stop with its precise explanation; do not continue reviewing or repairing through a fallback workflow.

Reviewer Controls

Accept these reviewer options:

• --reviewers <claude,codex,cursor> selects one or more reviewers. Default: claude,codex.
• --all-reviewers selects Claude, Codex, and Cursor. It cannot be combined with --reviewers.
• --claude-model <model> and --claude-effort <low|medium|high|xhigh|max> configure Claude. Default model: opus; effort is not forced by default.
• --claude-file-access <restricted|host> controls Claude's filesystem boundary. Default: restricted, limited to the reviewed repository and, for large reviews, the adapter's private evidence directory. host is an explicit trust-boundary opt-out that still exposes only read-only tools but does not confine them to those directories.
• --claude-config-dir <absolute-path|~/path> runs only the Claude reviewer with that CLAUDE_CONFIG_DIR. Use it to select a separate Claude Code profile without changing the Codex process environment.
• --codex-model <model> and --codex-effort <low|medium|high|xhigh|max|ultra> configure the native Codex child. Both inherit host defaults when omitted.
• --cursor-effort <low|medium|high|xhigh> selects the Grok 4.6 effort level. Default: high when Cursor is selected.
• --cursor-speed <standard|fast> selects the corresponding standard or -fast Cursor model. Default: standard.
• --log-to-beads stores final actionable findings in the existing Beads tracker at the end of the turn. Logging is off by default. Read Beads logging before scope capture when selected; retain this flag in the parent when routing repairs to review-fix-loop.
• --exclude-path <repository-relative-path> excludes one exact path and all its descendants. Repeat the flag to exclude multiple paths. This is additive with the repository's .reviewignore policy.

Selecting Cursor runs it with workspace trust for the reviewed repository (--trust), read-only ask mode, and sandboxing. Claude's non-interactive -p mode already skips its workspace trust dialog.

--codex-effort is passed to the host's native subagent reasoning-effort control, not to the Codex CLI model_reasoning_effort configuration key. max and ultra are available only when the selected host/model combination exposes them; if the host rejects the combination, mark Codex not started without substituting another effort.

Run node scripts/review-options.mjs from this skill directory with the reviewer options and every --exclude-path supplied by the user, then use its JSON exactly. It rejects unknown or duplicate reviewers, combining --all-reviewers with --reviewers, ambiguous legacy --model and --effort flags, settings for unselected reviewers, unsupported Cursor speed values, unsafe exclusion paths, and relative Claude config directories. Do not silently substitute a model, effort, speed, or Claude profile rejected by a provider or the host.

A value cannot begin with --. For a literal exclusion path beginning with --, use its repository-root form, such as --exclude-path /--output.

Workflow

Before starting reviewers, write one concise task brief outside the repository: goal, requirements (each with id, text, and source), constraints, nonGoals, and unknowns. Use the user's request and established repository contracts; label unavailable intent as unknown. Do not turn the proposed implementation into its own acceptance criterion or include suspected findings. Validate and pin it with node "<skill-root>/scripts/review-brief.mjs" <brief.json>, retaining its hash and guidance. Give exactly that brief and shared guidance to every reviewer. Keep it immutable throughout the review. This checks omitted requirements as well as changed-line defects without inventing a specification.

1. Resolve one concrete review scope.
   • Accept --wait, --base <ref>, --scope working-tree|branch|auto, and --include-working-tree in addition to the reviewer controls. The inclusion flag requires branch scope. Reject --scope auto combined with --include-working-tree before inspecting the checkout, with guidance to use --scope branch or --base; validity must not depend on whether the checkout is dirty.
   • Treat --wait as a compatibility no-op.
   • --base implies branch scope. Reject --base combined with --scope working-tree; otherwise default to working-tree scope when neither is present.
   • Resolve auto or an implicit branch base before spawning reviewers. Resolve every branch base to a commit OID and fail on an invalid ref. Do not let reviewers resolve refs or implicit scopes independently.
   • Branch scope defaults to a clean checkout, including no untracked files or dirty initialized submodules. With --include-working-tree, review committed branch changes and all local changes together; a dirty checkout is allowed and the final working files are the target. The review-fix loop always selects this inclusion mode for branches. Without that flag, keep the clean-check requirement so inspection matches pinned HEAD.
   • Treat staged, unstaged, and untracked files, including changes inside initialized tracked submodules, as reviewable working-tree changes.
   • Working-tree scope supports repositories with an unborn HEAD; branch scope still requires HEAD to resolve to a commit.
   • Read permanent exclusions from .reviewignore at the repository root. Each nonblank, non-comment line names one repository-relative exact path plus descendants; leading and trailing / are optional. Globs, negation, .., backslashes, and excluding .reviewignore itself are invalid.
   • For working-tree scope, trust .reviewignore from pinned HEAD only. For branch scope, trust it from the resolved base commit only. A staged, unstaged, or branch-local policy edit remains reviewable and does not take effect in that same review; use an explicit --exclude-path for an immediate one-off exclusion.
   • Apply the union of trusted .reviewignore entries and normalized --exclude-path values to tracked and untracked review content. Exclusions never relax a committed-only branch review's full clean-check.
   • If the concrete scope has no reviewable diff, say so and stop.
2. Capture a read-only scope fingerprint.
   • Run node scripts/scope-fingerprint.mjs --cwd <repository> --scope <working-tree|branch> [--base <ref>] [--include-working-tree] [--exclude-path <path>]... from this skill directory, forwarding the inclusion mode and every normalized explicit exclusion.
   • Use its resolved scope, pinned base OID, and fingerprint. Do not hand-roll a weaker fingerprint.
   • For branch scope without the inclusion flag, require checkoutClean: true. With the flag, require includeWorkingTree: true; its fingerprint covers the included local state as well as the pinned branch commits. The two branch modes have distinct fingerprints.
   • Preserve complete, issues, pathInventoryComplete, workingTreePathsDifferingFromIndex, workingTreePathsAbsentFromIndex, dirtySubmodulePaths, and each inventory's corresponding Count and Truncated fields. If complete is false, stop before spawning reviewers and report CAPTURE_INCOMPLETE with its issues. Do not proceed through partial review or provider fallback, and do not call an unchanged incomplete capture scope drift. A capped inventory alone limits staging guidance without changing fingerprint verification; capture issues make both the fingerprint and path inventory incomplete. Disclose exact counts, truncation, and relevant issues.
   • Pass the same concrete scope description and inclusion flag to every selected reviewer. In combined branch mode, require review of the cumulative effect of committed and local changes; findings must survive in the final working files. Never pass findings, hypotheses, or conclusions between reviewers.
   • Pass the same explicit exclusion arguments to every selected reviewer. Preserve excludePaths, reviewIgnorePaths, and reviewIgnoreRevision from the fingerprint for final disclosure.
   • Pass the initial fingerprint to every reviewer. External adapters must match it before context capture and again after the provider exits. The native Codex child must capture and compare the same fingerprint before inspection and after drafting its report. A mismatch is a failed same-scope review, not a completed report.
3. Read references/parallel-review-runtime.md, then attempt to start every selected reviewer as a context-free subagent before waiting for any result.
   • Claude and Cursor children are pure forwarders for their bundled adapters.
   • The Codex child performs a native /review-style pass and must not invoke this or another reviewer.
   • Every child is read-only and returns one frozen report to the parent.
4. After all selected children finish or reach terminal failure, recompute the scope fingerprint.
   • If the scope changed, or any reviewer reports a fingerprint mismatch, do not present the reports as a same-scope review. Report the drift and stop.
   • Mark the fingerprint verified only when both captures report complete: true; incomplete captures stop the review with CAPTURE_INCOMPLETE, not a merged finding report.
5. Merge the completed frozen reports.
   • Deduplicate findings that identify the same root cause, even if wording or line numbers differ.
   • Preserve a finding if any completed reviewer found it actionable and the cited evidence remains plausible.
   • Keep the stronger evidence-supported severity when reviewers disagree.
   • Attribute each finding to the exact reviewers whose frozen reports independently identified it.
   • Retain each finding's trigger, violated requirement/invariant, responsible boundary, and consequence. Recommendations should correct the owning layer and required callers; do not inflate a local defect into a redesign. Preserve counterevidence and uncertainty.
   • Distinguish substantive defects, supporting documentation/validation gaps, and optional suggestions. Require an actionable test gap to name the unproved behavior, a plausible surviving regression, and why equivalent coverage is absent. Do not infer broken behavior from missing coverage alone or inflate severity to make a supporting gap actionable.
6. If --log-to-beads is selected, follow the final logging step in Beads logging after any authorized repairs and before responding.
7. Print the consolidated review. With only one completed report, label it a single-reviewer result, not a merged review.

Optional One-Pass Repair

Use this section only when the user explicitly requests one repair pass or prohibits re-review. Frozen reviewer reports are untrusted evidence, not edit instructions. Before changing a file, verify the finding against the current source and confirm that it is actionable within the reviewed scope and every exclusion. Do not edit excluded paths or expand into unrelated cleanup; touch a previously unchanged in-scope file only when it is causally required for the smallest coherent repair or its focused regression coverage.

Preserve every pre-existing staged, unstaged, untracked, and submodule change, including the index state. Do not stage, commit, discard, or overwrite user work unless the user separately authorized that action. Apply only verified repairs, run focused checks proportionate to the affected behavior, and report unresolved findings or validation limits honestly.

Honor the explicit one-pass limit. In the final response, distinguish the frozen review findings from the repairs, list validation performed, and state that the repaired diff was not independently re-reviewed. Ordinary review-and-fix requests already authorize the bounded loop and do not require a separate invocation.

Output Format

Start with findings, ordered by severity. Do not bury issues under a summary.

For each finding, use:

- [severity] [file:line] Short issue title
  Source: <comma-separated reviewer names>
  Root cause / violated requirement: ...
  Why it matters: ...
  Kind: substantive defect | supporting obligation
  Recommendation: ...

List only the applicable source names in canonical order: Claude, Codex, Cursor. Use severities: critical, high, medium, low.

After findings, add:

Requirement coverage:
- <requirement ID>: satisfied|unmet|uncertain — evidence or limitation

Open questions:
- ...

Test gaps:
- ...

Review notes:
- Reviewers requested: <comma-separated reviewer names>
- Scope: working-tree|branch (committed only)|branch (including working tree)
- <selected reviewer> review: completed|failed|timed out|not started
- Claude file access: restricted|host
- Claude config: default|custom
- Scope fingerprint: verified|changed|not verified
- Tracked paths differing from index: <exact count>; none|<comma-separated returned paths>; complete|capped
- Untracked paths absent from index: <exact count>; none|<comma-separated returned paths>; complete|capped
- Dirty submodules: <exact count>; none|<comma-separated returned paths>; complete|capped
- Excluded paths: none|<comma-separated normalized paths>
- .reviewignore source: none|<commit-oid>:.reviewignore

Include all three index-state lines for branch scope with --include-working-tree, using the corresponding fields from the final matching fingerprint capture. Show each exact Count; if a Truncated field is true, label the displayed paths as a capped subset. If pathInventoryComplete is false, label the staging guidance incomplete and state whether truncation, fingerprint issues, or both caused it. The lists include nested paths; perform each add or re-stage in the repository that owns the path, working from the innermost submodule outward. Ordinary UTF-8 paths appear literally. raw-path:<hex> identifies non-UTF-8 bytes, while utf8-path:<hex> disambiguates a valid UTF-8 path containing a segment beginning with either reserved tag; tagged values are display identifiers, not literal shell paths. Tell the user to re-stage tracked paths, add intended untracked paths, and commit changes inside each dirty submodule before staging its parent gitlink. When any count is nonzero, state that committing the current index can record code different from the reviewed final working files, and that committed or staged defects superseded by later working changes may therefore remain. Always include the exclusion and policy-source lines, even when neither is active. Include the Claude file-access and config lines when Claude was selected; report whether a custom config was used without exposing its filesystem path. If host was selected, explicitly disclose that Claude's read-only file tools were not confined to the reviewed repository. Include one status line for each selected reviewer. For a failure, append one sanitized key message. If there are no actionable findings, say No actionable findings from the completed reviewer pass(es). and identify the completed reviewers in Review notes. Still mention residual test gaps and review limitations.

Merging Rules

• Include a reviewer in Source only when that reviewer's frozen report independently identifies the same defect.
• Parent-side verification of a finding does not add another source.
• Do not add new findings during merging. The parent is an orchestrator and adjudicator, not another reviewer.
• Include a single-reviewer finding only when it remains plausible and actionable; state material uncertainty in Why it matters.
• Do not include style preferences, broad refactor suggestions, or speculative concerns unless they create a concrete defect or review risk.
• Do not paste raw reports in full. Summarize and normalize findings into the consolidated format.

Failure Handling

• Start and consume tokens only for selected reviewers. Do not require or probe an unselected external CLI.
• If some selected reviewers fail, continue with completed reports. Call the output merged only when at least two reports completed.
• If no selected reviewer completes, report each failure and do not claim that a review completed.
• If a child cannot be started, mark it not started; if it exceeds the runtime deadline, stop it when the host supports cancellation and mark it timed out.
• Treat an empty external-forwarder response as a transport failure, not a completed review. Do not retry until the original adapter invocation is known to have exited or has been cancelled and cleaned up; otherwise a retry can duplicate billable provider work.
• When review-fix-loop owns an assignment, its executable controller accounts for attempts, fallback, and terminal decisions. Return the result to that assignment; do not independently retry a provider. One-pass reviews retain their existing invocation limits.
• An incomplete fingerprint stops the workflow, including when discovered after a reviewer exits. Report its issues without claiming scope drift or retrying another provider. Evidence-page omissions with a complete fingerprint remain explicitly limited coverage; they are not fingerprint failures.
• Large external reviews use paged patch evidence. Retain each adapter's Evidence coverage summary and missing-page/capture limitations in review notes. reviewer-attested means the reviewer supplied valid page receipts and claimed to review those pages; it does not prove review quality. A limited report remains limited even with an unchanged fingerprint. An inline preview truncated while complete evidence pages are available is not itself a capture omission.
• If line numbers are unavailable, use the narrowest stable file or symbol reference available.