fix: reject a repeated --expect-head, and say that older releases ignore it

parseValueFlag keeps the first occurrence, so a second --expect-head was
ignored without a word. Releases from before the flag ignore it like any
unknown argument, so the README and LLP 0007 say to pin a release that has it.

README.md

@@ -372,6 +372,16 @@ Enforced in code and by an independent CI guard step, not by convention:
   rosters, prompts, or routing is reviewed under the **previous** config, and a
   missing base fails closed — never a fallback to the checkout. (Temporary escape
   hatch: `ecr ci --unsafe-config-from-head`, with a loud warning; to be removed.)
+- **The review covers one commit, and can be pinned to the one you asked for.**
+  `ecr ci` reads the PR's base and head commits once. The tree it reads is checked
+  out at that head, and the diff compares those two commits, so a push during the
+  run cannot change either. `ecr ci --expect-head <sha>` also stops when the head is
+  not `<sha>`, so a workflow can pass the commit a maintainer labeled
+  (`github.event.pull_request.head.sha`) and a push before the run starts is not
+  reviewed in its place. With a `routing.jsonc`, the run only logs the reason,
+  since a note would replace the aggregate comment and its per-scope state.
+  Releases without the flag ignore it, so pin a release whose `ecr ci --help`
+  lists it.
 - **The model runtime never sees PR-owned ambient config.** The head worktree is
   scrubbed of `opencode.json{,c}`, `.opencode/`, `AGENTS.md`, `CLAUDE.md`,
   `.claude/`, `.mcp.json`, `.cursor*`, and `.env*` before the engine starts, so a

llp/0007-cli-commands-and-ci.explainer.md

@@ -77,6 +77,8 @@ publishing stale findings or rendering them under rules the model run never saw.
 
 **Automated reruns reuse unchanged complete results.** Before launching agents, CI hashes the review job's scoped diff and stable review inputs and compares it with the hash hidden in the newest author-verified reviewer comment. An exact match re-renders the stored result (so links, dismissals, and live annotation-only feedback still update) without spending model budget. Routed CI does this independently per scope. Manual `/review`/`--force`, stack-aware runs, model-backed feedback runs, partial/failed results, and truncated aggregate state all bypass reuse and run fresh. The hash contract and durable-state rules are owned by LLP 0005's Review Result Cache section.
 
+**`--expect-head` pins the run to the commit the request named.** A workflow on a label event knows the commit the maintainer labeled (`github.event.pull_request.head.sha`), but `ecr ci` starts minutes later and reads the PR's head then, so a push in between would be reviewed with the secrets in place of the labeled code. With `--expect-head <sha>`, both CI paths compare the head they read with `<sha>` after the break-glass check and before any model call, and stop when it differs or is unknown (`expectedHeadMismatch` in [ci.ts](../src/commands/ci.ts)). The single-config path posts a `couldNotComplete` "not reviewed" note (`postNotReviewed`). The routed path only logs and removes the reviewer's ready label: its root comment holds the aggregate and its per-scope state, which a single-review render would replace, so a later `--scopes` run would lose the other scopes' results. A malformed flag (`--expect-head=<sha>`, an empty value, a short hash, the flag twice) is a usage error, never ignored, since an ignored flag would review an unpinned head without a word (`parseExpectHead`). Releases from before the flag ignore it like any unknown argument, so a workflow that passes it must pin a release that has it. The diff compares the same base and head commits the trees are checked out at (see [LLP 0008](./0008-sources-and-reporters.explainer.md#pr-head-materialization)), so a push after the check cannot swap the diff either [observed] (tests `src/__tests__/github-pr-head.test.ts`).
+
 `--unsafe-config-from-head` is a deliberate escape hatch that reverses this model, loading config from the checkout. It is never scaffolded by `ecr init`, always prints a loud `⚠ SECURITY` warning, and is scheduled for removal on a minor version boundary [observed: `src/commands/ci.ts:85-91` `CI_USAGE`, `155-162` warning].
 
 `--context-file` in CI takes the opposite read-error policy from `ecr review`, and the split is dictated by the never-fail-checks invariant. `ci` reads the file once before any fan-out and, on a missing/oversized/unreadable path, WARNs to stderr and continues with no context — it never fails, because a broken Atlantis-provided plan file must not turn the PR's check red [observed: `src/commands/ci.ts` `--context-file` read → stderr warn, no `exitCode`, `contextText` stays undefined]. The same reader (`readContextFile`, capped at `MAX_CONTEXT_FILE_BYTES`) backs both commands; only the catch policy differs — fatal locally, warn in CI. The byte-bounded raw text threads through the shared `CiRunOptions` into both `runLegacyCi` and `runRoutedCi`, and from there into every per-scope `runReview` as `contextText` (char-capped at prompt build) [observed: `src/commands/ci.ts` `CiRunOptions.contextText`; `runReview` calls in `runLegacyCi` and the fan-out loop].

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

@@ -37,6 +37,8 @@ The checkout targets the specific OID, never `FETCH_HEAD`. `FETCH_HEAD` would wo
 
 **Token handling.** Git fetches APPEND a gh credential helper (`-c credential.helper=!gh auth git-credential`) rather than replacing the credential config [observed] ([github-pr.ts:26-32, 120](../src/sources/github-pr.ts)). Appending, not overriding, has two reasons: `GH_TOKEN` flows via the credential-helper protocol so it never lands in argv or `.git/config` (works even under `persist-credentials: false`), and a developer's own existing helpers still run first, so local behavior is unchanged [observed] ([github-pr.ts:26-30](../src/sources/github-pr.ts); commit 601b19a). Replacing it with a sole `credential.helper` override would break developer machines that rely on their own helpers.
 
+**The diff is pinned to the same OIDs.** `gh pr diff` has no commit argument and returns the PR's diff as it is now, so a push during the run would give the agents a diff from another commit than the tree they read. `getChangedFiles` instead fetches the compare of the base and head OIDs `getMetadata` read (`GET /repos/{owner}/{repo}/compare/{base}...{head}` with the `application/vnd.github.diff` media type). That is the PR's three-dot diff: on one same-repository PR and three fork PRs it matched `gh pr diff` byte for byte [observed: 2026-11-02]. Only when an OID is unknown, or the metadata read failed, does it fall back to `gh pr diff` [observed] ([github-pr.ts](../src/sources/github-pr.ts) `getChangedFiles`; tests `src/__tests__/github-pr-head.test.ts`). A push after the metadata read can still stop the run: the head tree fetches `refs/pull/<n>/head` at depth 1 and then checks out the OID, which fails when the runner does not already have that commit (see `materializeWorktreeAsync`).
+
 ## The Trusted Base Root
 
 `prepareTrustedConfigRootAsync` materializes the PR **BASE** commit — not the head — as a separate worktree [observed] ([github-pr.ts:194-210](../src/sources/github-pr.ts); commit 601b19a). This is the whole trust story of CI in one method. Review policy, prompts, routing, and auth mapping all load from the base tree, so a PR cannot change the reviewer that evaluates it — config changes only take effect after they merge into the base branch [observed] ([github-pr.ts:195-197](../src/sources/github-pr.ts); commit 601b19a: "loads all review configuration from the PR's immutable base commit, never from the PR head"). The base tree is trusted, so it is *not* scrubbed; the head tree is untrusted, so it is. Two worktrees, two trust levels, by construction.

src/__tests__/github-pr-head.test.ts

@@ -0,0 +1,113 @@
+import { test, expect, afterEach, beforeEach } from "bun:test";
+import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import path, { delimiter } from "node:path";
+
+import { expectedHeadMismatch, parseExpectHead } from "../commands/ci.js";
+import { resetTrustedToolCache } from "../core/exec.js";
+import { GitHubPRSource } from "../sources/github-pr.js";
+
+const HEAD = "0123456789abcdef0123456789abcdef01234567";
+const BASE = "89abcdef0123456789abcdef0123456789abcdef";
+
+test("parseExpectHead: a full commit hash, lowercased; anything else is a usage error", () => {
+  expect(parseExpectHead([])).toBeUndefined();
+  expect(parseExpectHead(["--expect-head", HEAD.toUpperCase()])).toBe(HEAD);
+  expect(() => parseExpectHead(["--expect-head", "0123456"])).toThrow("40-character");
+  expect(() => parseExpectHead(["--expect-head", "--force"])).toThrow("requires a value");
+  expect(() => parseExpectHead(["--expect-head"])).toThrow("requires a value");
+  // Forms that would otherwise be ignored, and leave the run unpinned, fail loudly.
+  expect(() => parseExpectHead([`--expect-head=${HEAD}`])).toThrow("separate argument");
+  expect(() => parseExpectHead(["--expect-head", ""])).toThrow("is empty");
+  expect(() => parseExpectHead(["--expect-head", HEAD, "--expect-head", ""])).toThrow(
+    "more than once",
+  );
+});
+
+test("expectedHeadMismatch: only a named commit that is not the head stops the run", () => {
+  expect(expectedHeadMismatch(undefined, HEAD)).toBeNull();
+  expect(expectedHeadMismatch(undefined, undefined)).toBeNull();
+  expect(expectedHeadMismatch(HEAD, HEAD)).toBeNull();
+  expect(expectedHeadMismatch(HEAD, BASE)).toBe(
+    `the PR's head is ${BASE}, not ${HEAD}, the commit this review was requested for`,
+  );
+  // An unknown head fails closed.
+  expect(expectedHeadMismatch(HEAD, undefined)).toContain("the PR's head is unknown");
+});
+
+// ---- the diff is pinned to the OIDs the metadata read, through a fake `gh` ----
+
+// A fake `gh` that logs each call. `pr view` prints FAKE_GH_VIEW (both OIDs when
+// unset) and fails when it is "fail"; `api` and `pr diff` print the same diff.
+const FAKE_GH = `#!/bin/sh
+printf '%s\\n' "$*" >> "$FAKE_GH_DIR/calls.log"
+case "$*" in
+  "api "*|"pr diff"*)
+    printf 'diff --git a/a.ts b/a.ts\\n'
+    printf 'index 1111111..2222222 100644\\n'
+    printf -- '--- a/a.ts\\n'
+    printf '+++ b/a.ts\\n'
+    printf '@@ -1 +1,2 @@\\n'
+    printf ' one\\n'
+    printf '+two\\n' ;;
+  "pr view"*)
+    [ "$FAKE_GH_VIEW" = fail ] && exit 1
+    printf '%s' "\${FAKE_GH_VIEW:-{\\"headRefOid\\": \\"${HEAD}\\", \\"baseRefOid\\": \\"${BASE}\\"}}" ;;
+  *) echo "unexpected gh call: $*" >&2; exit 1 ;;
+esac
+`;
+
+let dir = "";
+const saved: Record<string, string | undefined> = {};
+
+beforeEach(async () => {
+  dir = await mkdtemp(path.join(tmpdir(), "ecr-gh-pr-head-"));
+  await writeFile(path.join(dir, "gh"), FAKE_GH, "utf8");
+  await chmod(path.join(dir, "gh"), 0o755);
+  await writeFile(path.join(dir, "calls.log"), "", "utf8");
+  for (const name of ["PATH", "FAKE_GH_DIR", "FAKE_GH_VIEW"]) {
+    saved[name] = process.env[name];
+  }
+  process.env.PATH = `${dir}${delimiter}${saved.PATH ?? ""}`;
+  process.env.FAKE_GH_DIR = dir;
+  delete process.env.FAKE_GH_VIEW;
+  resetTrustedToolCache();
+});
+
+afterEach(async () => {
+  for (const [name, value] of Object.entries(saved)) {
+    if (value === undefined) {
+      delete process.env[name];
+    } else {
+      process.env[name] = value;
+    }
+  }
+  resetTrustedToolCache();
+  await rm(dir, { recursive: true, force: true });
+});
+
+/** Run getChangedFiles on a fresh source and return the gh call that fetched the diff. */
+async function diffCall(options: { repo?: string }): Promise<string> {
+  await writeFile(path.join(dir, "calls.log"), "", "utf8");
+  const files = await new GitHubPRSource({ prNumber: 7, ...options }).getChangedFiles();
+  expect(files.map((file) => file.path)).toEqual(["a.ts"]);
+  const calls = (await readFile(path.join(dir, "calls.log"), "utf8")).trim().split("\n");
+  expect(calls).toHaveLength(2);
+  expect(calls[0]).toStartWith("pr view");
+  return calls[1]!;
+}
+
+test("getChangedFiles: the diff compares the base and head OIDs the metadata read", async () => {
+  expect(await diffCall({ repo: "o/r" })).toBe(
+    `api repos/o/r/compare/${BASE}...${HEAD} -H Accept: application/vnd.github.diff`,
+  );
+  // Without --repo, gh resolves the placeholders from the checkout, as `gh pr` does.
+  expect(await diffCall({})).toStartWith(`api repos/{owner}/{repo}/compare/${BASE}...${HEAD} `);
+});
+
+test("getChangedFiles: an unknown OID or a failed metadata read falls back to gh pr diff", async () => {
+  process.env.FAKE_GH_VIEW = `{"baseRefOid": "${BASE}"}`;
+  expect(await diffCall({ repo: "o/r" })).toStartWith("pr diff 7 --repo o/r");
+  process.env.FAKE_GH_VIEW = "fail";
+  expect(await diffCall({ repo: "o/r" })).toStartWith("pr diff 7 --repo o/r");
+});

src/commands/ci.ts

@@ -36,7 +36,7 @@ import {
   reviewInputHash,
   reviewMatchesInput,
 } from "../core/review-cache.js";
-import { GitHubPRSource } from "../sources/github-pr.js";
+import { GitHubPRSource, isCommitOid } from "../sources/github-pr.js";
 import { memoizeSource, stackConfirmFromConfig, stackWalkFromConfig } from "../sources/source.js";
 import type { PreparedReadRoot, ReviewSource, StackWalkOptions } from "../sources/source.js";
 import { GitHubReporter } from "../reporters/github.js";
@@ -127,6 +127,10 @@ Options:
   --force              Manual override: review even if the trigger policy (label
                        trigger / ai-review:skip) would skip. Break-glass and the
                        auth lock still apply. A /review comment command implies this.
+  --expect-head <sha>  Review only when the PR's head is this full commit hash,
+                       for example \${{ github.event.pull_request.head.sha }} on
+                       a label event. Otherwise stop with a "not reviewed" note
+                       (logged only, with a routing.jsonc).
   -h, --help           Show this help
 
 Env: GITHUB_REPOSITORY, GITHUB_EVENT_PATH/GITHUB_REF (PR number), GH_TOKEN,
@@ -152,9 +156,11 @@ export async function ciCommand(argv: string[] = []): Promise<void> {
   // the default .expo-code-review/. Applies to config.jsonc AND routing.jsonc.
   let configDir: string | undefined;
   let contextFile: string | undefined;
+  let expectHead: string | undefined;
   try {
     configDir = parseValueFlag(argv, "--config-dir");
     contextFile = parseValueFlag(argv, "--context-file");
+    expectHead = parseExpectHead(argv);
   } catch (error) {
     process.stderr.write(`${errorMessage(error)}\n\n${CI_USAGE}`);
     process.exitCode = 2;
@@ -261,6 +267,7 @@ export async function ciCommand(argv: string[] = []): Promise<void> {
         configDir,
         contextText,
         noStackAware,
+        expectHead,
       });
       return;
     }
@@ -275,6 +282,7 @@ export async function ciCommand(argv: string[] = []): Promise<void> {
         configDir,
         contextText,
         noStackAware,
+        expectHead,
       });
     } catch (error) {
       // Fan-out failures stay non-blocking (single-writer property is the point).
@@ -485,6 +493,47 @@ interface CiRunOptions {
   contextText: string | undefined;
   /** --no-stack-aware: force stack-aware requalification off for this run. */
   noStackAware: boolean;
+  /** --expect-head: the full head commit hash this run may review, or undefined. */
+  expectHead?: string | undefined;
+}
+
+// @ref LLP 0007#ecr-ci-the-trusted-root-run [implements] — --expect-head pins the review to the commit the request named
+/**
+ * Why the run must stop when `--expect-head` names a commit that is not the PR's head
+ * (unknown counts as different, so a metadata failure fails closed), else null. The
+ * reason is safe to post.
+ */
+export function expectedHeadMismatch(
+  expectHead: string | undefined,
+  headOid: string | undefined,
+): string | null {
+  if (expectHead === undefined || headOid === expectHead) {
+    return null;
+  }
+  return (
+    `the PR's head is ${headOid ?? "unknown"}, not ${expectHead}, ` +
+    "the commit this review was requested for"
+  );
+}
+
+/** Post the "not reviewed" note for a head that is not the one this run may review. */
+async function postNotReviewed(reporter: GitHubReporter, reason: string): Promise<void> {
+  process.stderr.write(`CI reviewer: not reviewing: ${reason}.\n`);
+  try {
+    await reporter.report({
+      decision: "approve_with_comments",
+      findings: [],
+      summary: `⚠️ This change was **not** reviewed: ${reason}.`,
+      incomplete: [],
+      // Not a review of anything: keeps the inline sync additive-only and removes
+      // the reviewer's ready label.
+      couldNotComplete: true,
+    });
+  } catch (postError) {
+    process.stderr.write(
+      `CI reviewer: also failed to post the not-reviewed note: ${errorMessage(postError)}\n`,
+    );
+  }
 }
 
 // @ref LLP 0010#config-and-cli-surface [implements] — under ci, stack-aware is gated by the trusted-base config; a stack walk failure only ever warns (source fails open), never a check failure
@@ -604,6 +653,11 @@ async function runLegacyCi(
   if (await isNewCommitsEvent()) {
     await reporter.removeReadyLabel();
   }
+  const headProblem = expectedHeadMismatch(options.expectHead, headSha);
+  if (headProblem) {
+    await postNotReviewed(reporter, headProblem);
+    return;
+  }
 
   const stack = resolveStackWalk(config.stack, noStackAware);
   const stackConfirm = resolveStackConfirm(config.stack, noStackAware);
@@ -818,6 +872,44 @@ async function runRoutedCi(
     return;
   }
 
+  // Break-glass: ONE check via a reporter on the root marker, before fan-out.
+  const bgReporter = new GitHubReporter({
+    prNumber,
+    repo,
+    commentTag: rootTag,
+    breakGlassMarker: rootConfig.breakGlassMarker,
+    cwd,
+    readyForHumanLabel: rootConfig.review.readyForHumanLabel,
+  });
+  try {
+    if (await bgReporter.checkBreakGlass()) {
+      process.stderr.write(`CI reviewer: ${rootConfig.breakGlassMarker} detected; skipping.\n`);
+      await bgReporter.postSkipNote();
+      return;
+    }
+  } catch (error) {
+    process.stderr.write(
+      `CI reviewer: break-glass check failed (continuing): ${errorMessage(error)}\n`,
+    );
+  }
+  // The ready label described the previous head; the review adds it back if this one
+  // is ready.
+  if (await isNewCommitsEvent()) {
+    await bgReporter.removeReadyLabel();
+  }
+
+  // In the routed path the root comment holds the aggregate and its per-scope state,
+  // which a single "not reviewed" render would replace, so a head that is not the one
+  // this run may review only logs, and the reviewer's ready label goes.
+  const headProblem = expectedHeadMismatch(
+    options.expectHead,
+    (await source.getMetadata().catch(() => undefined))?.headOid,
+  );
+  if (headProblem) {
+    process.stderr.write(`CI reviewer: not reviewing: ${headProblem}.\n`);
+    await bgReporter.removeReadyLabel();
+    return;
+  }
   const changed = await source.getChangedFiles();
   const resolution = resolveScopes(
     manifest,
@@ -852,32 +944,6 @@ async function runRoutedCi(
 
   const mode = commentOverride ?? manifest.comment;
 
-  // Break-glass: ONE check via a reporter on the root marker, before fan-out.
-  const bgReporter = new GitHubReporter({
-    prNumber,
-    repo,
-    commentTag: rootTag,
-    breakGlassMarker: rootConfig.breakGlassMarker,
-    cwd,
-    readyForHumanLabel: rootConfig.review.readyForHumanLabel,
-  });
-  try {
-    if (await bgReporter.checkBreakGlass()) {
-      process.stderr.write(`CI reviewer: ${rootConfig.breakGlassMarker} detected; skipping.\n`);
-      await bgReporter.postSkipNote();
-      return;
-    }
-  } catch (error) {
-    process.stderr.write(
-      `CI reviewer: break-glass check failed (continuing): ${errorMessage(error)}\n`,
-    );
-  }
-  // The ready label described the previous head; the review adds it back if this one
-  // is ready.
-  if (await isNewCommitsEvent()) {
-    await bgReporter.removeReadyLabel();
-  }
-
   // Build ONE link context for all scopes (rate-limit hygiene): diff lines from the
   // already-fetched changed files, base OID from the memoized PR metadata (the same
   // immutable OID the trusted config root was materialized from).
@@ -1384,6 +1450,33 @@ function parseListFlag(argv: string[], flag: string): string[] | undefined {
     .filter(Boolean);
 }
 
+/**
+ * Parse `--expect-head <sha>`: a full commit hash, lowercased, or undefined. Every
+ * malformed form throws instead of being ignored, because an ignored flag would
+ * review an unpinned head without a word.
+ */
+export function parseExpectHead(argv: string[]): string | undefined {
+  if (argv.some((arg) => arg.startsWith("--expect-head="))) {
+    throw new Error("--expect-head takes its value as a separate argument: --expect-head <sha>");
+  }
+  if (argv.filter((arg) => arg === "--expect-head").length > 1) {
+    throw new Error("--expect-head was given more than once");
+  }
+  const value = parseValueFlag(argv, "--expect-head");
+  if (value === undefined) {
+    return undefined;
+  }
+  if (value === "") {
+    throw new Error(
+      "--expect-head is empty; the workflow likely passed a PR head from an event that has none",
+    );
+  }
+  if (!isCommitOid(value)) {
+    throw new Error(`--expect-head needs a full 40-character commit hash, got "${value}"`);
+  }
+  return value.toLowerCase();
+}
+
 /** Parse a single-value flag (`--flag value`); undefined when absent, throws when
  * present without a value (matching review.ts's requireValue). */
 function parseValueFlag(argv: string[], flag: string): string | undefined {

src/sources/github-pr.ts

@@ -223,16 +223,31 @@ export class GitHubPRSource implements ReviewSource {
     };
   }
 
+  // @ref LLP 0008#pr-head-materialization [implements] — the diff is pinned to the same base and head OIDs as the trees
+  /**
+   * The PR's diff between the base and head OIDs that getMetadata() read, the same
+   * commits the trusted config root and the head tree are checked out at. `gh pr diff`
+   * takes no commit and returns the PR as it is now, so it is used only when an OID
+   * is unknown.
+   */
   async getChangedFiles(): Promise<DiffEntry[]> {
+    const metadata = await this.getMetadata().catch(() => undefined);
+    const base = metadata?.baseOid;
+    const head = metadata?.headOid;
     const gh = await resolveTrustedTool("gh");
-    // gh colors the diff even into a pipe when the environment forces it
-    // (CLICOLOR_FORCE, GH_FORCE_TTY), and the escape codes before `diff --git` would
-    // make the whole PR parse as no files.
-    const { stdout } = await run(
-      gh,
-      ["pr", "diff", String(this.options.prNumber), ...this.repoArgs(), "--color", "never"],
-      { cwd: this.options.cwd },
-    );
+    const args =
+      isCommitOid(base) && isCommitOid(head)
+        ? [
+            "api",
+            `repos/${this.options.repo ?? "{owner}/{repo}"}/compare/${base}...${head}`,
+            "-H",
+            "Accept: application/vnd.github.diff",
+          ]
+        : // gh colors the diff even into a pipe when the environment forces it
+          // (CLICOLOR_FORCE, GH_FORCE_TTY), and the escape codes before `diff --git`
+          // would make the whole PR parse as no files.
+          ["pr", "diff", String(this.options.prNumber), ...this.repoArgs(), "--color", "never"];
+    const { stdout } = await run(gh, args, { cwd: this.options.cwd });
     return parseUnifiedDiff(stdout);
   }