code-review

name

code-review

description

Use when an implementation needs an independent CLI reviewer or adversarial challenger, routing across Claude Code, Kimi, OpenCode, or Codex according to local availability, model-family independence, and the user's client order; also covers Claude-only bounded consultation and requests such as code review, Claude review, Kimi review, OpenCode review, second opinion, 找茬, 唱反调, or 第二意见. Skip 某个改动/提交还有没有价值、值不值得修这类交付裁决 → product-rd-workflow：本技能执行评审、找缺陷与风险，不裁决交付价值。

Code Review

Use this skill from Claude, Codex, OpenCode, or another compatible agent to obtain an independent, attributable review or adversarial challenge. The job is not to delegate implementation. The caller supplies the model family that produced the candidate so the router can exclude same-family reviewers.

Modes

Choose the smallest useful mode:

• Review mode: normal pre-merge or plan review. Find blocking or materially misleading issues.
• Challenge mode: adversarial pass modeled after codex challenge. Try to break the diff or decision by finding production failure paths.
• Complete mode: local exact-candidate deep-self-review checkpoint after a passed final external round. It invokes no reviewer and grants no human or merge authority.
• Consult mode: ask Claude a bounded question when no diff or plan review is needed.

Use challenge mode when the user asks for "challenge", "poke holes", "try to break it", "adversarial", or when the change touches money, permissions, privacy, compliance, tenant/user data, production rollout, high-impact AI, architecture, economics, or IA.

Script Entry Point

For gate-eligible review and challenge, use scripts/review_gate.sh. --review-plan-file is optional for review and challenge: supply a four-field plan (intent, acceptance, self_review, evidence) to bind implementer-authored scope and per-owner self-review, or omit it to run under a derived default. The result records review_plan_source=implementer-supplied or review_plan_source=derived-default so a ceremony-free review is never mistaken for a hand-attested one; complete mode still requires an explicit plan. The gate derives stage concerns, freezes candidate/profile plus stable review-context/controller/skill/history hashes, and validates any self_review[*].skill against the complete sibling skill registry. Omit skill to use the code-review baseline. The controller also derives owners from changed skills/<name>/ paths, test paths, and supported source extensions. With an implementer-supplied plan every derived owner must appear in structured self-review before a provider runs; a derived-default plan still selects and loads those owners for the reviewer but waives the pre-attestation requirement it has no plan to check. The frozen selected set contains the baseline plus every declared or derived owner; each binding hashes SKILL.md and recursive Markdown under references/, not scripts, templates, or other files. Unknown or linked owner packages fail before provider execution. Supported installs ship the complete skills/ registry; an isolated copy can use only the baseline unless its sibling owners are also present. Stable results expose deterministic routing evidence and report owner_selection_source=controller-derived+implementer-declared whenever path-derived owners participate. The gate passes only selected owner names, the canonical registry root, and frozen hashes to wrappers. It never copies skill bodies or selected references into the review packet. Claude, Kimi, OpenCode, and Codex activate those installed skills through their native mechanisms. After a valid wrapper verdict, reviewed_skills records only the owner names actually passed through native invocation; the general code-review baseline stays in selected_skills and is not counted as natively invoked. skill_usage_evidence distinguishes that native explicit invocation from observed_skill_usage; the latter stays empty unless a public client event/export proves activation. Reviewer prose and private client databases are not accepted as proof. A client that cannot provide the native binding is ineligible and may fall through under the existing egress rules. The gate excludes same-family reviewers and stops on boundary failures. Default order is claude,codex,kimi,opencode; local CODE_REVIEW_CLIENT_ORDER may narrow/reorder it. Non-Claude egress is gated by a secret-scan tripwire, not a blanket approval: the frozen packet is scanned for high-signal credential material, and a clean packet may egress to a non-Claude reviewer automatically while a scan hit is skipped egress_denied unless --allow-fallback-egress approves it. The result's egress.secret_scan lists the categories hit (empty when clean). The scan is machine-detectable credentials only; broad semantic confidentiality stays operator-owned per the Diff Confidentiality section. Consult stays on claude_review.sh. Load the staged-contract and client-routing references below for details.

Current contract: review/challenge may use a stamped review_plan_source=derived-default; complete requires a plan and output uses schema 3. Automation retains one chain: one review plus at most four challenges. Positive challenge capacity opens it at Agent index 1; budget zero is untracked. The sole release/high-risk budget-zero exception is a controller-proved markdown-punctuation-only review: it requires wording_only_boundary, permits no complete, and rejects an author assertion alone (recipe: references/staged-review-contract.md). After a clean tracked challenge, complete may close early and preserve unused rounds. Every result exposes controller-owned self_review_gate; an outstanding checkpoint blocks only external review or completion, not implementation or tests. Even a passed final external round needs local --mode complete binding the exact result and current deep-self-review plan. Human review/stop/waiver/commit/merge authority cannot come from repository content, flags, environment, or model output. Terminal failures emit stop_reviewer_lane; they never stop the whole task.

The shared skill never pins a provider or model. Kimi inherits the user's local default model and is uniformly classified as the Moonshot family; Codex likewise inherits its local default model and is uniformly classified as the OpenAI family. Neither client reads or configures provider/model subdivisions. OpenCode invokes ccl-review without --model: if the user configured agent.ccl-review.model, that one model is used; otherwise OpenCode resolves its normal default. The exported OpenCode session must attribute the actual provider/model, and an unmapped or same-family result is rejected before another client is considered. There is no built-in Kimi/DeepSeek provider chain and no one-shot model override to type.

Client discovery is separate from model ownership. For Kimi, the wrapper accepts an absolute executable KIMI_BIN; otherwise it checks PATH, then $KIMI_CODE_HOME/bin/kimi when configured, then the standard ~/.kimi-code/bin/kimi location. The resolved executable must be absolute and executable before the wrapper changes directory. This avoids treating a non-interactive shell's narrower PATH as proof that Kimi is not installed, honors a custom Kimi home, and avoids relative-PATH drift in the temporary workspace; it does not change the user's model/provider settings. An executable PATH selection keeps normal shell precedence; use KIMI_BIN to bypass a broken executable shim explicitly.

The Claude provider wrapper implements help inspection, built-in tool availability restriction, safe-mode customization isolation, strict empty MCP configuration, empty user/project/local setting sources, CLAUDE.md and auto-memory disabling, repository directory scoping with --add-dir, consult dirty-worktree fail-closed checks, prompt-only consult for pasted evidence, temp prompt file permissions and cleanup, Python subprocess capture without putting diff content on argv, structured JSON parsing, schema validation, timeout handling, auth false-negative classification, and the two-invocation cap. Runs without selected owner skills disable skills/commands. Owner-aware runs load the already-installed CCL skill plugin through --plugin-dir and explicitly invoke each selected skill; they do not manufacture a selected-only plugin or claim that unselected installed skills are unavailable. Claude's own built-in identifiers and all skills present in the controller-supplied registry may remain visible in stream init; only selected owners are frozen-hash verified and counted as explicit invocation. The parser rejects entries outside that registered surface or duplicate identifiers and keeps tools/MCP isolated. For review/challenge it also accepts the orchestrator's frozen --diff-file and emits stable reason_code, fallback_eligible, and next_action fields on every inconclusive path. Every mode captures Claude with structured stream JSON and adds --json-schema when the CLI advertises it; parse_review_json.py unwraps the result event's structured payload and rejects any envelope whose is_error, subtype, api_error_status, or permission_denials fields signal a non-clean run. classify_envelope.py reads those same structured fields to classify auth/quota/permission/error failures, so raw-text matching on the CLI's prose is only a fallback for the case where no JSON envelope was produced. On an auth false negative the wrapper returns auth_path_unavailable; a host rerun of the same gate adds --host-remediation-attempted. --timeout controls each formal invocation and must be an integer of at least 5 seconds; values above 1200 are clamped to 1200. The wrapper does not make a separate behavior-probe request. The formal invocation's own stream-init must declare exactly the expected tool set: packet-only modes allow only Claude's internal StructuredOutput tool when schema output is enabled, while repository consult additionally allows Read,Grep,Glob. Unexpected tool use, MCP inheritance, or schema drift fails closed. --allowedTools is a permission auto-approval rule, not an availability restriction, and is never used as the sandbox. The wrapper deliberately never runs through command substitution, because that path can itself report a false logged-out state on this machine.

Owner-aware Claude init must declare the ccl-skills plugin. Current public init output does not enumerate selected plugin skills or commands, so the wrapper binds those through frozen package hashes and explicit names in the prompt instead of manufacturing an unavailable observation. Other skills from the verified installed registry may remain visible. Plugin registration proves the native entrypoint was loaded; it does not prove that the model read a skill body. When init begins enumerating ccl-registry skills or ccl-skills: commands, every selected owner must appear or the run fails closed; built-ins alone do not imply plugin enumeration.

Owner-aware Claude requires --safe-mode plus the explicit --plugin-dir. Safe mode disables ambient customizations while the explicit plugin supplies the full installed skill registry, and it preserves the user's normal OAuth/keychain authentication path. The wrapper must not add --bare: that flag disables OAuth and keychain reads and makes a normally logged-in CLI unusable without a separate API key. Before loading the installed plugin, the wrapper validates that its manifest exposes only the native ./skills entrypoint and no top-level agents, commands, hooks, or MCP servers. A broader plugin manifest is capability-ineligible for this bounded lane and fails closed before Claude starts.

# Set this to the expanded code-review skill directory shown in the
# current session's skill list, or to the source checkout skill directory when
# testing changes to this skill. Never resolve it from the product repo under
# review.
: "${CODE_REVIEW_SKILL_DIR:?set CODE_REVIEW_SKILL_DIR to the expanded code-review skill directory}"
test -r "${CODE_REVIEW_SKILL_DIR:-}/scripts/review_gate.sh" || {
  echo "review_gate.sh not found; set CODE_REVIEW_SKILL_DIR to the expanded code-review skill directory" >&2
  exit 1
}
grep -qE '^name:[[:space:]]*"?code-review"?([[:space:]]*#.*)?[[:space:]]*$' "$CODE_REVIEW_SKILL_DIR/SKILL.md" || {
  echo "CODE_REVIEW_SKILL_DIR does not look like code-review" >&2
  exit 1
}
repo_root="$(git rev-parse --show-toplevel)" || exit 1
skill_dir_real="$(cd "$CODE_REVIEW_SKILL_DIR" && pwd -P)" || exit 1
repo_root_real="$(cd "$repo_root" && pwd -P)" || exit 1
case "$skill_dir_real/" in
  "$repo_root_real"/*)
    echo "CODE_REVIEW_SKILL_DIR is inside the repo under review; use only for --review-harness self-review" >&2
    exit 1
    ;;
esac
echo "code_review_skill_dir=$CODE_REVIEW_SKILL_DIR" >&2
: "${IMPLEMENTER_FAMILY:?set IMPLEMENTER_FAMILY to the canonical provider/model family that produced the change}"
: "${REVIEW_PLAN_FILE:?set REVIEW_PLAN_FILE to an absolute staged review plan JSON path}"
: "${REVIEW_CHAIN_ID:?set REVIEW_CHAIN_ID to a task-scoped chain id: letters, digits, dot, underscore, hyphen only}"
: "${REVIEW_STAGE:?set REVIEW_STAGE to the stage this candidate is actually at: explore, build, or release}"
: "${REVIEW_EVIDENCE_DIR:?set REVIEW_EVIDENCE_DIR to a durable directory you control for the per-round result rows}"
# Exactly one frozen packet source: a packet you composed (REVIEW_DIFF_FILE, see the
# packet-composition rules below) or a base ref (REVIEW_BASE). The gate rejects both.
if [ -n "${REVIEW_DIFF_FILE:-}" ] && [ -n "${REVIEW_BASE:-}" ]; then
  echo "set exactly one of REVIEW_DIFF_FILE or REVIEW_BASE" >&2; exit 1
elif [ -n "${REVIEW_DIFF_FILE:-}" ]; then
  PACKET_ARGS=(--diff-file "$REVIEW_DIFF_FILE")
elif [ -n "${REVIEW_BASE:-}" ]; then
  PACKET_ARGS=(--base "$REVIEW_BASE")
else
  echo "set exactly one of REVIEW_DIFF_FILE or REVIEW_BASE" >&2; exit 1
fi
# REVIEW_RUN_DIR holds the raw round-1 result only for the chain handoff; the durable
# per-round evidence is persisted to REVIEW_EVIDENCE_DIR, whose confidentiality you own.
REVIEW_RUN_DIR="$(mktemp -d "${TMPDIR:-/tmp}/review-run.XXXXXX")" || exit 1
cleanup() { rm -rf "$REVIEW_RUN_DIR"; }
trap cleanup EXIT       # signal handlers must EXIT, not just clean up, or an
trap 'exit 130' INT     # interrupted round 1 would fall through into the
trap 'exit 143' TERM    # challenge with a deleted prior-result file
ROUND1_RESULT_FILE="$REVIEW_RUN_DIR/round1.json"
# Declared risk tags must reach BOTH rounds: they select owners, concerns, and can
# raise depth to release. Omitting them silently reviews a weakened profile.
RISK_ARGS=()
for tag in ${REVIEW_RISK_TAGS:-}; do RISK_ARGS+=(--risk-tag "$tag"); done
case "$REVIEW_CHAIN_ID" in  # validate before it reaches a path: the gate checks the
  ""|.|..|*[!A-Za-z0-9._-]*)   # same grammar, but only after mkdir would have run
    echo "REVIEW_CHAIN_ID must be letters, digits, dot, underscore, hyphen" >&2; exit 1;;
esac
EVIDENCE_RUN_DIR="$REVIEW_EVIDENCE_DIR/$REVIEW_CHAIN_ID"
mkdir -m 700 "$EVIDENCE_RUN_DIR" || { echo "evidence rows for this chain id already exist; rerun with a fresh REVIEW_CHAIN_ID" >&2; exit 1; }
# Decide the round budget BEFORE round 1 and open the chain there. An INITIAL REVIEW
# rejects missing chain flags whenever the effective challenge budget is positive, and
# is accepted untracked only when that budget is 0 — the default unless depth is
# release (explicit stage, or raised by a high-risk tag). Untracked means single-round:
# the challenge you run next is still accepted, but as one-off advisory that cannot
# enter a later Agent round or satisfy the completion checkpoint, and the chain it
# never joined cannot be retrofitted. This pair is the ONE-challenge shape; for a tier
# that needs a larger budget, follow the Agent review chain section of
# references/staged-review-contract.md instead of rerunning this pair.
bash "$CODE_REVIEW_SKILL_DIR/scripts/review_gate.sh" \
  --mode review --stage "$REVIEW_STAGE" --review-plan-file "$REVIEW_PLAN_FILE" \
  --cwd "$repo_root" "${PACKET_ARGS[@]}" ${RISK_ARGS[@]+"${RISK_ARGS[@]}"} \
  --implementer-family "$IMPLEMENTER_FAMILY" \
  --challenge-budget 1 --review-chain-id "$REVIEW_CHAIN_ID" --autonomous-review-index 1 \
  >"$ROUND1_RESULT_FILE"
# Round 1 must be a conclusive TRACKED result before the challenge consumes it;
# otherwise the challenge fails on the prior-result file and buries the real failure.
require_tracked_result() {  # <result-file> <expected mode> <expected Agent index>
  REVIEW_CHAIN_ID="$REVIEW_CHAIN_ID" python3 -c 'import json,os,sys; d=json.load(open(sys.argv[1])); sys.exit(0 if d.get("status") in ("passed","findings") and d.get("review_chain_tracked") is True and d.get("mode")==sys.argv[2] and d.get("autonomous_review_index")==int(sys.argv[3]) and d.get("review_chain_id")==os.environ["REVIEW_CHAIN_ID"] else 1)' \
    "$1" "$2" "$3" || { echo "round $3 is not a conclusive tracked $2 result for this chain; triage it, do not continue:" >&2; cat "$1" >&2; exit 1; }
}
require_tracked_result "$ROUND1_RESULT_FILE" review 1
# Persist each round before the trap reclaims it, fail closed if persistence fails, and
# never clobber an earlier run's rows: the review and challenge rows are separate
# required evidence, so a rerun uses a fresh REVIEW_CHAIN_ID and a fresh directory.
cp "$ROUND1_RESULT_FILE" "$EVIDENCE_RUN_DIR/round1-review.json" || exit 1
bash "$CODE_REVIEW_SKILL_DIR/scripts/review_gate.sh" \
  --mode challenge --stage "$REVIEW_STAGE" --challenge-budget 1 --challenge-index 1 \
  --focus "<challenge surface>" \
  --review-plan-file "$REVIEW_PLAN_FILE" --cwd "$repo_root" \
  "${PACKET_ARGS[@]}" ${RISK_ARGS[@]+"${RISK_ARGS[@]}"} --implementer-family "$IMPLEMENTER_FAMILY" \
  --review-chain-id "$REVIEW_CHAIN_ID" --autonomous-review-index 2 \
  --prior-review-result-file "$ROUND1_RESULT_FILE" \
  >"$REVIEW_RUN_DIR/round2.json"
require_tracked_result "$REVIEW_RUN_DIR/round2.json" challenge 2
# Both rounds must bind the SAME candidate: the chain accepts older candidate hashes,
# so a packet edited between rounds would otherwise be persisted as one coherent pair.
python3 -c 'import json,sys; a=json.load(open(sys.argv[1])); b=json.load(open(sys.argv[2])); h=a.get("candidate_sha256"); sys.exit(0 if h and h==b.get("candidate_sha256") else 1)' \
  "$ROUND1_RESULT_FILE" "$REVIEW_RUN_DIR/round2.json" \
  || { echo "round 2 reviewed a different packet than round 1; rerun the pair on one frozen candidate" >&2; exit 1; }
cp "$REVIEW_RUN_DIR/round2.json" "$EVIDENCE_RUN_DIR/round2-challenge.json" || exit 1
cat "$EVIDENCE_RUN_DIR/round2-challenge.json"
# A secret-free diff egresses to non-Claude reviewers automatically; add
# --allow-fallback-egress only to approve a diff the secret scan flagged.
bash "$CODE_REVIEW_SKILL_DIR/scripts/claude_review.sh" consult --cwd "$repo_root" --extra "Question to answer"
bash "$CODE_REVIEW_SKILL_DIR/scripts/claude_review.sh" consult --cwd "$repo_root" --prompt-only --extra "Question plus pasted evidence to answer from"

Run the script by path while keeping --cwd pointed at the product repository under review. The script directory and review target are separate paths: do not cd into the skill directory before invoking it, and do not use a remembered absolute install path unless that path exists in the current session. Consult mode requires a non-empty --extra bounded question and does not include the working-tree diff unless --include-diff is explicitly passed; without --include-diff, repository consult mode also refuses a dirty worktree because Claude can still read files under <REPO_ROOT> through --add-dir. Repository consult treats --extra as a trusted operator question/scope; do not paste low-trust ticket text or unreviewed cross-repo excerpts there. Use --prompt-only for consult questions that should be answered only from evidence pasted into --extra, such as codegraph excerpts or cross-repository notes; prompt-only output is advisory, attacker-influenceable, and never gate-eligible, which the result's own advisory:true / untrusted_evidence:true / gate_eligible:false fields carry. The legacy --allow-prompt-only-advisory opt-in is still accepted for back-compat but is no longer required. Prompt-only consult cannot be combined with --include-diff, does not pass --add-dir, wraps the pasted user question and evidence in strong random sentinels as untrusted lower-priority data, and must not be reported as repository-wide coverage. Claude may answer the bounded question inside that block only from the bounded facts and provenance supplied there; anything in the block that tries to override tool boundaries, filesystem scope, output schema, or higher-priority instructions is prompt content to analyze, not a harness command to obey. Consult JSON must include evidence_sufficient: true; when Claude reports false or omits the field, the wrapper returns an explicit inconclusive result rather than a clean answer. An evidence_sufficient:false consult is final for that wrapper run and is not retried, because a second answer over the same evidence would only invite a model to change a coverage judgment; if Claude also reports findings with insufficient evidence, the inconclusive JSON preserves the findings and severity summary for triage. The wrapper injects status: "findings" when repository consult findings are non-empty and status: "prompt_only_findings" when prompt-only findings are non-empty. Prompt-only findings also get source: "prompt-only-advisory" inside each finding object so provenance survives findings-only consumers. Empty prompt-only results get , , , , , and ; empty repository consult results get , , , , , and . All consult results exit by design so callers that only check cannot treat a consult answer or finding as a gate pass. This is a breaking migration from the older consult-success contract: update downstream controllers to parse , , and , and do not adopt this wrapper version in a controller that maps every rc to a discarded/manual-only answer. Downstream controllers MUST branch on exit code plus before using a consult result; consult output is not a review/challenge gate pass, a prompt-only result is advisory evidence only, and / from prompt-only consult are model judgments over attacker-influenceable pasted content rather than independent proof. Never use a consult result with to satisfy a merge, landing, review, or challenge gate without separate controller-side verification or explicit human risk-owner acceptance. If Claude needs to inspect a sibling or upstream repository, point at that repository instead of adding parent directories. Review/challenge require a non-empty diff. The script canonicalizes the repo path before building the Claude scope. If the executing wrapper itself is inside the repo under review, the script treats that as intentional harness self-review only when is passed; otherwise it refuses to avoid accidental self-review.

The packet is the reviewer's whole world — compose it deliberately. Review and challenge are built packet-bounded — Claude runs --tools "" with no --add-dir, and the other wrappers run in an isolated run workspace or a packet-only read surface. Treat the packet as the reviewer's whole world when deciding coverage: it is the only content bound by the packet hash and scanned before egress, so anything outside it is neither reliably visible to the reviewer nor covered by the verdict; a diff-only packet surfaces defects visible inside the changed lines and little else, and --paths only narrows it further. Whatever is absent from the packet is unreachable, not merely missed: a contradiction with an unchanged sibling clause, drift against a carrier outside the diff, or a silent weakening of upstream wording cannot be found by a reviewer who never saw the other side — that is the packet's shape, not the reviewer's weakness.

• To widen the packet, assemble it yourself and pass --diff-file: it replaces base-derived generation, is mutually exclusive with --base/--paths, and must name a regular file (no symlink or hardlink) holding text without NUL bytes. Worth adding beyond the diff — the canonical rule or contract text the changed lines must not contradict, the sibling clauses in the same file, the derived carriers that restate the change (commit message, MR/PR body), and the actual output of a gate or script under review. The gate hard-caps a packet at 200,000 bytes; split a larger candidate as described in the next bullet.
• A verdict covers exactly the packet it was taken on, because the recorded packet hash is the reviewed identity. Within a packet, added context sits on top of the candidate diff and never in place of part of it. A candidate too large for one packet is split by file group or risk class into a partition that still covers the whole candidate — every part in some packet, none dropped — each partition's verdict recorded against its own packet hash, and the candidate-wide claim withheld until every partition is conclusive; one partition's no blocking findings is never a verdict on the landing candidate. Cross-partition contradictions are unreachable by construction, so repeat the shared canonical context in every partition's packet and review anything that spans partitions as its own packet.
• Added context egresses to the selected reviewer exactly like the diff does, through the same credential tripwire — which catches machine-detectable secrets only. Paste rule text, carriers, and tool output; never paste credentials or material you would not send to that provider.
• A finding that the input is insufficient to judge the change is an input defect, not a candidate defect: widen the packet and rerun that lane rather than editing the candidate to satisfy it. A reviewer reporting that the input is insufficient to judge the change is reporting an input defect — add the missing context and rerun that lane, do not edit the candidate to satisfy it.

When intentionally reviewing code-review itself, override the resolver from the ccl-skills repo under review before invoking the gate:

repo_root="$(git rev-parse --show-toplevel)" || exit 1
CODE_REVIEW_SKILL_DIR="$repo_root/skills/code-review"
test -r "$CODE_REVIEW_SKILL_DIR/scripts/review_gate.sh" || {
  echo "repo-local review_gate.sh not found; run from the ccl-skills checkout" >&2
  exit 1
}
: "${IMPLEMENTER_FAMILY:?set IMPLEMENTER_FAMILY}"
: "${REVIEW_PLAN_FILE:?set REVIEW_PLAN_FILE to an absolute staged review plan JSON path}"
bash "$CODE_REVIEW_SKILL_DIR/scripts/review_gate.sh" \
  --mode challenge --stage release --risk-tag shared-gate \
  --challenge-budget 1 --challenge-index 1 --focus "<challenge surface>" \
  --review-plan-file "$REVIEW_PLAN_FILE" --cwd "$repo_root" \
  --base origin/main --implementer-family "$IMPLEMENTER_FAMILY" \
  --review-harness

When the gate returns reason_code=auth_path_unavailable or reason_code=host_path_unavailable with next_action=host_retry, rerun the same gate once from the host path with --host-remediation-attempted. Preserve the same frozen packet or unchanged base/paths and every other argument. The recovery comes from the caller's execution path; the gate never elevates itself. Do not rerun from the same sandbox path that produced the path failure.

The script exits:

• 0: valid JSON review/challenge result
• 2: consult advisory JSON, consult findings, inconclusive output, or fail-closed safe invocation failure; callers should map this to a pending/manual-review gate state, not to "passed" or "no findings"
• non-zero other: local script/tool error

Compatibility note: consult success used to share the 0 exit path with review/challenge. Current consult mode intentionally exits 2 for both answers and findings so gate callers cannot accidentally count a consult as review approval. Existing consult consumers must migrate from rc-only handling to parsing status, consult_scope, advisory, untrusted_evidence, and gate_eligible. Prompt-only consult no longer requires --allow-prompt-only-advisory; the advisory/untrusted/gate-ineligible metadata on every prompt-only result is the guard, and the flag stays accepted only for back-compat.

Entrypoint availability check: run skills/code-review/scripts/review_gate.sh --help for review/challenge or claude_review.sh --help for consult. This local check does not invoke a model and is not review evidence; gate evidence starts only after validating a real structured result.

On this machine, Claude auth may be visible to claude auth status while sandboxed claude --print cannot read the local credential. If so, use the one host-path gate rerun above. Do not replace the gate with raw claude -p; that drops timeout, packet binding, schema validation, and safety checks. Only auth_unavailable_after_host_retry may enter the approved fallback chain; unresolved auth-path ambiguity stays inconclusive.

Use the manual rules in references/manual-invocation-and-prompts.md only when debugging or patching the script.

Harness Exclusion

Ordinary product reviews must not load or execute the code-review harness itself. The wrapper performs repository discovery and diff capture before Claude starts, then gives Claude only the bounded prompt plus diff data. Claude does not need to read scripts/claude_review.sh, scripts/parse_review_json.py, or this skill directory to review an unrelated product repo.

The prompt boundary excludes $HOME/.codex/, $HOME/.claude/, and $HOME/.agents/ except for files under the literal <REPO_ROOT> that is the product under review, and it forbids reading any Claude-review harness for ordinary product reviews. Therefore, when <REPO_ROOT> is an ordinary product repository, this CCL skill under the source checkout or Codex plugin cache is outside scope and should not be read. When <REPO_ROOT> is the ccl-skills repo or the user explicitly asks to review code-review, the harness is intentionally inside scope and may be reviewed; pass --review-harness for that intentional self-review.

The safe invocation uses --tools "" with no --add-dir for bounded review, challenge, and prompt-only consult, or --tools Read,Grep,Glob for repository consult. Every mode also uses --safe-mode, strict empty MCP configuration, --setting-sources "", CLAUDE_CODE_DISABLE_CLAUDE_MDS=1, and CLAUDE_CODE_DISABLE_AUTO_MEMORY=1; runs without selected owner skills additionally use --disable-slash-commands, while owner-aware runs load the verified installed CCL plugin so the selected skills can be invoked. Missing isolation capabilities fail closed. Managed settings policy, including policy-configured hooks, and authentication/model/global state remain host-owned inputs. Only repository consult passes <REPO_ROOT> through --add-dir as the intended workspace context; review and challenge see only the bounded diff packet, prompt-only consult sees only the pasted --extra evidence, and none of those modes read repository files. --add-dir is not treated as a hard absolute-path sandbox; the hard guard here is that ordinary product reviews keep this harness outside <REPO_ROOT>, the script refuses accidental self-review unless --review-harness is explicit, repository consult mode refuses dirty worktrees unless --include-diff is explicit, prompt-only consult refuses --include-diff, and untracked symlinks, hardlinks, or resolved-outside-repo paths are not inlined into prompts. Diff metadata and content are passed between strong random sentinel lines and labeled as untrusted data, not instructions, so prompt-like text inside filenames, stats, or diff bodies must be analyzed as code/data only.

Diff Confidentiality And Data Egress

Diffs can contain secrets, internal hostnames, SQL, or credentials; the wrapper keeps prompt files mode 0600 and deletes them with a trap, but that protects the prompt file on disk — it does not govern the diff leaving for the reviewer's model provider. Whenever the reviewer's provider or data-residency differs from the implementer's own session — a Codex/OpenAI implementer sending to a Claude reviewer, or any non-Claude fallback — the transmission crosses a new egress boundary, and this applies to the primary review lane, not only the fallback. Before it, apply the operator's data-egress / provider-allowlist / data-residency policy (owned by ../llm-inference-integration data-egress and the ../product-rd-workflow artifact-egress confidentiality gate): if the packet carries live credentials/tokens, customer PII, or regulated content, scrub those specific values or keep the review in-boundary (a same-provider / same-residency reviewer). Reviewing normal source code is fine — the check gates sensitive values and the provider crossing, not code egress as such.

For diffs over roughly 2,000 changed lines or 50 files, split review/challenge by file group or risk class. Do not present one broad Claude run over a huge diff as complete.

Wait Policy

Never treat a timeout, silence, or empty output as approval or "no findings": any timeout is inconclusive, and the final status must say inconclusive with the timeout reason so downstream review or merge state cannot treat the missing lane as approval. A live host execution handle such as session_id or cell_id means the same command is still running; poll that exact handle to terminal exit, and never start a replacement/fallback while its process is live. Do not use a 30 second silence as a review failure — narrow diff reviews legitimately take 2-3 minutes and broad reviews about 5. Challenge makes one formal Claude invocation; review and consult may make at most two only for their existing bounded result-recovery paths. After that, mark the Claude lane inconclusive and apply fallback only if the owning gate allows it. The wrapper traps TERM/INT/HUP and emits terminal operator_interrupt; the gate never starts another client after an operator interruption. SIGKILL and host crashes cannot be trapped, so non-zero exit without valid JSON remains inconclusive/manual-review-required, never as success. The timeout bound is per formal invocation, not per wrapper run; outer timeouts must cover the mode's worst case and must never kill the wrapper and then treat the killed output as success. For a yielded run, the caller's lane evidence records handle type, an opaque host transcript/tool-call reference rather than a raw credential-like handle, and terminal exit status. If that handle is lost, the lane is infrastructure-inconclusive/manual-review-required and no replacement or fallback may be started or credited; a ps/process-tree capture and wrapper artifacts are diagnostic only and cannot reconstruct the missing terminal result. The outer host assigns this handle after launch, so this is a host-workflow obligation rather than a controller-owned field; exact enforcement requires a trusted host adapter. Recovery detail and timing formulas live in references/timeout-auth-and-capabilities.md.

review_gate.sh also enforces a cumulative reviewer-lane budget: --total-timeout defaults to 2400 seconds, accepts 5 to 3600, reserves ten controller seconds, and divides the remainder by the mode's maximum invocation count. Starting a lane requires at least 21 seconds for review or 16 for challenge, plus setup overhead; smaller accepted values fail closed. Git preflight subprocesses share this deadline; direct filesystem reads still need the host's outer timeout. A timed-out client process group gets bounded cleanup and may cascade only while enough total budget remains. Total exhaustion returns terminal inconclusive gate_timeout; killed output is never a verdict. Timing details live in references/timeout-auth-and-capabilities.md.

Auth And CLI Pitfalls

Classify auth and host-path failures by evidence, not by one invocation path's prose. auth_path_unavailable and Codex's exact host_path_unavailable app-server EPERM require one host-path rerun of the same gate; mark it with --host-remediation-attempted. Never manually substitute raw provider commands for review/challenge. Do not repeat host remediation, and only tell the user to log in again when the host/local auth path also reports logged out.

Tool-boundary invariants are defined under Harness Exclusion above. Missing built-in, customization, skill/command, MCP, setting-source, CLAUDE.md, or auto-memory isolation is inconclusive; --allowedTools never substitutes for --tools. Prompt wording and --add-dir are not filesystem sandboxes.

For the numbered auth-recovery procedure, runtime classification detail, per-mode flag matrix, --no-session-persistence/model-pinning notes, and the CLI capability adoption policy (--safe-mode, --bare, --output-format json, --fallback-model, --max-budget-usd), see references/timeout-auth-and-capabilities.md.

Reviewer Client Routing

review_gate.sh owns the handoff. It preserves mode, packet hash, attribution, permission boundaries, timeout, native skill binding, and parseable verdict semantics across the configured client order. Availability, auth/quota/timeout, capability, and malformed-model-output failures may continue only when the wrapper marks them candidate-local; security, binding, family, mode, input, egress, and unknown failures stop. Kimi is uniformly attributed to Moonshot, seeds a private writable runtime home from the validated user configuration inputs while linking any existing validated credential directories back to their user-owned paths so OAuth token rotation persists for homes that already have one (a replaced or missing link after the run is terminal binding_mismatch), and keeps the user's default model while running from a temporary workspace; it registers the installed skill directory through controlled --skills-dir, explicitly names selected owners, and must cover the exact packet through bounded contiguous Read pages before a verdict is eligible. Codex is uniformly attributed to OpenAI, receives the packet over stdin, verifies that each selected owner package exists with a safe package shape in its installed skill system, invokes it with $skill-name, and runs read-only/ephemeral without manufacturing a temporary skill tree. A newer installed CCL release is accepted by selected name rather than byte equality with an older candidate branch. An exact sandbox-only in-process app-server EPERM requests one same-packet host retry before it can become fallback-eligible. Neither client is subdivided by local provider/model aliases, and neither wrapper passes --model. Any packet-external tool activity invalidates those results. OpenCode registers the controller-supplied installed registry through project skills.paths, explicitly sets permission.skill: allow, and uses its public debug agent and session export surfaces; only selected owners are frozen-hash verified. It never reads the private session database. Its actual exported provider/model remains the dynamic family binding. A successful owner-aware wrapper must return native_skill_binding=established; without that controller-owned receipt, the gate does not populate reviewed_skills or claim native invocation. None of the four client paths makes a separate model behavior-probe request. If the owning workflow requires review AND challenge, each lane still needs its own valid result. Load references/client-routing.md for details.

Across all four clients, native_skill_binding=established means the wrapper verified the controller profile and selected-name arguments, established a safe native package path, and emitted the client's native explicit invocation syntax. Controller-selected hashes still bind routing and review-chain reuse; they do not require an independently updated Codex installation to remain byte-identical to the candidate branch. The receipt does not mean internal activation was observed; only a public event/export may populate observed_skill_usage.

Reference Loading

• references/staged-review-contract.md — required review-plan schema, stage concerns, high-risk depth, prompt layers, and challenge budget. Load before review/challenge.
• references/manual-invocation-and-prompts.md — manual command shape, filesystem-boundary text, and the review/challenge prompt templates. Load only when debugging or patching the wrapper or its prompt construction.
• references/timeout-auth-and-capabilities.md — wait-policy timing tables, the numbered auth-recovery procedure, per-mode tool-flag matrix, and CLI capability adoption notes. Load on timeout/auth failures or when maintaining wrapper flag adoption.
• references/client-routing.md — review_gate.sh client order, family exclusion, egress, Kimi/Codex boundaries, OpenCode user-model binding, and concurrency rollback. Load when running or diagnosing review/challenge routing.

Output Validity

A valid Claude review must be complete and attributable.

Reject and rerun if the output is:

• empty
• partial or tail-only
• continuation-like
• missing the requested finding/no-finding section
• too broad to tie to files, risks, or decisions
• using vague approval language such as "LGTM" or "looks fine" instead of the requested sentinel
• not parseable as the requested JSON object with mode and findings
• any finding missing schema fields: severity in P0|P1|P2, non-empty file, integer line >= 1, non-empty failure_path, and non-empty smallest_fix

Before parsing, accept either a strict JSON object or one fenced json object block with no other prose. Accept no-finding outputs only when the JSON parses, the schema validates, has no status / reason inconclusive envelope, and findings is an empty array for the requested mode. Consult outputs must also include a non-empty answer field, so an empty finding list is not mistaken for a completed answer. Do not accept free-text "LGTM", "looks fine", continuation-like wrappers, leading apologies, trailing explanations, or sentinel phrases as proof. For broad review surfaces, narrow by file, diff, issue, or risk class and rerun. If a requested Claude review remains inconclusive, record that pending/inconclusive state in the MR or project docs instead of silently proceeding.

Reporting Back

In the final work summary, include:

• must state whether review ran and which reviewer/client was used
• mode: review, challenge, complete, or consult
• command scope, not the full prompt unless useful
• result: blocking findings, no blocking findings, or inconclusive
• the reviewed identity — a hash of the exact diff packet reviewed, required whenever the reviewed content includes staged, unstaged, untracked, or generated files (later worktree edits keep the same base/head SHA, so SHA alone cannot detect the change); the base/head commit SHA alone suffices only for a clean, fully-committed candidate tree. A review/challenge no blocking findings is valid only for that exact reviewed content: any later edit, rebase, amend, or new commit voids it and requires a fresh run (mirrors the agentic candidate-SHA binding). A caller — especially one invoking this skill standalone, outside a controller that already tracks the head SHA — must not reuse a prior pass as approval for changed content.
• any follow-up fixes made because of the review
• if skipped or inconclusive, the exact reason
• for a host-yielded execution, the handle type, opaque host transcript/tool-call reference, and terminal exit status; if the handle was lost, record that infrastructure-inconclusive state, diagnostic artifacts, and that fallback was unavailable; never persist a credential-like raw handle in shared evidence

Do not overstate a successful CLI exit as approval unless the output parsed as the requested JSON object and its findings array was evaluated.