fix(ce-code-review): isolate testing reviewer mutation work

docs/solutions/skill-design/verify-harness-worktree-snapshot-fidelity.md

@@ -0,0 +1,56 @@
+---
+title: Harness worktree isolation is not a faithful snapshot — verify HEAD before acting
+date: 2026-08-31
+category: skill-design
+module: skills (isolated-worktree dispatch across harnesses)
+problem_type: design_pattern
+component: tooling
+severity: high
+applies_when:
+  - "a skill dispatches a subagent with harness worktree isolation (e.g. Claude Code Agent isolation: \"worktree\")"
+  - "the dispatched work mutates or verifies files and assumes it holds the session's reviewed tree"
+  - "the session itself runs inside a managed or linked worktree (Codex desktop, Cursor, Orca)"
+tags: [worktree-isolation, subagent-dispatch, snapshot-fidelity, cross-harness, mutation-testing, ce-code-review]
+---
+
+# Harness worktree isolation is not a faithful snapshot — verify HEAD before acting
+
+## Context
+
+`ce-code-review` dispatches its `testing` persona into an isolated worktree (Claude Code's Agent `isolation: "worktree"`) so mutation testing cannot write transient lines into the shared checkout that concurrent read-only reviewers observe (PR #1584, issue #1566). The dispatch prose originally assumed that when the reviewed tree is committed `HEAD`, "the harness supplies the snapshot" — that an isolated worktree is automatically a faithful copy of the tree the session is reviewing.
+
+A live probe falsified that assumption. Most harness apps (Codex desktop, Cursor, Orca) run the whole session inside a managed/linked worktree, so skills routinely execute from one. Spawning a worktree-isolated subagent from inside such a session on this repo produced an isolated copy cut from the **primary checkout's default branch**, not from the session worktree's checked-out branch: the isolated HEAD matched neither the session's HEAD nor the reviewed commit, and its tree differed from the reviewed tree by 53 files. A mutation-testing reviewer dropped there would have silently tested the wrong code.
+
+## Guidance
+
+When a skill hands work to a harness-created isolated worktree, treat snapshot fidelity as a condition to verify, never a property of the mechanism:
+
+1. **Pass the intended commit SHA** to the isolated worker as part of its dispatch.
+2. **Have the worker verify before acting**: its copy's `HEAD` must equal that commit (`git rev-parse HEAD`).
+3. **Fall back on any mismatch** to an explicit scratch copy of the intended tree, made by the worker itself.
+4. Uncommitted state never survives worktree isolation: a review scope that includes staged/unstaged changes (`local-aligned`) needs a scratch copy that preserves them regardless of what the harness supplies.
+
+The fix on PR #1584's branch (open as of this writing) states this in both files that govern the behavior — the dispatch layer in `skills/ce-code-review/references/dispatch-reviewers.md` ("Exception — tree-mutating reviewers"), and independently in the persona prompt in `skills/ce-code-review/references/personas/testing-reviewer.md`, because a fresh subagent receives only the persona file and never inherits the dispatch reference. The verify condition is pinned in `tests/review-skill-contract.test.ts` (`"HEAD equals the reviewed commit"`).
+
+## Why This Matters
+
+The failure is silent and wrong-by-content, not loud: the isolated worker runs happily, the suite passes or fails, and every conclusion refers to a tree nobody asked about. Worktree-based session managers are the default setup for a growing share of users, so "session runs inside a linked worktree" is the common case, not the edge. And because git worktrees share one object store, the wrong-base copy looks completely normal from inside — only comparing `HEAD` against the intended commit exposes it.
+
+## When to Apply
+
+- Any skill or dispatch path that uses a harness isolation feature (Claude Code Agent `isolation: "worktree"` or an analogue) to give a worker its own copy of the repo.
+- Any worker whose job depends on operating over a specific tree: mutation testing, build verification, refactoring probes, benchmark runs.
+- Reviews or checks whose scope includes uncommitted changes — isolation from committed `HEAD` is stale for those by construction, on every harness.
+
+## Examples
+
+The probe that demonstrated the trap, runnable from any session already inside a linked worktree: spawn a subagent with `isolation: "worktree"` and have it report `pwd`, `git rev-parse HEAD`, and `git worktree list`. In the observed run, the session worktree sat at the reviewed commit on the PR branch, while the isolated copy's `HEAD` was a commit on top of the default branch tip from the primary checkout — the two trees differed by 53 files (`git diff --stat <session-head> <isolated-head>`).
+
+The corrected dispatch shape, from `dispatch-reviewers.md`: dispatch with `isolation: "worktree"` *and* the reviewed commit SHA; the persona verifies `HEAD` equals that SHA before mutating and falls back to a scratch copy on any mismatch.
+
+## Related
+
+- [sandbox-workers-must-not-write-linked-worktree-git-index.md](sandbox-workers-must-not-write-linked-worktree-git-index.md) — sibling worktree-dispatch pitfall: a worktree handed to a sandboxed worker cannot take git index writes.
+- [bundled-script-path-resolution-across-harnesses.md](bundled-script-path-resolution-across-harnesses.md) — same meta-pattern: a cross-harness assumption falsified empirically, then generalized into an authoring rule.
+- [harness-agent-gate-workaround.md](harness-agent-gate-workaround.md) — lifecycle rule for harness workarounds; if harnesses later anchor isolation worktrees to the session worktree's HEAD, its exit condition applies to this guard.
+- Issue #1566 (originating), PR #1584 (carrier).

skills/ce-code-review/references/dispatch-reviewers.md

@@ -73,6 +73,8 @@ For each selected reviewer, and only for those, read the corresponding local pro
 
 Persona sub-agents are **read-only** with respect to the project: they review and return structured JSON. They do not edit project files or propose refactors. The one permitted write is saving their full analysis to the resolved run-artifact path specified in the output contract.
 
+**Exception — tree-mutating reviewers.** A persona whose method mutates the tree (`testing`, when it runs mutation testing) operates only on a faithful snapshot of the reviewed tree, never the shared checkout. Mutation testing on the shared tree is forbidden: a concurrent reviewer can observe a transient write as if it were the diff. When the reviewed tree is committed `HEAD`, dispatch that persona with `isolation: "worktree"` and pass it the reviewed commit SHA — a harness-supplied worktree is not automatically faithful (when the session itself runs inside a managed worktree, the harness may cut it from the primary checkout or default branch instead; observed live), so before mutating, the persona verifies its copy's HEAD equals the reviewed commit and falls back to a scratch copy on any mismatch. When the reviewed tree includes staged or unstaged changes (`local-aligned` scope), or the host cannot isolate, have the persona materialize a scratch copy that preserves those changes and mutate only the copy. Read-only siblings stay on the shared checkout.
+
 Read-only here means **non-mutating**, not "no shell access." Reviewer sub-agents may use non-mutating inspection commands when needed to gather evidence or verify scope, including read-oriented `git` / `gh` usage such as `git diff`, `git show`, `git blame`, `git log`, and `gh pr view`. When a finding's claim depends on line history (`pre_existing`, intent, introduced-by-this-diff, or P0/P1 confidence that depends on authorship/age), reviewers are expected to attach one concise provenance evidence line from targeted blame/log on the cited line — additional to the quote-the-line gate, never a full-file dump, and omitted when the finding is justified from the diff alone. In **`pr-remote`** or **`branch-remote`** scope (see Stage 1), inspect changed files via `git show <remote-head-ref>:<path>` or diff hunks — do not Read/Grep workspace paths for files in scope; gather blame/log against that reviewed head ref. They must not edit project files, change branches, commit, push, create PRs, or otherwise mutate the checkout or repository state.
 
 Each persona sub-agent writes full JSON (all schema fields) to `{run_dir}/{reviewer_name}.json` and returns compact JSON with merge-tier fields only:

skills/ce-code-review/references/personas/testing-reviewer.md

@@ -13,6 +13,8 @@ You are a test architecture and coverage expert who evaluates whether the tests
 - **Missing edge case coverage for error paths** -- new code has error handling (catch blocks, error returns, fallback branches) but no test verifies the error path fires correctly. The happy path is tested; the sad path is not.
 - **Behavioral changes with no test additions** -- the diff modifies behavior (new logic branches, state mutations, changed API contracts, altered control flow, or error behavior) but adds or modifies zero test files. This is distinct from untested branches above, which checks coverage *within* code that has tests. This check flags when the diff contains behavioral changes with no corresponding test work at all. Non-behavioral changes (formatting, comments, type-only annotations, or dependency/config metadata that does not alter runtime behavior) are excluded.
 
+If you use mutation testing (edit a production file, run the suite, revert), do it only in an isolated worktree or a scratch copy that is a faithful snapshot of the reviewed tree — verify before mutating: your copy's HEAD must equal the reviewed commit (a harness-created worktree may be cut from the primary checkout or default branch instead), and `local-aligned` scope needs the staged/unstaged changes a committed-`HEAD` worktree lacks. On any mismatch, fall back to a scratch copy of the reviewed tree. Never mutate the shared checkout the rest of the reviewer batch is reading.
+
 ## Confidence calibration
 
 Use the anchored confidence rubric in the subagent template. Persona-specific guidance:

tests/review-skill-contract.test.ts

@@ -525,6 +525,22 @@ describe("ce-code-review contract", () => {
     expect(content).toMatch(/still waiting/i)
   })
 
+  test("Stage 4 isolates tree-mutating testing reviewers from the shared checkout (#1566)", async () => {
+    const dispatch = await readRepoFile(
+      "skills/ce-code-review/references/dispatch-reviewers.md",
+    )
+    expect(dispatch).toContain('isolation: "worktree"')
+    expect(dispatch).toContain("HEAD equals the reviewed commit")
+    expect(dispatch).toContain("Mutation testing on the shared tree is forbidden")
+    expect(dispatch).toContain("faithful snapshot of the reviewed tree")
+    expect(dispatch).toContain("local-aligned")
+    const testing = await readRepoFile(
+      "skills/ce-code-review/references/personas/testing-reviewer.md",
+    )
+    expect(testing).toContain("Never mutate the shared checkout")
+    expect(testing).toContain("faithful snapshot of the reviewed tree")
+  })
+
   test("Stage 4 collects by observed return shape and fails closed without a collector", async () => {
     const skill = await readRepoFile("skills/ce-code-review/SKILL.md")
     const content = await readRepoFile(