fix: show submodule pointer changes whatever the diff config, and isolate the local-git tests

- Local diffs also pass --submodule=short and --ignore-submodules=dirty, so
  diff.submodule=log|diff, diff.ignoreSubmodules=all and ignore = all in
  .gitmodules can't hide or rewrite a submodule pointer change, and edits
  inside a submodule whose pointer didn't move aren't sent for review.
- The tests remove every GIT_* variable and point HOME and XDG_CONFIG_HOME
  at an empty directory, so a hook's GIT_DIR/GIT_INDEX_FILE or the user's
  global git files can't touch another repo or fail the fixtures.
- New tests: deletions under diff.noprefix and diff.mnemonicPrefix (the only
  case --src-prefix protects), and the submodule settings above.
- LLP 0004 and 0008 line references updated.

llp/0003-model-runtimes-and-credentials.explainer.md

@@ -165,7 +165,10 @@ attempt and accounts for both attempts; it never accepts unvalidated fallback te
 `--allowedTools` list default-**allows** reads; it does not default-deny. So two things
 happen together: every **granted** read tool (`Read`/`Grep`/`Glob`) is path-scoped to
 the review tree with a `//<cwd>/**` allow rule, and every read tool **not** granted is
-denied explicitly **by name** via `--disallowedTools`. Assuming standard allow-list
+denied explicitly **by name** via `--disallowedTools`. The one scope beyond the tree is
+the run's patch directory (`extraReadRoots`): in CI it sits under the base checkout,
+outside the PR-head tree, and holds only patch text ecr wrote itself. The rest of
+`.runs/` stays unreadable (see LLP 0004 "Patch Workspace"). Assuming standard allow-list
 semantics (unlisted = denied) would silently reopen every unscoped read tool — a bare
 `Grep` no less than a bare `Read` would let an injected instruction read
 `~/.claude/.credentials.json`, `.env*`, or SSH keys and emit them into a finding. This

llp/0004-diff-noise-and-prompts.explainer.md

@@ -77,8 +77,8 @@ path.
 
 `parseUnifiedDiff` has three independent callers with different trust needs, so
 a parsing bug is not contained to the reviewer feed: the two sources
-(`src/sources/local-git.ts:97`, `src/sources/github-pr.ts:98`) turn raw diff
-into the review input, and the GitHub reporter (`src/reporters/github.ts:214`,
+(`src/sources/local-git.ts:118`, `src/sources/github-pr.ts:233`) turn raw diff
+into the review input, and the GitHub reporter (`src/reporters/github.ts:1094`,
 `buildDiffLineIndex`) re-parses to anchor inline PR comments to line numbers
 [observed] (grep of `parseUnifiedDiff` call sites). A path or hunk-offset bug
 would corrupt comment placement, not just review content.
@@ -186,17 +186,27 @@ decides which reviewer agents run [observed] (`src/core/agent-paths.ts`).
 ## Patch Workspace
 
 `writePatchWorkspace` materializes the kept diffs as one `.patch` file per
-changed file plus a `context.md` manifest, all written **inside** the repo run
-directory so the agent's file-read tool can reach them [observed]
-(`src/core/noise.ts:199-231`). Agents are pointed at these paths instead of
-having the full diff inlined into every prompt [observed]
-(`src/core/noise.ts:200-202` docstring).
+changed file plus a `context.md` manifest under the run directory [observed]
+(`src/core/noise.ts:215-245`). Agents are pointed at these paths for files whose
+diff is not inlined into their prompt: a reviewer's context-only files, and the
+cross-cutting pass's files past its inline budget [observed]
+(`src/core/prompts.ts` `buildReviewerTask`, `buildCrossCuttingTask`).
+
+The run directory is **not** inside the tree the passes read in CI. `.runs/` lives
+under the trusted base checkout, while the passes run in the PR-head tree that
+`ecr ci` materializes in a temp directory. The Claude Code engine scopes every read
+tool to that tree, so it grants the patch directory (`patchDirFor(runDir)`) as a
+second read scope, and only that directory [observed] (`src/core/noise.ts:205`,
+`src/core/claude-code.ts` `buildClaudeArgs` `extraReadRoots`). Before that grant,
+every patch read in CI was denied, verified 2026-10-20 against `claude 2.1.280`.
+The OpenCode engine sets no read permissions of its own, so whether it can read
+outside the tree depends on OpenCode's defaults [assumed].
 
 The token/context saving is the stated motive. There is likely a second motive:
 delivering diffs as files the agent reads on demand also keeps untrusted PR
 content out of the prompt text that must be sanitized inline, shrinking the
 injection surface [inferred] — the diff/noise code states only the
-context-budget reason (`src/core/noise.ts:200-202`), and AGENTS.md states diffs
+context-budget reason (`src/core/noise.ts:211-213`), and AGENTS.md states diffs
 are untrusted and must go through `prompts.ts`, but the code never states
 file-delivery as an isolation choice. Note the cross-cutting pass reverses this
 default and inlines diffs anyway (see [Prompt Layout for Cache and
@@ -204,7 +214,7 @@ Coverage](#prompt-layout-for-cache-and-coverage)), which is why the isolation
 motive is a hypothesis, not a stated rule.
 
 Patch filenames are sanitized to `[a-zA-Z0-9._-]` (everything else → `__`) and
-zero-padded index-prefixed [observed] (`src/core/noise.ts:214-217`). This is a
+zero-padded index-prefixed [observed] (`src/core/noise.ts:226-229`). This is a
 security control, not cosmetics: `entry.path` comes from untrusted PR diff
 content, and an unsanitized path could be used for traversal or collision when
 writing files to disk.

llp/0008-sources-and-reporters.explainer.md

@@ -47,13 +47,15 @@ The base OID is fetchable by SHA even though the base ref may have moved since t
 
 ## Local Git Source
 
-`LocalGitSource` has no network calls; it reads local git state and computes its own diff base [observed] ([local-git.ts:17-21](../src/sources/local-git.ts)). Three small decisions carry the weight.
+`LocalGitSource` has no network calls; it reads local git state and computes its own diff base [observed] ([local-git.ts:35-39](../src/sources/local-git.ts)). Four small decisions carry the weight.
 
-**Default branch resolution** tries `origin/HEAD` via `symbolic-ref` first, then falls back through a fixed guess list (`origin/main`, `origin/master`, `main`, `master`), then `main`, because not every checkout has `origin/HEAD` set [observed] ([local-git.ts:31-50](../src/sources/local-git.ts)).
+**Default branch resolution** tries `origin/HEAD` via `symbolic-ref` first, then falls back through a fixed guess list (`origin/main`, `origin/master`, `main`, `master`), then `main`, because not every checkout has `origin/HEAD` set [observed] ([local-git.ts:49-68](../src/sources/local-git.ts)).
 
-**Diff base is `merge-base(defaultBranch, HEAD)`, not the branch tip.** This scopes the review to the current branch's own changes and excludes commits merged into the default branch after the feature branch diverged — you review what you wrote, not what trunk moved on to [observed] ([local-git.ts:53-68](../src/sources/local-git.ts)).
+**Diff base is `merge-base(defaultBranch, HEAD)`, not the branch tip.** This scopes the review to the current branch's own changes and excludes commits merged into the default branch after the feature branch diverged — you review what you wrote, not what trunk moved on to [observed] ([local-git.ts:71-86](../src/sources/local-git.ts)).
 
-**Untracked files are diffed without mutating the index.** Running a review must have no side effects on the user's working state, so untracked files are synthesized as add-diffs via `git diff --no-index -- /dev/null <file>` rather than being staged [observed] ([local-git.ts:101-103, 114](../src/sources/local-git.ts)). The listing uses `git ls-files -z` (NUL-terminated) so filenames containing newlines parse correctly [observed] ([local-git.ts:106-107](../src/sources/local-git.ts)), and the `--` separator before the filename stops a name beginning with `-` from being read as a git option — argument injection through an untracked filename [observed] ([local-git.ts:112, 114](../src/sources/local-git.ts)).
+**Untracked files are diffed without mutating the index.** Running a review must have no side effects on the user's working state, so untracked files are synthesized as add-diffs via `git diff --no-index -- /dev/null <file>` rather than being staged [observed] ([local-git.ts:122-124, 137](../src/sources/local-git.ts)). The listing uses `git ls-files -z` (NUL-terminated) so filenames containing newlines parse correctly [observed] ([local-git.ts:127-128](../src/sources/local-git.ts)), and the `--` separator before the filename stops a name beginning with `-` from being read as a git option — argument injection through an untracked filename [observed] ([local-git.ts:133, 137](../src/sources/local-git.ts)).
+
+**Every parsed `git diff` overrides the user's diff config.** `git diff` follows the user's git config, and several settings change what it prints. An external diff tool (`diff.external`) replaces the unified diff entirely, forced color (`color.diff=always`) puts escape codes before `diff --git`, and `diff.noprefix` or `diff.mnemonicPrefix` drop or change the `a/` and `b/` prefixes that `parseUnifiedDiff` matches. Any of them made a local review parse no files and report "No reviewable changes". Two more change the content rather than break the parse: a textconv driver shows converted text instead of the file the reviewers and verifier read (or nothing, when both sides convert alike), and copy detection (`diff.renames=copies`) turns a copied file into a hunk-less `copy from`/`copy to` entry, so its contents go unreviewed. `diff.submodule` does both to a submodule change: `log` prints commit subjects that parse as nothing, and `diff` prints the submodule's own file diffs instead of the pointer change. `diff.ignoreSubmodules=all`, or `ignore = all` for a submodule in `.gitmodules`, drops the pointer change entirely. Each call therefore passes `--no-ext-diff --no-color --src-prefix=a/ --dst-prefix=b/ --no-textconv --find-renames --submodule=short --ignore-submodules=dirty` [observed]. `dirty` rather than git's default `none`: a submodule then shows only when its pointer moves, so edits or untracked files inside one never reach the reviewers as a `-dirty` pseudo-commit [observed] ([local-git.ts](../src/sources/local-git.ts) `PARSEABLE_DIFF_FLAGS`; tests `src/__tests__/local-git.test.ts`, which fail when any one flag is removed). Repository attributes (`binary`, `-diff`) still apply: in local mode the checkout is trusted (see [LLP 0001](./0001-trust-model.principles.md)), and a file they mark binary is dropped by noise filtering as before.
 
 ## GitHub Reporter Identity
 

src/__tests__/claude-code.test.ts

@@ -627,6 +627,61 @@ test("buildClaudeArgs: empty tools (coordinator/no-tools fallback) deny every re
   );
 });
 
+test("buildClaudeArgs: extraReadRoots add a second scope for each granted read tool only", () => {
+  const patchDir = "/work/base/.expo-code-review/.runs/r1/patches";
+  const args = buildClaudeArgs({
+    model: "claude-opus-5",
+    system: "SYS",
+    cwd: "/work/tree",
+    tools: ["read", "grep"],
+    extraReadRoots: [patchDir],
+  });
+  const joined = args.join(" ");
+  // In CI the patch files live under the base checkout, outside the PR-head tree, so
+  // without this scope every "read the patch file" instruction is denied by dontAsk.
+  expect(joined).toContain(
+    `--allowedTools Read(//work/tree/**) Read(/${patchDir}/**) Grep(//work/tree/**) Grep(/${patchDir}/**)`,
+  );
+  // Only the patch directory: the rest of the run directory stays unreadable.
+  expect(joined).not.toContain("/.runs/r1/**");
+  expect(joined).not.toContain("Glob(/");
+
+  // A pass with no read tools gets no rule for the extra root either.
+  const none = buildClaudeArgs({
+    model: "claude-opus-5",
+    system: "SYS",
+    cwd: "/work/tree",
+    tools: [],
+    extraReadRoots: [patchDir],
+  });
+  expect(none.join(" ")).not.toContain("--allowedTools");
+  expect(none.join(" ")).not.toContain(patchDir);
+});
+
+test("runClaudePrompt: the handle's patch directory reaches the CLI's read scope", async () => {
+  const body = JSON.stringify({
+    type: "result",
+    subtype: "success",
+    is_error: false,
+    result: "ok",
+  });
+  const { bin, argsLog, cleanup } = await sequencedFakeClaude([body]);
+  const patchDir = "/work/base/.expo-code-review/.runs/r1/patches";
+  try {
+    await runClaudePrompt(testHandle(bin, { patchDir }), {
+      agent: "reviewer",
+      system: "SYS",
+      text: "hi",
+      title: "t",
+    });
+    const args = (await argsLog(1)).split("\n");
+    expect(args).toContain(`Read(/${patchDir}/**)`);
+    expect(args).toContain(`Grep(/${patchDir}/**)`);
+  } finally {
+    await cleanup();
+  }
+});
+
 test("buildClaudeArgs: maxTurns is forwarded as the CLI --max-turns bound", () => {
   const args = buildClaudeArgs({
     model: "claude-opus-5",

src/__tests__/local-git.test.ts

@@ -0,0 +1,270 @@
+import { test, expect, afterAll, afterEach, beforeAll } from "bun:test";
+import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import path from "node:path";
+
+import { git } from "../core/exec.js";
+import { LocalGitSource } from "../sources/local-git.js";
+
+// This file's fixture git must stay inside its temp repos. Git exports GIT_DIR,
+// GIT_INDEX_FILE and friends to hooks, so `bun test` run from a pre-commit hook would
+// otherwise write these fixtures into the commit being made. The user's global and
+// system git files (signing, a hooks path, init templates, a global ignore or
+// attributes file) could also fail the fixtures before any assertion runs, so HOME and
+// XDG_CONFIG_HOME point at an empty directory, which works on every git version.
+const savedEnv: Record<string, string | undefined> = {};
+// Set only once mkdtemp succeeds: afterAll runs even when beforeAll throws, and must
+// never remove whatever HOME still points at.
+let fakeHome: string | undefined;
+beforeAll(async () => {
+  for (const [name, value] of Object.entries(process.env)) {
+    if (name.startsWith("GIT_")) {
+      savedEnv[name] = value;
+      delete process.env[name];
+    }
+  }
+  fakeHome = await mkdtemp(path.join(tmpdir(), "ecr-local-git-home-"));
+  for (const [name, value] of [
+    ["HOME", fakeHome],
+    ["XDG_CONFIG_HOME", fakeHome],
+    ["GIT_CONFIG_NOSYSTEM", "1"],
+  ] as const) {
+    savedEnv[name] ??= process.env[name];
+    process.env[name] = value;
+  }
+});
+afterAll(async () => {
+  for (const [name, value] of Object.entries(savedEnv)) {
+    if (value === undefined) {
+      delete process.env[name];
+    } else {
+      process.env[name] = value;
+    }
+  }
+  if (fakeHome) {
+    await rm(fakeHome, { recursive: true, force: true });
+  }
+});
+
+const dirs: string[] = [];
+afterEach(async () => {
+  for (const dir of dirs.splice(0)) {
+    await rm(dir, { recursive: true, force: true });
+  }
+});
+
+async function tempDir(prefix: string): Promise<string> {
+  const dir = await mkdtemp(path.join(tmpdir(), prefix));
+  dirs.push(dir);
+  return dir;
+}
+
+function commit(root: string, message: string): Promise<string> {
+  return git(["-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "-m", message], root);
+}
+
+async function executable(dir: string, name: string, script: string): Promise<string> {
+  const file = path.join(dir, name);
+  await writeFile(file, script, "utf8");
+  await chmod(file, 0o755);
+  return file;
+}
+
+// A committed repo with `a.txt` and `b/inner.txt`. The `b/` path matters: with no
+// prefixes, `+++ b/inner.txt` loses its directory to the parser's `b/` strip.
+async function baseRepo(): Promise<string> {
+  const root = await tempDir("ecr-local-git-");
+  await git(["init", "-q"], root);
+  await mkdir(path.join(root, "b"));
+  await writeFile(path.join(root, "a.txt"), "one\n", "utf8");
+  await writeFile(path.join(root, "b", "inner.txt"), "inner\n", "utf8");
+  await git(["add", "-A"], root);
+  await commit(root, "base");
+  return root;
+}
+
+// A repo whose local git config changes what `git diff` prints: an external diff
+// tool, forced color, and no `a/` and `b/` prefixes. All of them break the parser
+// unless the engine overrides them.
+async function repoWithDiffConfig(): Promise<string> {
+  const root = await baseRepo();
+  const tools = await tempDir("ecr-local-git-tools-");
+  const external = await executable(
+    tools,
+    "external-diff",
+    '#!/bin/sh\necho "external diff of $1"\n',
+  );
+  await git(["config", "diff.external", external], root);
+  await git(["config", "color.diff", "always"], root);
+  await git(["config", "diff.noprefix", "true"], root);
+  return root;
+}
+
+async function repoWithMnemonicPrefix(): Promise<string> {
+  const root = await baseRepo();
+  await git(["config", "diff.mnemonicPrefix", "true"], root);
+  return root;
+}
+
+async function editBoth(root: string): Promise<void> {
+  await writeFile(path.join(root, "a.txt"), "one\ntwo\n", "utf8");
+  await writeFile(path.join(root, "b", "inner.txt"), "inner\nmore\n", "utf8");
+}
+
+test("LocalGitSource: working-tree diffs parse despite the user's diff config", async () => {
+  const root = await repoWithDiffConfig();
+  await editBoth(root);
+  await writeFile(path.join(root, "b", "new.txt"), "new\n", "utf8");
+  const files = await new LocalGitSource({ cwd: root, base: "HEAD" }).getChangedFiles();
+  expect(files.map((file) => file.path).sort()).toEqual(["a.txt", "b/inner.txt", "b/new.txt"]);
+  expect(files.every((file) => file.patch.includes("@@"))).toBe(true);
+});
+
+test("LocalGitSource: a base...head diff parses despite the user's diff config", async () => {
+  const root = await repoWithDiffConfig();
+  await editBoth(root);
+  await git(["add", "-A"], root);
+  await commit(root, "change");
+  const files = await new LocalGitSource({
+    cwd: root,
+    base: "HEAD~1",
+    head: "HEAD",
+  }).getChangedFiles();
+  expect(files.map((file) => file.path)).toEqual(["a.txt", "b/inner.txt"]);
+  expect(files[0]!.patch).toContain("+two");
+});
+
+test("LocalGitSource: a staged diff parses despite the user's diff config", async () => {
+  const root = await repoWithDiffConfig();
+  await editBoth(root);
+  await git(["add", "-A"], root);
+  const files = await new LocalGitSource({ cwd: root, staged: true }).getChangedFiles();
+  expect(files.map((file) => file.path)).toEqual(["a.txt", "b/inner.txt"]);
+  expect(files[0]!.patch).toContain("+two");
+});
+
+test("LocalGitSource: diff.mnemonicPrefix doesn't rename working-tree, untracked or staged paths", async () => {
+  // Mnemonic prefixes are `c/` and `w/` for the working tree, `2/` for --no-index,
+  // and `i/` for the index, none of which the parser strips.
+  const root = await repoWithMnemonicPrefix();
+  await editBoth(root);
+  await writeFile(path.join(root, "new.txt"), "new\n", "utf8");
+  const working = await new LocalGitSource({ cwd: root, base: "HEAD" }).getChangedFiles();
+  expect(working.map((file) => file.path).sort()).toEqual(["a.txt", "b/inner.txt", "new.txt"]);
+  await git(["add", "-A"], root);
+  const staged = await new LocalGitSource({ cwd: root, staged: true }).getChangedFiles();
+  expect(staged.map((file) => file.path)).toEqual(["a.txt", "b/inner.txt", "new.txt"]);
+});
+
+// Only a deletion takes its path from the `---` line, so only a deletion shows a wrong
+// source prefix: `--- b/inner.txt` would lose its directory, and `--- c/b/inner.txt`
+// would keep a `c/` that isn't part of the path.
+test.each([
+  ["diff.noprefix", repoWithDiffConfig],
+  ["diff.mnemonicPrefix", repoWithMnemonicPrefix],
+])("LocalGitSource: a deleted file keeps its path under %s", async (_config, makeRepo) => {
+  const root = await makeRepo();
+  await rm(path.join(root, "b", "inner.txt"));
+  const working = await new LocalGitSource({ cwd: root, base: "HEAD" }).getChangedFiles();
+  await git(["add", "-A"], root);
+  const staged = await new LocalGitSource({ cwd: root, staged: true }).getChangedFiles();
+  await commit(root, "delete");
+  const range = await new LocalGitSource({
+    cwd: root,
+    base: "HEAD~1",
+    head: "HEAD",
+  }).getChangedFiles();
+  for (const files of [working, staged, range]) {
+    expect(files.map((file) => [file.status, file.path])).toEqual([["D", "b/inner.txt"]]);
+  }
+});
+
+test("LocalGitSource: a textconv driver can't replace or hide the real change", async () => {
+  // A driver that prints a constant makes both sides identical, so with textconv on
+  // the change to a.txt vanishes from the review.
+  const root = await baseRepo();
+  const tools = await tempDir("ecr-local-git-tools-");
+  const mask = await executable(tools, "mask", "#!/bin/sh\necho masked\n");
+  await writeFile(path.join(root, ".git", "info", "attributes"), "*.txt diff=mask\n", "utf8");
+  await git(["config", "diff.mask.textconv", mask], root);
+  await editBoth(root);
+  const files = await new LocalGitSource({ cwd: root, base: "HEAD" }).getChangedFiles();
+  expect(files.map((file) => file.path)).toEqual(["a.txt", "b/inner.txt"]);
+  expect(files[0]!.patch).toContain("+two");
+  expect(files[0]!.patch).not.toContain("masked");
+});
+
+test("LocalGitSource: copy detection can't turn a new file into an empty modification", async () => {
+  // With diff.renames=copies, an exact copy of a file the change also edits comes out
+  // as `copy from`/`copy to` with no hunk, so the new file's contents go unreviewed.
+  const root = await baseRepo();
+  await git(["config", "diff.renames", "copies"], root);
+  await writeFile(path.join(root, "b", "copy.txt"), "inner\n", "utf8");
+  await writeFile(path.join(root, "b", "inner.txt"), "inner\nmore\n", "utf8");
+  await git(["add", "-A"], root);
+  const files = await new LocalGitSource({ cwd: root, staged: true }).getChangedFiles();
+  const copy = files.find((file) => file.path === "b/copy.txt");
+  expect(copy?.status).toBe("A");
+  expect(copy?.patch).toContain("+inner");
+});
+
+// `diff.submodule=log` prints `Submodule sub <old>..<new>:` and commit subjects instead
+// of a diff, so the change parses as nothing. `diff.submodule=diff` prints the
+// submodule's own file diffs, so the review sees `sub/s.txt` instead of the pointer
+// change the commit makes. `diff.ignoreSubmodules=all`, or `ignore = all` in
+// `.gitmodules`, drops the change entirely.
+async function repoWithSubmodule(): Promise<{ root: string; sub: string }> {
+  const root = await baseRepo();
+  const sub = path.join(root, "sub");
+  await mkdir(sub);
+  await git(["init", "-q"], sub);
+  await writeFile(path.join(sub, "s.txt"), "one\n", "utf8");
+  await git(["add", "-A"], sub);
+  await commit(sub, "s1");
+  await git(["add", "sub"], root);
+  await commit(root, "add sub");
+  return { root, sub };
+}
+
+test.each([
+  ["diff.submodule=log", ["config", "diff.submodule", "log"]],
+  ["diff.submodule=diff", ["config", "diff.submodule", "diff"]],
+  ["diff.ignoreSubmodules=all", ["config", "diff.ignoreSubmodules", "all"]],
+  [".gitmodules ignore = all", ["config", "-f", ".gitmodules", "submodule.sub.ignore", "all"]],
+])("LocalGitSource: %s still shows a submodule change as its pointer", async (_name, setting) => {
+  const { root, sub } = await repoWithSubmodule();
+  await writeFile(path.join(sub, "s.txt"), "two\n", "utf8");
+  await git(["add", "-A"], sub);
+  await commit(sub, "s2");
+  await git(setting, root);
+  if (setting.includes(".gitmodules")) {
+    await git(["config", "-f", ".gitmodules", "submodule.sub.path", "sub"], root);
+    await git(["add", ".gitmodules"], root);
+    await commit(root, "ignore sub");
+  }
+  const working = await new LocalGitSource({ cwd: root, base: "HEAD" }).getChangedFiles();
+  // `git add` skips a submodule that `.gitmodules` ignores unless forced.
+  await git(["add", "--force", "sub"], root);
+  const staged = await new LocalGitSource({ cwd: root, staged: true }).getChangedFiles();
+  await commit(root, "bump sub");
+  const range = await new LocalGitSource({
+    cwd: root,
+    base: "HEAD~1",
+    head: "HEAD",
+  }).getChangedFiles();
+  for (const files of [working, staged, range]) {
+    expect(files.map((file) => [file.status, file.path])).toEqual([["M", "sub"]]);
+    expect(files[0]!.patch).toContain("+Subproject commit");
+  }
+});
+
+// Edits inside a submodule whose pointer didn't move aren't a change to review, even
+// under a user's `diff.ignoreSubmodules=none`.
+test("LocalGitSource: a submodule with local edits but the same pointer isn't a change", async () => {
+  const { root, sub } = await repoWithSubmodule();
+  await git(["config", "diff.ignoreSubmodules", "none"], root);
+  await writeFile(path.join(sub, "s.txt"), "edited\n", "utf8");
+  await writeFile(path.join(sub, "untracked.txt"), "new\n", "utf8");
+  const files = await new LocalGitSource({ cwd: root, base: "HEAD" }).getChangedFiles();
+  expect(files).toEqual([]);
+});

src/core/claude-code.ts

@@ -53,6 +53,8 @@ export interface ClaudeCodeHandle extends OpencodeHandle {
   researchMcpConfigPath?: string;
   /** Only reviewer and cross-cutting roles may receive the documentation tools. */
   researchAgents?: ReadonlySet<string>;
+  /** The run's patch directory, readable by every pass that has read tools. */
+  patchDir?: string;
 }
 
 /** Prompt args, structurally shared with promptAgent/promptAndParse in opencode.ts. */
@@ -260,7 +262,9 @@ export function claudeModelMatches(requested: string, actualKey: string): boolea
  * (Glob withheld — directory crawling is what made them wander), and the coordinator
  * plus the no-tools fallback get none. Whatever the role, every GRANTED read tool is
  * path-scoped to the review tree (`//<cwd>/**`, Claude Code's absolute-path rule)
- * with `dontAsk` denying any call that matches no allow rule. This narrows the
+ * plus the run's own patch directory (`extraReadRoots` — ecr writes it, and it holds
+ * only patch text the passes are already shown), with `dontAsk` denying any call
+ * that matches no allow rule. This narrows the
  * prompt-injection exfil path for DIRECT out-of-tree reads: untrusted PR content is
  * the review input and findings are posted as PR comments, so any unscoped
  * read-capable tool (a bare `Grep` no less than a bare `Read`) would let an injected
@@ -306,6 +310,12 @@ export function buildClaudeArgs(opts: {
    * by omission.
    */
   tools?: readonly string[];
+  /**
+   * Absolute directories the granted read tools may ALSO read, beyond the tree: the
+   * run's patch directory, which in CI sits under the base checkout and outside the
+   * PR-head tree. Without it every "read the patch file" instruction is denied.
+   */
+  extraReadRoots?: readonly string[];
   researchMcpConfigPath?: string;
   maxTurns?: number;
   jsonSchema?: Record<string, unknown>;
@@ -313,8 +323,10 @@ export function buildClaudeArgs(opts: {
   // Permission rules are gitignore-style with forward slashes; a Windows cwd
   // (`C:\Users\dev\repo`) must be normalized or every rule silently matches
   // nothing and dontAsk denies all reads.
-  const scopeRoot = opts.cwd.replace(/\\/g, "/");
-  const scope = (tool: string): string => `${tool}(/${scopeRoot}/**)`;
+  const scopeRoots = [opts.cwd, ...(opts.extraReadRoots ?? [])].map((root) =>
+    root.replace(/\\/g, "/"),
+  );
+  const scope = (tool: string): string[] => scopeRoots.map((root) => `${tool}(/${root}/**)`);
   const requested = opts.tools ?? ["read", "grep", "glob"];
   const enabled = ALL_READ_TOOLS.filter((claudeName) =>
     requested.some((name) => READ_TOOL_MAP[name] === claudeName),
@@ -323,7 +335,7 @@ export function buildClaudeArgs(opts: {
   // scoped allow rules alone don't deny them when the allow list is empty.
   const deniedReadTools = ALL_READ_TOOLS.filter((tool) => !enabled.includes(tool));
   const researchTools = opts.researchMcpConfigPath ? [...CLAUDE_RESEARCH_TOOLS] : [];
-  const allowedTools = [...enabled.map(scope), ...researchTools];
+  const allowedTools = [...enabled.flatMap(scope), ...researchTools];
   return [
     "-p",
     "--output-format",
@@ -822,6 +834,7 @@ export async function runClaudePrompt(
         system: args.system,
         cwd: process.cwd(),
         tools,
+        ...(handle.patchDir ? { extraReadRoots: [handle.patchDir] } : {}),
         researchMcpConfigPath,
         maxTurns,
         jsonSchema: args.jsonSchema,
@@ -1108,6 +1121,7 @@ export function claudeTokenCredential(
 export async function startClaudeCode(
   config: LoadedConfig,
   research?: ResearchMcpRuntime,
+  patchDir?: string,
 ): Promise<ClaudeCodeHandle> {
   const cliPath = await resolveOnPath("claude");
   if (!cliPath) {
@@ -1222,5 +1236,6 @@ export async function startClaudeCode(
     childEnv,
     ...(research ? { researchMcpConfigPath: research.claudeConfigPath } : {}),
     researchAgents,
+    ...(patchDir ? { patchDir } : {}),
   };
 }

src/core/noise.ts

@@ -197,18 +197,27 @@ export interface PatchWorkspace {
   files: PatchWorkspaceFile[];
 }
 
+/**
+ * Where a run's patch files live. In CI that is under the trusted base checkout,
+ * OUTSIDE the PR-head tree the review passes read, so the Claude Code engine grants
+ * this one directory as a second read scope (see buildClaudeArgs).
+ */
+export function patchDirFor(rootDir: string): string {
+  return path.join(rootDir, "patches");
+}
+
 // @ref LLP 0004#patch-workspace [implements] — filenames sanitized against traversal/collisions from untrusted diff paths
 /**
- * Write one patch file per changed file plus a shared manifest, all inside the
- * repo (so the OpenCode read tool can reach them). Agents are pointed at these
- * paths instead of having the full diff inlined into every prompt.
+ * Write one patch file per changed file plus a shared manifest under the run
+ * directory. Agents are pointed at these paths instead of having every diff
+ * inlined into every prompt.
  */
 export async function writePatchWorkspace(
   kept: DiffEntry[],
   metadata: ReviewMetadata,
   rootDir: string,
 ): Promise<PatchWorkspace> {
-  const patchDir = path.join(rootDir, "patches");
+  const patchDir = patchDirFor(rootDir);
   await mkdir(patchDir, { recursive: true });
 
   const files: PatchWorkspaceFile[] = [];

src/core/review.ts

@@ -12,7 +12,7 @@ import { prepareAuth } from "./auth.js";
 import { coordinate } from "./coordinator.js";
 import { writeRunLog } from "./log.js";
 import type { RunLogRecord } from "./log.js";
-import { filterNoise, writePatchWorkspace } from "./noise.js";
+import { filterNoise, patchDirFor, writePatchWorkspace } from "./noise.js";
 import type { PriorReview } from "./prior-review.js";
 import type { PatchWorkspaceFile } from "./noise.js";
 import {
@@ -456,7 +456,7 @@ export async function runReview(
   }
   try {
     if (usesClaude) {
-      claudeHandle = await startClaudeCode(config, researchRuntime);
+      claudeHandle = await startClaudeCode(config, researchRuntime, patchDirFor(runDir));
     }
   } catch (error) {
     opencodeHandle?.close();

src/sources/local-git.ts

@@ -4,6 +4,25 @@ import { parseUnifiedDiff } from "../core/diff.js";
 import type { DiffEntry, ReviewMetadata } from "../core/schema.js";
 import type { ReviewSource } from "./source.js";
 
+// `git diff` honors the user's config: an external diff tool (`diff.external`), forced
+// color, other path prefixes (`diff.noprefix`, `diff.mnemonicPrefix`), textconv
+// drivers, copy detection (`diff.renames=copies`), submodule logs or inline diffs
+// (`diff.submodule`), and ignored submodules (`diff.ignoreSubmodules`, or `ignore`
+// in `.gitmodules`). These flags restore the plain unified diff of the real file
+// contents, with the `a/` and `b/` prefixes that parseUnifiedDiff reads. A submodule
+// shows only when its pointer moves, never for edits inside it.
+// @ref LLP 0008#local-git-source [implements] — every parsed `git diff` overrides the user's diff config
+const PARSEABLE_DIFF_FLAGS = [
+  "--no-ext-diff",
+  "--no-color",
+  "--src-prefix=a/",
+  "--dst-prefix=b/",
+  "--no-textconv",
+  "--find-renames",
+  "--submodule=short",
+  "--ignore-submodules=dirty",
+];
+
 export interface LocalGitOptions {
   /** Base ref to diff against. Defaults to the merge-base with the default branch. */
   base?: string;
@@ -83,13 +102,16 @@ export class LocalGitSource implements ReviewSource {
   async getChangedFiles(): Promise<DiffEntry[]> {
     let raw: string;
     if (this.options.staged) {
-      raw = await git(["diff", "--staged"], this.cwd);
+      raw = await git(["diff", ...PARSEABLE_DIFF_FLAGS, "--staged"], this.cwd);
     } else {
       const base = await this.resolveBase();
       if (this.options.head) {
-        raw = await git(["diff", `${base}...${this.options.head}`], this.cwd);
+        raw = await git(
+          ["diff", ...PARSEABLE_DIFF_FLAGS, `${base}...${this.options.head}`],
+          this.cwd,
+        );
       } else {
-        const tracked = await git(["diff", base], this.cwd);
+        const tracked = await git(["diff", ...PARSEABLE_DIFF_FLAGS, base], this.cwd);
         const untracked = await this.untrackedDiffs();
         raw = [tracked, untracked].filter((chunk) => chunk.trim()).join("\n");
       }
@@ -111,10 +133,11 @@ export class LocalGitSource implements ReviewSource {
     for (const file of files) {
       // `--` so a filename beginning with `-` can't be read as a git option.
       const gitPath = await resolveTrustedTool("git");
-      const { stdout } = await run(gitPath, ["diff", "--no-index", "--", "/dev/null", file], {
-        cwd: this.cwd,
-        check: false,
-      });
+      const { stdout } = await run(
+        gitPath,
+        ["diff", ...PARSEABLE_DIFF_FLAGS, "--no-index", "--", "/dev/null", file],
+        { cwd: this.cwd, check: false },
+      );
       if (stdout.trim()) {
         chunks.push(stdout);
       }