From 6d8908a54ab0b901c65a03e0c1644ac2a2f53ec0 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 18 May 2026 21:11:28 -0700 Subject: [PATCH] fix(review): diff from git merge-base, not git diff origin/ (#1492) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git diff origin/ shows everything since the common ancestor in both directions — it includes commits that landed on origin/ after this branch was created as deletions. That made /review and /ship's pre-landing structured review report inflated diff totals and flagged "removed" code that was actually still present in the working tree. Fix: compute DIFF_BASE via git merge-base origin/ HEAD and diff the working tree against that point. Same coverage of uncommitted edits, no phantom deletions from out-of-order base advancement. Applies to /review's Step 1 (diff existence check), Step 3 (get the diff), the build-on-intent scope-creep check, the structured review DIFF_INS/DIFF_DEL stats, and the Claude adversarial subagent prompt. Same change flows into ship/SKILL.md via the shared resolver. Touches: - review/SKILL.md.tmpl + regenerated review/SKILL.md, ship/SKILL.md - scripts/resolvers/review.ts - scripts/resolvers/review-army.ts Contributed by @mvanhorn via #1492. Co-Authored-By: Claude Opus 4.7 (1M context) --- review/SKILL.md | 31 ++++++++++++++++++++----------- review/SKILL.md.tmpl | 11 +++++++++-- scripts/resolvers/review-army.ts | 9 +++++---- scripts/resolvers/review.ts | 11 ++++++----- ship/SKILL.md | 20 +++++++++++--------- 5 files changed, 51 insertions(+), 31 deletions(-) diff --git a/review/SKILL.md b/review/SKILL.md index d178c182e..ee065f032 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -813,7 +813,7 @@ You are running the `/review` workflow. Analyze the current branch's diff agains 1. Run `git branch --show-current` to get the current branch. 2. If on the base branch, output: **"Nothing to review — you're on the base branch or have no changes against it."** and stop. -3. Run `git fetch origin --quiet && git diff origin/ --stat` to check if there's a diff. If no diff, output the same message and stop. +3. Run `git fetch origin --quiet && DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE" --stat` to check if there's a diff. If no diff, output the same message and stop. --- @@ -825,7 +825,7 @@ Before reviewing code quality, check: **did they build what was requested — no Read commit messages (`git log origin/..HEAD --oneline`). **If no PR exists:** rely on commit messages and TODOS.md for stated intent — this is the common case since /review runs before /ship creates the PR. 2. Identify the **stated intent** — what was this branch supposed to accomplish? -3. Run `git diff origin/...HEAD --stat` and compare the files changed against the stated intent. +3. Run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE" --stat` and compare the files changed against the stated intent. 4. Evaluate with skepticism (incorporating plan completion results if available from an earlier step or adjacent section): @@ -1080,7 +1080,14 @@ Fetch the latest base branch to avoid false positives from stale local state: git fetch origin --quiet ``` -Run `git diff origin/` to get the full diff. This includes both committed and uncommitted changes against the latest base branch. +Compute the merge base, then diff the working tree against that point: + +```bash +DIFF_BASE=$(git merge-base origin/ HEAD) +git diff "$DIFF_BASE" +``` + +This includes both committed and uncommitted changes while excluding commits that landed on the base branch after this branch was created. ## Step 3.4: Workspace-aware queue status (advisory) @@ -1216,8 +1223,9 @@ STACK="" [ -f go.mod ] && STACK="${STACK}go " [ -f Cargo.toml ] && STACK="${STACK}rust " echo "STACK: ${STACK:-unknown}" -DIFF_INS=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") -DIFF_DEL=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") +DIFF_BASE=$(git merge-base origin/ HEAD) +DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") +DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") DIFF_LINES=$((DIFF_INS + DIFF_DEL)) echo "DIFF_LINES: $DIFF_LINES" # Detect test framework for specialist test stub generation @@ -1291,7 +1299,7 @@ If learnings are found, include them: "Past learnings for this domain: {learning 4. Instructions: "You are a specialist code reviewer. Read the checklist below, then run -`git diff origin/` to get the full diff. Apply the checklist against the diff. +`DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"` to get the full diff. Apply the checklist against the diff. For each finding, output a JSON object on its own line: {\"severity\":\"CRITICAL|INFORMATIONAL\",\"confidence\":N,\"path\":\"file\",\"line\":N,\"category\":\"category\",\"summary\":\"description\",\"fix\":\"recommended fix\",\"fingerprint\":\"path:line:category\",\"specialist\":\"name\"} @@ -1394,7 +1402,7 @@ The Red Team subagent receives: Prompt: "You are a red team reviewer. The code has already been reviewed by N specialists who found the following issues: {merged findings summary}. Your job is to find what they -MISSED. Read the checklist, run `git diff origin/`, and look for gaps. +MISSED. Read the checklist, run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"`, and look for gaps. Output findings as JSON objects (same schema as the specialists). Focus on cross-cutting concerns, integration boundary issues, and failure modes that specialist checklists don't cover." @@ -1566,8 +1574,9 @@ Every diff gets adversarial review from both Claude and Codex. LOC is not a prox **Detect diff size and tool availability:** ```bash -DIFF_INS=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") -DIFF_DEL=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") +DIFF_BASE=$(git merge-base origin/ HEAD) +DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") +DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") DIFF_TOTAL=$((DIFF_INS + DIFF_DEL)) which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" # Legacy opt-out — only gates Codex passes, Claude always runs @@ -1587,7 +1596,7 @@ If `OLD_CFG` is `disabled`: skip Codex passes only. Claude adversarial subagent Dispatch via the Agent tool. The subagent has fresh context — no checklist bias from the structured review. This genuine independence catches things the primary reviewer is blind to. Subagent prompt: -"Read the diff for this branch with `git diff origin/`. Think like an attacker and a chaos engineer. Your job is to find ways this code will fail in production. Look for: edge cases, race conditions, security holes, resource leaks, failure modes, silent data corruption, logic errors that produce wrong results silently, error handling that swallows failures, and trust boundary violations. Be adversarial. Be thorough. No compliments — just the problems. For each finding, classify as FIXABLE (you know how to fix it) or INVESTIGATE (needs human judgment). After listing findings, end your output with ONE line in the canonical format `Recommendation: because ` — examples: `Recommendation: Fix the unbounded retry at queue.ts:78 because it'll DoS the worker pool under sustained 429s` or `Recommendation: Ship as-is because the strongest finding is a theoretical race that requires conditions we can't trigger in production`. The reason must point to a specific finding (or no-fix rationale). Generic reasons like 'because it's safer' do not qualify." +"Read the diff for this branch with `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"`. Think like an attacker and a chaos engineer. Your job is to find ways this code will fail in production. Look for: edge cases, race conditions, security holes, resource leaks, failure modes, silent data corruption, logic errors that produce wrong results silently, error handling that swallows failures, and trust boundary violations. Be adversarial. Be thorough. No compliments — just the problems. For each finding, classify as FIXABLE (you know how to fix it) or INVESTIGATE (needs human judgment). After listing findings, end your output with ONE line in the canonical format `Recommendation: because ` — examples: `Recommendation: Fix the unbounded retry at queue.ts:78 because it'll DoS the worker pool under sustained 429s` or `Recommendation: Ship as-is because the strongest finding is a theoretical race that requires conditions we can't trigger in production`. The reason must point to a specific finding (or no-fix rationale). Generic reasons like 'because it's safer' do not qualify." Present findings under an `ADVERSARIAL REVIEW (Claude subagent):` header. **FIXABLE findings** flow into the same Fix-First pipeline as the structured review. **INVESTIGATE findings** are presented as informational. @@ -1602,7 +1611,7 @@ If Codex is available AND `OLD_CFG` is NOT `disabled`: ```bash TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems. End your output with ONE line in the canonical format `Recommendation: because `. Generic reasons like 'because it's safer' do not qualify; the reason must point to a specific finding or no-fix rationale." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" +codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE" to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems. End your output with ONE line in the canonical format `Recommendation: because `. Generic reasons like 'because it's safer' do not qualify; the reason must point to a specific finding or no-fix rationale." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" ``` Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. After the command completes, read stderr: diff --git a/review/SKILL.md.tmpl b/review/SKILL.md.tmpl index fada69112..ae480da3d 100644 --- a/review/SKILL.md.tmpl +++ b/review/SKILL.md.tmpl @@ -38,7 +38,7 @@ You are running the `/review` workflow. Analyze the current branch's diff agains 1. Run `git branch --show-current` to get the current branch. 2. If on the base branch, output: **"Nothing to review — you're on the base branch or have no changes against it."** and stop. -3. Run `git fetch origin --quiet && git diff origin/ --stat` to check if there's a diff. If no diff, output the same message and stop. +3. Run `git fetch origin --quiet && DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE" --stat` to check if there's a diff. If no diff, output the same message and stop. --- @@ -72,7 +72,14 @@ Fetch the latest base branch to avoid false positives from stale local state: git fetch origin --quiet ``` -Run `git diff origin/` to get the full diff. This includes both committed and uncommitted changes against the latest base branch. +Compute the merge base, then diff the working tree against that point: + +```bash +DIFF_BASE=$(git merge-base origin/ HEAD) +git diff "$DIFF_BASE" +``` + +This includes both committed and uncommitted changes while excluding commits that landed on the base branch after this branch was created. ## Step 3.4: Workspace-aware queue status (advisory) diff --git a/scripts/resolvers/review-army.ts b/scripts/resolvers/review-army.ts index 516ce3c8d..5c8766e30 100644 --- a/scripts/resolvers/review-army.ts +++ b/scripts/resolvers/review-army.ts @@ -30,8 +30,9 @@ STACK="" [ -f go.mod ] && STACK="\${STACK}go " [ -f Cargo.toml ] && STACK="\${STACK}rust " echo "STACK: \${STACK:-unknown}" -DIFF_INS=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") -DIFF_DEL=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") +DIFF_BASE=$(git merge-base origin/ HEAD) +DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") +DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") DIFF_LINES=$((DIFF_INS + DIFF_DEL)) echo "DIFF_LINES: $DIFF_LINES" # Detect test framework for specialist test stub generation @@ -105,7 +106,7 @@ If learnings are found, include them: "Past learnings for this domain: {learning 4. Instructions: "You are a specialist code reviewer. Read the checklist below, then run -\`git diff origin/\` to get the full diff. Apply the checklist against the diff. +\`DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"\` to get the full diff. Apply the checklist against the diff. For each finding, output a JSON object on its own line: {\\"severity\\":\\"CRITICAL|INFORMATIONAL\\",\\"confidence\\":N,\\"path\\":\\"file\\",\\"line\\":N,\\"category\\":\\"category\\",\\"summary\\":\\"description\\",\\"fix\\":\\"recommended fix\\",\\"fingerprint\\":\\"path:line:category\\",\\"specialist\\":\\"name\\"} @@ -217,7 +218,7 @@ The Red Team subagent receives: Prompt: "You are a red team reviewer. The code has already been reviewed by N specialists who found the following issues: {merged findings summary}. Your job is to find what they -MISSED. Read the checklist, run \`git diff origin/\`, and look for gaps. +MISSED. Read the checklist, run \`DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"\`, and look for gaps. Output findings as JSON objects (same schema as the specialists). Focus on cross-cutting concerns, integration boundary issues, and failure modes that specialist checklists don't cover." diff --git a/scripts/resolvers/review.ts b/scripts/resolvers/review.ts index 40916115d..9839a2023 100644 --- a/scripts/resolvers/review.ts +++ b/scripts/resolvers/review.ts @@ -423,7 +423,7 @@ Before reviewing code quality, check: **did they build what was requested — no Read commit messages (\`git log origin/..HEAD --oneline\`). **If no PR exists:** rely on commit messages and TODOS.md for stated intent — this is the common case since /review runs before /ship creates the PR. 2. Identify the **stated intent** — what was this branch supposed to accomplish? -3. Run \`git diff origin/...HEAD --stat\` and compare the files changed against the stated intent. +3. Run \`DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE" --stat\` and compare the files changed against the stated intent. 4. Evaluate with skepticism (incorporating plan completion results if available from an earlier step or adjacent section): @@ -467,8 +467,9 @@ Every diff gets adversarial review from both Claude and Codex. LOC is not a prox **Detect diff size and tool availability:** \`\`\`bash -DIFF_INS=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") -DIFF_DEL=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") +DIFF_BASE=$(git merge-base origin/ HEAD) +DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") +DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") DIFF_TOTAL=$((DIFF_INS + DIFF_DEL)) which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" # Legacy opt-out — only gates Codex passes, Claude always runs @@ -488,7 +489,7 @@ If \`OLD_CFG\` is \`disabled\`: skip Codex passes only. Claude adversarial subag Dispatch via the Agent tool. The subagent has fresh context — no checklist bias from the structured review. This genuine independence catches things the primary reviewer is blind to. Subagent prompt: -"Read the diff for this branch with \`git diff origin/\`. Think like an attacker and a chaos engineer. Your job is to find ways this code will fail in production. Look for: edge cases, race conditions, security holes, resource leaks, failure modes, silent data corruption, logic errors that produce wrong results silently, error handling that swallows failures, and trust boundary violations. Be adversarial. Be thorough. No compliments — just the problems. For each finding, classify as FIXABLE (you know how to fix it) or INVESTIGATE (needs human judgment). After listing findings, end your output with ONE line in the canonical format \`Recommendation: because \` — examples: \`Recommendation: Fix the unbounded retry at queue.ts:78 because it'll DoS the worker pool under sustained 429s\` or \`Recommendation: Ship as-is because the strongest finding is a theoretical race that requires conditions we can't trigger in production\`. The reason must point to a specific finding (or no-fix rationale). Generic reasons like 'because it's safer' do not qualify." +"Read the diff for this branch with \`DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"\`. Think like an attacker and a chaos engineer. Your job is to find ways this code will fail in production. Look for: edge cases, race conditions, security holes, resource leaks, failure modes, silent data corruption, logic errors that produce wrong results silently, error handling that swallows failures, and trust boundary violations. Be adversarial. Be thorough. No compliments — just the problems. For each finding, classify as FIXABLE (you know how to fix it) or INVESTIGATE (needs human judgment). After listing findings, end your output with ONE line in the canonical format \`Recommendation: because \` — examples: \`Recommendation: Fix the unbounded retry at queue.ts:78 because it'll DoS the worker pool under sustained 429s\` or \`Recommendation: Ship as-is because the strongest finding is a theoretical race that requires conditions we can't trigger in production\`. The reason must point to a specific finding (or no-fix rationale). Generic reasons like 'because it's safer' do not qualify." Present findings under an \`ADVERSARIAL REVIEW (Claude subagent):\` header. **FIXABLE findings** flow into the same Fix-First pipeline as the structured review. **INVESTIGATE findings** are presented as informational. @@ -503,7 +504,7 @@ If Codex is available AND \`OLD_CFG\` is NOT \`disabled\`: \`\`\`bash TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "${CODEX_BOUNDARY}Review the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems. End your output with ONE line in the canonical format \`Recommendation: because \`. Generic reasons like 'because it's safer' do not qualify; the reason must point to a specific finding or no-fix rationale." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" +codex exec "${CODEX_BOUNDARY}Review the changes on this branch against the base branch. Run DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE" to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems. End your output with ONE line in the canonical format \`Recommendation: because \`. Generic reasons like 'because it's safer' do not qualify; the reason must point to a specific finding or no-fix rationale." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" \`\`\` Set the Bash tool's \`timeout\` parameter to \`300000\` (5 minutes). Do NOT use the \`timeout\` shell command — it doesn't exist on macOS. After the command completes, read stderr: diff --git a/ship/SKILL.md b/ship/SKILL.md index fe2500506..0cb0fe529 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -1860,7 +1860,7 @@ Before reviewing code quality, check: **did they build what was requested — no Read commit messages (`git log origin/..HEAD --oneline`). **If no PR exists:** rely on commit messages and TODOS.md for stated intent — this is the common case since /review runs before /ship creates the PR. 2. Identify the **stated intent** — what was this branch supposed to accomplish? -3. Run `git diff origin/...HEAD --stat` and compare the files changed against the stated intent. +3. Run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE" --stat` and compare the files changed against the stated intent. 4. Evaluate with skepticism (incorporating plan completion results if available from an earlier step or adjacent section): @@ -1998,8 +1998,9 @@ STACK="" [ -f go.mod ] && STACK="${STACK}go " [ -f Cargo.toml ] && STACK="${STACK}rust " echo "STACK: ${STACK:-unknown}" -DIFF_INS=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") -DIFF_DEL=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") +DIFF_BASE=$(git merge-base origin/ HEAD) +DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") +DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") DIFF_LINES=$((DIFF_INS + DIFF_DEL)) echo "DIFF_LINES: $DIFF_LINES" # Detect test framework for specialist test stub generation @@ -2073,7 +2074,7 @@ If learnings are found, include them: "Past learnings for this domain: {learning 4. Instructions: "You are a specialist code reviewer. Read the checklist below, then run -`git diff origin/` to get the full diff. Apply the checklist against the diff. +`DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"` to get the full diff. Apply the checklist against the diff. For each finding, output a JSON object on its own line: {\"severity\":\"CRITICAL|INFORMATIONAL\",\"confidence\":N,\"path\":\"file\",\"line\":N,\"category\":\"category\",\"summary\":\"description\",\"fix\":\"recommended fix\",\"fingerprint\":\"path:line:category\",\"specialist\":\"name\"} @@ -2176,7 +2177,7 @@ The Red Team subagent receives: Prompt: "You are a red team reviewer. The code has already been reviewed by N specialists who found the following issues: {merged findings summary}. Your job is to find what they -MISSED. Read the checklist, run `git diff origin/`, and look for gaps. +MISSED. Read the checklist, run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"`, and look for gaps. Output findings as JSON objects (same schema as the specialists). Focus on cross-cutting concerns, integration boundary issues, and failure modes that specialist checklists don't cover." @@ -2312,8 +2313,9 @@ Every diff gets adversarial review from both Claude and Codex. LOC is not a prox **Detect diff size and tool availability:** ```bash -DIFF_INS=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") -DIFF_DEL=$(git diff origin/ --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") +DIFF_BASE=$(git merge-base origin/ HEAD) +DIFF_INS=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+' || echo "0") +DIFF_DEL=$(git diff "$DIFF_BASE" --stat | tail -1 | grep -oE '[0-9]+ deletion' | grep -oE '[0-9]+' || echo "0") DIFF_TOTAL=$((DIFF_INS + DIFF_DEL)) which codex 2>/dev/null && echo "CODEX_AVAILABLE" || echo "CODEX_NOT_AVAILABLE" # Legacy opt-out — only gates Codex passes, Claude always runs @@ -2333,7 +2335,7 @@ If `OLD_CFG` is `disabled`: skip Codex passes only. Claude adversarial subagent Dispatch via the Agent tool. The subagent has fresh context — no checklist bias from the structured review. This genuine independence catches things the primary reviewer is blind to. Subagent prompt: -"Read the diff for this branch with `git diff origin/`. Think like an attacker and a chaos engineer. Your job is to find ways this code will fail in production. Look for: edge cases, race conditions, security holes, resource leaks, failure modes, silent data corruption, logic errors that produce wrong results silently, error handling that swallows failures, and trust boundary violations. Be adversarial. Be thorough. No compliments — just the problems. For each finding, classify as FIXABLE (you know how to fix it) or INVESTIGATE (needs human judgment). After listing findings, end your output with ONE line in the canonical format `Recommendation: because ` — examples: `Recommendation: Fix the unbounded retry at queue.ts:78 because it'll DoS the worker pool under sustained 429s` or `Recommendation: Ship as-is because the strongest finding is a theoretical race that requires conditions we can't trigger in production`. The reason must point to a specific finding (or no-fix rationale). Generic reasons like 'because it's safer' do not qualify." +"Read the diff for this branch with `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"`. Think like an attacker and a chaos engineer. Your job is to find ways this code will fail in production. Look for: edge cases, race conditions, security holes, resource leaks, failure modes, silent data corruption, logic errors that produce wrong results silently, error handling that swallows failures, and trust boundary violations. Be adversarial. Be thorough. No compliments — just the problems. For each finding, classify as FIXABLE (you know how to fix it) or INVESTIGATE (needs human judgment). After listing findings, end your output with ONE line in the canonical format `Recommendation: because ` — examples: `Recommendation: Fix the unbounded retry at queue.ts:78 because it'll DoS the worker pool under sustained 429s` or `Recommendation: Ship as-is because the strongest finding is a theoretical race that requires conditions we can't trigger in production`. The reason must point to a specific finding (or no-fix rationale). Generic reasons like 'because it's safer' do not qualify." Present findings under an `ADVERSARIAL REVIEW (Claude subagent):` header. **FIXABLE findings** flow into the same Fix-First pipeline as the structured review. **INVESTIGATE findings** are presented as informational. @@ -2348,7 +2350,7 @@ If Codex is available AND `OLD_CFG` is NOT `disabled`: ```bash TMPERR_ADV=$(mktemp /tmp/codex-adv-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run git diff origin/ to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems. End your output with ONE line in the canonical format `Recommendation: because `. Generic reasons like 'because it's safer' do not qualify; the reason must point to a specific finding or no-fix rationale." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" +codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE" to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems. End your output with ONE line in the canonical format `Recommendation: because `. Generic reasons like 'because it's safer' do not qualify; the reason must point to a specific finding or no-fix rationale." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV" ``` Set the Bash tool's `timeout` parameter to `300000` (5 minutes). Do NOT use the `timeout` shell command — it doesn't exist on macOS. After the command completes, read stderr: