fix(review,cso): SHA-pin PR diff to prevent worktree-flip-during-review

CHANGELOG.md

@@ -1,5 +1,60 @@
 # Changelog
 
+## [1.26.3.0] - 2026-05-04
+
+## **`/review` and `/cso` stop reading the wrong branch's diff when a subagent flips the worktree.**
+
+The bug, observed three times in one session against a downstream project's PR
+reviewers: a long-running review skill renders findings against whatever branch
+the local worktree happens to be checked out to at the moment each `git diff`
+runs — not against the branch the user asked to review. Inside Agent SDK
+sessions where nested subagents share the worktree, a stray `git checkout`
+anywhere in the call tree silently re-targets every later diff command. The
+review then reports vulnerabilities, race conditions, or dead code on
+unrelated work, and the human reviewer has no way to know.
+
+The fix is mechanical and contained: a new `{{PR_DIFF_PIN}}` template fragment
+runs at Step 0.5 of each review skill, resolves `BASE_SHA` and `HEAD_SHA` to
+immutable commit identifiers via `gh pr view --json headRefOid` (in PR
+context) or `git rev-parse origin/<base>` (out of PR context), and forces
+every subsequent `git diff`, `git log`, and `git show` to reference those
+SHAs by value. Commit SHAs are not symbolic refs — they don't move when the
+worktree flips. `/review` and `/cso` both adopt the new fragment in this
+release. A regression test (`test/pr-diff-pin-regression.test.ts`) builds a
+real two-branch fixture, simulates the worktree flip, and asserts that bare
+`git diff main` reports the wrong branch while SHA-pinned `git diff
+"$BASE_SHA" "$HEAD_SHA"` reports the right one — proving both the failure
+mode and the fix are real.
+
+This is not the upstream Claude Code `/security-review` built-in (which has
+the same class of bug — it uses `git diff origin/HEAD...` against working-tree
+HEAD — but lives in `cli.js` and is out of gstack's reach). The gstack `/cso`
+skill is now a strictly safer alternative for security audits run from inside
+multi-agent sessions.
+
+### What you can now do
+
+- **Run `/review` from a session that spawns subagents and trust the diff is the right one.** Step 0.5 prints the pinned `BASE_SHA` and `HEAD_SHA` so you can verify before findings start. If the SHAs ever look wrong, you'll see it before the review reports anything.
+- **Run `/cso --diff` from the same kind of multi-agent session** and get the same guarantee for the secrets-archaeology and OWASP phases.
+- **Catch a regression in code review.** `test/pr-diff-pin-regression.test.ts` runs in the free `bun test` tier (~6s) and will fail loudly if anyone re-introduces a bare `git diff origin/<base>` into either skill template.
+
+### Itemized changes
+
+#### Added
+
+- `{{PR_DIFF_PIN}}` template resolver in `scripts/resolvers/utility.ts`. Generates a Step 0.5 block that resolves `BASE_SHA`/`HEAD_SHA` from PR metadata when available, from `origin/<base>` and local `HEAD` otherwise, fetches the head commit so the SHA is local, and aborts with a descriptive error when SHAs cannot be resolved (refusing to proceed beats silently rendering against the wrong branch).
+- `test/pr-diff-pin-regression.test.ts` (8 tests, ~6s, free tier). Builds a fresh git repo with two divergent feature branches, exercises the bug end-to-end, and asserts the SHA-pinned form is stable across worktree flips. Includes template smell-tests that fail if `{{PR_DIFF_PIN}}` is removed or if a bare-ref diff command is reintroduced into `review/SKILL.md.tmpl` or `cso/SKILL.md.tmpl`.
+
+#### Changed
+
+- `review/SKILL.md.tmpl` Step 1 and Step 3 use `git diff "$BASE_SHA" "$HEAD_SHA"` instead of `git diff origin/<base>`. The Step 3.4 workspace-aware queue check uses `git show "$HEAD_SHA:VERSION"` and `git show "$BASE_SHA:VERSION"` instead of `git show HEAD:VERSION` and `git show origin/$BASE_BRANCH:VERSION`. The Step 3.5 slop-scan reads against the pinned base. The skill's preamble explicitly names the `shared-checkout-branch-flip-during-review` failure mode the change closes.
+- `cso/SKILL.md.tmpl` adds `{{BASE_BRANCH_DETECT}}` and `{{PR_DIFF_PIN}}` to its preamble (it had neither). The Phase 2 secrets-archaeology `--diff` mode line replaces `git log -p <base>..HEAD` with `git log -p "$BASE_SHA..$HEAD_SHA"`.
+- `review/checklist.md` and `review/greptile-triage.md` reference the pinned SHAs and explain why `git diff origin/main` alone is unsafe inside multi-agent sessions.
+
+#### For contributors
+
+- Other skills with similar bare-ref diff patterns (`ship`, `codex`, `document-release`) are unchanged in this release. They are reachable from outside multi-agent reviews and the worktree-flip risk is lower there; a separate PR will sweep them once a per-skill verification is done. The new `{{PR_DIFF_PIN}}` resolver is reusable — adopting it elsewhere is one line in the `.tmpl` plus running `bun run gen:skill-docs`.
+
 ## [1.26.2.0] - 2026-05-03
 
 ## **`/plan-eng-review` always asks. Never silently writes findings to your plan first.**

VERSION

@@ -1 +1 @@
-1.26.2.0
+1.26.3.0

cso/SKILL.md

@@ -678,8 +678,182 @@ PLAN MODE EXCEPTION — always allowed (it's the plan file).
 
 
 
+## Step 0: Detect platform and base branch
+
+First, detect the git hosting platform from the remote URL:
+
+```bash
+git remote get-url origin 2>/dev/null
+```
+
+- If the URL contains "github.com" → platform is **GitHub**
+- If the URL contains "gitlab" → platform is **GitLab**
+- Otherwise, check CLI availability:
+  - `gh auth status 2>/dev/null` succeeds → platform is **GitHub** (covers GitHub Enterprise)
+  - `glab auth status 2>/dev/null` succeeds → platform is **GitLab** (covers self-hosted)
+  - Neither → **unknown** (use git-native commands only)
+
+Determine which branch this PR/MR targets, or the repo's default branch if no
+PR/MR exists. Use the result as "the base branch" in all subsequent steps.
+
+**If GitHub:**
+1. `gh pr view --json baseRefName -q .baseRefName` — if succeeds, use it
+2. `gh repo view --json defaultBranchRef -q .defaultBranchRef.name` — if succeeds, use it
+
+**If GitLab:**
+1. `glab mr view -F json 2>/dev/null` and extract the `target_branch` field — if succeeds, use it
+2. `glab repo view -F json 2>/dev/null` and extract the `default_branch` field — if succeeds, use it
+
+**Git-native fallback (if unknown platform, or CLI commands fail):**
+1. `git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/origin/||'`
+2. If that fails: `git rev-parse --verify origin/main 2>/dev/null` → use `main`
+3. If that fails: `git rev-parse --verify origin/master 2>/dev/null` → use `master`
+
+If all fail, fall back to `main`.
+
+Print the detected base branch name. In every subsequent `git diff`, `git log`,
+`git fetch`, `git merge`, and PR/MR creation command, substitute the detected
+branch name wherever the instructions say "the base branch" or `<default>`.
+
+---
+
+## Step 0.5: Pin diff context to immutable SHAs (anti-branch-flip)
+
+A long-running review skill is **not safe** to read git state through symbolic
+refs like `HEAD`, `origin/<base>`, or `origin/HEAD`. Inside an Agent SDK
+session — and especially across nested subagents that share a worktree — the
+working tree, the symbolic-ref `HEAD`, and even the checked-out branch can
+flip mid-skill (e.g., another tool runs `git checkout` to inspect a file,
+then forgets to switch back). When that happens, every later `git diff`
+command silently re-renders against the new branch, and the review reports
+findings on the wrong code.
+
+The fix is to **resolve diff endpoints to immutable commit SHAs at the very
+start of the skill**, then use those SHAs in every subsequent `git diff`,
+`git log`, and `git show` invocation. SHAs do not move when the working
+tree flips.
+
+Run this **once, before any other diff/log step**:
+
+```bash
+# Resolve the PR (or branch) we're reviewing. Prefer explicit PR context.
+PR_NUMBER=$(gh pr view --json number -q .number 2>/dev/null || echo "")
+
+# REVIEW_DIRTY governs whether uncommitted local changes count as part of the
+# review. Default OFF in PR context (review committed work only); default ON
+# for local /review pre-PR (preserves the pre-fix behavior where dirty edits
+# were included in the diff). Override by exporting REVIEW_DIRTY=1 / 0 before
+# invoking the skill.
+if [ -z "${REVIEW_DIRTY+x}" ]; then
+  if [ -n "$PR_NUMBER" ]; then REVIEW_DIRTY=0; else REVIEW_DIRTY=1; fi
+fi
+
+if [ -n "$PR_NUMBER" ]; then
+  # In-PR review: prefer the PR's *own* recorded base/head SHAs over the
+  # local origin/<base> tracking ref. baseRefOid and headRefOid are
+  # immutable for the PR's current state — they are the SHAs GitHub renders
+  # against, regardless of local fetch staleness.
+  PR_META=$(gh pr view "$PR_NUMBER" --json baseRefName,headRefName,headRefOid,baseRefOid 2>/dev/null)
+  BASE_BRANCH=$(echo "$PR_META" | jq -r '.baseRefName // empty')
+  HEAD_BRANCH=$(echo "$PR_META" | jq -r '.headRefName // empty')
+  HEAD_SHA=$(echo "$PR_META" | jq -r '.headRefOid // empty')
+  BASE_SHA=$(echo "$PR_META" | jq -r '.baseRefOid // empty')
+  # Fetch BOTH SHAs so they are present in the local object store. \
+  # Without this, `git diff "$BASE_SHA" "$HEAD_SHA"` errors out.
+  if [ -n "$HEAD_SHA" ]; then
+    git fetch origin "$HEAD_SHA" --quiet 2>/dev/null || \
+      git fetch origin "pull/$PR_NUMBER/head" --quiet 2>/dev/null || \
+      git fetch origin "$HEAD_BRANCH" --quiet 2>/dev/null || true
+  fi
+  if [ -n "$BASE_SHA" ]; then
+    git fetch origin "$BASE_SHA" --quiet 2>/dev/null || \
+      git fetch origin "$BASE_BRANCH" --quiet 2>/dev/null || true
+  fi
+else
+  # No PR context: fall back to local-branch review against detected base branch.
+  # Reuse "the base branch" detected in Step 0; pin to its current origin SHA + local HEAD SHA.
+  HEAD_BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "")
+  HEAD_SHA=$(git rev-parse HEAD 2>/dev/null || echo "")
+  if ! git fetch origin "$BASE_BRANCH" --quiet 2>/dev/null; then
+    echo "WARNING: could not fetch origin/$BASE_BRANCH. Pinning to whatever local origin/$BASE_BRANCH points at — may be stale." >&2
+  fi
+  BASE_SHA=$(git rev-parse "origin/$BASE_BRANCH" 2>/dev/null || echo "")
+fi
+
+# Soft-validate the SHAs. If a skill REQUIRES a diff context (`/review`, `/cso --diff`),
+# it should add an explicit `[ -n "$BASE_SHA" ] && [ -n "$HEAD_SHA" ]` assertion before
+# its first diff/log step. Skills that operate without a diff (`/cso --infra`,
+# `/cso --supply-chain`, etc.) can proceed with empty SHAs and simply skip diff-mode
+# substeps. Returning early via `exit 1` here would break those scope-flag modes.
+if [ -z "$BASE_SHA" ] || [ -z "$HEAD_SHA" ]; then
+  echo "WARNING: could not resolve BASE_SHA / HEAD_SHA — diff-dependent steps will be skipped." >&2
+  echo "  PR_NUMBER=$PR_NUMBER  BASE_BRANCH=$BASE_BRANCH  HEAD_BRANCH=$HEAD_BRANCH" >&2
+fi
+
+# If we DID resolve SHAs, verify both actually exist in the local object store.
+# (cat-file probe is a no-op when SHA is empty.)
+for _SHA in "$BASE_SHA" "$HEAD_SHA"; do
+  if [ -n "$_SHA" ] && ! git cat-file -e "$_SHA" 2>/dev/null; then
+    echo "WARNING: SHA $_SHA is not present in the local repo — re-run after `git fetch origin` if review covers committed changes." >&2
+  fi
+done
+
+echo "Pinned review context:"
+echo "  PR:           ${PR_NUMBER:-<none — local-branch review>}"
+echo "  Base branch:  $BASE_BRANCH @ $BASE_SHA"
+echo "  Head branch:  $HEAD_BRANCH @ $HEAD_SHA"
+echo "  Dirty edits:  ${REVIEW_DIRTY:-0} (1 = include uncommitted working-tree changes in diff)"
+```
+
+**For the rest of this skill, use these pinned SHAs** in every diff/log
+command. Concretely:
+
+| Don't (working-tree dependent — bug) | Do (SHA-pinned — correct)                |
+|--------------------------------------|------------------------------------------|
+| `git diff origin/<base>`           | `git diff "$BASE_SHA" "$HEAD_SHA"`     |
+| `git diff origin/<base>...HEAD`    | `git diff "$BASE_SHA" "$HEAD_SHA"`     |
+| `git diff <base>..HEAD`            | `git diff "$BASE_SHA" "$HEAD_SHA"`     |
+| `git log origin/<base>..HEAD`      | `git log "$BASE_SHA..$HEAD_SHA"`       |
+| `git diff --name-only origin/HEAD...` | `git diff --name-only "$BASE_SHA" "$HEAD_SHA"` |
+| `git show HEAD:VERSION`            | `git show "$HEAD_SHA:VERSION"`         |
+
+**Avoid `gh pr diff "$PR_NUMBER"`** even in PR-review context: that endpoint
+re-resolves `HEAD` and `BASE` server-side at every call, so a force-push of
+the PR head or a fast-forward of the PR base mid-review will silently change
+its output. Use the SHA-pinned local `git diff "$BASE_SHA" "$HEAD_SHA"`
+instead — it is immutable both against worktree flips AND against PR-state
+drift on the remote.
+
+If you genuinely need the PR-rendered diff (e.g., to compare against
+GitHub's UI), append `--patch` and a SHA boundary explicitly:
+`gh api "/repos/<owner>/<repo>/compare/$BASE_SHA...$HEAD_SHA"`.
+
+**Do not** use bare `HEAD`, `origin/HEAD`, or `origin/<base>` (without
+`...$HEAD_SHA`) anywhere else in this skill. Even if those refs are correct
+right now, a later subagent may flip the worktree underneath you.
+
+This step is named `shared-checkout-branch-flip-during-review` in
+`CLAUDE.md` failure-mode tracking.
+
+---
+
 # /cso — Chief Security Officer Audit (v2)
 
+**Diff context is SHA-pinned — see Step 0.5.** Whenever this skill scans a
+PR/branch diff (any phase running with `--diff`, plus Phase 2 secrets-archaeology
+diff mode), it must use `$BASE_SHA` and `$HEAD_SHA` resolved in Step 0.5, **not**
+bare `HEAD` / `origin/<base>` / `<base>..HEAD`. A subagent flipping the worktree
+mid-audit otherwise causes findings to render against the wrong code (named
+failure mode `shared-checkout-branch-flip-during-review`).
+
+When `/cso` runs WITHOUT `--diff` (e.g., `/cso --infra`, `/cso --supply-chain`,
+`/cso --owasp`), Step 0.5's WARN-on-missing-SHA behavior is acceptable: those
+scope flags don't read PR diffs, so empty `$BASE_SHA` / `$HEAD_SHA` is fine.
+Diff-mode substeps that DO need them (Phase 2 with `--diff`, etc.) must check
+`[ -n "$BASE_SHA" ] && [ -n "$HEAD_SHA" ]` and skip with a clear note when not
+resolvable. Do not silently fall back to bare `HEAD`/`origin/<base>` — that
+re-introduces the bug this Step 0.5 block exists to close.
+
 You are a **Chief Security Officer** who has led incident response on real breaches and testified before boards about security posture. You think like an attacker but report like a defender. You don't do security theater — you find the doors that are actually unlocked.
 
 The real attack surface isn't your code — it's your dependencies. Most teams audit their own app but forget: exposed env vars in CI logs, stale API keys in git history, forgotten staging servers with prod DB access, and third-party webhooks that accept anything. Start there, not at the code level.
@@ -864,7 +1038,7 @@ done 2>/dev/null
 
 **FP rules:** Placeholders ("your_", "changeme", "TODO") excluded. Test fixtures excluded unless same value in non-test code. Rotated secrets still flagged (they were exposed). `.env.local` in `.gitignore` is expected.
 
-**Diff mode:** Replace `git log -p --all` with `git log -p <base>..HEAD`.
+**Diff mode:** Replace `git log -p --all` with `git log -p "$BASE_SHA..$HEAD_SHA"` (SHAs pinned in Step 0.5 — never use bare `<base>..HEAD` or `origin/<base>..HEAD` because a subagent flipping the worktree would silently re-target the diff at the wrong branch).
 
 ### Phase 3: Dependency Supply Chain
 

cso/SKILL.md.tmpl

@@ -35,8 +35,27 @@ triggers:
 
 {{GBRAIN_CONTEXT_LOAD}}
 
+{{BASE_BRANCH_DETECT}}
+
+{{PR_DIFF_PIN}}
+
 # /cso — Chief Security Officer Audit (v2)
 
+**Diff context is SHA-pinned — see Step 0.5.** Whenever this skill scans a
+PR/branch diff (any phase running with `--diff`, plus Phase 2 secrets-archaeology
+diff mode), it must use `$BASE_SHA` and `$HEAD_SHA` resolved in Step 0.5, **not**
+bare `HEAD` / `origin/<base>` / `<base>..HEAD`. A subagent flipping the worktree
+mid-audit otherwise causes findings to render against the wrong code (named
+failure mode `shared-checkout-branch-flip-during-review`).
+
+When `/cso` runs WITHOUT `--diff` (e.g., `/cso --infra`, `/cso --supply-chain`,
+`/cso --owasp`), Step 0.5's WARN-on-missing-SHA behavior is acceptable: those
+scope flags don't read PR diffs, so empty `$BASE_SHA` / `$HEAD_SHA` is fine.
+Diff-mode substeps that DO need them (Phase 2 with `--diff`, etc.) must check
+`[ -n "$BASE_SHA" ] && [ -n "$HEAD_SHA" ]` and skip with a clear note when not
+resolvable. Do not silently fall back to bare `HEAD`/`origin/<base>` — that
+re-introduces the bug this Step 0.5 block exists to close.
+
 You are a **Chief Security Officer** who has led incident response on real breaches and testified before boards about security posture. You think like an attacker but report like a defender. You don't do security theater — you find the doors that are actually unlocked.
 
 The real attack surface isn't your code — it's your dependencies. Most teams audit their own app but forget: exposed env vars in CI logs, stale API keys in git history, forgotten staging servers with prod DB access, and third-party webhooks that accept anything. Start there, not at the code level.
@@ -185,7 +204,7 @@ done 2>/dev/null
 
 **FP rules:** Placeholders ("your_", "changeme", "TODO") excluded. Test fixtures excluded unless same value in non-test code. Rotated secrets still flagged (they were exposed). `.env.local` in `.gitignore` is expected.
 
-**Diff mode:** Replace `git log -p --all` with `git log -p <base>..HEAD`.
+**Diff mode:** Replace `git log -p --all` with `git log -p "$BASE_SHA..$HEAD_SHA"` (SHAs pinned in Step 0.5 — never use bare `<base>..HEAD` or `origin/<base>..HEAD` because a subagent flipping the worktree would silently re-target the diff at the wrong branch).
 
 ### Phase 3: Dependency Supply Chain
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "gstack",
-  "version": "1.26.2.0",
+  "version": "1.26.3.0",
   "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
   "license": "MIT",
   "type": "module",