Fix cwd/worktree leak if prepareAuth throws after chdir

The reviewer (dogfooding this PR) caught it: after chdir into the throwaway
worktree, prepareAuth() sat outside both the startOpencode catch and the main
try/finally, so if it threw (e.g. forbidden token env) restoreCwd() never ran —
leaking the temp worktree and leaving process.cwd() pointing at it.

Move prepareAuth above the chdir (it has no cwd dependency) so nothing that can
throw sits between the chdir and the guarded blocks.

README.md

@@ -84,9 +84,11 @@ Options (most to least common):
 | `--no-fail` | Always exit 0 (otherwise a `request_changes` decision exits non-zero). |
 | `-h`, `--help` | Show help. |
 
-`--pr` uses the PR's diff (authoritative) but reads your checked-out files for
-surrounding context; for full fidelity, `gh pr checkout <n>` first and run a plain
-`ecr review`.
+`--pr` uses the PR's diff (authoritative) and checks the PR head out into a
+throwaway worktree so the agents' surrounding-source reads and the verifier see the
+PR's versions of files — no manual `gh pr checkout` needed, and your working tree is
+left untouched. (If that materialization can't run — e.g. not a git checkout — it
+falls back to reading the current working directory.)
 
 In CI it runs automatically from the scaffolded workflows — by label or a `/review`
 comment (see **CI usage**). From Claude Code (or another agent), add a slash command

ROADMAP.md

@@ -529,21 +529,21 @@ plausible-but-wrong claim ships as-is. Fixes, cheapest/highest-leverage first:
    evidence didn't ground (see #1). Parallel, 3-min cap, fails open (keeps the finding
    if verification itself errors). Decision is re-derived after drops (no criticals
    left → soften `request_changes`).
-3. **`--pr` verification fidelity — verifier must read the PR-HEAD version, not the
-   checked-out tree.** *(Open — found 2026-07-23 re-testing #4057.)* On a `--pr` run
-   the diff is authoritative from `gh`, but the agents' and the verifier's
-   surrounding-source reads come from whatever is checked out. Re-reviewing #4057
-   (the PR that *adds* `--account`) from a `main` checkout, the verifier read `main`'s
-   `init.ts` — which has no `--account` — and **false-refuted** the real "--account
-   ignored when already linked" finding ("the flag is rejected during parsing"). So
-   `--pr` verification silently depends on the local branch: it can refute real
-   findings by reading the wrong version — the mirror image of the quote-grounding
-   over-drop we just fixed (#1). Fix: for `--pr`, make the reviewed tree the PR head
-   — check out the head into a temp worktree, or fetch head blobs for the files the
-   agents/verifier open — so reads match the diff. (Also surfaced: the reviewer
-   *config* itself isn't on an older PR's branch, reinforcing the base-ref-config
-   work in the merge-boundary section — the review needs config-from-a-known-ref and
-   source-from-PR-head, resolved independently.)
+3. ✅ **`--pr` verification fidelity — read the PR-HEAD tree, not the checkout.**
+   *(Shipped 2026-07-23; found re-testing #4057.)* On a `--pr` run the diff is
+   authoritative from `gh`, but the agents' and the verifier's surrounding-source
+   reads came from whatever was checked out. Re-reviewing #4057 (the PR that *adds*
+   `--account`) from a `main` checkout, the verifier read `main`'s `init.ts` — no
+   `--account` — and **false-refuted** the real "--account ignored when already
+   linked" finding — the mirror image of the quote-grounding over-drop (#1). **Fix:**
+   a source can now `prepareReadRootAsync()`; `GitHubPRSource` checks the PR head
+   (`refs/pull/<n>/head`, fetched from the repo URL so forks work) out into a
+   throwaway git worktree, and `runReview` chdirs into it for the agents + verifier
+   (config is already in memory; run-log/patch paths are absolute; fails soft to the
+   current directory). This also fixes CI's `/review` **command** workflow, which
+   checks out the base ref — it was reviewing base, not the PR. (The config-not-on-an
+   -old-PR-branch problem is sidestepped: config loads from the checkout, source
+   reads from the head worktree — two trees, resolved independently.)
 4. **Oracle checks.** A finding that asserts "won't compile / type error" can be
    validated against `tsc`; "crashes"/"breaks tests" against the test suite. At
    minimum, never surface a compile-error claim when the package compiles.

src/core/review.ts

@@ -111,14 +111,37 @@ export async function runReview(
     return output;
   }
 
+  // Prepare auth BEFORE the chdir below: it doesn't depend on the working directory,
+  // and doing it first means nothing that can throw sits between the chdir and the
+  // guarded blocks — so a prepareAuth failure can't leak the worktree or leave cwd
+  // pointing at it.
   const auth = await prepareAuth(config);
 
+  // Read the PR-head tree (not the current checkout) when the source can materialize
+  // it, so the agents' surrounding-source reads and the verifier's re-reads see the
+  // versions that match the diff. Config is already fully loaded in memory, so the
+  // chdir doesn't affect it; run-log/patch paths are absolute; gh/git calls already
+  // ran above. Fails soft to the current directory.
+  const originalCwd = process.cwd();
+  const readRoot = (await source.prepareReadRootAsync?.()) ?? null;
+  const restoreCwd = async (): Promise<void> => {
+    if (readRoot) {
+      process.chdir(originalCwd);
+      await readRoot.cleanup();
+    }
+  };
+  if (readRoot) {
+    progress('Reviewing the PR-head tree (so reads match the PR, not the checkout).');
+    process.chdir(readRoot.dir);
+  }
+
   progress('Starting OpenCode server…');
   let handle: OpencodeHandle | null = null;
   try {
     handle = await startOpencode(buildOpencodeConfig(config));
   } catch (error) {
     await auth.cleanup();
+    await restoreCwd();
     throw new Error(
       `Failed to start the OpenCode server. Ensure the \`opencode\` CLI is installed and ` +
         `model credentials are configured.\n${errorMessage(error)}`
@@ -515,6 +538,7 @@ export async function runReview(
   } finally {
     handle?.close();
     await auth.cleanup();
+    await restoreCwd();
   }
 }
 

src/sources/github-pr.ts

@@ -1,7 +1,11 @@
+import { mkdtemp, rm } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+
 import { run } from '../core/exec.js';
 import { parseUnifiedDiff } from '../core/diff.js';
 import type { DiffEntry, ReviewMetadata } from '../core/schema.js';
-import type { ReviewSource } from './source.js';
+import type { PreparedReadRoot, ReviewSource } from './source.js';
 
 export interface GitHubPRSourceOptions {
   prNumber: number;
@@ -56,4 +60,46 @@ export class GitHubPRSource implements ReviewSource {
     );
     return parseUnifiedDiff(stdout);
   }
+
+  /**
+   * Check the PR HEAD out into a throwaway git worktree so the agents and verifier
+   * read the PR's versions of files (not whatever branch happens to be checked out).
+   * Fetches the head from the repo's own URL — `refs/pull/<n>/head`, which the base
+   * repo hosts even for fork PRs — so it's always the correct PR, independent of the
+   * local `origin`. Fails SOFT: any problem (not a git repo, fetch/worktree error)
+   * returns null, and the review falls back to reading the current checkout.
+   */
+  async prepareReadRootAsync(): Promise<PreparedReadRoot | null> {
+    const cwd = this.options.cwd;
+    if (!this.options.repo) {
+      // Without an explicit owner/repo we can't build the fetch URL safely.
+      return null;
+    }
+    const url = `https://github.com/${this.options.repo}.git`;
+    const ref = `refs/pull/${this.options.prNumber}/head`;
+    let parent: string | undefined;
+    try {
+      await run('git', ['fetch', '--no-tags', '--depth=1', url, ref], { cwd });
+      parent = await mkdtemp(path.join(tmpdir(), 'ecr-prhead-'));
+      const dir = path.join(parent, 'head'); // must not pre-exist for `worktree add`
+      await run('git', ['worktree', 'add', '--detach', dir, 'FETCH_HEAD'], { cwd });
+      const removeParent = parent;
+      return {
+        dir,
+        cleanup: async () => {
+          try {
+            await run('git', ['worktree', 'remove', '--force', dir], { cwd });
+          } catch {
+            // best effort — fall through to removing the temp dir
+          }
+          await rm(removeParent, { recursive: true, force: true });
+        },
+      };
+    } catch {
+      if (parent) {
+        await rm(parent, { recursive: true, force: true }).catch(() => {});
+      }
+      return null;
+    }
+  }
 }

src/sources/source.ts

@@ -8,4 +8,22 @@ export interface ReviewSource {
   getMetadata(): Promise<ReviewMetadata>;
   /** Changed files as path + patch text per file. */
   getChangedFiles(): Promise<DiffEntry[]>;
+  /**
+   * Optionally materialize the exact tree the review should READ from and return its
+   * directory + a cleanup. The review core chdirs into it while running the agents
+   * and the verifier, so their read/grep tools see the right versions of files.
+   *
+   * This matters for a GitHub PR: the diff is authoritative from the API, but the
+   * agents also read surrounding source and the verifier re-reads cited files — and
+   * those must be the PR-HEAD versions, not whatever happens to be checked out.
+   * Return `null` (or omit the method) to just review the current working directory.
+   */
+  prepareReadRootAsync?(): Promise<PreparedReadRoot | null>;
+}
+
+export interface PreparedReadRoot {
+  /** Directory the review should read from (chdir target). */
+  dir: string;
+  /** Remove the materialized tree; always called in a finally. */
+  cleanup: () => Promise<void>;
 }