From d23bc1970ff6d2e33498029cb4f7f67e73785664 Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sat, 29 Aug 2026 21:11:26 -0700
Subject: [PATCH 01/12] Add a Local Review Engine That Records What a Review
 Covered

The fleet's rule that PR-bound work gets a local adversarial review before every
push is already written in three Skills, including the fix-push moment, and is
still not followed (#1104). It is not a wording gap, so this adds the capability
a capture point can actually check: a review pass records a receipt keyed on the
content it reviewed, and a later caller asks whether that receipt still covers
what is about to be pushed.

The key is over the net content the branch introduces against its target, never
over diff text or HEAD, which is what lets a review run before the commit while
the check runs at the push. Reviewing untracked work and committing it unchanged
leaves the key identical; changing one byte moves it.

Every identity in the key is computed by git rather than reconstructed. That is
the load-bearing detail: a blob id built by hashing raw working-tree bytes cannot
equal the one git stores wherever a text attribute or clean filter sits between
them, and this repository applies one to every text file, so `git add` moved the
key on any CRLF file. So the working tree's side is read by staging it into a
throwaway index that git builds, with object writes redirected into a throwaway
object directory and the real one attached as an alternate. Without that
redirection a read would deposit the content of every unignored untracked file
into the repository permanently.

The engine holds no review logic. It drives backends: agent-skill is the
local-strict-review subagent pass, which only a live session can run, so the
engine records it; coderabbit-cli is headless and is the one backend a git hook
can execute by itself. A pass records that a review ran over this content, never
that the content is clean.

Exit codes are three-valued, so an execution boundary is never reported as a
verdict. The capture points that will call this, a git pre-push hook and the
Skill steps, are a separate change.

Part of #1104 and #1083. The capture points that close them come next.
---
 scripts/README.md                  |  27 +-
 scripts/local_review.py            | 724 +++++++++++++++++++++++++++++
 scripts/tests/test_local_review.py | 688 +++++++++++++++++++++++++++
 3 files changed, 1437 insertions(+), 2 deletions(-)
 create mode 100755 scripts/local_review.py
 create mode 100755 scripts/tests/test_local_review.py

diff --git a/scripts/README.md b/scripts/README.md
index 3941be23..56f2b4cf 100644
--- a/scripts/README.md
+++ b/scripts/README.md
@@ -4,14 +4,15 @@ The fleet's checks and review tooling, run by hand, with the deterministic ones
 
 **Hosted here and reached, never carried.** These are not declared in [`spec/files.json`][files], so the audit does not expect a downstream repo to ship them, the same footing as `spec/audit.py`. That is the fleet model rather than an omission: a gate holding no per-repo content has one canonical implementation for the fleet. The prose and repository gate implementations live beside their composite actions under `.github/actions/`, which lets reusable workflows reach the implementation at their pinned hub commit through `$/.github/actions/` without checking out the hub. The same-named files in `scripts/` are thin local entry points that call those action-owned implementations, so maintainers and hooks retain the established commands from a hub checkout without creating a second copy of either gate. A repository that cannot reach the hub reports a local check as not run rather than reconstructing it, since a rebuilt gate encodes its author's reading of the rule and agrees with no other repository.
 
-Python only, standard library only, no third-party packages. Every check script is read-only and exits non-zero on a finding. `build_dist.py` and `skills_install.py` below are the two exceptions, since a generator and an installer both exist to write, and each still offers a read-only mode (`--check`, `--report`) for CI and for asking without changing anything.
+Python only, standard library only, no third-party packages. Every check script is read-only and exits non-zero on a finding. `build_dist.py`, `local_review.py`, and `skills_install.py` below are the exceptions, since a generator, a recorder, and an installer all exist to write, and each still offers a read-only mode (`--check`, `status`, `--report`) for CI and for asking without changing anything.
 
-The directory separates its kinds by name and by tree. A gate checks and exits non-zero on a finding, and its name carries a `_lint` or `_gate` suffix saying what it gates. The `prose_lint.py` and `repo_gate.py` entry points delegate to the action-owned implementations that gate this tree in CI, and `host_gate.py` gates the machine it runs on. A utility does work rather than gating and carries no suffix: `build_dist.py`, `pr_review.py`, `skills_install.py`. The unit tests live apart under [`scripts/tests/`][tests], one `test_<script>.py` per script, driving the canonical implementations against input they must reject. A gate nobody has watched fail is a gate nobody knows works. Where a case covers a table it reads the live table rather than restating it, and each one asserts a floor on what a healthy run reaches, since a check whose scan matches nothing reports zero findings and reads exactly like a pass.
+The directory separates its kinds by name and by tree. A gate checks and exits non-zero on a finding, and its name carries a `_lint` or `_gate` suffix saying what it gates. The `prose_lint.py` and `repo_gate.py` entry points delegate to the action-owned implementations that gate this tree in CI, and `host_gate.py` gates the machine it runs on. A utility does work rather than gating and carries no suffix: `build_dist.py`, `local_review.py`, `pr_review.py`, `skills_install.py`. The unit tests live apart under [`scripts/tests/`][tests], one `test_<script>.py` per script, driving the canonical implementations against input they must reject. A gate nobody has watched fail is a gate nobody knows works. Where a case covers a table it reads the live table rather than restating it, and each one asserts a floor on what a healthy run reaches, since a check whose scan matches nothing reports zero findings and reads exactly like a pass.
 
 ```sh
 python3 scripts/tests/test_prose_lint.py
 python3 scripts/tests/test_repo_gate.py
 python3 scripts/tests/test_pr_review.py
+python3 scripts/tests/test_local_review.py
 python3 scripts/tests/test_build_dist.py
 python3 scripts/tests/test_skills_install.py
 python3 -m unittest discover -s scripts/tests    # all of them, and exits 5 if the suite vanishes
@@ -217,6 +218,27 @@ Undecided is a third answer here for the reason it is one in `repo_gate.py`. A r
 
 The match is on the block's heading rather than anywhere in the body, and on the runbook's alternation rather than on one phrasing, since the wording has already appeared two ways. A case asserts the script's pattern is the one the runbook publishes rather than a copy of it that can drift. Reading the whole body was the first implementation and its own review caught it: a review whose overview prose discusses suppressed findings carries none, and reporting that as a finding trains the reader to skim the field. A heading outside any `<details>` wrapper is still read, because reporting zero when the markup moves is the same false clean one level up, and that fallback takes a count so ordinary prose does not become one.
 
+## `local_review.py`
+
+Records and verifies that a local review pass covered a branch's current content, so the fleet's review-before-every-push rule has something a capture point can actually check. That rule was already written in three Skills and still went unfollowed, which is the gap this closes: a pass records a receipt keyed on the content it reviewed, and a capture point asks whether that receipt still covers what is about to be pushed. The capture points themselves (a git pre-push hook, the Skill steps that invoke the pass) are a separate change, so this script is the capability they will call rather than a gate that is already wired up.
+
+The key is over the content of the changed files, plus the merge-base commit that fixes what "changed" means. It is deliberately not over the diff text and not over `HEAD`, which is what lets a review run before the commit while the check runs at the push: reviewing untracked work and then committing it unchanged leaves the key identical. Each path contributes the state it is in on each side, the working tree's and the index's, both as git's own mode-and-blob identity so the two compare directly. Where they agree the path reads as one state, which is what makes `git add` and `git commit` invisible. Where they diverge, because content is staged that the working tree no longer holds, or because a file mode changed, the key moves. Reading the index rather than the working tree alone is what stops content being committed that the key never saw.
+
+Every identity is computed by git rather than reconstructed, which matters more than it sounds: a blob id built by hashing raw working-tree bytes cannot equal the one git stores wherever a `text` attribute or any clean filter sits between them, and this repository applies one to every text file, so `git add` moved the key on any CRLF file until that was fixed. The working tree's side is therefore read by staging it into a throwaway index that git builds, with its object writes redirected into a throwaway object directory and the real one attached as an alternate. That redirection is not incidental: staging writes blobs, so without it a read would permanently deposit the content of every unignored untracked file, a stray secret among them, into the repository. Nothing about the real index, the working tree, or the object database is modified. One requirement this places on a repository is that a `clean` filter be deterministic, since the same content is staged once when a pass is recorded and again when it is checked.
+
+What the key covers is the net content the branch introduces against its target, not the commit series that produced it. A branch that adds a file and later deletes it, or a rebase that rewrites only messages, keeps its key. That is the intended scope, since a feature branch lands as a squash and the net diff is what a reviewer reads, but a capture point built on this does not cover the commit series. The receipt lives in the worktree's own git directory rather than in the working tree, so it is per-task, can never be committed by accident, and needs no `.gitignore` entry of its own.
+
+It holds no review logic. It drives backends: `agent-skill` is the `local-strict-review` subagent pass, which only a live agent session can run, so the engine records it rather than invoking it, and `coderabbit-cli` is headless and is the one backend a git hook can execute by itself. A CodeRabbit CLI run draws on the same hourly budget as this account's pull request reviews, so a run that is rate limited, errors, or ends without a completion event records no pass rather than a clean one. A recorded pass means a review ran over exactly this content, never that the content is clean: disposing of what a review found is judgment, per the `pr-review-conduct` Skill's five outcomes, so a pass is recorded whether it raised findings or none.
+
+```shell
+python3 scripts/local_review.py status                                      # what covers the current content. 0 either way, 2 if it cannot run
+python3 scripts/local_review.py record --reviewer agent-skill --findings 3
+python3 scripts/local_review.py check                                       # 0 covered, 1 not, 2 could not run
+python3 scripts/local_review.py run --backend coderabbit-cli
+```
+
+The exit code is three-valued on purpose, per [`AGENTS.md`][agents] "Report an execution boundary separately from a check finding". `0` and `1` are findings a caller acts on, and `2` means the check itself never ran, whether because no git repository resolved, the target ref is absent from this checkout, the receipt could not be read off disk, or a backend binary is not installed. Any unexpected failure reports `2` as well, rather than falling through to the interpreter's own exit `1`, which a caller would read as the "not covered" verdict. `status` reports rather than gating, so it exits `0` whether or not the content is covered, and only reports `2` where it could not run at all.
+
 ## `build_dist.py`
 
 Regenerates [`.github/skills/`][github-skills-dist] and [`.claude-plugin/fleet-skills/`][fleet-skills-dist] from [`.agents/skills/`][agents-skills], the hub's own hand-authored fleet Skills. Codex and opencode read `.agents/skills/` directly, GitHub Copilot reads `.github/skills/`, and Claude Code reads the generated plugin published through [`.claude-plugin/marketplace.json`][marketplace]. `.agents/skills/` stays the one place a skill is hand-edited. Both generated trees are never hand-edited.
@@ -244,6 +266,7 @@ Installs the fleet's Skills for the current machine, cross-platform and idempote
 
 [agent-safety-install]: ../host-setup/agent-safety/claude/install.py
 [agents-skills]: ../.agents/skills/README.md
+[agents]: ../AGENTS.md
 [audit]: ../spec/audit.py
 [copilot-instructions]: ../.github/copilot-instructions.md
 [editorconfig]: ../.editorconfig
diff --git a/scripts/local_review.py b/scripts/local_review.py
new file mode 100755
index 00000000..74c25bd0
--- /dev/null
+++ b/scripts/local_review.py
@@ -0,0 +1,724 @@
+#!/usr/bin/env python3
+"""Record and verify that a local review pass covered this branch's current content.
+
+The fleet's rule is that PR-bound work gets an adversarial local review before it is pushed, and
+again before every fix push, so a PR-hosted reviewer is not used as the first reviewer. That rule
+was already written in three skills and still went unfollowed, so this script gives it something
+mechanically checkable: a review pass records a receipt keyed on the content it reviewed, and a
+capture point (a git pre-push hook, a skill step, a session about to claim done) asks whether a
+receipt still covers what is about to be pushed.
+
+The key is over the content of the changed paths, plus the merge-base commit that fixes what
+"changed" means. It is deliberately not over the diff text and not over HEAD, which is what lets a
+review run before the commit while the check runs at the push: reviewing untracked work and then
+committing it unchanged leaves the key identical, while changing one byte moves it.
+
+Every identity in the key is computed by git rather than reconstructed here, which is the point
+worth stating plainly, because reconstructing one is where this went wrong twice. A blob id read
+off the working tree by hashing its raw bytes does not equal the id git stores whenever a filter
+sits between them, and this repository's own `.gitattributes` applies one to every text file, so
+`git add` moved the key on any CRLF file. A file mode derived from `stat` disagrees with git's on
+any checkout where `core.fileMode` is off. So the working tree's own state is read by staging it
+into a throwaway index that git builds, and the result is compared against the real index that git
+already maintains. Both sides then carry git's own answer, filters, modes, renames, deletions,
+symlinks, and submodules included.
+
+Each path contributes the set of distinct states it is in, the throwaway index's and the real
+one's. When they agree the set collapses to one member, which is what makes `git add` and
+`git commit` invisible to the key. When they diverge, because content is staged that the working
+tree no longer holds, or because a mode changed, the set has more than one member and the key
+moves. Reading the real index rather than the working tree alone is what stops content being
+committed that the key never saw.
+
+What the key covers is the net content this branch introduces against its target, not the series
+of commits that produced it, and the difference is worth stating plainly because two consequences
+follow. An interactive rebase or an amend that leaves the tree byte-identical, changing only commit
+messages or authorship, keeps the same key. So does a branch that adds a file in one commit and
+deletes it in a later one: the commits carry content, the net result carries none, and the key
+follows the net result.
+
+That is the intended scope rather than a gap. A feature branch reaches `develop` as a squash and
+`develop` reaches `main` as a merge, so the net diff is what actually lands, and it is what a
+reviewer reads. Commit messages are governed by `git-commit-conventions` rather than by a content
+review. But a capture point built on this must not be described as covering the commit series, and
+a reviewer who needs to see intermediate churn has to read the range itself.
+
+Two known cases move the key though nothing that would be pushed changed, both erring toward
+demanding another review rather than skipping one. An intent-to-add entry (`git add -N`, which
+some tooling issues implicitly) sits in the real index as the empty blob while the throwaway index
+holds the real content. And a new untracked file that `.gitignore` does not cover enters the
+changed set, since a file not yet added is exactly what a review has to read.
+
+This script holds no review logic. It drives backends: `agent-skill` is the `local-strict-review`
+subagent pass, which only a live agent session can run, so the engine records it rather than
+invoking it; `coderabbit-cli` is headless and is the one backend a git hook can execute by itself.
+
+A recorded pass means a review ran over exactly this content. It never means the content is clean.
+Disposing of what a review found is judgment, per `pr-review-conduct`'s five outcomes, so a pass is
+recorded whether the pass raised findings or none.
+
+Usage: python3 scripts/local_review.py status [--target <branch>]
+           what the current content digest is, and which backends hold a pass on it.
+       python3 scripts/local_review.py record --reviewer <id> [--target <branch>] [--findings N]
+           record that <id> reviewed the current content.
+       python3 scripts/local_review.py check [--target <branch>]
+           exit 0 covered, 1 not covered, 2 could not run.
+       python3 scripts/local_review.py run --backend coderabbit-cli [--target <branch>]
+           execute a headless backend and record its pass.
+
+Exit codes are three-valued on purpose, per AGENTS.md "Report an execution boundary separately
+from a check finding". 0 and 1 are findings the caller acts on. 2 means the check itself did not
+run (no git repository, an unresolvable target ref, a receipt that cannot be read off disk, a
+missing backend binary), which a gate must not silently read as either answer. Every unexpected
+failure reports 2 as well, since a crash is the check not having run rather than a verdict of not
+covered. `status` reports rather than gating, so it is 0 whether or not the content is covered,
+and 2 only when it could not run at all.
+"""
+
+from __future__ import annotations
+
+import argparse
+import hashlib
+import json
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
+from datetime import UTC, datetime
+from pathlib import Path
+
+RECEIPT_VERSION = 1
+RECEIPT_NAME = "local-review-receipt.json"
+# Kept in the git directory rather than the working tree, so staging the tree does not stage these.
+STAGING_INDEX_PREFIX = "local-review-index."
+STAGING_OBJECTS_PREFIX = "local-review-objects."
+DEFAULT_TARGET = "develop"
+
+EXIT_COVERED = 0
+EXIT_NOT_COVERED = 1
+EXIT_CANNOT_RUN = 2
+
+# Bounded so a hung git or CLI fails the capture point loudly instead of hanging a push forever.
+# Staging the whole working tree is the slowest call here, which is why the git bound is generous.
+GIT_TIMEOUT = 300
+BACKEND_TIMEOUT = 900
+
+# The backends the engine knows, and whether it can execute one itself.
+# An agent-driven backend is recorded by the session that ran it, since no script spawns an agent.
+# A headless backend is one a hook can run with no session attached.
+BACKENDS = {
+    "agent-skill": {"headless": False, "why": "the local-strict-review subagent pass"},
+    "coderabbit-cli": {"headless": True, "why": "coderabbit review --agent, structured JSON"},
+}
+
+
+class CannotRun(Exception):
+    """The check could not be performed, as distinct from being performed and failing.
+
+    Raised for an execution boundary the caller must not read as a verdict: no git repository, a
+    target ref that does not resolve, a receipt that cannot be read off disk, a backend binary
+    that is not installed.
+    """
+
+
+# Cleared from every call's environment unless that call sets them itself.
+# A git hook runs with GIT_INDEX_FILE pointing at the commit it is gating, measured on a real pre-commit hook.
+# A partial commit points it at a pending index rather than the real one.
+# Inheriting that would silently read a different index than the one this branch would push.
+INHERITED_REDIRECTS = ("GIT_INDEX_FILE", "GIT_OBJECT_DIRECTORY", "GIT_ALTERNATE_OBJECT_DIRECTORIES")
+
+
+def git(*args: str, root: Path | None = None, env: dict[str, str] | None = None) -> str:
+    """Run a git command from `root` and return its stdout, raising CannotRun when git fails.
+
+    Every call names its working directory rather than inheriting the process's own, because git
+    reports paths relative to different places depending on the subcommand, so running two of them
+    from different directories silently keys a path under a name that does not resolve.
+
+    Decoding is UTF-8 with surrogateescape rather than the locale's encoding, so a path holding a
+    non-ASCII byte round-trips to the same name on disk instead of decoding into a name that
+    misses. `core.quotePath=false` stops git escaping those bytes before they are ever decoded.
+    """
+    run_env = {k: v for k, v in os.environ.items() if k not in INHERITED_REDIRECTS}
+    run_env.update(env or {})
+    try:
+        proc = subprocess.run(
+            ["git", "-c", "core.quotePath=false", *args],
+            cwd=str(root) if root else None,
+            env=run_env,
+            capture_output=True,
+            check=False,
+            timeout=GIT_TIMEOUT,
+            encoding="utf-8",
+            errors="surrogateescape",
+        )
+    except (OSError, subprocess.SubprocessError) as e:
+        raise CannotRun(f"git {' '.join(args)} could not be run ({e})") from e
+    if proc.returncode != 0:
+        raise CannotRun(f"git {' '.join(args)} failed: {proc.stderr.strip()}")
+    return proc.stdout
+
+
+def repo_root() -> Path:
+    """The working tree this invocation belongs to."""
+    return Path(git("rev-parse", "--show-toplevel").strip())
+
+
+def git_dir(root: Path) -> Path:
+    """This worktree's own administrative directory.
+
+    `--absolute-git-dir` resolves per worktree for a linked worktree, so two tasks in two worktrees
+    of one repository never share a receipt or a staging index.
+    """
+    return Path(git("rev-parse", "--absolute-git-dir", root=root).strip())
+
+
+def receipt_path(root: Path) -> Path:
+    """Where this worktree's receipt lives.
+
+    Inside the git directory rather than the working tree, so the receipt can never be committed
+    by accident, is never picked up as untracked content, and needs no .gitignore entry.
+    """
+    return git_dir(root) / RECEIPT_NAME
+
+
+def target_ref(target: str) -> str:
+    """The remote-tracking ref a target name means.
+
+    A bare name is the ordinary fleet case and resolves under `origin`. A value already carrying a
+    slash is used as written, so a fork-based flow can name an upstream remote's branch rather
+    than being told the engine understands only one remote.
+    """
+    return target if "/" in target else f"origin/{target}"
+
+
+def merge_base(target: str, root: Path) -> str:
+    """The fork point this branch's diff is measured from.
+
+    A merge-base is a fork point rather than a tip, so fetching a newer target does not move it.
+    """
+    ref = target_ref(target)
+    try:
+        git("rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}", root=root)
+    except CannotRun as e:
+        raise CannotRun(
+            f"{ref} does not resolve in this checkout, so the review scope cannot be determined"
+        ) from e
+    return git("merge-base", ref, "HEAD", root=root).strip()
+
+
+def _zsplit(out: str) -> list[str]:
+    """The non-empty records of a NUL-delimited git output."""
+    return [f for f in out.split("\0") if f]
+
+
+def _ls_files_states(out: str) -> dict[str, set[str]]:
+    """Parse `ls-files -s` into path to the set of mode-and-blob states it holds.
+
+    An unmerged path emits one record per stage, so the states are collected into a set rather
+    than assigned. Keeping only the last record would silently reduce a conflicted index to one
+    side of the conflict, and a pass recorded mid-conflict would then cover the resolution.
+
+    The stage number is part of the state whenever it is not the ordinary zero. This is defense
+    rather than a fix for a demonstrated case: a review argued that two stages of one conflict can
+    carry an identical mode and object, which dropping the stage would collapse into a state
+    reading as no conflict, but no construction produced it. Where our side's content equals the
+    base, git records no modification and applies the other side's delete cleanly rather than
+    conflicting, checked against a delete opposite an untouched file, an identical rewrite, and a
+    mode-only change. The stage costs nothing to carry, so it stays.
+    """
+    states: dict[str, set[str]] = {}
+    for record in _zsplit(out):
+        meta, _, path = record.partition("\t")
+        fields = meta.split()
+        if len(fields) >= 3 and path:
+            stage = f"{fields[2]}:" if fields[2] != "0" else ""
+            states.setdefault(path, set()).add(f"{stage}{fields[0]}:{fields[1]}")
+    return states
+
+
+def base_states(base: str, root: Path) -> dict[str, str]:
+    """Every path in the merge-base commit, as its mode-and-blob."""
+    states = {}
+    for record in _zsplit(git("ls-tree", "-r", "-z", base, root=root)):
+        meta, _, path = record.partition("\t")
+        fields = meta.split()
+        if len(fields) >= 3 and path:
+            states[path] = f"{fields[0]}:{fields[2]}"
+    return states
+
+
+def index_states(root: Path) -> dict[str, set[str]]:
+    """Every path in the real index, with every stage it holds.
+
+    The whole index is listed rather than a path list, which also keeps the call clear of the
+    argument-length ceiling a branch touching tens of thousands of paths would otherwise hit.
+    """
+    return _ls_files_states(git("ls-files", "-s", "-z", root=root))
+
+
+def worktree_states(root: Path) -> dict[str, set[str]]:
+    """Every path as git would stage it right now, read by actually staging it.
+
+    A throwaway index is seeded from the real one and `git add -A` is run against that, so git
+    computes each blob and mode exactly as a real `git add` would, with every filter, attribute,
+    and `core.fileMode` setting applied. Seeding from the real index rather than starting empty
+    keeps git's cached stat information, so the scan is a refresh rather than a full re-hash.
+
+    Staging writes objects, and those writes are redirected into a throwaway object directory with
+    the real one as an alternate, so git can still read every existing object while writing none
+    of its own. Without that redirection this read would permanently deposit the content of every
+    untracked file into the repository, an unignored `.env` or key file among them, recoverable
+    long afterwards from a command the caller had every reason to believe only looked.
+
+    Both throwaway paths carry a unique name and live in the git directory. A fixed name would let
+    two invocations in one worktree delete each other's index mid-read, and a name under the
+    working tree would be staged as untracked content by the very command being run.
+
+    One requirement this places on the repository: a `clean` filter has to be deterministic, since
+    the same content is staged once when a pass is recorded and again when it is checked. A filter
+    embedding a timestamp would move the key between two invocations that changed nothing.
+    """
+    gd = git_dir(root)
+    real = gd / "index"
+    fd, staging = tempfile.mkstemp(dir=str(gd), prefix=STAGING_INDEX_PREFIX)
+    os.close(fd)
+    objects = tempfile.mkdtemp(dir=str(gd), prefix=STAGING_OBJECTS_PREFIX)
+    try:
+        if real.is_file():
+            shutil.copyfile(real, staging)
+        env = {
+            "GIT_INDEX_FILE": staging,
+            "GIT_OBJECT_DIRECTORY": objects,
+            "GIT_ALTERNATE_OBJECT_DIRECTORIES": str(gd / "objects"),
+        }
+        git("add", "-A", "--", ".", root=root, env=env)
+        return _ls_files_states(git("ls-files", "-s", "-z", root=root, env=env))
+    except OSError as e:
+        raise CannotRun(f"the staging index could not be prepared ({e})") from e
+    finally:
+        Path(staging).unlink(missing_ok=True)
+        Path(staging + ".lock").unlink(missing_ok=True)
+        shutil.rmtree(objects, ignore_errors=True)
+
+
+def state_mark(index: set[str], work: set[str]) -> str:
+    """One path's state, with the working tree's side named separately from the index's.
+
+    A missing index entry contributes nothing rather than a state of its own, which is what keeps
+    `git add` invisible to the key: an untracked file, and that same file staged unchanged, both
+    read as the working tree's state alone.
+
+    The index is named only where it actually disagrees with the working tree. Naming it that way
+    rather than merging both sides into one set is what tells staging A over a working tree of B
+    apart from the reverse, which a plain union renders identically while `git commit` pushes a
+    different one of the two.
+    """
+    mark = "w:" + ",".join(sorted(work)) if work else "w:absent"
+    if index and index != work:
+        mark += "|i:" + ",".join(sorted(index))
+    return mark
+
+
+def fingerprints(base: str, root: Path) -> dict[str, str]:
+    """Every changed path and the state it is in.
+
+    A path is changed when either side, the real index or the staged working tree, disagrees with
+    the merge base. Comparing states rather than reading a diff listing means renames, deletions,
+    mode changes, and submodule bumps are all just a state that differs, with no per-case handling
+    and no rename detection to disable.
+    """
+    at_base = base_states(base, root)
+    at_index = index_states(root)
+    at_work = worktree_states(root)
+    out = {}
+    for path in {*at_base, *at_index, *at_work}:
+        base_state = {at_base[path]} if path in at_base else set()
+        index = at_index.get(path, set())
+        work = at_work.get(path, set())
+        if index == work == base_state:
+            continue
+        out[path] = state_mark(index, work)
+    return out
+
+
+def content_digest(base: str, marks: dict[str, str]) -> str:
+    """One digest over the merge base and every changed path's state.
+
+    Paths are hashed in sorted order and a NUL follows every field, so two different path sets
+    cannot collide by concatenating to the same string. The digest is derived from the repository
+    on every call and never read back from the receipt, which is what makes a hand-edited receipt
+    report stale rather than current.
+    """
+    h = hashlib.sha256()
+    h.update(f"v{RECEIPT_VERSION}\0{base}\0".encode())
+    for path in sorted(marks):
+        h.update(path.encode("utf-8", "surrogateescape"))
+        h.update(b"\0")
+        h.update(marks[path].encode("utf-8"))
+        h.update(b"\0")
+    return h.hexdigest()
+
+
+# Checked before a receipt is read, so a hand-edited file gives a verdict rather than a traceback.
+# Shape rather than presence, because a partial write leaves keys missing.
+# A hand edit instead leaves a key holding the wrong type, which a key-only check passes and then crashes on.
+RECEIPT_SHAPE = {
+    "receiptVersion": int,
+    "target": str,
+    "mergeBase": str,
+    "contentDigest": str,
+    "passes": list,
+}
+
+
+def receipt_problems(receipt: object) -> list[str]:
+    """What makes this receipt unusable, in reading order, or an empty list where it is fine."""
+    if not isinstance(receipt, dict):
+        return [f"its root is {type(receipt).__name__} where an object is required"]
+    out = []
+    for key, want in RECEIPT_SHAPE.items():
+        if key not in receipt:
+            out.append(f"{key} is missing")
+        elif not isinstance(receipt[key], want):
+            out.append(f"{key} is {type(receipt[key]).__name__} where {want.__name__} is required")
+    version = receipt.get("receiptVersion")
+    # The version carries the format rather than the content, so a mismatch either way is unreadable.
+    # A newer receipt holds fields this code does not read, and an older one lacks fields it does.
+    # Carrying the field and never checking it is the version telling nobody anything.
+    if isinstance(version, int) and version != RECEIPT_VERSION:
+        out.append(f"receiptVersion is {version} where this script writes {RECEIPT_VERSION}")
+    passes = receipt.get("passes")
+    # Each pass is later read for its reviewer name and formatted into a message.
+    # Reaching that line with a non-string name raises, and a raise is reported as a verdict.
+    if isinstance(passes, list):
+        for i, entry in enumerate(passes):
+            if not isinstance(entry, dict):
+                out.append(f"passes[{i}] is {type(entry).__name__} where an object is required")
+            elif not isinstance(entry.get("reviewer"), str):
+                out.append(f"passes[{i}].reviewer is missing or not a string")
+    return out
+
+
+def read_receipt(root: Path) -> tuple[dict | None, list[str]]:
+    """The receipt on disk and what is wrong with it, or (None, []) where there is none.
+
+    A receipt that cannot be read off disk at all, rather than one that parses into something
+    unusable, is an execution boundary and raises. The difference matters: unusable content is a
+    real answer about coverage, while an unreadable file means this check never ran.
+    """
+    path = receipt_path(root)
+    try:
+        if not path.is_file():
+            return None, []
+        text = path.read_text(encoding="utf-8")
+    except OSError as e:
+        raise CannotRun(f"the receipt at {path} could not be read ({e})") from e
+    try:
+        data = json.loads(text or "{}")
+    # ValueError rather than JSONDecodeError, since it also covers UnicodeDecodeError.
+    # A partially written or non-UTF-8 file raises that before the JSON parser is reached.
+    except ValueError as e:
+        return None, [f"the receipt does not parse ({e})"]
+    problems = receipt_problems(data)
+    return (data if not problems else None), problems
+
+
+def current_state(target: str, root: Path) -> tuple[str, str, int]:
+    """The merge base, digest, and changed-path count this invocation is talking about."""
+    base = merge_base(target, root)
+    marks = fingerprints(base, root)
+    return base, content_digest(base, marks), len(marks)
+
+
+def covering_passes(receipt: dict | None, base: str, digest: str, target: str) -> list[dict]:
+    """The passes in `receipt` that actually cover the current content, which may be none.
+
+    A pass covers the content only when the receipt agrees on all three of target, merge base, and
+    digest. The target is checked because a review against one branch says nothing about the same
+    branch retargeted at another, where the diff under review is a different one. It is compared
+    as the ref it resolves to, so naming the same branch two ways is not a mismatch.
+    """
+    if not receipt:
+        return []
+    if target_ref(str(receipt.get("target"))) != target_ref(target):
+        return []
+    if receipt.get("mergeBase") != base or receipt.get("contentDigest") != digest:
+        return []
+    return [p for p in receipt.get("passes", []) if isinstance(p, dict) and p.get("reviewer")]
+
+
+def write_pass(
+    root: Path, target: str, base: str, digest: str, reviewer: str, findings: int | None
+) -> dict:
+    """Add one backend's pass to the receipt, dropping any pass that no longer covers the content.
+
+    Passes accumulate so a sequence of backends over one unchanged diff builds up on one receipt.
+    A pass recorded against a different digest is dropped rather than kept, since it reviewed
+    content that no longer exists and would otherwise vote for a diff nobody read.
+
+    The file is replaced rather than rewritten in place, so a reader never observes a half-written
+    receipt and a process killed mid-write leaves the previous one intact.
+    """
+    receipt, _ = read_receipt(root)
+    kept = [p for p in covering_passes(receipt, base, digest, target) if p["reviewer"] != reviewer]
+    kept.append(
+        {
+            "reviewer": reviewer,
+            "recordedUtc": datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%SZ"),
+            "findings": findings,
+        }
+    )
+    out = {
+        "receiptVersion": RECEIPT_VERSION,
+        "target": target,
+        "mergeBase": base,
+        "contentDigest": digest,
+        "passes": kept,
+    }
+    path = receipt_path(root)
+    fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=path.name + ".")
+    try:
+        with os.fdopen(fd, "w", encoding="utf-8") as fh:
+            fh.write(json.dumps(out, indent=2) + "\n")
+        os.replace(tmp, path)
+    except BaseException:
+        Path(tmp).unlink(missing_ok=True)
+        raise
+    return out
+
+
+def run_coderabbit(base: str, root: Path) -> tuple[int, str]:
+    """Run the CodeRabbit CLI over this branch's diff and return the findings count and an error.
+
+    `--agent` is the CLI's documented structured-JSON mode for agent integrations. The base handed
+    to it is the merge-base commit itself rather than the target's tip, because the two differ the
+    moment the target moves: reviewing against the tip would cover the reverse of every commit the
+    target gained since this branch forked, which is not what the receipt claims.
+
+    A completion event is required rather than assumed. A CLI build that does not understand
+    `--agent`, or a run truncated part way, emits no findings and exits zero, which would
+    otherwise be recorded as a clean review of content nothing read. A rate-limited or errored run
+    likewise returns an error and the caller records no pass, since the CLI shares its hourly
+    budget with this account's pull request reviews and a budget exhaustion must never look like a
+    review.
+    """
+    exe = shutil.which("coderabbit") or shutil.which("cr")
+    if not exe:
+        raise CannotRun("the coderabbit CLI is not installed, so this backend cannot run")
+    try:
+        proc = subprocess.run(
+            [exe, "review", "--agent", "--base", base],
+            cwd=str(root),
+            capture_output=True,
+            check=False,
+            timeout=BACKEND_TIMEOUT,
+            encoding="utf-8",
+            errors="replace",
+        )
+    except (OSError, subprocess.SubprocessError) as e:
+        raise CannotRun(f"the coderabbit CLI could not be run ({e})") from e
+    findings = 0
+    complete = False
+    error = ""
+    for raw in proc.stdout.splitlines():
+        line = raw.strip()
+        if not line.startswith("{"):
+            continue
+        try:
+            event = json.loads(line)
+        except ValueError:
+            continue
+        if not isinstance(event, dict):
+            continue
+        kind = event.get("type") or event.get("event")
+        if kind == "finding":
+            findings += 1
+        elif kind == "complete":
+            complete = True
+        elif kind == "error":
+            error = str(event.get("message") or "the CLI reported an error event")
+    if proc.returncode != 0 and not error:
+        error = f"the CLI exited {proc.returncode}: {proc.stderr.strip() or 'no error text'}"
+    if not error and not complete:
+        error = "the CLI emitted no completion event, so the run cannot be treated as a review"
+    return findings, error
+
+
+def resolve_target(explicit: str | None) -> str:
+    """The branch this work targets.
+
+    The receipt is never consulted here. Reading the target back out of the receipt and then
+    comparing the receipt against it is self-fulfilling, so a branch retargeted after a pass was
+    recorded would verify against the branch it used to target and pass. Defaulting `record` from
+    the receipt while `check` defaults to the fleet branch would also strand a receipt that can
+    never cover, so both resolve the same way.
+
+    An explicitly empty value is rejected rather than treated as absent, since a hook written as
+    `--target "$VAR"` with the variable unset would otherwise silently gate the wrong branch.
+    """
+    if explicit is None:
+        return DEFAULT_TARGET
+    if not explicit.strip():
+        raise CannotRun("--target was given an empty value, so the review scope is ambiguous")
+    return explicit
+
+
+def cmd_status(args: argparse.Namespace) -> int:
+    root = repo_root()
+    target = resolve_target(args.target)
+    base, digest, changed = current_state(target, root)
+    receipt, problems = read_receipt(root)
+    passes = covering_passes(receipt, base, digest, target)
+    print(
+        json.dumps(
+            {
+                "target": target,
+                "mergeBase": base,
+                "contentDigest": digest,
+                "changedPaths": changed,
+                "covered": bool(passes),
+                "reviewers": [p["reviewer"] for p in passes],
+                "receiptProblems": problems,
+            },
+            indent=2,
+        )
+    )
+    # Reports rather than gating, so a caller under `set -e` can run it whatever the answer is.
+    return EXIT_COVERED
+
+
+def cmd_record(args: argparse.Namespace) -> int:
+    if args.reviewer not in BACKENDS:
+        known = ", ".join(sorted(BACKENDS))
+        print(f"unknown reviewer '{args.reviewer}'. Known backends: {known}", file=sys.stderr)
+        return EXIT_CANNOT_RUN
+    root = repo_root()
+    target = resolve_target(args.target)
+    base, digest, changed = current_state(target, root)
+    out = write_pass(root, target, base, digest, args.reviewer, args.findings)
+    reviewers = ", ".join(p["reviewer"] for p in out["passes"])
+    print(
+        f"recorded {args.reviewer} over {changed} changed path(s) against {target}"
+        f" at {digest[:12]}. Passes now: {reviewers}"
+    )
+    return EXIT_COVERED
+
+
+def cmd_check(args: argparse.Namespace) -> int:
+    root = repo_root()
+    target = resolve_target(args.target)
+    base, digest, changed = current_state(target, root)
+    receipt, problems = read_receipt(root)
+    if covering_passes(receipt, base, digest, target):
+        return EXIT_COVERED
+    print(
+        "No local review covers this branch's current content.\n"
+        f"  target        {target}\n"
+        f"  changed paths {changed}\n"
+        f"  digest        {digest[:12]}",
+        file=sys.stderr,
+    )
+    if receipt:
+        print(
+            f"  receipt       covers {receipt['target']} at {receipt['contentDigest'][:12]},"
+            " which is not this content",
+            file=sys.stderr,
+        )
+    for problem in problems:
+        print(f"  receipt       {problem}", file=sys.stderr)
+    print(
+        "\nRun the local-strict-review pass over this diff, then record it:\n"
+        "  python3 scripts/local_review.py record --reviewer agent-skill"
+        f" --target {target} --findings <n>",
+        file=sys.stderr,
+    )
+    return EXIT_NOT_COVERED
+
+
+def cmd_run(args: argparse.Namespace) -> int:
+    backend = BACKENDS.get(args.backend)
+    if backend is None:
+        known = ", ".join(sorted(k for k, v in BACKENDS.items() if v["headless"]))
+        print(f"unknown backend '{args.backend}'. Headless backends: {known}", file=sys.stderr)
+        return EXIT_CANNOT_RUN
+    if not backend["headless"]:
+        print(
+            f"'{args.backend}' is {backend['why']}, which only a live agent session can run."
+            " Run the pass, then record it with the record subcommand.",
+            file=sys.stderr,
+        )
+        return EXIT_CANNOT_RUN
+    root = repo_root()
+    target = resolve_target(args.target)
+    base, digest, _ = current_state(target, root)
+    findings, error = run_coderabbit(base, root)
+    if error:
+        print(f"{args.backend} did not complete, so no pass was recorded: {error}", file=sys.stderr)
+        return EXIT_CANNOT_RUN
+    # Re-read the content the backend actually finished against.
+    # An edit during a run that takes minutes would otherwise be recorded as reviewed.
+    after_base, after_digest, _ = current_state(target, root)
+    if (after_base, after_digest) != (base, digest):
+        print(
+            f"{args.backend} finished but the content changed during the run,"
+            " so no pass was recorded. Re-run it.",
+            file=sys.stderr,
+        )
+        # The check ran and the honest answer is that nothing covers the content now.
+        # Reporting the boundary code would let a gate that warns and continues on 2 wave it through.
+        return EXIT_NOT_COVERED
+    write_pass(root, target, base, digest, args.backend, findings)
+    print(
+        f"recorded {args.backend} against {target} at {digest[:12]}, {findings} finding(s)."
+        " A pass records that a review ran, never that the content is clean."
+    )
+    return EXIT_COVERED
+
+
+def main(argv: list[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(description=__doc__, add_help=True)
+    sub = parser.add_subparsers(dest="command", required=True)
+
+    p_status = sub.add_parser("status", help="what covers the current content, as JSON")
+    p_status.set_defaults(func=cmd_status)
+
+    p_record = sub.add_parser("record", help="record that a backend reviewed the current content")
+    p_record.add_argument(
+        "--reviewer", required=True, help=f"one of: {', '.join(sorted(BACKENDS))}"
+    )
+    p_record.add_argument("--findings", type=int, default=None, help="how many findings it raised")
+    p_record.set_defaults(func=cmd_record)
+
+    p_check = sub.add_parser("check", help="exit 0 covered, 1 not covered, 2 could not run")
+    p_check.set_defaults(func=cmd_check)
+
+    p_run = sub.add_parser("run", help="execute a headless backend and record its pass")
+    p_run.add_argument("--backend", required=True, help="a headless backend, e.g. coderabbit-cli")
+    p_run.set_defaults(func=cmd_run)
+
+    for p in (p_status, p_record, p_check, p_run):
+        p.add_argument("--target", default=None, help=f"target branch (default {DEFAULT_TARGET})")
+
+    args = parser.parse_args(argv)
+    try:
+        return int(args.func(args))
+    except CannotRun as e:
+        print(f"local_review: {e}", file=sys.stderr)
+        return EXIT_CANNOT_RUN
+    # A reader that closes early, `| head -1` being the ordinary case, otherwise raises here.
+    # It then raises again during the interpreter's own shutdown flush, which exits 120, outside the contract.
+    except BrokenPipeError:
+        devnull = os.open(os.devnull, os.O_WRONLY)
+        os.dup2(devnull, sys.stdout.fileno())
+        return EXIT_CANNOT_RUN
+    # A crash is the check not having run, so it reports the boundary code.
+    # Falling through to the interpreter's own exit 1 would read as the not-covered verdict.
+    except Exception as e:  # noqa: BLE001
+        print(f"local_review: unexpected failure ({type(e).__name__}: {e})", file=sys.stderr)
+        return EXIT_CANNOT_RUN
+
+
+if __name__ == "__main__":
+    sys.exit(main())
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
new file mode 100755
index 00000000..db1e2d2f
--- /dev/null
+++ b/scripts/tests/test_local_review.py
@@ -0,0 +1,688 @@
+#!/usr/bin/env python3
+"""Exercise local_review.py's content key, receipt handling, and exit codes against real git trees.
+
+Every case builds a throwaway repository with real remote-tracking refs, because the whole
+mechanism rests on git's own merge-base, index, and attribute behavior rather than on anything
+this module could stub convincingly.
+
+The fixture isolates git's global and system configuration. Without that, a host with commit
+signing on cannot commit inside the fixture at all, and a host with a different `core.autocrlf`
+answers the attribute cases differently than CI does.
+
+Run as `python3 scripts/tests/test_local_review.py`, or under
+`python3 -m unittest discover -s scripts/tests`.
+"""
+
+from __future__ import annotations
+
+import contextlib
+import io
+import json
+import os
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+import local_review
+
+
+def run(cwd: Path, *args: str) -> str:
+    """A checked git call for test setup, loud on failure so a broken fixture is never silent."""
+    proc = subprocess.run(["git", *args], cwd=str(cwd), capture_output=True, text=True, check=False)
+    if proc.returncode != 0:
+        raise AssertionError(f"git {' '.join(args)} failed: {proc.stderr.strip()}")
+    return proc.stdout
+
+
+class RepoCase(unittest.TestCase):
+    """A throwaway repository with one base commit and origin/develop plus origin/main refs.
+
+    The remote refs are planted with update-ref rather than by adding a real remote and fetching,
+    so the fixture needs no network and no second repository on disk. origin/main exists so a case
+    can record against one target and verify against another.
+    """
+
+    target = "develop"
+
+    def setUp(self) -> None:
+        self.tmp = Path(self.enterContext(tempfile.TemporaryDirectory())).resolve()
+        # Anything a case needs outside the repository still lives under a managed directory.
+        # Reaching for the repository's own parent would write into the shared system temp root.
+        # The file then outlives the run, and two concurrent runs collide on one name.
+        self.outside = Path(self.enterContext(tempfile.TemporaryDirectory())).resolve()
+        self.isolate_git_config()
+        run(self.tmp, "init", "--initial-branch=develop", ".")
+        run(self.tmp, "config", "user.email", "test@example.invalid")
+        run(self.tmp, "config", "user.name", "Test")
+        run(self.tmp, "config", "commit.gpgsign", "false")
+        (self.tmp / "base.txt").write_text("base\n", encoding="utf-8")
+        run(self.tmp, "add", "base.txt")
+        run(self.tmp, "commit", "-m", "base")
+        head = run(self.tmp, "rev-parse", "HEAD").strip()
+        run(self.tmp, "update-ref", "refs/remotes/origin/develop", head)
+        run(self.tmp, "update-ref", "refs/remotes/origin/main", head)
+        run(self.tmp, "checkout", "-b", "task")
+        self.prev = Path.cwd()
+        os.chdir(self.tmp)
+        self.addCleanup(os.chdir, self.prev)
+
+    def isolate_git_config(self) -> None:
+        """Keep the host's own git configuration out of every case."""
+        empty = self.outside / "empty-gitconfig"
+        empty.write_text("", encoding="utf-8")
+        for name in ("GIT_CONFIG_GLOBAL", "GIT_CONFIG_SYSTEM"):
+            prev = os.environ.get(name)
+            os.environ[name] = str(empty)
+            self.addCleanup(self.restore_env, name, prev)
+
+    @staticmethod
+    def restore_env(name: str, prev: str | None) -> None:
+        if prev is None:
+            os.environ.pop(name, None)
+        else:
+            os.environ[name] = prev
+
+    def digest(self, target: str | None = None) -> str:
+        _, digest, _ = local_review.current_state(target or self.target, self.tmp)
+        return digest
+
+    def marks(self) -> dict[str, str]:
+        return local_review.fingerprints(local_review.merge_base(self.target, self.tmp), self.tmp)
+
+    def record(self, reviewer: str, findings: int | None = 0, target: str | None = None) -> dict:
+        t = target or self.target
+        base, digest, _ = local_review.current_state(t, self.tmp)
+        return local_review.write_pass(self.tmp, t, base, digest, reviewer, findings)
+
+    def main_quiet(self, argv: list[str]) -> int:
+        """Run the CLI with its diagnostics captured, for a case asserting only the exit code."""
+        with contextlib.redirect_stderr(io.StringIO()), contextlib.redirect_stdout(io.StringIO()):
+            return local_review.main(argv)
+
+
+class ContentKeyCase(RepoCase):
+    def test_key_survives_the_commit_boundary(self) -> None:
+        """The property the whole design rests on: review untracked work, commit it, same key."""
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        untracked = self.digest()
+        run(self.tmp, "add", "new.py")
+        self.assertEqual(self.digest(), untracked, "staging changed the key")
+        run(self.tmp, "commit", "-m", "add new")
+        self.assertEqual(self.digest(), untracked, "committing changed the key")
+
+    def test_key_survives_the_commit_boundary_under_a_text_filter(self) -> None:
+        """The same property where git rewrites content on the way into the index.
+
+        A blob id computed by hashing raw working-tree bytes cannot equal the one git stores for a
+        CRLF file under `text` attributes, so the two states never collapse and `git add` moves
+        the key. This repository's own .gitattributes applies such a filter to every text file, so
+        this is the configuration the engine actually runs under, not an exotic one.
+        """
+        (self.tmp / ".gitattributes").write_text("* text eol=crlf\n", encoding="utf-8")
+        run(self.tmp, "add", ".gitattributes")
+        run(self.tmp, "commit", "-m", "attributes")
+        (self.tmp / "crlf.txt").write_bytes(b"line one\r\nline two\r\n")
+        # Without this the case proves nothing.
+        # A typo in .gitattributes, or a git build ignoring it, leaves the filter inactive.
+        # The assertions below would then pass for the trivial reason.
+        raw = run(self.tmp, "hash-object", "--no-filters", "--", "crlf.txt").strip()
+        filtered = run(self.tmp, "hash-object", "--", "crlf.txt").strip()
+        self.assertNotEqual(raw, filtered, "the text filter is not active, so this proves nothing")
+        before = self.digest()
+        run(self.tmp, "add", "crlf.txt")
+        self.assertEqual(self.digest(), before, "staging a CRLF file moved the key")
+        run(self.tmp, "commit", "-m", "add crlf")
+        self.assertEqual(self.digest(), before, "committing a CRLF file moved the key")
+
+    def test_one_byte_changes_the_key(self) -> None:
+        """A fix to a review finding has to invalidate the receipt, or the gate proves nothing."""
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        before = self.digest()
+        (self.tmp / "new.py").write_text("print('y')\n", encoding="utf-8")
+        self.assertNotEqual(self.digest(), before)
+
+    def test_index_only_content_reaches_the_fingerprint(self) -> None:
+        """Content staged that the working tree no longer holds still gets committed and pushed.
+
+        The assertion is on the fingerprint holding two states rather than only on the digest
+        moving. A digest-only assertion also passes when the path merely enters the changed set,
+        which would leave the index side of the fingerprint unproven.
+        """
+        (self.tmp / "base.txt").write_text("staged\n", encoding="utf-8")
+        run(self.tmp, "add", "base.txt")
+        (self.tmp / "base.txt").write_text("base\n", encoding="utf-8")
+        self.assertEqual(
+            run(self.tmp, "diff", "--name-only", "HEAD"), "", "fixture is not the case"
+        )
+        mark = self.marks()["base.txt"]
+        self.assertEqual(
+            len(mark.split("|")), 2, f"index state absent from the fingerprint: {mark}"
+        )
+
+    def test_a_mode_change_changes_the_key(self) -> None:
+        """A chmod is a real diff git records, so a receipt taken before it must not still cover.
+
+        The file is already edited, so it is already in the changed-path set before the chmod.
+        Otherwise the chmod would move the key merely by adding a path, and the case would pass
+        without the mode ever reaching the fingerprint.
+        """
+        target = self.tmp / "base.txt"
+        target.write_text("edited\n", encoding="utf-8")
+        before = self.digest()
+        self.assertIn("base.txt", self.marks())
+        target.chmod(target.stat().st_mode | 0o111)
+        if "100755" not in run(self.tmp, "diff", "--raw", "HEAD"):
+            self.skipTest("this filesystem does not carry an executable bit git can see")
+        self.assertNotEqual(self.digest(), before)
+
+    def test_a_copy_and_a_move_do_not_share_a_key(self) -> None:
+        """Comparing states against the merge base means a deletion is always visible.
+
+        A diff listing with rename detection reports only a rename's destination, so turning a
+        copy into a move would leave the source's deletion out of the key entirely.
+        """
+        run(self.tmp, "mv", "base.txt", "moved.txt")
+        moved = self.digest()
+        self.assertEqual(self.marks()["base.txt"], "w:absent")
+        run(self.tmp, "reset", "--hard", "HEAD")
+        (self.tmp / "moved.txt").write_text("base\n", encoding="utf-8")
+        self.assertNotIn("base.txt", self.marks())
+        self.assertNotEqual(moved, self.digest())
+
+    def test_deleting_a_tracked_file_changes_the_key(self) -> None:
+        before = self.digest()
+        run(self.tmp, "rm", "-q", "base.txt")
+        self.assertNotEqual(self.digest(), before)
+        self.assertEqual(self.marks()["base.txt"], "w:absent")
+
+    def test_an_unchanged_path_is_not_in_the_key(self) -> None:
+        """Otherwise every file in the repository would be reviewed content."""
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.assertEqual(sorted(self.marks()), ["new.py"])
+
+    def test_the_key_depends_on_the_merge_base(self) -> None:
+        """Without this, a build whose digest ignored the base would pass every other case."""
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        # Drive current_state against two real and different bases.
+        # Passing a made-up base straight to content_digest is passed by any hash that mixes its first argument.
+        run(self.tmp, "add", "-A")
+        run(self.tmp, "commit", "-m", "on task")
+        run(
+            self.tmp,
+            "update-ref",
+            "refs/remotes/origin/main",
+            run(self.tmp, "rev-parse", "HEAD").strip(),
+        )
+        self.assertNotEqual(
+            local_review.merge_base("develop", self.tmp), local_review.merge_base("main", self.tmp)
+        )
+        (self.tmp / "after.py").write_text("print('a')\n", encoding="utf-8")
+        self.assertNotEqual(self.digest("develop"), self.digest("main"))
+
+    def test_an_unmerged_path_keeps_every_stage(self) -> None:
+        """Keeping only the last stage reduces a conflicted index to one side of the conflict.
+
+        A pass recorded mid-conflict would then carry the same key as the resolved tree, so it
+        would cover a resolution nobody reviewed.
+        """
+        clean = self.digest()
+        run(self.tmp, "checkout", "-b", "side")
+        (self.tmp / "c.txt").write_text("theirs\n", encoding="utf-8")
+        run(self.tmp, "add", "c.txt")
+        run(self.tmp, "commit", "-m", "theirs")
+        run(self.tmp, "checkout", "task")
+        (self.tmp / "c.txt").write_text("ours\n", encoding="utf-8")
+        run(self.tmp, "add", "c.txt")
+        run(self.tmp, "commit", "-m", "ours")
+        subprocess.run(
+            ["git", "merge", "side"], cwd=str(self.tmp), capture_output=True, check=False
+        )
+        states = local_review.index_states(self.tmp)["c.txt"]
+        self.assertGreaterEqual(len(states), 2, f"stages collapsed to {states}")
+        # The property that matters is the digest rather than the parse.
+        # A conflicted tree must not carry the key of any resolution of it.
+        # Otherwise a pass taken mid-conflict covers the resolution nobody reviewed.
+        conflicted = self.digest()
+        run(self.tmp, "checkout", "--theirs", "--", "c.txt")
+        run(self.tmp, "add", "c.txt")
+        self.assertNotEqual(
+            self.digest(), conflicted, "resolving the conflict did not move the key"
+        )
+        self.assertNotEqual(self.digest(), clean)
+
+    def test_a_symlink_is_keyed_by_its_target_not_its_content(self) -> None:
+        """Reading through a link would fold bytes from outside the reviewed tree into the key."""
+        outside = self.outside / "outside.txt"
+        outside.write_text("secret\n", encoding="utf-8")
+        try:
+            (self.tmp / "link").symlink_to(outside)
+        except (OSError, NotImplementedError):
+            self.skipTest("this platform does not permit creating a symlink")
+        before = self.digest()
+        self.assertIn("120000:", self.marks()["link"])
+        # Rewriting the bytes at the far end must not move the key.
+        # Those bytes are outside the tree and were never under review.
+        outside.write_text("different content entirely\n", encoding="utf-8")
+        self.assertEqual(self.digest(), before, "the key followed the link")
+
+    def test_a_non_ascii_path_is_keyed_under_its_real_name(self) -> None:
+        """git quotes such a path by default, so the key could carry a name that is not on disk."""
+        # Escaped rather than literal so this source file stays ASCII.
+        # The name on disk is still non-ASCII, which is the property under test.
+        name = "docs/caf\u00e9.md"
+        (self.tmp / "docs").mkdir()
+        (self.tmp / name).write_text("x\n", encoding="utf-8")
+        self.assertIn(name, self.marks())
+        run(self.tmp, "add", "--", name)
+        run(self.tmp, "commit", "-m", "add")
+        self.assertIn(name, self.marks())
+
+    def test_a_submodule_bump_is_a_state_not_a_crash(self) -> None:
+        """A gitlink is a directory on disk, so reading it as a file would raise."""
+        sub = self.outside / "sub"
+        sub.mkdir()
+        run(sub, "init", "--initial-branch=main", ".")
+        run(sub, "config", "user.email", "test@example.invalid")
+        run(sub, "config", "user.name", "Test")
+        run(sub, "config", "commit.gpgsign", "false")
+        (sub / "f.txt").write_text("x\n", encoding="utf-8")
+        run(sub, "add", "f.txt")
+        run(sub, "commit", "-m", "sub")
+        added = subprocess.run(
+            ["git", "-c", "protocol.file.allow=always", "submodule", "add", str(sub), "sub"],
+            cwd=str(self.tmp),
+            capture_output=True,
+            text=True,
+            check=False,
+        )
+        if added.returncode != 0:
+            self.skipTest(f"submodules unavailable here: {added.stderr.strip()}")
+        before = self.digest()
+        self.assertIn("160000:", self.marks()["sub"])
+        # Adding a submodule is not bumping one, and only the bump exercises the gitlink state.
+        (sub / "f.txt").write_text("moved on\n", encoding="utf-8")
+        run(sub, "commit", "-am", "advance")
+        run(self.tmp / "sub", "pull", "--ff-only", str(sub), "main")
+        self.assertNotEqual(self.digest(), before, "a submodule bump did not move the key")
+
+    def test_staging_the_worktree_writes_nothing_into_the_object_database(self) -> None:
+        """Reading the tree stages it, and staging writes blobs, so the writes are redirected.
+
+        Without the redirection this read permanently deposits the content of every unignored
+        untracked file into the repository, a stray secret among them, from a command whose whole
+        contract is that it only looks.
+        """
+        (self.tmp / ".env").write_text("TOKEN=constructed-not-real\n", encoding="utf-8")
+        objects = local_review.git_dir(self.tmp) / "objects"
+        before = sorted(p.name for p in objects.rglob("*") if p.is_file())
+        states = local_review.worktree_states(self.tmp)
+        after = sorted(p.name for p in objects.rglob("*") if p.is_file())
+        self.assertEqual(before, after, "staging leaked objects into the real database")
+        oid = next(iter(states[".env"])).split(":")[-1]
+        probe = subprocess.run(
+            ["git", "cat-file", "-e", oid], cwd=str(self.tmp), capture_output=True, check=False
+        )
+        self.assertNotEqual(probe.returncode, 0, "the untracked file is readable from the repo")
+
+    def test_staging_leaves_no_throwaway_paths_behind(self) -> None:
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        local_review.worktree_states(self.tmp)
+        gd = local_review.git_dir(self.tmp)
+        leftovers = [p.name for p in gd.iterdir() if p.name.startswith("local-review-")]
+        self.assertEqual(leftovers, [])
+
+    def test_two_readers_do_not_share_a_throwaway_name(self) -> None:
+        """A fixed name lets one invocation delete another's index mid-read.
+
+        `git ls-files` against a missing index exits 0 with empty output, so the loser computes a
+        confident wrong digest rather than failing.
+        """
+        gd = local_review.git_dir(self.tmp)
+        seen = set()
+        real_git = local_review.git
+
+        def capture(*args: str, **kwargs: object) -> str:
+            env = kwargs.get("env")
+            if isinstance(env, dict) and "GIT_INDEX_FILE" in env:
+                seen.add(env["GIT_INDEX_FILE"])
+            return real_git(*args, **kwargs)  # type: ignore[arg-type]
+
+        local_review.git = capture  # type: ignore[assignment]
+        self.addCleanup(setattr, local_review, "git", real_git)
+        local_review.worktree_states(self.tmp)
+        local_review.worktree_states(self.tmp)
+        self.assertEqual(len(seen), 2, f"both reads used the same index path: {seen}")
+        self.assertTrue(all(str(gd) in p for p in seen))
+
+    def test_an_inherited_index_redirect_is_ignored(self) -> None:
+        """A git hook runs with GIT_INDEX_FILE set, and a hook is the intended capture point.
+
+        Inheriting it would make the real-index read answer for whatever index the caller pointed
+        at, which during a partial commit is a pending one holding different content.
+        """
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        expected = self.digest()
+        decoy = local_review.git_dir(self.tmp) / "decoy-index"
+        decoy.write_bytes(b"")
+        prev = os.environ.get("GIT_INDEX_FILE")
+        os.environ["GIT_INDEX_FILE"] = str(decoy)
+        self.addCleanup(self.restore_env, "GIT_INDEX_FILE", prev)
+        self.assertEqual(self.digest(), expected, "the inherited index redirect was honored")
+
+    def test_staging_over_a_worktree_is_told_from_the_reverse(self) -> None:
+        """A plain union of the two sides renders these identically, and they push different bytes."""
+        f = self.tmp / "base.txt"
+        f.write_text("A\n", encoding="utf-8")
+        run(self.tmp, "add", "base.txt")
+        f.write_text("B\n", encoding="utf-8")
+        a_over_b = self.digest()
+        run(self.tmp, "reset", "--hard", "HEAD")
+        f.write_text("B\n", encoding="utf-8")
+        run(self.tmp, "add", "base.txt")
+        f.write_text("A\n", encoding="utf-8")
+        self.assertNotEqual(self.digest(), a_over_b, "the two sides collapsed into one state")
+
+    def test_the_engine_is_correct_from_a_subdirectory(self) -> None:
+        """git reports paths relative to different places depending on the subcommand.
+
+        An engine that inherited the process's own directory would key untracked files under names
+        that do not resolve, and drop every one outside that directory.
+        """
+        (self.tmp / "sub").mkdir()
+        (self.tmp / "sub" / "inner.py").write_text("print('i')\n", encoding="utf-8")
+        (self.tmp / "top.py").write_text("print('t')\n", encoding="utf-8")
+        from_root = self.digest()
+        os.chdir(self.tmp / "sub")
+        self.assertEqual(local_review.repo_root(), self.tmp)
+        self.assertEqual(self.digest(), from_root, "the key changed with the working directory")
+
+
+class StagingIsolationCase(RepoCase):
+    def test_reading_the_worktree_does_not_touch_the_real_index(self) -> None:
+        """The read stages the tree into a throwaway index, which must stay throwaway."""
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        before = run(self.tmp, "ls-files", "-s")
+        local_review.worktree_states(self.tmp)
+        self.assertEqual(run(self.tmp, "ls-files", "-s"), before, "the real index was modified")
+        self.assertEqual(run(self.tmp, "status", "--porcelain"), "?? new.py\n")
+
+    def test_the_staging_index_leaves_nothing_behind(self) -> None:
+        """Left in the working tree it would stage itself, and its lock, as untracked content."""
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        local_review.worktree_states(self.tmp)
+        gd = local_review.git_dir(self.tmp)
+        leftovers = [p.name for p in gd.iterdir() if p.name.startswith("local-review-index")]
+        self.assertEqual(leftovers, [])
+        self.assertNotIn("local-review", run(self.tmp, "status", "--porcelain"))
+
+
+class ReceiptCase(RepoCase):
+    def test_check_reports_not_covered_then_covered(self) -> None:
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
+        self.record("agent-skill")
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 0)
+
+    def test_a_stale_pass_is_dropped_rather_than_carried(self) -> None:
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.record("agent-skill")
+        (self.tmp / "new.py").write_text("print('y')\n", encoding="utf-8")
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
+        out = self.record("coderabbit-cli")
+        self.assertEqual([p["reviewer"] for p in out["passes"]], ["coderabbit-cli"])
+
+    def test_backends_accumulate_on_one_unchanged_diff(self) -> None:
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.record("agent-skill", 3)
+        out = self.record("coderabbit-cli", 1)
+        self.assertEqual(
+            sorted(p["reviewer"] for p in out["passes"]), ["agent-skill", "coderabbit-cli"]
+        )
+
+    def test_recording_the_same_backend_twice_replaces_its_pass(self) -> None:
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.record("agent-skill", 3)
+        out = self.record("agent-skill", 0)
+        self.assertEqual(len(out["passes"]), 1)
+        self.assertEqual(out["passes"][0]["findings"], 0)
+
+    def test_no_subcommand_takes_its_target_from_the_receipt(self) -> None:
+        """Reading the target out of the receipt and comparing against it is self-fulfilling.
+
+        A branch retargeted after a pass was recorded would otherwise verify against the branch it
+        used to target and pass, never having been reviewed against the diff being pushed.
+        """
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.record("agent-skill", target="main")
+        self.assertEqual(local_review.resolve_target(None), "develop")
+        self.assertEqual(self.main_quiet(["check"]), 1)
+
+    def test_the_same_target_named_two_ways_still_covers(self) -> None:
+        """A skill step and a hook that spell the target differently must not deadlock."""
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.record("agent-skill", target="develop")
+        self.assertEqual(self.main_quiet(["check", "--target", "origin/develop"]), 0)
+
+    def test_a_receipt_for_another_target_does_not_cover_this_one(self) -> None:
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.record("agent-skill", target="main")
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
+
+    def test_a_hand_edited_receipt_gives_a_verdict_not_a_traceback(self) -> None:
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.record("agent-skill")
+        local_review.receipt_path(self.tmp).write_text(
+            '{"receiptVersion": "one"}', encoding="utf-8"
+        )
+        receipt, problems = local_review.read_receipt(self.tmp)
+        self.assertIsNone(receipt)
+        self.assertTrue(problems)
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
+
+    def test_an_unparseable_receipt_gives_a_verdict_not_a_traceback(self) -> None:
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        local_review.receipt_path(self.tmp).write_text("{not json", encoding="utf-8")
+        receipt, problems = local_review.read_receipt(self.tmp)
+        self.assertIsNone(receipt)
+        self.assertTrue(problems)
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
+
+    def test_a_malformed_pass_entry_is_caught_before_it_is_formatted(self) -> None:
+        """A non-string reviewer name would otherwise reach a formatting line and raise."""
+        problems = local_review.receipt_problems(
+            {
+                "receiptVersion": local_review.RECEIPT_VERSION,
+                "target": "develop",
+                "mergeBase": "x",
+                "contentDigest": "y",
+                "passes": [{"reviewer": 1}],
+            }
+        )
+        self.assertTrue(any("reviewer" in p for p in problems), problems)
+
+    def test_an_unreadable_receipt_file_is_a_boundary_not_a_verdict(self) -> None:
+        """The tool's own state file being unreadable means this check never ran."""
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.record("agent-skill")
+        path = local_review.receipt_path(self.tmp)
+        path.chmod(0o000)
+        self.addCleanup(path.chmod, 0o644)
+        if os.access(path, os.R_OK):
+            self.skipTest("this process can read a mode-000 file, so the case cannot be built")
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 2)
+
+    def test_a_future_receipt_version_reads_as_unusable(self) -> None:
+        problems = local_review.receipt_problems(
+            {
+                "receiptVersion": local_review.RECEIPT_VERSION + 1,
+                "target": "develop",
+                "mergeBase": "x",
+                "contentDigest": "y",
+                "passes": [],
+            }
+        )
+        self.assertTrue(any("receiptVersion" in p for p in problems))
+
+    def test_the_receipt_lives_outside_the_working_tree(self) -> None:
+        """So it can never be committed by accident and needs no .gitignore entry."""
+        path = local_review.receipt_path(self.tmp)
+        self.assertEqual(path.parent, local_review.git_dir(self.tmp))
+        path.write_text("{}", encoding="utf-8")
+        self.assertEqual(run(self.tmp, "status", "--porcelain", "--ignored"), "")
+
+    def test_a_linked_worktree_keeps_its_own_receipt(self) -> None:
+        """Two tasks in two worktrees of one repository must not share a coverage answer."""
+        other = self.outside / "linked"
+        run(self.tmp, "worktree", "add", "-b", "other", str(other))
+        self.addCleanup(run, self.tmp, "worktree", "remove", "--force", str(other))
+        self.assertNotEqual(local_review.receipt_path(self.tmp), local_review.receipt_path(other))
+
+
+class BackendCase(RepoCase):
+    """Drives run_coderabbit against a stand-in binary, so the parsing contract is tested."""
+
+    def fake_cli(self, stdout: str, code: int = 0) -> None:
+        bin_dir = self.outside / "fakebin"
+        bin_dir.mkdir(exist_ok=True)
+        script = bin_dir / "coderabbit"
+        script.write_text(
+            f"#!/usr/bin/env python3\nimport sys\nsys.stdout.write({stdout!r})\nsys.exit({code})\n",
+            encoding="utf-8",
+        )
+        script.chmod(0o755)
+        prev = os.environ.get("PATH", "")
+        os.environ["PATH"] = f"{bin_dir}{os.pathsep}{prev}"
+        self.addCleanup(self.restore_env, "PATH", prev)
+
+    def test_a_completed_run_counts_findings(self) -> None:
+        self.fake_cli('{"type":"finding"}\n{"type":"finding"}\n{"type":"complete"}\n')
+        self.assertEqual(local_review.run_coderabbit(self.target, self.tmp), (2, ""))
+
+    def test_a_run_with_no_completion_event_is_not_a_review(self) -> None:
+        """A build that does not understand --agent prints prose and exits zero.
+
+        Treating that as a clean review records a pass over content nothing read.
+        """
+        self.fake_cli("Reviewing your changes...\nLooks good!\n")
+        findings, error = local_review.run_coderabbit(self.target, self.tmp)
+        self.assertEqual(findings, 0)
+        self.assertIn("completion event", error)
+
+    def test_a_truncated_run_is_not_a_review(self) -> None:
+        self.fake_cli('{"type":"finding"}\n')
+        _, error = local_review.run_coderabbit(self.target, self.tmp)
+        self.assertTrue(error)
+
+    def test_an_error_event_is_reported(self) -> None:
+        self.fake_cli('{"type":"error","message":"rate limited"}\n{"type":"complete"}\n')
+        _, error = local_review.run_coderabbit(self.target, self.tmp)
+        self.assertEqual(error, "rate limited")
+
+    def test_a_non_zero_exit_with_no_error_event_still_reports(self) -> None:
+        self.fake_cli('{"type":"complete"}\n', code=3)
+        _, error = local_review.run_coderabbit(self.target, self.tmp)
+        self.assertIn("exited 3", error)
+
+    def test_a_rate_limited_run_records_no_pass(self) -> None:
+        """A budget exhaustion shares its shape with a clean review, so it must not become one."""
+        self.fake_cli('{"type":"error","message":"rate limited"}\n')
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.assertEqual(
+            self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 2
+        )
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
+
+    def test_a_completed_run_records_a_pass_through_main(self) -> None:
+        """The whole cmd_run path, including the digest re-check and the receipt write."""
+        self.fake_cli('{"type":"finding"}\n{"type":"complete"}\n')
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.assertEqual(
+            self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 0
+        )
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 0)
+        receipt, _ = local_review.read_receipt(self.tmp)
+        assert receipt is not None
+        self.assertEqual(receipt["passes"][0]["findings"], 1)
+
+    def test_a_missing_binary_is_a_boundary_through_main(self) -> None:
+        prev = os.environ.get("PATH", "")
+        os.environ["PATH"] = str(self.tmp / "definitely-empty")
+        self.addCleanup(self.restore_env, "PATH", prev)
+        self.assertEqual(
+            self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 2
+        )
+
+
+class ExitCodeCase(RepoCase):
+    def test_an_unknown_reviewer_is_a_boundary_not_a_finding(self) -> None:
+        """2 says the check did not run, which a gate must not read as either verdict."""
+        self.assertEqual(
+            self.main_quiet(["record", "--reviewer", "nope", "--target", self.target]), 2
+        )
+
+    def test_an_agent_backend_cannot_be_run_headlessly(self) -> None:
+        self.assertEqual(
+            self.main_quiet(["run", "--backend", "agent-skill", "--target", self.target]), 2
+        )
+
+    def test_an_unresolvable_target_is_a_boundary_not_a_finding(self) -> None:
+        self.assertEqual(self.main_quiet(["check", "--target", "no-such-branch"]), 2)
+
+    def test_an_empty_target_is_a_boundary_not_a_silent_default(self) -> None:
+        """A hook written with an unset variable must not gate the wrong diff.
+
+        The exit code alone does not prove the guard: an empty target also fails later, when the
+        ref it builds does not resolve. The diagnostic is what the guard actually changes, so the
+        assertion is on the message naming the empty value rather than an unresolvable ref.
+        """
+        err = io.StringIO()
+        with contextlib.redirect_stderr(err), contextlib.redirect_stdout(io.StringIO()):
+            code = local_review.main(["check", "--target", ""])
+        self.assertEqual(code, 2)
+        self.assertIn("empty value", err.getvalue())
+
+    def test_an_unexpected_crash_reports_the_boundary_not_a_verdict(self) -> None:
+        """Falling through to the interpreter would exit 1, which reads as a real finding."""
+        original = local_review.current_state
+
+        def boom(*_args: object, **_kwargs: object) -> tuple[str, str, int]:
+            raise RuntimeError("something nobody anticipated")
+
+        local_review.current_state = boom  # type: ignore[assignment]
+        self.addCleanup(setattr, local_review, "current_state", original)
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 2)
+
+    def test_status_does_not_gate_on_coverage(self) -> None:
+        """It reports, so a hook or skill step running it under `set -e` must not abort."""
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.assertEqual(self.main_quiet(["status", "--target", self.target]), 0)
+
+    def test_status_emits_json_carrying_the_reviewers(self) -> None:
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.record("agent-skill", 2)
+        buf = io.StringIO()
+        with contextlib.redirect_stdout(buf):
+            code = local_review.main(["status", "--target", self.target])
+        self.assertEqual(code, 0)
+        data = json.loads(buf.getvalue())
+        self.assertEqual(data["reviewers"], ["agent-skill"])
+        self.assertTrue(data["covered"])
+
+    def test_record_reports_what_it_recorded(self) -> None:
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        buf = io.StringIO()
+        with contextlib.redirect_stdout(buf):
+            code = local_review.main(["record", "--reviewer", "agent-skill", "--findings", "2"])
+        self.assertEqual(code, 0)
+        self.assertIn("agent-skill", buf.getvalue())
+        receipt, problems = local_review.read_receipt(self.tmp)
+        self.assertEqual(problems, [])
+        assert receipt is not None
+        self.assertEqual(receipt["passes"][0]["findings"], 2)
+
+
+if __name__ == "__main__":
+    unittest.main()

From 2555142349c8a0394fbf7727cb3a08111c0c4812 Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sat, 29 Aug 2026 21:32:58 -0700
Subject: [PATCH 02/12] Bind a Recorded Pass to the Content the Review Actually
 Saw

Review findings on the pull request, plus a fourth local pass over the full
diff. Every one is a defect the earlier passes missed.

The serious one: `record` computed the digest at the moment it ran, and nothing
tied that to the moment the review finished. A format-on-save or a hook autofix
in between was stamped as reviewed content no reviewer had seen, which is the one
claim the receipt exists to make. `record` now takes `--expect-digest`, the value
`status` reported before the pass, and refuses when the content has moved.

Also fixed:

- The object store resolved as the worktree git directory plus `objects`, which
  does not exist in a linked worktree, where the store lives in the common
  directory. That is the fleet's standard working mode, so the alternate pointed
  at nothing and every existing object was unreadable. Asked of git now.
- A non-UTF-8 receipt raised past both handlers into the generic one and reported
  the boundary code for what is unusable content, so decoding moved in with
  parsing.
- The broken-pipe guard never fired: stdout to a pipe is block-buffered, so the
  payload only failed during interpreter shutdown, exiting 120. Flushed inside
  the guarded region.
- `record` accepted a headless backend, forging the pass whose integrity the
  completion check defends.
- The CodeRabbit subprocess inherited the redirects the rest of the engine
  strips, so run from a hook it reviewed a different tree than the receipt keyed.
- Receipt accumulation was a read-modify-replace with no lock, so two concurrent
  records dropped one.
- The alternate object directory is a path-separator list, now quoted.
- Dropped the `cr` binary fallback rather than run whatever holds that name.
- A semicolon in the module docstring, which `prose_lint` cannot see: its
  semicolon check is Markdown-only and never scans Python docstrings.
- The submodule test's `pull` needed the file-protocol allowance its `add` had.

Test quality, from the same passes. Reworked four cases that passed for
incidental reasons: the merge-base case moved on differing path sets rather than
on the base, the non-ASCII case used a name that is valid UTF-8 and so exercised
neither the quoting nor the decoding, the symlink case named a mechanism the
engine no longer implements, and the linked-worktree case never ran the engine
inside one, which is why the object-store defect above was invisible. Added
cases for the ignored-file exclusion, the net-content scope boundary, staging
determinism, and the backend argument contract.

Part of #1104 and #1083.
---
 scripts/README.md                  |   6 +-
 scripts/local_review.py            | 125 +++++++++++--
 scripts/tests/test_local_review.py | 274 ++++++++++++++++++++++-------
 3 files changed, 327 insertions(+), 78 deletions(-)

diff --git a/scripts/README.md b/scripts/README.md
index 56f2b4cf..b21cdf48 100644
--- a/scripts/README.md
+++ b/scripts/README.md
@@ -4,7 +4,7 @@ The fleet's checks and review tooling, run by hand, with the deterministic ones
 
 **Hosted here and reached, never carried.** These are not declared in [`spec/files.json`][files], so the audit does not expect a downstream repo to ship them, the same footing as `spec/audit.py`. That is the fleet model rather than an omission: a gate holding no per-repo content has one canonical implementation for the fleet. The prose and repository gate implementations live beside their composite actions under `.github/actions/`, which lets reusable workflows reach the implementation at their pinned hub commit through `$/.github/actions/` without checking out the hub. The same-named files in `scripts/` are thin local entry points that call those action-owned implementations, so maintainers and hooks retain the established commands from a hub checkout without creating a second copy of either gate. A repository that cannot reach the hub reports a local check as not run rather than reconstructing it, since a rebuilt gate encodes its author's reading of the rule and agrees with no other repository.
 
-Python only, standard library only, no third-party packages. Every check script is read-only and exits non-zero on a finding. `build_dist.py`, `local_review.py`, and `skills_install.py` below are the exceptions, since a generator, a recorder, and an installer all exist to write, and each still offers a read-only mode (`--check`, `status`, `--report`) for CI and for asking without changing anything.
+Python only, standard library only, no third-party packages. Every check script is read-only and exits non-zero on a finding. `build_dist.py`, `local_review.py`, and `skills_install.py` below are the exceptions, since a generator, a recorder, and an installer all exist to write. `build_dist.py --check` and `skills_install.py --report` are read-only modes for CI and for asking without changing anything. `local_review.py status` and `check` answer without recording anything, but they are not read-only in that same sense: reading the working tree stages it, which writes inside `.git` and runs any `filter.*.clean` the repository configures, and only the receipt is left untouched.
 
 The directory separates its kinds by name and by tree. A gate checks and exits non-zero on a finding, and its name carries a `_lint` or `_gate` suffix saying what it gates. The `prose_lint.py` and `repo_gate.py` entry points delegate to the action-owned implementations that gate this tree in CI, and `host_gate.py` gates the machine it runs on. A utility does work rather than gating and carries no suffix: `build_dist.py`, `local_review.py`, `pr_review.py`, `skills_install.py`. The unit tests live apart under [`scripts/tests/`][tests], one `test_<script>.py` per script, driving the canonical implementations against input they must reject. A gate nobody has watched fail is a gate nobody knows works. Where a case covers a table it reads the live table rather than restating it, and each one asserts a floor on what a healthy run reaches, since a check whose scan matches nothing reports zero findings and reads exactly like a pass.
 
@@ -228,11 +228,13 @@ Every identity is computed by git rather than reconstructed, which matters more
 
 What the key covers is the net content the branch introduces against its target, not the commit series that produced it. A branch that adds a file and later deletes it, or a rebase that rewrites only messages, keeps its key. That is the intended scope, since a feature branch lands as a squash and the net diff is what a reviewer reads, but a capture point built on this does not cover the commit series. The receipt lives in the worktree's own git directory rather than in the working tree, so it is per-task, can never be committed by accident, and needs no `.gitignore` entry of its own.
 
+`record` takes `--expect-digest`, the digest `status` reported before the pass ran, and refuses when the content has moved since. Nothing else binds the moment a review finished to the moment it is recorded, so a format-on-save or a hook autofix in between would otherwise be stamped as reviewed content no reviewer saw, which is the one claim the receipt exists to make. A headless backend is recorded by running it rather than by hand, since recording it directly would bypass the completion check that makes its pass mean anything.
+
 It holds no review logic. It drives backends: `agent-skill` is the `local-strict-review` subagent pass, which only a live agent session can run, so the engine records it rather than invoking it, and `coderabbit-cli` is headless and is the one backend a git hook can execute by itself. A CodeRabbit CLI run draws on the same hourly budget as this account's pull request reviews, so a run that is rate limited, errors, or ends without a completion event records no pass rather than a clean one. A recorded pass means a review ran over exactly this content, never that the content is clean: disposing of what a review found is judgment, per the `pr-review-conduct` Skill's five outcomes, so a pass is recorded whether it raised findings or none.
 
 ```shell
 python3 scripts/local_review.py status                                      # what covers the current content. 0 either way, 2 if it cannot run
-python3 scripts/local_review.py record --reviewer agent-skill --findings 3
+python3 scripts/local_review.py record --reviewer agent-skill --findings 3 --expect-digest <digest>
 python3 scripts/local_review.py check                                       # 0 covered, 1 not, 2 could not run
 python3 scripts/local_review.py run --backend coderabbit-cli
 ```
diff --git a/scripts/local_review.py b/scripts/local_review.py
index 74c25bd0..4c074a76 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -51,7 +51,7 @@
 
 This script holds no review logic. It drives backends: `agent-skill` is the `local-strict-review`
 subagent pass, which only a live agent session can run, so the engine records it rather than
-invoking it; `coderabbit-cli` is headless and is the one backend a git hook can execute by itself.
+invoking it. `coderabbit-cli` is headless and is the one backend a git hook can execute by itself.
 
 A recorded pass means a review ran over exactly this content. It never means the content is clean.
 Disposing of what a review found is judgment, per `pr-review-conduct`'s five outcomes, so a pass is
@@ -85,6 +85,7 @@
 import subprocess
 import sys
 import tempfile
+import time
 from datetime import UTC, datetime
 from pathlib import Path
 
@@ -174,6 +175,19 @@ def git_dir(root: Path) -> Path:
     return Path(git("rev-parse", "--absolute-git-dir", root=root).strip())
 
 
+def objects_dir(root: Path) -> Path:
+    """The object store this worktree actually reads and writes.
+
+    Asked of git rather than joined onto the git directory. A linked worktree's own git directory
+    holds no `objects` at all, since the store lives in the common directory shared with every
+    other worktree, and the fleet runs every task in a linked worktree. Joining the path by hand
+    names a directory that does not exist there, which silently attaches nothing as an alternate.
+    """
+    return Path(
+        git("rev-parse", "--path-format=absolute", "--git-path", "objects", root=root).strip()
+    )
+
+
 def receipt_path(root: Path) -> Path:
     """Where this worktree's receipt lives.
 
@@ -258,6 +272,17 @@ def index_states(root: Path) -> dict[str, set[str]]:
     return _ls_files_states(git("ls-files", "-s", "-z", root=root))
 
 
+def alternate_entry(path: Path) -> str:
+    """One entry for GIT_ALTERNATE_OBJECT_DIRECTORIES, quoted where it has to be.
+
+    The variable holds a list separated by the platform's path separator, so a repository checked
+    out under a path containing one would otherwise split into two directories that do not exist.
+    Git reads a double-quoted entry as a single path.
+    """
+    text = str(path)
+    return f'"{text}"' if os.pathsep in text else text
+
+
 def worktree_states(root: Path) -> dict[str, set[str]]:
     """Every path as git would stage it right now, read by actually staging it.
 
@@ -267,8 +292,8 @@ def worktree_states(root: Path) -> dict[str, set[str]]:
     keeps git's cached stat information, so the scan is a refresh rather than a full re-hash.
 
     Staging writes objects, and those writes are redirected into a throwaway object directory with
-    the real one as an alternate, so git can still read every existing object while writing none
-    of its own. Without that redirection this read would permanently deposit the content of every
+    the real store attached as an alternate, so git can still read every existing object, and
+    every alternate that store itself chains to, while writing none of its own. Without that redirection this read would permanently deposit the content of every
     untracked file into the repository, an unignored `.env` or key file among them, recoverable
     long afterwards from a command the caller had every reason to believe only looked.
 
@@ -291,7 +316,7 @@ def worktree_states(root: Path) -> dict[str, set[str]]:
         env = {
             "GIT_INDEX_FILE": staging,
             "GIT_OBJECT_DIRECTORY": objects,
-            "GIT_ALTERNATE_OBJECT_DIRECTORIES": str(gd / "objects"),
+            "GIT_ALTERNATE_OBJECT_DIRECTORIES": alternate_entry(objects_dir(root)),
         }
         git("add", "-A", "--", ".", root=root, env=env)
         return _ls_files_states(git("ls-files", "-s", "-z", root=root, env=env))
@@ -412,13 +437,15 @@ def read_receipt(root: Path) -> tuple[dict | None, list[str]]:
     try:
         if not path.is_file():
             return None, []
-        text = path.read_text(encoding="utf-8")
+        raw = path.read_bytes()
     except OSError as e:
         raise CannotRun(f"the receipt at {path} could not be read ({e})") from e
+    # Decoding belongs with parsing rather than with reading.
+    # A non-UTF-8 receipt is unusable content, which is a verdict, not a file this process failed to read.
+    # Decoding inside the block above would raise UnicodeDecodeError past both handlers and report the boundary code.
     try:
-        data = json.loads(text or "{}")
+        data = json.loads(raw.decode("utf-8") or "{}")
     # ValueError rather than JSONDecodeError, since it also covers UnicodeDecodeError.
-    # A partially written or non-UTF-8 file raises that before the JSON parser is reached.
     except ValueError as e:
         return None, [f"the receipt does not parse ({e})"]
     problems = receipt_problems(data)
@@ -449,6 +476,27 @@ def covering_passes(receipt: dict | None, base: str, digest: str, target: str) -
     return [p for p in receipt.get("passes", []) if isinstance(p, dict) and p.get("reviewer")]
 
 
+def held_lock(path: Path, timeout: float = 10.0) -> int:
+    """Take an exclusive lock beside the receipt, or raise once waiting has gone on too long.
+
+    Recording is a read, a merge, and a replace. The replace is atomic on its own, but two
+    backends recording concurrently over one unchanged diff would both read a receipt without the
+    other's pass and the second write would drop it, which is exactly the accumulation this file
+    promises. The lock is what makes the promise true rather than usually true.
+    """
+    lock = Path(str(path) + ".lock")
+    deadline = time.monotonic() + timeout
+    while True:
+        try:
+            return os.open(lock, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600)
+        except FileExistsError:
+            if time.monotonic() >= deadline:
+                raise CannotRun(f"another process holds {lock} and did not release it") from None
+            time.sleep(0.05)
+        except OSError as e:
+            raise CannotRun(f"{lock} could not be created ({e})") from e
+
+
 def write_pass(
     root: Path, target: str, base: str, digest: str, reviewer: str, findings: int | None
 ) -> dict:
@@ -461,6 +509,19 @@ def write_pass(
     The file is replaced rather than rewritten in place, so a reader never observes a half-written
     receipt and a process killed mid-write leaves the previous one intact.
     """
+    path = receipt_path(root)
+    fd_lock = held_lock(path)
+    try:
+        return _write_pass_locked(root, target, base, digest, reviewer, findings)
+    finally:
+        os.close(fd_lock)
+        Path(str(path) + ".lock").unlink(missing_ok=True)
+
+
+def _write_pass_locked(
+    root: Path, target: str, base: str, digest: str, reviewer: str, findings: int | None
+) -> dict:
+    """The read, merge, and replace that `write_pass` holds the lock around."""
     receipt, _ = read_receipt(root)
     kept = [p for p in covering_passes(receipt, base, digest, target) if p["reviewer"] != reviewer]
     kept.append(
@@ -504,13 +565,19 @@ def run_coderabbit(base: str, root: Path) -> tuple[int, str]:
     budget with this account's pull request reviews and a budget exhaustion must never look like a
     review.
     """
-    exe = shutil.which("coderabbit") or shutil.which("cr")
+    # Only the documented binary name.
+    # A bare `cr` is a common short name for unrelated tools.
+    # Running whichever one is on PATH with a 900 second budget is not a fallback worth having.
+    exe = shutil.which("coderabbit")
     if not exe:
         raise CannotRun("the coderabbit CLI is not installed, so this backend cannot run")
     try:
         proc = subprocess.run(
             [exe, "review", "--agent", "--base", base],
             cwd=str(root),
+            # The same redirects the rest of the engine refuses to inherit.
+            # Run from a hook that sets GIT_INDEX_FILE, the CLI would review the wrong tree.
+            env={k: v for k, v in os.environ.items() if k not in INHERITED_REDIRECTS},
             capture_output=True,
             check=False,
             timeout=BACKEND_TIMEOUT,
@@ -590,13 +657,35 @@ def cmd_status(args: argparse.Namespace) -> int:
 
 
 def cmd_record(args: argparse.Namespace) -> int:
-    if args.reviewer not in BACKENDS:
+    backend = BACKENDS.get(args.reviewer)
+    if backend is None:
         known = ", ".join(sorted(BACKENDS))
         print(f"unknown reviewer '{args.reviewer}'. Known backends: {known}", file=sys.stderr)
         return EXIT_CANNOT_RUN
+    if backend["headless"]:
+        print(
+            f"'{args.reviewer}' is headless, so it is recorded by running it rather than by hand."
+            f" Use: run --backend {args.reviewer}",
+            file=sys.stderr,
+        )
+        return EXIT_CANNOT_RUN
     root = repo_root()
     target = resolve_target(args.target)
     base, digest, changed = current_state(target, root)
+    # The digest is read now, and the review finished some time earlier.
+    # A format-on-save or a hook autofix in between would otherwise be stamped as reviewed.
+    # That is content no reviewer saw, and attesting to it is the one claim this receipt makes.
+    # The caller passes back what `status` reported before the pass ran.
+    # A mismatch is the content having moved rather than an answer about coverage.
+    if args.expect_digest and args.expect_digest != digest:
+        print(
+            "the content changed between the review and this record, so nothing was recorded.\n"
+            f"  reviewed  {args.expect_digest[:12]}\n"
+            f"  now       {digest[:12]}\n"
+            "Re-run the review over the current content.",
+            file=sys.stderr,
+        )
+        return EXIT_CANNOT_RUN
     out = write_pass(root, target, base, digest, args.reviewer, args.findings)
     reviewers = ", ".join(p["reviewer"] for p in out["passes"])
     print(
@@ -689,6 +778,11 @@ def main(argv: list[str] | None = None) -> int:
         "--reviewer", required=True, help=f"one of: {', '.join(sorted(BACKENDS))}"
     )
     p_record.add_argument("--findings", type=int, default=None, help="how many findings it raised")
+    p_record.add_argument(
+        "--expect-digest",
+        default=None,
+        help="the contentDigest the review was run against, refusing to record if it has moved",
+    )
     p_record.set_defaults(func=cmd_record)
 
     p_check = sub.add_parser("check", help="exit 0 covered, 1 not covered, 2 could not run")
@@ -703,15 +797,22 @@ def main(argv: list[str] | None = None) -> int:
 
     args = parser.parse_args(argv)
     try:
-        return int(args.func(args))
+        code = int(args.func(args))
+        # Flushed inside the guarded region on purpose.
+        # Piping into a reader that exits first leaves the payload sitting in a block buffer.
+        # That buffer only fails during interpreter shutdown, which reports 120.
+        # The handler below is never reached at all without this flush.
+        sys.stdout.flush()
+        return code
     except CannotRun as e:
         print(f"local_review: {e}", file=sys.stderr)
         return EXIT_CANNOT_RUN
     # A reader that closes early, `| head -1` being the ordinary case, otherwise raises here.
     # It then raises again during the interpreter's own shutdown flush, which exits 120, outside the contract.
     except BrokenPipeError:
-        devnull = os.open(os.devnull, os.O_WRONLY)
-        os.dup2(devnull, sys.stdout.fileno())
+        # Redirected so the interpreter's own shutdown flush has somewhere to go.
+        # Letting it raise again is what turns this into an exit outside the contract.
+        os.dup2(os.open(os.devnull, os.O_WRONLY), sys.stdout.fileno())
         return EXIT_CANNOT_RUN
     # A crash is the check not having run, so it reports the boundary code.
     # Falling through to the interpreter's own exit 1 would read as the not-covered verdict.
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index db1e2d2f..bf07b5f2 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -22,6 +22,8 @@
 import subprocess
 import sys
 import tempfile
+import threading
+import time
 import unittest
 from pathlib import Path
 
@@ -203,80 +205,54 @@ def test_an_unchanged_path_is_not_in_the_key(self) -> None:
         (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
         self.assertEqual(sorted(self.marks()), ["new.py"])
 
-    def test_the_key_depends_on_the_merge_base(self) -> None:
-        """Without this, a build whose digest ignored the base would pass every other case."""
+    def test_current_state_uses_the_real_merge_base(self) -> None:
+        """One half of the claim: the base in the key is the fork point git computes."""
         (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
-        # Drive current_state against two real and different bases.
-        # Passing a made-up base straight to content_digest is passed by any hash that mixes its first argument.
-        run(self.tmp, "add", "-A")
-        run(self.tmp, "commit", "-m", "on task")
-        run(
-            self.tmp,
-            "update-ref",
-            "refs/remotes/origin/main",
-            run(self.tmp, "rev-parse", "HEAD").strip(),
-        )
-        self.assertNotEqual(
-            local_review.merge_base("develop", self.tmp), local_review.merge_base("main", self.tmp)
-        )
-        (self.tmp / "after.py").write_text("print('a')\n", encoding="utf-8")
-        self.assertNotEqual(self.digest("develop"), self.digest("main"))
+        base, _, _ = local_review.current_state(self.target, self.tmp)
+        self.assertEqual(base, run(self.tmp, "merge-base", "origin/develop", "HEAD").strip())
 
-    def test_an_unmerged_path_keeps_every_stage(self) -> None:
-        """Keeping only the last stage reduces a conflicted index to one side of the conflict.
+    def test_the_digest_mixes_the_merge_base(self) -> None:
+        """The other half: two different bases over one identical mark set differ.
 
-        A pass recorded mid-conflict would then carry the same key as the resolved tree, so it
-        would cover a resolution nobody reviewed.
+        Asserting only that two targets give different digests is weaker than it looks, since the
+        changed-path sets usually differ too and the digest would move on that alone.
         """
-        clean = self.digest()
-        run(self.tmp, "checkout", "-b", "side")
-        (self.tmp / "c.txt").write_text("theirs\n", encoding="utf-8")
-        run(self.tmp, "add", "c.txt")
-        run(self.tmp, "commit", "-m", "theirs")
-        run(self.tmp, "checkout", "task")
-        (self.tmp / "c.txt").write_text("ours\n", encoding="utf-8")
-        run(self.tmp, "add", "c.txt")
-        run(self.tmp, "commit", "-m", "ours")
-        subprocess.run(
-            ["git", "merge", "side"], cwd=str(self.tmp), capture_output=True, check=False
-        )
-        states = local_review.index_states(self.tmp)["c.txt"]
-        self.assertGreaterEqual(len(states), 2, f"stages collapsed to {states}")
-        # The property that matters is the digest rather than the parse.
-        # A conflicted tree must not carry the key of any resolution of it.
-        # Otherwise a pass taken mid-conflict covers the resolution nobody reviewed.
-        conflicted = self.digest()
-        run(self.tmp, "checkout", "--theirs", "--", "c.txt")
-        run(self.tmp, "add", "c.txt")
-        self.assertNotEqual(
-            self.digest(), conflicted, "resolving the conflict did not move the key"
-        )
-        self.assertNotEqual(self.digest(), clean)
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        base, digest, _ = local_review.current_state(self.target, self.tmp)
+        marks = local_review.fingerprints(base, self.tmp)
+        other = local_review.content_digest("0" * 40, marks)
+        self.assertNotEqual(digest, other)
 
-    def test_a_symlink_is_keyed_by_its_target_not_its_content(self) -> None:
-        """Reading through a link would fold bytes from outside the reviewed tree into the key."""
+    def test_a_symlink_is_keyed_as_a_link_not_as_its_pointee(self) -> None:
+        """git stores a link as its target path, so the key must not follow it out of the tree."""
         outside = self.outside / "outside.txt"
         outside.write_text("secret\n", encoding="utf-8")
         try:
             (self.tmp / "link").symlink_to(outside)
         except (OSError, NotImplementedError):
             self.skipTest("this platform does not permit creating a symlink")
+        mark = self.marks().get("link", "")
+        if "120000:" not in mark:
+            self.skipTest(f"this checkout does not store links as links: {mark}")
         before = self.digest()
-        self.assertIn("120000:", self.marks()["link"])
-        # Rewriting the bytes at the far end must not move the key.
-        # Those bytes are outside the tree and were never under review.
         outside.write_text("different content entirely\n", encoding="utf-8")
-        self.assertEqual(self.digest(), before, "the key followed the link")
+        self.assertEqual(self.digest(), before, "the key followed the link out of the tree")
+
+    def test_a_path_that_is_not_valid_utf8_is_keyed_under_its_real_name(self) -> None:
+        """A name holding a raw high byte is what exercises the decoding, and a valid one is not.
 
-    def test_a_non_ascii_path_is_keyed_under_its_real_name(self) -> None:
-        """git quotes such a path by default, so the key could carry a name that is not on disk."""
-        # Escaped rather than literal so this source file stays ASCII.
-        # The name on disk is still non-ASCII, which is the property under test.
-        name = "docs/caf\u00e9.md"
+        A merely non-ASCII name such as an accented one is valid UTF-8, so it round-trips under a
+        strict decoder and proves nothing about the surrogateescape handling the engine relies on.
+        """
+        raw = b"docs/broken-\xff-name.md"
         (self.tmp / "docs").mkdir()
-        (self.tmp / name).write_text("x\n", encoding="utf-8")
+        try:
+            (self.tmp / os.fsdecode(raw)).write_bytes(b"x\n")
+        except (OSError, UnicodeError):
+            self.skipTest("this filesystem does not accept a non-UTF-8 name")
+        name = os.fsdecode(raw)
         self.assertIn(name, self.marks())
-        run(self.tmp, "add", "--", name)
+        run(self.tmp, "add", "-A")
         run(self.tmp, "commit", "-m", "add")
         self.assertIn(name, self.marks())
 
@@ -305,7 +281,17 @@ def test_a_submodule_bump_is_a_state_not_a_crash(self) -> None:
         # Adding a submodule is not bumping one, and only the bump exercises the gitlink state.
         (sub / "f.txt").write_text("moved on\n", encoding="utf-8")
         run(sub, "commit", "-am", "advance")
-        run(self.tmp / "sub", "pull", "--ff-only", str(sub), "main")
+        # The same file-protocol allowance the add above needed.
+        # Without it this call fails outright on a git that disables the protocol by default.
+        run(
+            self.tmp / "sub",
+            "-c",
+            "protocol.file.allow=always",
+            "pull",
+            "--ff-only",
+            str(sub),
+            "main",
+        )
         self.assertNotEqual(self.digest(), before, "a submodule bump did not move the key")
 
     def test_staging_the_worktree_writes_nothing_into_the_object_database(self) -> None:
@@ -327,6 +313,27 @@ def test_staging_the_worktree_writes_nothing_into_the_object_database(self) -> N
         )
         self.assertNotEqual(probe.returncode, 0, "the untracked file is readable from the repo")
 
+    def test_the_object_store_resolves_inside_a_linked_worktree(self) -> None:
+        """The fleet runs every task in a linked worktree, whose git directory holds no objects.
+
+        The store lives in the common directory there, so joining `objects` onto the worktree's own
+        git directory names a path that does not exist and attaches nothing as an alternate. The
+        containment case above cannot catch this: its fixture is a primary checkout, where the
+        joined path happens to be right.
+        """
+        linked = self.outside / "linked-tree"
+        run(self.tmp, "worktree", "add", "-b", "linked-task", str(linked))
+        self.addCleanup(run, self.tmp, "worktree", "remove", "--force", str(linked))
+        self.assertFalse(
+            (local_review.git_dir(linked) / "objects").exists(),
+            "fixture is not the case: this worktree has its own objects directory",
+        )
+        store = local_review.objects_dir(linked)
+        self.assertTrue(store.is_dir(), f"the object store did not resolve: {store}")
+        # And the read still works from inside it, which a broken alternate would not guarantee.
+        (linked / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.assertIn("new.py", local_review.worktree_states(linked))
+
     def test_staging_leaves_no_throwaway_paths_behind(self) -> None:
         (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
         local_review.worktree_states(self.tmp)
@@ -490,6 +497,19 @@ def test_an_unparseable_receipt_gives_a_verdict_not_a_traceback(self) -> None:
         self.assertTrue(problems)
         self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
 
+    def test_a_non_utf8_receipt_is_a_verdict_not_a_boundary(self) -> None:
+        """Unusable content is a real answer about coverage, unlike a file that cannot be read.
+
+        Decoding inside the block guarding the disk read raises UnicodeDecodeError past both
+        handlers, so the boundary code is reported for a receipt that was read perfectly well.
+        """
+        (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        local_review.receipt_path(self.tmp).write_bytes(b"\xff\xfe not utf-8 at all")
+        receipt, problems = local_review.read_receipt(self.tmp)
+        self.assertIsNone(receipt)
+        self.assertTrue(any("parse" in p for p in problems), problems)
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
+
     def test_a_malformed_pass_entry_is_caught_before_it_is_formatted(self) -> None:
         """A non-string reviewer name would otherwise reach a formatting line and raise."""
         problems = local_review.receipt_problems(
@@ -514,6 +534,99 @@ def test_an_unreadable_receipt_file_is_a_boundary_not_a_verdict(self) -> None:
             self.skipTest("this process can read a mode-000 file, so the case cannot be built")
         self.assertEqual(self.main_quiet(["check", "--target", self.target]), 2)
 
+    def test_record_refuses_content_that_moved_since_the_review(self) -> None:
+        """The load-bearing path: a pass must attest to what a reviewer actually saw.
+
+        Nothing binds the moment the review finished to the moment it is recorded, so a
+        format-on-save or a hook autofix in between would otherwise be stamped as reviewed.
+        """
+        (self.tmp / "a.py").write_text("reviewed content\n", encoding="utf-8")
+        reviewed = self.digest()
+        (self.tmp / "a.py").write_text("something else entirely\n", encoding="utf-8")
+        self.assertEqual(
+            self.main_quiet(["record", "--reviewer", "agent-skill", "--expect-digest", reviewed]),
+            2,
+        )
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
+
+    def test_record_accepts_the_digest_the_review_saw(self) -> None:
+        (self.tmp / "a.py").write_text("reviewed content\n", encoding="utf-8")
+        reviewed = self.digest()
+        self.assertEqual(
+            self.main_quiet(["record", "--reviewer", "agent-skill", "--expect-digest", reviewed]),
+            0,
+        )
+
+    def test_record_will_not_forge_a_headless_backend(self) -> None:
+        """Recording one by hand bypasses the completion check that makes its pass mean anything."""
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        self.assertEqual(self.main_quiet(["record", "--reviewer", "coderabbit-cli"]), 2)
+
+    def test_concurrent_records_do_not_drop_a_pass(self) -> None:
+        """Accumulation is a read, a merge, and a replace, so it needs more than an atomic write."""
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        base, digest, _ = local_review.current_state(self.target, self.tmp)
+        real = local_review._write_pass_locked
+
+        def slow(*args: object, **kwargs: object) -> dict:
+            # Widen the read-modify-write window the lock exists to close.
+            time.sleep(0.2)
+            return real(*args, **kwargs)  # type: ignore[arg-type]
+
+        local_review._write_pass_locked = slow  # type: ignore[assignment]
+        self.addCleanup(setattr, local_review, "_write_pass_locked", real)
+        done: list[dict] = []
+        threads = [
+            threading.Thread(
+                target=lambda r=r: done.append(
+                    local_review.write_pass(self.tmp, self.target, base, digest, r, 0)
+                )
+            )
+            for r in ("agent-skill", "coderabbit-cli")
+        ]
+        for t in threads:
+            t.start()
+        for t in threads:
+            t.join()
+        receipt, problems = local_review.read_receipt(self.tmp)
+        self.assertEqual(problems, [])
+        assert receipt is not None
+        self.assertEqual(
+            sorted(p["reviewer"] for p in receipt["passes"]),
+            ["agent-skill", "coderabbit-cli"],
+            "a concurrent record dropped the other pass",
+        )
+
+    def test_an_ignored_file_stays_out_of_the_key(self) -> None:
+        """Otherwise build output would demand a review, and the docstring claims it does not."""
+        (self.tmp / ".gitignore").write_text("build/\n", encoding="utf-8")
+        run(self.tmp, "add", ".gitignore")
+        run(self.tmp, "commit", "-m", "ignore build")
+        before = self.digest()
+        (self.tmp / "build").mkdir()
+        (self.tmp / "build" / "out.o").write_text("junk\n", encoding="utf-8")
+        self.assertEqual(self.digest(), before, "an ignored file entered the key")
+
+    def test_adding_then_deleting_a_file_keeps_the_key(self) -> None:
+        """The documented scope boundary: the key follows net content, not the commit series.
+
+        Asserted rather than left implicit, because it is the property a reader is most likely to
+        assume the other way round.
+        """
+        before = self.digest()
+        (self.tmp / "temp.py").write_text("print('x')\n", encoding="utf-8")
+        run(self.tmp, "add", "temp.py")
+        run(self.tmp, "commit", "-m", "add temp")
+        self.assertNotEqual(self.digest(), before)
+        run(self.tmp, "rm", "-q", "temp.py")
+        run(self.tmp, "commit", "-m", "remove temp")
+        self.assertEqual(self.digest(), before, "the net-content scope is not what is documented")
+
+    def test_two_staging_runs_over_unchanged_content_agree(self) -> None:
+        """Recording then checking stages the same content twice, so the read has to be stable."""
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        self.assertEqual(self.digest(), self.digest())
+
     def test_a_future_receipt_version_reads_as_unusable(self) -> None:
         problems = local_review.receipt_problems(
             {
@@ -544,14 +657,19 @@ def test_a_linked_worktree_keeps_its_own_receipt(self) -> None:
 class BackendCase(RepoCase):
     """Drives run_coderabbit against a stand-in binary, so the parsing contract is tested."""
 
-    def fake_cli(self, stdout: str, code: int = 0) -> None:
+    def fake_cli(
+        self, stdout: str, code: int = 0, argv_log: Path | None = None, also: str = ""
+    ) -> None:
         bin_dir = self.outside / "fakebin"
         bin_dir.mkdir(exist_ok=True)
         script = bin_dir / "coderabbit"
-        script.write_text(
-            f"#!/usr/bin/env python3\nimport sys\nsys.stdout.write({stdout!r})\nsys.exit({code})\n",
-            encoding="utf-8",
-        )
+        body = "#!/usr/bin/env python3\nimport sys\n"
+        if argv_log is not None:
+            body += f"open({str(argv_log)!r}, 'w').write(chr(10).join(sys.argv[1:]))\n"
+        if also:
+            body += also + "\n"
+        body += f"sys.stdout.write({stdout!r})\nsys.exit({code})\n"
+        script.write_text(body, encoding="utf-8")
         script.chmod(0o755)
         prev = os.environ.get("PATH", "")
         os.environ["PATH"] = f"{bin_dir}{os.pathsep}{prev}"
@@ -607,6 +725,34 @@ def test_a_completed_run_records_a_pass_through_main(self) -> None:
         assert receipt is not None
         self.assertEqual(receipt["passes"][0]["findings"], 1)
 
+    def test_the_backend_is_given_the_merge_base_not_the_target_tip(self) -> None:
+        """The two differ the moment the target moves, and only one is what the receipt claims.
+
+        The fake CLI records its own argv, since asserting on the exit code alone would let the
+        argument contract drift back to the tip with nothing objecting.
+        """
+        argv_log = self.outside / "argv.txt"
+        self.fake_cli('{"type":"complete"}\n', argv_log=argv_log)
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        base, _, _ = local_review.current_state(self.target, self.tmp)
+        self.assertEqual(
+            self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 0
+        )
+        argv = argv_log.read_text(encoding="utf-8").split("\n")
+        self.assertIn("--agent", argv)
+        self.assertIn(base, argv, f"the backend was not given the merge base: {argv}")
+        self.assertNotIn("origin/develop", argv, "the backend was given the target tip")
+
+    def test_content_changing_during_a_run_is_a_verdict(self) -> None:
+        """The review covered content that no longer exists, which is an answer, not a boundary."""
+        marker = self.tmp / "moved.py"
+        self.fake_cli('{"type":"complete"}\n', also=f"open({str(marker)!r}, 'w').write('later\\n')")
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        self.assertEqual(
+            self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 1
+        )
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
+
     def test_a_missing_binary_is_a_boundary_through_main(self) -> None:
         prev = os.environ.get("PATH", "")
         os.environ["PATH"] = str(self.tmp / "definitely-empty")

From 2f01d9c2a44b0259bac3229eaa24a362b7c9906d Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sat, 29 Aug 2026 21:36:23 -0700
Subject: [PATCH 03/12] Refuse a CodeRabbit CLI Run Whose Scope It May Not
 Cover

A reviewer found that `run_coderabbit` invokes a `--base` review, which covers
the tracked diff, while the receipt's own scope includes non-ignored untracked
files. A pass could therefore claim coverage of files the CLI never read, and a
changed set of only untracked files would record a clean review of nothing.

The CLI documents `--include-untracked` alongside `--uncommitted`, but whether
that composes with `--base` is not something this host can establish: there is no
CLI here to run it against, and asserting a tool's behavior that was never
executed is exactly what the fleet's own verification rules forbid. So the case
is refused rather than guessed at. `run` now stops while any untracked path is in
scope, naming the first one and pointing at the agent-skill backend, and the
restriction is documented as needing a real CLI and a test before it is lifted.

Part of #1104 and #1083.
---
 scripts/README.md                  |  2 +-
 scripts/local_review.py            | 20 ++++++++++++++++++++
 scripts/tests/test_local_review.py | 30 ++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/scripts/README.md b/scripts/README.md
index b21cdf48..e02210e4 100644
--- a/scripts/README.md
+++ b/scripts/README.md
@@ -230,7 +230,7 @@ What the key covers is the net content the branch introduces against its target,
 
 `record` takes `--expect-digest`, the digest `status` reported before the pass ran, and refuses when the content has moved since. Nothing else binds the moment a review finished to the moment it is recorded, so a format-on-save or a hook autofix in between would otherwise be stamped as reviewed content no reviewer saw, which is the one claim the receipt exists to make. A headless backend is recorded by running it rather than by hand, since recording it directly would bypass the completion check that makes its pass mean anything.
 
-It holds no review logic. It drives backends: `agent-skill` is the `local-strict-review` subagent pass, which only a live agent session can run, so the engine records it rather than invoking it, and `coderabbit-cli` is headless and is the one backend a git hook can execute by itself. A CodeRabbit CLI run draws on the same hourly budget as this account's pull request reviews, so a run that is rate limited, errors, or ends without a completion event records no pass rather than a clean one. A recorded pass means a review ran over exactly this content, never that the content is clean: disposing of what a review found is judgment, per the `pr-review-conduct` Skill's five outcomes, so a pass is recorded whether it raised findings or none.
+It holds no review logic. It drives backends: `agent-skill` is the `local-strict-review` subagent pass, which only a live agent session can run, so the engine records it rather than invoking it, and `coderabbit-cli` is headless and is the one backend a git hook can execute by itself. A CodeRabbit CLI run draws on the same hourly budget as this account's pull request reviews, so a run that is rate limited, errors, or ends without a completion event records no pass rather than a clean one. It also refuses outright while any non-ignored untracked path is in the review scope: the receipt covers those, a `--base` review covers the tracked diff, and whether the CLI's own untracked flag composes with `--base` has not been verified against a real CLI, so a pass there would claim coverage the run may not have. Lifting that restriction needs the binary and a test that drives it. A recorded pass means a review ran over exactly this content, never that the content is clean: disposing of what a review found is judgment, per the `pr-review-conduct` Skill's five outcomes, so a pass is recorded whether it raised findings or none.
 
 ```shell
 python3 scripts/local_review.py status                                      # what covers the current content. 0 either way, 2 if it cannot run
diff --git a/scripts/local_review.py b/scripts/local_review.py
index 4c074a76..f4d9c06f 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -550,6 +550,11 @@ def _write_pass_locked(
     return out
 
 
+def untracked_in(root: Path) -> list[str]:
+    """The non-ignored untracked paths, which the receipt's own scope includes."""
+    return _zsplit(git("ls-files", "--others", "--exclude-standard", "-z", root=root))
+
+
 def run_coderabbit(base: str, root: Path) -> tuple[int, str]:
     """Run the CodeRabbit CLI over this branch's diff and return the findings count and an error.
 
@@ -571,6 +576,21 @@ def run_coderabbit(base: str, root: Path) -> tuple[int, str]:
     exe = shutil.which("coderabbit")
     if not exe:
         raise CannotRun("the coderabbit CLI is not installed, so this backend cannot run")
+    # The receipt's scope includes non-ignored untracked files.
+    # A `--base` review covers the tracked diff instead.
+    # Recording a pass over a changed set holding untracked paths would claim coverage of files the CLI never read.
+    # The CLI documents `--include-untracked` alongside `--uncommitted`.
+    # Whether that composes with `--base` is unverified, since this host carries no CLI to run it against.
+    # Asserting a tool's behavior that was never executed is what `agent-conduct` forbids.
+    # So the case is refused rather than guessed at.
+    # Lifting this restriction needs a real CLI and a test that drives it.
+    untracked = untracked_in(root)
+    if untracked:
+        raise CannotRun(
+            f"{len(untracked)} untracked path(s) are in the review scope, and this backend's"
+            " coverage of them is unverified. Commit or stage them, or use the agent-skill"
+            f" backend. First: {untracked[0]}"
+        )
     try:
         proc = subprocess.run(
             [exe, "review", "--agent", "--base", base],
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index bf07b5f2..6398ed48 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -99,6 +99,10 @@ def record(self, reviewer: str, findings: int | None = 0, target: str | None = N
         base, digest, _ = local_review.current_state(t, self.tmp)
         return local_review.write_pass(self.tmp, t, base, digest, reviewer, findings)
 
+    def commit_all(self, message: str) -> None:
+        run(self.tmp, "add", "-A")
+        run(self.tmp, "commit", "-m", message)
+
     def main_quiet(self, argv: list[str]) -> int:
         """Run the CLI with its diagnostics captured, for a case asserting only the exit code."""
         with contextlib.redirect_stderr(io.StringIO()), contextlib.redirect_stdout(io.StringIO()):
@@ -717,6 +721,7 @@ def test_a_completed_run_records_a_pass_through_main(self) -> None:
         """The whole cmd_run path, including the digest re-check and the receipt write."""
         self.fake_cli('{"type":"finding"}\n{"type":"complete"}\n')
         (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
+        self.commit_all("new")
         self.assertEqual(
             self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 0
         )
@@ -734,6 +739,7 @@ def test_the_backend_is_given_the_merge_base_not_the_target_tip(self) -> None:
         argv_log = self.outside / "argv.txt"
         self.fake_cli('{"type":"complete"}\n', argv_log=argv_log)
         (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        self.commit_all("a")
         base, _, _ = local_review.current_state(self.target, self.tmp)
         self.assertEqual(
             self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 0
@@ -748,11 +754,35 @@ def test_content_changing_during_a_run_is_a_verdict(self) -> None:
         marker = self.tmp / "moved.py"
         self.fake_cli('{"type":"complete"}\n', also=f"open({str(marker)!r}, 'w').write('later\\n')")
         (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        self.commit_all("a")
         self.assertEqual(
             self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 1
         )
         self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
 
+    def test_the_cli_backend_refuses_a_scope_it_may_not_cover(self) -> None:
+        """The receipt scope includes untracked files, and a --base review covers the tracked diff.
+
+        Recording a pass over a changed set holding untracked paths would claim coverage of files
+        the CLI never read. Whether the CLI's own untracked flag composes with --base is unverified
+        on this host, so the case is refused rather than assumed either way.
+        """
+        self.fake_cli('{"type":"complete"}\n')
+        (self.tmp / "untracked.py").write_text("print('x')\n", encoding="utf-8")
+        self.assertEqual(
+            self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 2
+        )
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
+
+    def test_the_cli_backend_runs_once_nothing_is_untracked(self) -> None:
+        self.fake_cli('{"type":"complete"}\n')
+        (self.tmp / "tracked.py").write_text("print('x')\n", encoding="utf-8")
+        run(self.tmp, "add", "tracked.py")
+        run(self.tmp, "commit", "-m", "tracked")
+        self.assertEqual(
+            self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 0
+        )
+
     def test_a_missing_binary_is_a_boundary_through_main(self) -> None:
         prev = os.environ.get("PATH", "")
         os.environ["PATH"] = str(self.tmp / "definitely-empty")

From 7d7e26e9c69ac926de76b054e28c9ab34403e5e4 Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sat, 29 Aug 2026 21:40:08 -0700
Subject: [PATCH 04/12] Follow the CodeRabbit CLI's Documented Flag Semantics

A reviewer ran a web query against the CLI's own documentation and surfaced
three things this host cannot establish by execution, one of them a defect the
previous commit introduced.

`--base` is documented as taking a branch name and `--base-commit` as taking a
commit hash, so handing a merge-base sha to `--base` was wrong. It is
`--base-commit` now, and the test asserts the flag rather than only the value,
which is why the earlier version passed while being incorrect.

The CLI excludes untracked files by default, and `--include-untracked` is the
documented way to include them, which the receipt's own scope requires. That
replaces the previous commit's blanket refusal of any untracked scope: the
refusal was the right call while the flag semantics were unknown, and the
vendor's documentation is better evidence than the guess it was avoiding.

A run that finds nothing still completes, reporting `review_skipped` with zero
findings, which the parser accepted as a real review. It is refused now, for the
same reason a missing completion event is.

None of this has been executed against a real CLI on this host, so the
invocation follows documented semantics rather than observed behavior, and the
backend stays opt-in and labelled as such.

Part of #1104 and #1083.
---
 scripts/README.md                  |  2 +-
 scripts/local_review.py            | 43 ++++++++++++++----------------
 scripts/tests/test_local_review.py | 39 +++++++++++++++------------
 3 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/scripts/README.md b/scripts/README.md
index e02210e4..3c4a9148 100644
--- a/scripts/README.md
+++ b/scripts/README.md
@@ -230,7 +230,7 @@ What the key covers is the net content the branch introduces against its target,
 
 `record` takes `--expect-digest`, the digest `status` reported before the pass ran, and refuses when the content has moved since. Nothing else binds the moment a review finished to the moment it is recorded, so a format-on-save or a hook autofix in between would otherwise be stamped as reviewed content no reviewer saw, which is the one claim the receipt exists to make. A headless backend is recorded by running it rather than by hand, since recording it directly would bypass the completion check that makes its pass mean anything.
 
-It holds no review logic. It drives backends: `agent-skill` is the `local-strict-review` subagent pass, which only a live agent session can run, so the engine records it rather than invoking it, and `coderabbit-cli` is headless and is the one backend a git hook can execute by itself. A CodeRabbit CLI run draws on the same hourly budget as this account's pull request reviews, so a run that is rate limited, errors, or ends without a completion event records no pass rather than a clean one. It also refuses outright while any non-ignored untracked path is in the review scope: the receipt covers those, a `--base` review covers the tracked diff, and whether the CLI's own untracked flag composes with `--base` has not been verified against a real CLI, so a pass there would claim coverage the run may not have. Lifting that restriction needs the binary and a test that drives it. A recorded pass means a review ran over exactly this content, never that the content is clean: disposing of what a review found is judgment, per the `pr-review-conduct` Skill's five outcomes, so a pass is recorded whether it raised findings or none.
+It holds no review logic. It drives backends: `agent-skill` is the `local-strict-review` subagent pass, which only a live agent session can run, so the engine records it rather than invoking it, and `coderabbit-cli` is headless and is the one backend a git hook can execute by itself. A CodeRabbit CLI run draws on the same hourly budget as this account's pull request reviews, so a run that is rate limited, errors, or ends without a completion event records no pass rather than a clean one. The invocation follows the CLI's documented flag semantics: `--base-commit` for the merge-base hash, since `--base` takes a branch name, and `--include-untracked`, since the CLI excludes untracked files by default while this receipt's scope includes them. A completion reporting `review_skipped` is refused too, that being the CLI saying it looked at nothing. None of it has been executed against a real CLI on this host, so the backend stays opt-in until someone runs it. A recorded pass means a review ran over exactly this content, never that the content is clean: disposing of what a review found is judgment, per the `pr-review-conduct` Skill's five outcomes, so a pass is recorded whether it raised findings or none.
 
 ```shell
 python3 scripts/local_review.py status                                      # what covers the current content. 0 either way, 2 if it cannot run
diff --git a/scripts/local_review.py b/scripts/local_review.py
index f4d9c06f..3e1c1caf 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -550,11 +550,6 @@ def _write_pass_locked(
     return out
 
 
-def untracked_in(root: Path) -> list[str]:
-    """The non-ignored untracked paths, which the receipt's own scope includes."""
-    return _zsplit(git("ls-files", "--others", "--exclude-standard", "-z", root=root))
-
-
 def run_coderabbit(base: str, root: Path) -> tuple[int, str]:
     """Run the CodeRabbit CLI over this branch's diff and return the findings count and an error.
 
@@ -563,9 +558,20 @@ def run_coderabbit(base: str, root: Path) -> tuple[int, str]:
     moment the target moves: reviewing against the tip would cover the reverse of every commit the
     target gained since this branch forked, which is not what the receipt claims.
 
+    The base is passed as `--base-commit`, which the CLI documents as taking a commit hash, rather
+    than `--base`, which it documents as taking a branch name. Handing a merge-base sha to the
+    branch flag was the earlier mistake here. `--include-untracked` is passed because the CLI
+    excludes untracked files by default while this receipt's own scope includes them, so without
+    it a pass would claim coverage of files the run never read.
+
     A completion event is required rather than assumed. A CLI build that does not understand
     `--agent`, or a run truncated part way, emits no findings and exits zero, which would
-    otherwise be recorded as a clean review of content nothing read. A rate-limited or errored run
+    otherwise be recorded as a clean review of content nothing read. A completion reporting
+    `review_skipped` is refused for the same reason: it is the CLI saying it looked at nothing.
+
+    None of this has been executed against a real CLI on this host, so the invocation follows the
+    vendor's documented flag semantics rather than observed behavior, and the backend stays opt-in
+    until someone runs it. A rate-limited or errored run
     likewise returns an error and the caller records no pass, since the CLI shares its hourly
     budget with this account's pull request reviews and a budget exhaustion must never look like a
     review.
@@ -576,24 +582,9 @@ def run_coderabbit(base: str, root: Path) -> tuple[int, str]:
     exe = shutil.which("coderabbit")
     if not exe:
         raise CannotRun("the coderabbit CLI is not installed, so this backend cannot run")
-    # The receipt's scope includes non-ignored untracked files.
-    # A `--base` review covers the tracked diff instead.
-    # Recording a pass over a changed set holding untracked paths would claim coverage of files the CLI never read.
-    # The CLI documents `--include-untracked` alongside `--uncommitted`.
-    # Whether that composes with `--base` is unverified, since this host carries no CLI to run it against.
-    # Asserting a tool's behavior that was never executed is what `agent-conduct` forbids.
-    # So the case is refused rather than guessed at.
-    # Lifting this restriction needs a real CLI and a test that drives it.
-    untracked = untracked_in(root)
-    if untracked:
-        raise CannotRun(
-            f"{len(untracked)} untracked path(s) are in the review scope, and this backend's"
-            " coverage of them is unverified. Commit or stage them, or use the agent-skill"
-            f" backend. First: {untracked[0]}"
-        )
     try:
         proc = subprocess.run(
-            [exe, "review", "--agent", "--base", base],
+            [exe, "review", "--agent", "--base-commit", base, "--include-untracked"],
             cwd=str(root),
             # The same redirects the rest of the engine refuses to inherit.
             # Run from a hook that sets GIT_INDEX_FILE, the CLI would review the wrong tree.
@@ -623,7 +614,13 @@ def run_coderabbit(base: str, root: Path) -> tuple[int, str]:
         if kind == "finding":
             findings += 1
         elif kind == "complete":
-            complete = True
+            # A run that found nothing to look at also completes, reporting review_skipped.
+            # Counting that as a review records a clean pass over content nothing read.
+            # That is the same false clean the missing-completion case below guards against.
+            if str(event.get("status") or "") == "review_skipped":
+                error = "the CLI reported review_skipped, so no content was actually reviewed"
+            else:
+                complete = True
         elif kind == "error":
             error = str(event.get("message") or "the CLI reported an error event")
     if proc.returncode != 0 and not error:
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index 6398ed48..086c239b 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -721,7 +721,6 @@ def test_a_completed_run_records_a_pass_through_main(self) -> None:
         """The whole cmd_run path, including the digest re-check and the receipt write."""
         self.fake_cli('{"type":"finding"}\n{"type":"complete"}\n')
         (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
-        self.commit_all("new")
         self.assertEqual(
             self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 0
         )
@@ -748,6 +747,10 @@ def test_the_backend_is_given_the_merge_base_not_the_target_tip(self) -> None:
         self.assertIn("--agent", argv)
         self.assertIn(base, argv, f"the backend was not given the merge base: {argv}")
         self.assertNotIn("origin/develop", argv, "the backend was given the target tip")
+        # The flag name matters as much as the value.
+        # The CLI documents --base as taking a branch and --base-commit as taking a commit hash.
+        # A sha handed to --base is the wrong call even though the sha itself is right.
+        self.assertEqual(argv[argv.index(base) - 1], "--base-commit", argv)
 
     def test_content_changing_during_a_run_is_a_verdict(self) -> None:
         """The review covered content that no longer exists, which is an answer, not a boundary."""
@@ -760,28 +763,30 @@ def test_content_changing_during_a_run_is_a_verdict(self) -> None:
         )
         self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
 
-    def test_the_cli_backend_refuses_a_scope_it_may_not_cover(self) -> None:
-        """The receipt scope includes untracked files, and a --base review covers the tracked diff.
+    def test_the_backend_is_asked_to_include_untracked_files(self) -> None:
+        """The receipt scope includes them and the CLI excludes them by default.
 
-        Recording a pass over a changed set holding untracked paths would claim coverage of files
-        the CLI never read. Whether the CLI's own untracked flag composes with --base is unverified
-        on this host, so the case is refused rather than assumed either way.
+        Without the flag a pass would claim coverage of files the run never read, and a changed
+        set of only untracked files would record a clean review of nothing.
         """
-        self.fake_cli('{"type":"complete"}\n')
+        argv_log = self.outside / "argv2.txt"
+        self.fake_cli('{"type":"complete"}\n', argv_log=argv_log)
         (self.tmp / "untracked.py").write_text("print('x')\n", encoding="utf-8")
-        self.assertEqual(
-            self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 2
-        )
-        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
-
-    def test_the_cli_backend_runs_once_nothing_is_untracked(self) -> None:
-        self.fake_cli('{"type":"complete"}\n')
-        (self.tmp / "tracked.py").write_text("print('x')\n", encoding="utf-8")
-        run(self.tmp, "add", "tracked.py")
-        run(self.tmp, "commit", "-m", "tracked")
         self.assertEqual(
             self.main_quiet(["run", "--backend", "coderabbit-cli", "--target", self.target]), 0
         )
+        self.assertIn("--include-untracked", argv_log.read_text(encoding="utf-8").split("\n"))
+
+    def test_a_skipped_review_is_not_a_review(self) -> None:
+        """The CLI completes with review_skipped when it found nothing to look at.
+
+        Counting that as a review records a clean pass over content nothing read, which is the
+        same false clean the missing-completion case guards against.
+        """
+        self.fake_cli('{"type":"complete","status":"review_skipped","findings":0}\n')
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        _, error = local_review.run_coderabbit("HEAD", self.tmp)
+        self.assertIn("review_skipped", error)
 
     def test_a_missing_binary_is_a_boundary_through_main(self) -> None:
         prev = os.environ.get("PATH", "")

From 20e2089a0d44f89698b1c26c1c645ccd267d9d8e Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sat, 29 Aug 2026 21:44:16 -0700
Subject: [PATCH 05/12] Point at the Canonical Review Rule Instead of Restating
 It

Style findings from the pull request, each checked against the rule it cites
rather than taken on assertion.

The module docstring and the README restated the before-push review obligation
that AGENTS.md and the local-strict-review Skill already hold. AGENTS.md is
explicit that cross-cutting rules are not restated elsewhere, so both now point
at it and keep only what is specific to this tool.

The comment above BACKENDS ran to three lines where the comment rule makes one
the default and earns a second only with a constraint the code cannot carry. It
is two lines now, and they carry the constraint.

Both modules dropped `from __future__ import annotations`. The Python style
reference allows it only where a forward reference needs it, and neither module
has one, since the project targets 3.13 where every annotation used here is
native.

Each type-ignore now names the constraint requiring it rather than only its
diagnostic code.

Declined, with precedent: a finding that worktree_states' docstring should move
its rationale into inline comments. This repository's own convention is a
docstring that explains why and not only what, and build_dist.py's tree_digest
is the nearest sibling doing exactly that.

Part of #1104 and #1083.
---
 scripts/README.md                  |  2 +-
 scripts/local_review.py            | 18 +++++++-----------
 scripts/tests/test_local_review.py |  8 ++++++--
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/scripts/README.md b/scripts/README.md
index 3c4a9148..3237eafd 100644
--- a/scripts/README.md
+++ b/scripts/README.md
@@ -220,7 +220,7 @@ The match is on the block's heading rather than anywhere in the body, and on the
 
 ## `local_review.py`
 
-Records and verifies that a local review pass covered a branch's current content, so the fleet's review-before-every-push rule has something a capture point can actually check. That rule was already written in three Skills and still went unfollowed, which is the gap this closes: a pass records a receipt keyed on the content it reviewed, and a capture point asks whether that receipt still covers what is about to be pushed. The capture points themselves (a git pre-push hook, the Skill steps that invoke the pass) are a separate change, so this script is the capability they will call rather than a gate that is already wired up.
+Records and verifies that a local review pass covered a branch's current content, so the review-before-every-push rule [`AGENTS.md`][agents] and the `local-strict-review` Skill hold has something a capture point can actually check rather than a second statement of the rule itself. A pass records a receipt keyed on the content it reviewed, and a capture point asks whether that receipt still covers what is about to be pushed. The capture points themselves (a git pre-push hook, the Skill steps that invoke the pass) are a separate change, so this script is the capability they will call rather than a gate that is already wired up.
 
 The key is over the content of the changed files, plus the merge-base commit that fixes what "changed" means. It is deliberately not over the diff text and not over `HEAD`, which is what lets a review run before the commit while the check runs at the push: reviewing untracked work and then committing it unchanged leaves the key identical. Each path contributes the state it is in on each side, the working tree's and the index's, both as git's own mode-and-blob identity so the two compare directly. Where they agree the path reads as one state, which is what makes `git add` and `git commit` invisible. Where they diverge, because content is staged that the working tree no longer holds, or because a file mode changed, the key moves. Reading the index rather than the working tree alone is what stops content being committed that the key never saw.
 
diff --git a/scripts/local_review.py b/scripts/local_review.py
index 3e1c1caf..05707dfa 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -1,12 +1,11 @@
 #!/usr/bin/env python3
 """Record and verify that a local review pass covered this branch's current content.
 
-The fleet's rule is that PR-bound work gets an adversarial local review before it is pushed, and
-again before every fix push, so a PR-hosted reviewer is not used as the first reviewer. That rule
-was already written in three skills and still went unfollowed, so this script gives it something
-mechanically checkable: a review pass records a receipt keyed on the content it reviewed, and a
-capture point (a git pre-push hook, a skill step, a session about to claim done) asks whether a
-receipt still covers what is about to be pushed.
+`AGENTS.md` and the `local-strict-review` skill hold the rule about when a local review is owed.
+This script is what makes that rule checkable rather than a second copy of it: a review pass
+records a receipt keyed on the content it reviewed, and a capture point (a git pre-push hook, a
+skill step, a session about to claim done) asks whether a receipt still covers what is about to
+be pushed.
 
 The key is over the content of the changed paths, plus the merge-base commit that fixes what
 "changed" means. It is deliberately not over the diff text and not over HEAD, which is what lets a
@@ -75,8 +74,6 @@
 and 2 only when it could not run at all.
 """
 
-from __future__ import annotations
-
 import argparse
 import hashlib
 import json
@@ -105,9 +102,8 @@
 GIT_TIMEOUT = 300
 BACKEND_TIMEOUT = 900
 
-# The backends the engine knows, and whether it can execute one itself.
-# An agent-driven backend is recorded by the session that ran it, since no script spawns an agent.
-# A headless backend is one a hook can run with no session attached.
+# `headless` says whether a hook can run this backend with no agent session attached.
+# An agent-driven one is recorded by the session that ran it, since no script can spawn an agent.
 BACKENDS = {
     "agent-skill": {"headless": False, "why": "the local-strict-review subagent pass"},
     "coderabbit-cli": {"headless": True, "why": "coderabbit review --agent, structured JSON"},
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index 086c239b..e4cfc371 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -13,8 +13,6 @@
 `python3 -m unittest discover -s scripts/tests`.
 """
 
-from __future__ import annotations
-
 import contextlib
 import io
 import json
@@ -361,6 +359,7 @@ def capture(*args: str, **kwargs: object) -> str:
                 seen.add(env["GIT_INDEX_FILE"])
             return real_git(*args, **kwargs)  # type: ignore[arg-type]
 
+        # Suppressing assignment here: the wrapper's signature is intentionally wider.
         local_review.git = capture  # type: ignore[assignment]
         self.addCleanup(setattr, local_review, "git", real_git)
         local_review.worktree_states(self.tmp)
@@ -575,8 +574,12 @@ def test_concurrent_records_do_not_drop_a_pass(self) -> None:
         def slow(*args: object, **kwargs: object) -> dict:
             # Widen the read-modify-write window the lock exists to close.
             time.sleep(0.2)
+            # Suppressing arg-type here: the captured original is typed by its own signature.
+            # This deliberately generic forwarding wrapper cannot restate that signature.
             return real(*args, **kwargs)  # type: ignore[arg-type]
 
+        # Suppressing assignment here: rebinding a module function to a stand-in is the case.
+        # The stand-in cannot match the original's exact signature.
         local_review._write_pass_locked = slow  # type: ignore[assignment]
         self.addCleanup(setattr, local_review, "_write_pass_locked", real)
         done: list[dict] = []
@@ -832,6 +835,7 @@ def test_an_unexpected_crash_reports_the_boundary_not_a_verdict(self) -> None:
         def boom(*_args: object, **_kwargs: object) -> tuple[str, str, int]:
             raise RuntimeError("something nobody anticipated")
 
+        # Suppressing assignment here: the stand-in raises rather than returning.
         local_review.current_state = boom  # type: ignore[assignment]
         self.addCleanup(setattr, local_review, "current_state", original)
         self.assertEqual(self.main_quiet(["check", "--target", self.target]), 2)

From 3857fe9c68501e35e06ca18aa715891b2a6d82db Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sat, 29 Aug 2026 21:51:25 -0700
Subject: [PATCH 06/12] Drop a Git 2.31 Dependency, and Close Three Gaps in the
 New Guards

Round three findings, all real, three of them in code added by the earlier
rounds of this same pull request.

`objects_dir` used `rev-parse --path-format=absolute`, which needs git 2.31,
while `spec/host-tools.json` declares no floor on git at all. The guard kit's own
README already records that dependency as a known rule-specific hazard. It uses
`--git-path` alone now, resolving the relative form a primary checkout returns
against the repository root, which needs nothing newer than the rest of the file.

`held_lock` treated any existing lock file as held, so a process killed while
holding one would block every later record permanently, with nothing saying why
and no way out but deleting a file by hand. A lock older than any plausible
record is now treated as abandoned. Both halves are tested: the stale one is
broken, the fresh one is still respected, since an escape hatch that always fires
would defeat the lock it exists to rescue.

`--expect-digest` was optional while the README described it as always enforced.
An optional guard is the one a caller omits, and the omission looks exactly like
a pass that was properly bound to its review, so it is required now and the docs
match the code.

`--findings` accepted a negative value straight into the receipt.

Part of #1104 and #1083.
---
 scripts/README.md                  |  2 +-
 scripts/local_review.py            | 36 +++++++++++++---
 scripts/tests/test_local_review.py | 69 ++++++++++++++++++++++++++++--
 3 files changed, 96 insertions(+), 11 deletions(-)

diff --git a/scripts/README.md b/scripts/README.md
index 3237eafd..34e7a5c3 100644
--- a/scripts/README.md
+++ b/scripts/README.md
@@ -228,7 +228,7 @@ Every identity is computed by git rather than reconstructed, which matters more
 
 What the key covers is the net content the branch introduces against its target, not the commit series that produced it. A branch that adds a file and later deletes it, or a rebase that rewrites only messages, keeps its key. That is the intended scope, since a feature branch lands as a squash and the net diff is what a reviewer reads, but a capture point built on this does not cover the commit series. The receipt lives in the worktree's own git directory rather than in the working tree, so it is per-task, can never be committed by accident, and needs no `.gitignore` entry of its own.
 
-`record` takes `--expect-digest`, the digest `status` reported before the pass ran, and refuses when the content has moved since. Nothing else binds the moment a review finished to the moment it is recorded, so a format-on-save or a hook autofix in between would otherwise be stamped as reviewed content no reviewer saw, which is the one claim the receipt exists to make. A headless backend is recorded by running it rather than by hand, since recording it directly would bypass the completion check that makes its pass mean anything.
+`record` requires `--expect-digest`, the digest `status` reported before the pass ran, and refuses when the content has moved since. It is required rather than optional because an optional guard is the one a caller in a hurry omits, and the omission looks exactly like a pass that was properly bound. Nothing else binds the moment a review finished to the moment it is recorded, so a format-on-save or a hook autofix in between would otherwise be stamped as reviewed content no reviewer saw, which is the one claim the receipt exists to make. A headless backend is recorded by running it rather than by hand, since recording it directly would bypass the completion check that makes its pass mean anything.
 
 It holds no review logic. It drives backends: `agent-skill` is the `local-strict-review` subagent pass, which only a live agent session can run, so the engine records it rather than invoking it, and `coderabbit-cli` is headless and is the one backend a git hook can execute by itself. A CodeRabbit CLI run draws on the same hourly budget as this account's pull request reviews, so a run that is rate limited, errors, or ends without a completion event records no pass rather than a clean one. The invocation follows the CLI's documented flag semantics: `--base-commit` for the merge-base hash, since `--base` takes a branch name, and `--include-untracked`, since the CLI excludes untracked files by default while this receipt's scope includes them. A completion reporting `review_skipped` is refused too, that being the CLI saying it looked at nothing. None of it has been executed against a real CLI on this host, so the backend stays opt-in until someone runs it. A recorded pass means a review ran over exactly this content, never that the content is clean: disposing of what a review found is judgment, per the `pr-review-conduct` Skill's five outcomes, so a pass is recorded whether it raised findings or none.
 
diff --git a/scripts/local_review.py b/scripts/local_review.py
index 05707dfa..586d1df9 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -58,8 +58,10 @@
 
 Usage: python3 scripts/local_review.py status [--target <branch>]
            what the current content digest is, and which backends hold a pass on it.
-       python3 scripts/local_review.py record --reviewer <id> [--target <branch>] [--findings N]
-           record that <id> reviewed the current content.
+       python3 scripts/local_review.py record --reviewer <id> --expect-digest <digest>
+                                             [--target <branch>] [--findings N]
+           record that <id> reviewed the content `status` reported as <digest>, refusing when the
+           content has moved since.
        python3 scripts/local_review.py check [--target <branch>]
            exit 0 covered, 1 not covered, 2 could not run.
        python3 scripts/local_review.py run --backend coderabbit-cli [--target <branch>]
@@ -100,6 +102,9 @@
 # Bounded so a hung git or CLI fails the capture point loudly instead of hanging a push forever.
 # Staging the whole working tree is the slowest call here, which is why the git bound is generous.
 GIT_TIMEOUT = 300
+# Past this, a lock is an abandoned one rather than a held one.
+# Longer than any single record, and short enough that a crash does not wedge the tool.
+STALE_LOCK_SECONDS = 300
 BACKEND_TIMEOUT = 900
 
 # `headless` says whether a hook can run this backend with no agent session attached.
@@ -179,9 +184,11 @@ def objects_dir(root: Path) -> Path:
     other worktree, and the fleet runs every task in a linked worktree. Joining the path by hand
     names a directory that does not exist there, which silently attaches nothing as an alternate.
     """
-    return Path(
-        git("rev-parse", "--path-format=absolute", "--git-path", "objects", root=root).strip()
-    )
+    # `--git-path` alone, without `--path-format=absolute`, because that option needs git 2.31 and
+    # `spec/host-tools.json` declares no floor on git at all.
+    # The result is relative in a primary checkout and absolute in a linked worktree.
+    # Joining an absolute path onto the root yields that absolute path unchanged.
+    return root / git("rev-parse", "--git-path", "objects", root=root).strip()
 
 
 def receipt_path(root: Path) -> Path:
@@ -486,6 +493,16 @@ def held_lock(path: Path, timeout: float = 10.0) -> int:
         try:
             return os.open(lock, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600)
         except FileExistsError:
+            # A process killed while holding the lock would block every later record permanently.
+            # Nothing would say why, and the only way out would be deleting a file by hand.
+            # A lock older than any plausible record is treated as abandoned rather than held.
+            try:
+                age = time.time() - lock.stat().st_mtime
+            except OSError:
+                age = 0.0
+            if age > STALE_LOCK_SECONDS:
+                lock.unlink(missing_ok=True)
+                continue
             if time.monotonic() >= deadline:
                 raise CannotRun(f"another process holds {lock} and did not release it") from None
             time.sleep(0.05)
@@ -682,6 +699,9 @@ def cmd_record(args: argparse.Namespace) -> int:
             file=sys.stderr,
         )
         return EXIT_CANNOT_RUN
+    if args.findings is not None and args.findings < 0:
+        print(f"--findings cannot be negative, got {args.findings}", file=sys.stderr)
+        return EXIT_CANNOT_RUN
     root = repo_root()
     target = resolve_target(args.target)
     base, digest, changed = current_state(target, root)
@@ -690,7 +710,9 @@ def cmd_record(args: argparse.Namespace) -> int:
     # That is content no reviewer saw, and attesting to it is the one claim this receipt makes.
     # The caller passes back what `status` reported before the pass ran.
     # A mismatch is the content having moved rather than an answer about coverage.
-    if args.expect_digest and args.expect_digest != digest:
+    # The flag is required rather than optional, since an optional guard is the one a caller omits.
+    # That omission looks exactly like a pass that was properly bound to its review.
+    if args.expect_digest != digest:
         print(
             "the content changed between the review and this record, so nothing was recorded.\n"
             f"  reviewed  {args.expect_digest[:12]}\n"
@@ -793,7 +815,7 @@ def main(argv: list[str] | None = None) -> int:
     p_record.add_argument("--findings", type=int, default=None, help="how many findings it raised")
     p_record.add_argument(
         "--expect-digest",
-        default=None,
+        required=True,
         help="the contentDigest the review was run against, refusing to record if it has moved",
     )
     p_record.set_defaults(func=cmd_record)
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index e4cfc371..99819996 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -552,6 +552,33 @@ def test_record_refuses_content_that_moved_since_the_review(self) -> None:
         )
         self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
 
+    def test_record_requires_the_digest_the_review_saw(self) -> None:
+        """An optional guard is the one a caller in a hurry omits.
+
+        The omission would then look exactly like a pass that was properly bound to its review.
+        """
+        with contextlib.redirect_stderr(io.StringIO()), self.assertRaises(SystemExit) as caught:
+            local_review.main(["record", "--reviewer", "agent-skill"])
+        self.assertNotEqual(caught.exception.code, 0)
+
+    def test_a_negative_finding_count_is_refused(self) -> None:
+        """It would otherwise land in the receipt and mean nothing to whoever reads it."""
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        self.assertEqual(
+            self.main_quiet(
+                [
+                    "record",
+                    "--reviewer",
+                    "agent-skill",
+                    "--findings",
+                    "-1",
+                    "--expect-digest",
+                    self.digest(),
+                ]
+            ),
+            2,
+        )
+
     def test_record_accepts_the_digest_the_review_saw(self) -> None:
         (self.tmp / "a.py").write_text("reviewed content\n", encoding="utf-8")
         reviewed = self.digest()
@@ -563,7 +590,9 @@ def test_record_accepts_the_digest_the_review_saw(self) -> None:
     def test_record_will_not_forge_a_headless_backend(self) -> None:
         """Recording one by hand bypasses the completion check that makes its pass mean anything."""
         (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
-        self.assertEqual(self.main_quiet(["record", "--reviewer", "coderabbit-cli"]), 2)
+        self.assertEqual(
+            self.main_quiet(["record", "--reviewer", "coderabbit-cli", "--expect-digest", "x"]), 2
+        )
 
     def test_concurrent_records_do_not_drop_a_pass(self) -> None:
         """Accumulation is a read, a merge, and a replace, so it needs more than an atomic write."""
@@ -604,6 +633,27 @@ def slow(*args: object, **kwargs: object) -> dict:
             "a concurrent record dropped the other pass",
         )
 
+    def test_a_stale_lock_does_not_wedge_the_tool(self) -> None:
+        """A process killed while holding it would otherwise block every later record forever."""
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        lock = Path(str(local_review.receipt_path(self.tmp)) + ".lock")
+        lock.write_text("", encoding="utf-8")
+        old = time.time() - local_review.STALE_LOCK_SECONDS - 60
+        os.utime(lock, (old, old))
+        base, digest, _ = local_review.current_state(self.target, self.tmp)
+        out = local_review.write_pass(self.tmp, self.target, base, digest, "agent-skill", 0)
+        self.assertEqual([p["reviewer"] for p in out["passes"]], ["agent-skill"])
+
+    def test_a_fresh_lock_is_respected(self) -> None:
+        """Otherwise the staleness escape hatch would defeat the lock it exists to rescue."""
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        lock = Path(str(local_review.receipt_path(self.tmp)) + ".lock")
+        lock.write_text("", encoding="utf-8")
+        self.addCleanup(lock.unlink, True)
+        # Driving held_lock directly, with a short budget, so the case does not spend the real wait.
+        with self.assertRaises(local_review.CannotRun):
+            local_review.held_lock(local_review.receipt_path(self.tmp), timeout=0.2)
+
     def test_an_ignored_file_stays_out_of_the_key(self) -> None:
         """Otherwise build output would demand a review, and the docstring claims it does not."""
         (self.tmp / ".gitignore").write_text("build/\n", encoding="utf-8")
@@ -804,7 +854,10 @@ class ExitCodeCase(RepoCase):
     def test_an_unknown_reviewer_is_a_boundary_not_a_finding(self) -> None:
         """2 says the check did not run, which a gate must not read as either verdict."""
         self.assertEqual(
-            self.main_quiet(["record", "--reviewer", "nope", "--target", self.target]), 2
+            self.main_quiet(
+                ["record", "--reviewer", "nope", "--target", self.target, "--expect-digest", "x"]
+            ),
+            2,
         )
 
     def test_an_agent_backend_cannot_be_run_headlessly(self) -> None:
@@ -860,7 +913,17 @@ def test_record_reports_what_it_recorded(self) -> None:
         (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
         buf = io.StringIO()
         with contextlib.redirect_stdout(buf):
-            code = local_review.main(["record", "--reviewer", "agent-skill", "--findings", "2"])
+            code = local_review.main(
+                [
+                    "record",
+                    "--reviewer",
+                    "agent-skill",
+                    "--findings",
+                    "2",
+                    "--expect-digest",
+                    self.digest(),
+                ]
+            )
         self.assertEqual(code, 0)
         self.assertIn("agent-skill", buf.getvalue())
         receipt, problems = local_review.read_receipt(self.tmp)

From 8a14151fbe230887de8f7d30d031f168854dae53 Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sat, 29 Aug 2026 21:53:54 -0700
Subject: [PATCH 07/12] Snapshot the Object Database by Path, and Verify the
 Alternates Quoting

Two suppressed findings and one grammar fix from the review.

The object-containment case snapshotted loose objects by bare filename. Two
objects in different two-hex directories can share the same 38-hex name, so the
snapshot could match while new objects had in fact been written, which is a false
clean in the case guarding against leaking untracked content into the repository.
It compares relative paths now.

A second suppressed finding claimed git splits GIT_ALTERNATE_OBJECT_DIRECTORIES
purely on the separator with no quoting support, which would make the quoting
added earlier useless. Two reviewers had asserted opposite things, so it was
checked rather than chosen: under an object directory whose path holds a colon,
the unquoted form fails with "object directory /tmp/... does not exist" and the
double-quoted form resolves the object. The quoting is correct and the docstring
now records that it was verified, along with what was observed.

The README sentence introduced in the previous commit mixed a plural subject with
a singular verb.

Part of #1104 and #1083.
---
 scripts/README.md                  |  2 +-
 scripts/local_review.py            |  4 +++-
 scripts/tests/test_local_review.py | 13 +++++++++++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/scripts/README.md b/scripts/README.md
index 34e7a5c3..15f4a5f8 100644
--- a/scripts/README.md
+++ b/scripts/README.md
@@ -220,7 +220,7 @@ The match is on the block's heading rather than anywhere in the body, and on the
 
 ## `local_review.py`
 
-Records and verifies that a local review pass covered a branch's current content, so the review-before-every-push rule [`AGENTS.md`][agents] and the `local-strict-review` Skill hold has something a capture point can actually check rather than a second statement of the rule itself. A pass records a receipt keyed on the content it reviewed, and a capture point asks whether that receipt still covers what is about to be pushed. The capture points themselves (a git pre-push hook, the Skill steps that invoke the pass) are a separate change, so this script is the capability they will call rather than a gate that is already wired up.
+Records and verifies that a local review pass covered a branch's current content. [`AGENTS.md`][agents] and the `local-strict-review` Skill hold the review-before-every-push rule itself, and this gives that rule something a capture point can actually check rather than restating it. A pass records a receipt keyed on the content it reviewed, and a capture point asks whether that receipt still covers what is about to be pushed. The capture points themselves (a git pre-push hook, the Skill steps that invoke the pass) are a separate change, so this script is the capability they will call rather than a gate that is already wired up.
 
 The key is over the content of the changed files, plus the merge-base commit that fixes what "changed" means. It is deliberately not over the diff text and not over `HEAD`, which is what lets a review run before the commit while the check runs at the push: reviewing untracked work and then committing it unchanged leaves the key identical. Each path contributes the state it is in on each side, the working tree's and the index's, both as git's own mode-and-blob identity so the two compare directly. Where they agree the path reads as one state, which is what makes `git add` and `git commit` invisible. Where they diverge, because content is staged that the working tree no longer holds, or because a file mode changed, the key moves. Reading the index rather than the working tree alone is what stops content being committed that the key never saw.
 
diff --git a/scripts/local_review.py b/scripts/local_review.py
index 586d1df9..f52547a7 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -280,7 +280,9 @@ def alternate_entry(path: Path) -> str:
 
     The variable holds a list separated by the platform's path separator, so a repository checked
     out under a path containing one would otherwise split into two directories that do not exist.
-    Git reads a double-quoted entry as a single path.
+    Git reads a double-quoted entry as a single path, which was verified rather than assumed: under
+    an object directory holding a colon, the unquoted form fails with "object directory /tmp/... does
+    not exist" while the quoted form resolves the object.
     """
     text = str(path)
     return f'"{text}"' if os.pathsep in text else text
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index 99819996..aca45cd0 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -305,9 +305,18 @@ def test_staging_the_worktree_writes_nothing_into_the_object_database(self) -> N
         """
         (self.tmp / ".env").write_text("TOKEN=constructed-not-real\n", encoding="utf-8")
         objects = local_review.git_dir(self.tmp) / "objects"
-        before = sorted(p.name for p in objects.rglob("*") if p.is_file())
+
+        # Relative paths rather than bare names.
+        # Two loose objects in different two-hex directories can share the same 38-hex filename.
+        # A name-only snapshot can therefore match while new objects were in fact written.
+        def snapshot() -> list[str]:
+            return sorted(
+                q.relative_to(objects).as_posix() for q in objects.rglob("*") if q.is_file()
+            )
+
+        before = snapshot()
         states = local_review.worktree_states(self.tmp)
-        after = sorted(p.name for p in objects.rglob("*") if p.is_file())
+        after = snapshot()
         self.assertEqual(before, after, "staging leaked objects into the real database")
         oid = next(iter(states[".env"])).split(":")[-1]
         probe = subprocess.run(

From 70d4425a93f8e2cdd56f3737125b964c3711c763 Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sat, 29 Aug 2026 22:01:52 -0700
Subject: [PATCH 08/12] Make the Check Failure's Remedy Command One That
 Actually Runs

Requiring --expect-digest in an earlier commit broke the one actionable line
this tool prints. The check failure told the reader to run a record command that
omitted the now-required flag, so following the instruction landed on a usage
error rather than a recorded pass, which is the whole point of printing it.

The remedy now carries the flag and the full digest rather than the abbreviated
one shown above it, so the line runs as it stands. The test does not read the
message, it extracts the printed command, runs it, and asserts the branch then
checks as covered, since asserting on the text alone would pass for any wording
that happens to mention the flag.

Part of #1104 and #1083.
---
 scripts/local_review.py            |  4 +++-
 scripts/tests/test_local_review.py | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/scripts/local_review.py b/scripts/local_review.py
index f52547a7..03af2524 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -754,10 +754,12 @@ def cmd_check(args: argparse.Namespace) -> int:
         )
     for problem in problems:
         print(f"  receipt       {problem}", file=sys.stderr)
+    # The digest is printed in full rather than abbreviated, so the command below runs as it stands.
+    # Abbreviating it, or omitting the required --expect-digest, makes this line a usage error.
     print(
         "\nRun the local-strict-review pass over this diff, then record it:\n"
         "  python3 scripts/local_review.py record --reviewer agent-skill"
-        f" --target {target} --findings <n>",
+        f" --target {target} --findings <n> --expect-digest {digest}",
         file=sys.stderr,
     )
     return EXIT_NOT_COVERED
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index aca45cd0..8bd8605d 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -561,6 +561,29 @@ def test_record_refuses_content_that_moved_since_the_review(self) -> None:
         )
         self.assertEqual(self.main_quiet(["check", "--target", self.target]), 1)
 
+    def test_the_check_failure_prints_a_command_that_actually_runs(self) -> None:
+        """The remedy line is the one actionable thing the failure emits.
+
+        It has to carry every required flag and the real digest, or following it lands the reader
+        on a usage error instead of a recorded pass.
+        """
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        err = io.StringIO()
+        with contextlib.redirect_stderr(err), contextlib.redirect_stdout(io.StringIO()):
+            self.assertEqual(local_review.main(["check", "--target", self.target]), 1)
+        printed = err.getvalue()
+        digest = self.digest()
+        self.assertIn("--expect-digest", printed)
+        self.assertIn(digest, printed, "the remedy carries an abbreviated digest, so it will fail")
+        # Run the command the failure printed, and confirm it records rather than erroring.
+        line = next(l for l in printed.splitlines() if "local_review.py record" in l)
+        argv = line.split()[2:]
+        argv = [a for a in argv if a != "<n>"]
+        argv[argv.index("--findings")] = "--findings"
+        argv.insert(argv.index("--findings") + 1, "0")
+        self.assertEqual(self.main_quiet(argv), 0, f"the printed command failed: {argv}")
+        self.assertEqual(self.main_quiet(["check", "--target", self.target]), 0)
+
     def test_record_requires_the_digest_the_review_saw(self) -> None:
         """An optional guard is the one a caller in a hurry omits.
 

From 3cd2035208b11dfc2f0ca59cab20cdb7c59f66ae Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sun, 30 Aug 2026 06:45:33 -0700
Subject: [PATCH 09/12] Break a Stale Lock Under an Exclusive Claim, Not a
 Check and a Guess

The stale-lock escape hatch added last round introduced a race of its own. It
read the lock's age and then unlinked, and between those two steps the holder can
release, another process can take a fresh lock, and the unlink then removes a
lock that is legitimately held. That is worse than the wedge it was fixing.

Breaking now happens under a second lock of its own, and the staleness is re-read
after that exclusive claim rather than before it, so the file removed is still
the abandoned one that was observed. A lock taken in the window is not old enough
to qualify and survives. A process that finds the breaker already held defers
rather than racing alongside it. Three cases cover it: a lock that turns fresh
mid-break is not removed, the breaker leaves nothing behind, and a held breaker
defers.

Two smaller findings from the same round:

The CodeRabbit executable was resolved against this process's working directory
while the child runs with its directory set to the repository root, so a relative
PATH entry would resolve in one place and execute in another. It is resolved to
an absolute path at the point of lookup.

The recovery command printed by a failed check still carried a `<n>` placeholder,
so it could not be pasted. --findings is optional, so it is gone from the command
and mentioned on its own line instead. The test now runs the printed command
verbatim and asserts no placeholder survives, where before it repaired the
placeholder itself and so could not have caught this.

Part of #1104 and #1083.
---
 scripts/local_review.py            | 60 ++++++++++++++++++++++++------
 scripts/tests/test_local_review.py | 53 ++++++++++++++++++++++++--
 2 files changed, 97 insertions(+), 16 deletions(-)

diff --git a/scripts/local_review.py b/scripts/local_review.py
index 03af2524..66ffe4f3 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -481,6 +481,39 @@ def covering_passes(receipt: dict | None, base: str, digest: str, target: str) -
     return [p for p in receipt.get("passes", []) if isinstance(p, dict) and p.get("reviewer")]
 
 
+def break_stale_lock(lock: Path) -> bool:
+    """Remove `lock` if it is abandoned, reporting whether anything was removed.
+
+    Breaking is done under a second lock of its own, and the staleness is re-read after that
+    exclusive claim rather than before it. A plain check-then-unlink races: the observing process
+    can be descheduled between the two, the holder can release, another process can take a fresh
+    lock, and the unlink then removes a lock that is legitimately held. Re-reading under the
+    breaker means the file being removed is still the same abandoned one that was observed, since
+    a lock taken in between is not old enough to qualify.
+    """
+    breaker = Path(str(lock) + ".break")
+    try:
+        fd = os.open(breaker, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600)
+    except FileExistsError:
+        # Another process is already breaking it, so this one waits rather than racing alongside.
+        return False
+    except OSError:
+        return False
+    try:
+        try:
+            stat = lock.stat()
+        except OSError:
+            # Already gone, which is the outcome this function exists to produce.
+            return True
+        if time.time() - stat.st_mtime <= STALE_LOCK_SECONDS:
+            return False
+        lock.unlink(missing_ok=True)
+        return True
+    finally:
+        os.close(fd)
+        breaker.unlink(missing_ok=True)
+
+
 def held_lock(path: Path, timeout: float = 10.0) -> int:
     """Take an exclusive lock beside the receipt, or raise once waiting has gone on too long.
 
@@ -498,12 +531,7 @@ def held_lock(path: Path, timeout: float = 10.0) -> int:
             # A process killed while holding the lock would block every later record permanently.
             # Nothing would say why, and the only way out would be deleting a file by hand.
             # A lock older than any plausible record is treated as abandoned rather than held.
-            try:
-                age = time.time() - lock.stat().st_mtime
-            except OSError:
-                age = 0.0
-            if age > STALE_LOCK_SECONDS:
-                lock.unlink(missing_ok=True)
+            if break_stale_lock(lock):
                 continue
             if time.monotonic() >= deadline:
                 raise CannotRun(f"another process holds {lock} and did not release it") from None
@@ -594,9 +622,13 @@ def run_coderabbit(base: str, root: Path) -> tuple[int, str]:
     # Only the documented binary name.
     # A bare `cr` is a common short name for unrelated tools.
     # Running whichever one is on PATH with a 900 second budget is not a fallback worth having.
-    exe = shutil.which("coderabbit")
-    if not exe:
+    found = shutil.which("coderabbit")
+    if not found:
         raise CannotRun("the coderabbit CLI is not installed, so this backend cannot run")
+    # Resolved to an absolute path here, where the lookup happened.
+    # The child runs with its working directory set to the repository root instead.
+    # A relative PATH entry would otherwise resolve against one directory and execute in another.
+    exe = str(Path(found).resolve())
     try:
         proc = subprocess.run(
             [exe, "review", "--agent", "--base-commit", base, "--include-untracked"],
@@ -754,12 +786,16 @@ def cmd_check(args: argparse.Namespace) -> int:
         )
     for problem in problems:
         print(f"  receipt       {problem}", file=sys.stderr)
-    # The digest is printed in full rather than abbreviated, so the command below runs as it stands.
-    # Abbreviating it, or omitting the required --expect-digest, makes this line a usage error.
+    # Printed so it runs exactly as it stands.
+    # That means the full digest rather than the abbreviated form above, the required
+    # --expect-digest present, and no placeholder left to substitute.
+    # --findings is optional, so it is left out rather than shown as something to fill in.
+    # A placeholder would make the one actionable line here a command nobody can paste.
     print(
         "\nRun the local-strict-review pass over this diff, then record it:\n"
-        "  python3 scripts/local_review.py record --reviewer agent-skill"
-        f" --target {target} --findings <n> --expect-digest {digest}",
+        f"  python3 scripts/local_review.py record --reviewer agent-skill --target {target}"
+        f" --expect-digest {digest}\n"
+        "Add --findings <count> to record how many it raised.",
         file=sys.stderr,
     )
     return EXIT_NOT_COVERED
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index 8bd8605d..49924dcd 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -23,6 +23,7 @@
 import threading
 import time
 import unittest
+import unittest.mock
 from pathlib import Path
 
 sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
@@ -575,12 +576,11 @@ def test_the_check_failure_prints_a_command_that_actually_runs(self) -> None:
         digest = self.digest()
         self.assertIn("--expect-digest", printed)
         self.assertIn(digest, printed, "the remedy carries an abbreviated digest, so it will fail")
-        # Run the command the failure printed, and confirm it records rather than erroring.
+        # Run the printed command verbatim.
+        # Repairing it here, by dropping or filling a placeholder, would hide the defect.
         line = next(l for l in printed.splitlines() if "local_review.py record" in l)
         argv = line.split()[2:]
-        argv = [a for a in argv if a != "<n>"]
-        argv[argv.index("--findings")] = "--findings"
-        argv.insert(argv.index("--findings") + 1, "0")
+        self.assertNotIn("<n>", argv, "the printed command carries a placeholder, so it cannot run")
         self.assertEqual(self.main_quiet(argv), 0, f"the printed command failed: {argv}")
         self.assertEqual(self.main_quiet(["check", "--target", self.target]), 0)
 
@@ -676,6 +676,51 @@ def test_a_stale_lock_does_not_wedge_the_tool(self) -> None:
         out = local_review.write_pass(self.tmp, self.target, base, digest, "agent-skill", 0)
         self.assertEqual([p["reviewer"] for p in out["passes"]], ["agent-skill"])
 
+    def test_breaking_a_stale_lock_does_not_remove_a_fresh_one(self) -> None:
+        """Checking staleness and then unlinking is a race the breaker lock exists to close.
+
+        The observing process can be descheduled between the two, the holder can release, and
+        another process can take a fresh lock, which the unlink would then remove.
+        """
+        lock = Path(str(local_review.receipt_path(self.tmp)) + ".lock")
+        lock.write_text("", encoding="utf-8")
+        old = time.time() - local_review.STALE_LOCK_SECONDS - 60
+        os.utime(lock, (old, old))
+        real_unlink = Path.unlink
+
+        def racing_stat(self_path: Path) -> os.stat_result:
+            # Stand in for the holder releasing and a fresh lock being taken in the window.
+            if self_path == lock:
+                os.utime(lock, None)
+            return os.stat(self_path)
+
+        with unittest.mock.patch.object(Path, "stat", racing_stat):
+            broke = local_review.break_stale_lock(lock)
+        self.assertFalse(broke, "a lock that became fresh was still broken")
+        self.assertTrue(lock.exists(), "the fresh lock was removed")
+        real_unlink(lock)
+
+    def test_the_breaker_leaves_nothing_behind(self) -> None:
+        lock = Path(str(local_review.receipt_path(self.tmp)) + ".lock")
+        lock.write_text("", encoding="utf-8")
+        old = time.time() - local_review.STALE_LOCK_SECONDS - 60
+        os.utime(lock, (old, old))
+        self.assertTrue(local_review.break_stale_lock(lock))
+        self.assertFalse(Path(str(lock) + ".break").exists())
+        self.assertFalse(lock.exists())
+
+    def test_a_breaker_already_held_defers_rather_than_racing(self) -> None:
+        lock = Path(str(local_review.receipt_path(self.tmp)) + ".lock")
+        lock.write_text("", encoding="utf-8")
+        old = time.time() - local_review.STALE_LOCK_SECONDS - 60
+        os.utime(lock, (old, old))
+        breaker = Path(str(lock) + ".break")
+        breaker.write_text("", encoding="utf-8")
+        self.addCleanup(breaker.unlink, True)
+        self.addCleanup(lock.unlink, True)
+        self.assertFalse(local_review.break_stale_lock(lock))
+        self.assertTrue(lock.exists(), "the lock was broken while another breaker held it")
+
     def test_a_fresh_lock_is_respected(self) -> None:
         """Otherwise the staleness escape hatch would defeat the lock it exists to rescue."""
         (self.tmp / "a.py").write_text("x\n", encoding="utf-8")

From 96d8c1d4b8c41b1d53930bcb08cc0b3348c07fe2 Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sun, 30 Aug 2026 06:57:32 -0700
Subject: [PATCH 10/12] Quote the Printed Remedy, and Report the Verdict a
 Closed Reader Interrupted

Three findings from the review, one of them a safety issue rather than a
correctness one.

The recovery command printed by a failed check interpolated the target ref raw.
Git permits a ref name to hold shell metacharacters, confirmed against a real
repository for a semicolon, a command substitution, and an ampersand, so pasting
that line could run whatever a branch name carried. Both interpolated values are
shell-quoted now.

The test for it first passed with the quoting removed, which is why it is worth
naming: splitting the line with shlex is not enough, since shlex does not treat a
semicolon as an operator and the raw name survives as one token either way. It
asserts the quoted form is what was printed.

A closed reader was reported as the boundary code. It is not: the command ran and
produced an answer, so that answer is what gets reported, while a failure that
came before a verdict still reports the boundary. Writing the test for it
surfaced a further defect in the handler, which assumed stdout always has a
descriptor to redirect, so the guard against a second failure raised one itself
under any caller that replaces stdout.

The descriptor opened for that redirect was never closed.

Part of #1104 and #1083.
---
 scripts/local_review.py            | 30 +++++++++++++++++---
 scripts/tests/test_local_review.py | 45 ++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/scripts/local_review.py b/scripts/local_review.py
index 66ffe4f3..2e5fe655 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -80,6 +80,7 @@
 import hashlib
 import json
 import os
+import shlex
 import shutil
 import subprocess
 import sys
@@ -791,10 +792,13 @@ def cmd_check(args: argparse.Namespace) -> int:
     # --expect-digest present, and no placeholder left to substitute.
     # --findings is optional, so it is left out rather than shown as something to fill in.
     # A placeholder would make the one actionable line here a command nobody can paste.
+    # The target is quoted because git permits a ref name to hold shell metacharacters.
+    # Confirmed against a real repository for a semicolon, a command substitution, and an ampersand.
+    # Interpolating one raw into a line the reader is invited to paste would let a branch name run commands.
     print(
         "\nRun the local-strict-review pass over this diff, then record it:\n"
-        f"  python3 scripts/local_review.py record --reviewer agent-skill --target {target}"
-        f" --expect-digest {digest}\n"
+        f"  python3 scripts/local_review.py record --reviewer agent-skill"
+        f" --target {shlex.quote(target)} --expect-digest {shlex.quote(digest)}\n"
         "Add --findings <count> to record how many it raised.",
         file=sys.stderr,
     )
@@ -871,6 +875,9 @@ def main(argv: list[str] | None = None) -> int:
         p.add_argument("--target", default=None, help=f"target branch (default {DEFAULT_TARGET})")
 
     args = parser.parse_args(argv)
+    # Holds the boundary code until a verdict is actually produced.
+    # A reader that closes before the command finished then reports that nothing was decided.
+    code = EXIT_CANNOT_RUN
     try:
         code = int(args.func(args))
         # Flushed inside the guarded region on purpose.
@@ -887,8 +894,23 @@ def main(argv: list[str] | None = None) -> int:
     except BrokenPipeError:
         # Redirected so the interpreter's own shutdown flush has somewhere to go.
         # Letting it raise again is what turns this into an exit outside the contract.
-        os.dup2(os.open(os.devnull, os.O_WRONLY), sys.stdout.fileno())
-        return EXIT_CANNOT_RUN
+        # The descriptor is closed after the duplicate is installed, since dup2 leaves it open.
+        # This module is importable into a long-lived process, where that would accumulate.
+        # Guarded because stdout is not always a real file.
+        # Embedded in another process, or under a test that replaces it, there is no descriptor.
+        # The redirect that exists to stop a second failure would then raise one itself.
+        try:
+            devnull = os.open(os.devnull, os.O_WRONLY)
+            try:
+                os.dup2(devnull, sys.stdout.fileno())
+            finally:
+                os.close(devnull)
+        except (OSError, ValueError):
+            pass
+        # A reader closing the pipe is not this check failing to run.
+        # Where a verdict was already produced, that verdict is what gets reported.
+        # Where the failure came first, `code` still holds the boundary it started as.
+        return code
     # A crash is the check not having run, so it reports the boundary code.
     # Falling through to the interpreter's own exit 1 would read as the not-covered verdict.
     except Exception as e:  # noqa: BLE001
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index 49924dcd..b0a82bdd 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -17,6 +17,7 @@
 import io
 import json
 import os
+import shlex
 import subprocess
 import sys
 import tempfile
@@ -584,6 +585,50 @@ def test_the_check_failure_prints_a_command_that_actually_runs(self) -> None:
         self.assertEqual(self.main_quiet(argv), 0, f"the printed command failed: {argv}")
         self.assertEqual(self.main_quiet(["check", "--target", self.target]), 0)
 
+    def test_the_remedy_quotes_a_target_holding_shell_metacharacters(self) -> None:
+        """git permits them in a ref name, confirmed for `;`, `$(...)`, `&`, and a quote.
+
+        Interpolating one raw into a line the reader is invited to paste would let a branch name
+        run commands, so the printed value has to survive a shell round trip as one argument.
+        """
+        name = "feat;echo-pwned"
+        run(self.tmp, "branch", name)
+        run(self.tmp, "update-ref", f"refs/remotes/origin/{name}", "HEAD")
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        err = io.StringIO()
+        with contextlib.redirect_stderr(err), contextlib.redirect_stdout(io.StringIO()):
+            self.assertEqual(local_review.main(["check", "--target", name]), 1)
+        line = next(l for l in err.getvalue().splitlines() if "local_review.py record" in l)
+        # The decisive assertion is that the quoted form is what was printed.
+        # Splitting with shlex alone is not enough, since it does not treat `;` as an operator.
+        # The raw name survives as one token there whether or not it was quoted.
+        self.assertIn(shlex.quote(name), line, f"the target was interpolated raw: {line}")
+        self.assertIn(name, shlex.split(line), "the target did not survive as one argument")
+
+    def test_a_closed_reader_reports_the_verdict_it_reached(self) -> None:
+        """A reader that exits first is not this check failing to run.
+
+        The command produced an answer, so that answer is what gets reported rather than the
+        boundary code, which a gate would read as tooling trouble.
+        """
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+
+        class ClosedPipe(io.StringIO):
+            def flush(self) -> None:
+                raise BrokenPipeError(32, "Broken pipe")
+
+        saved = os.dup(1)
+        try:
+            with (
+                contextlib.redirect_stdout(ClosedPipe()),
+                contextlib.redirect_stderr(io.StringIO()),
+            ):
+                code = local_review.main(["status", "--target", self.target])
+        finally:
+            os.dup2(saved, 1)
+            os.close(saved)
+        self.assertEqual(code, 0, "a closed reader turned a completed status into a boundary")
+
     def test_record_requires_the_digest_the_review_saw(self) -> None:
         """An optional guard is the one a caller in a hurry omits.
 

From 52dea3356ccf9bb123a09fd62cb403902a794fb5 Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sun, 30 Aug 2026 07:06:01 -0700
Subject: [PATCH 11/12] Resolve a Target Against origin First, Not by Guessing
 From a Slash

A reviewer noted the --target help text did not describe the slash semantics.
Investigating it turned up something worse than a documentation gap.

Treating any slashed value as an already-qualified ref meant a perfectly ordinary
branch name such as release/v1 resolved to the LOCAL branch of that name rather
than origin's. Confirmed against a constructed repository where the two differ:
the merge base came back as the local commit, so a local branch that has moved on
would define the review scope with no error at all. That is a silent wrong
answer, which is the exact class this tool exists to prevent, and it is worse
than the confusing failure the finding predicted.

A target now resolves to origin/<value> wherever that exists, and is used as
written otherwise, which keeps a fork-based flow able to name another remote's
branch directly. A value resolving neither way is a boundary that says so, naming
both refs it tried.

Comparing a receipt's target no longer resolves refs, since a receipt can name a
branch no longer present. It ignores an optional origin/ prefix instead, so
develop and origin/develop agree while main and upstream/main stay distinct.

The help text describes the resolution, which is what was actually asked for.

Part of #1104 and #1083.
---
 scripts/local_review.py            | 69 ++++++++++++++++++++++--------
 scripts/tests/test_local_review.py | 34 +++++++++++++++
 2 files changed, 85 insertions(+), 18 deletions(-)

diff --git a/scripts/local_review.py b/scripts/local_review.py
index 2e5fe655..8fa44460 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -201,14 +201,36 @@ def receipt_path(root: Path) -> Path:
     return git_dir(root) / RECEIPT_NAME
 
 
-def target_ref(target: str) -> str:
-    """The remote-tracking ref a target name means.
+def ref_exists(ref: str, root: Path) -> bool:
+    """Whether `ref` resolves to a commit in this checkout."""
+    try:
+        git("rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}", root=root)
+    except CannotRun:
+        return False
+    return True
+
 
-    A bare name is the ordinary fleet case and resolves under `origin`. A value already carrying a
-    slash is used as written, so a fork-based flow can name an upstream remote's branch rather
-    than being told the engine understands only one remote.
+def target_ref(target: str, root: Path) -> str:
+    """The ref a target name means, preferring the remote-tracking one.
+
+    `origin/<target>` is tried first and used whenever it resolves, so an ordinary fleet branch
+    name works and so does one holding a slash. Treating any slash as "already a full ref", which
+    an earlier version did, silently measured a target such as `release/v1` against the local
+    branch of that name rather than the remote one, and a local branch that has moved on then
+    defines the review scope with no error at all.
+
+    A value that resolves only as written is used as written, which is what lets a fork-based flow
+    name another remote's branch.
     """
-    return target if "/" in target else f"origin/{target}"
+    remote = f"origin/{target}"
+    if ref_exists(remote, root):
+        return remote
+    if ref_exists(target, root):
+        return target
+    raise CannotRun(
+        f"neither {remote} nor {target} resolves in this checkout,"
+        " so the review scope cannot be determined"
+    )
 
 
 def merge_base(target: str, root: Path) -> str:
@@ -216,14 +238,7 @@ def merge_base(target: str, root: Path) -> str:
 
     A merge-base is a fork point rather than a tip, so fetching a newer target does not move it.
     """
-    ref = target_ref(target)
-    try:
-        git("rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}", root=root)
-    except CannotRun as e:
-        raise CannotRun(
-            f"{ref} does not resolve in this checkout, so the review scope cannot be determined"
-        ) from e
-    return git("merge-base", ref, "HEAD", root=root).strip()
+    return git("merge-base", target_ref(target, root), "HEAD", root=root).strip()
 
 
 def _zsplit(out: str) -> list[str]:
@@ -465,17 +480,28 @@ def current_state(target: str, root: Path) -> tuple[str, str, int]:
     return base, content_digest(base, marks), len(marks)
 
 
+def same_target(a: str, b: str) -> bool:
+    """Whether two target names mean the same branch.
+
+    A pure name comparison rather than a ref lookup, since this runs on a receipt that may name a
+    branch no longer present. Only the `origin/` prefix is optional, matching what `target_ref`
+    treats as the default remote, so `develop` and `origin/develop` agree while `main` and
+    `upstream/main` stay distinct.
+    """
+    return a.removeprefix("origin/") == b.removeprefix("origin/")
+
+
 def covering_passes(receipt: dict | None, base: str, digest: str, target: str) -> list[dict]:
     """The passes in `receipt` that actually cover the current content, which may be none.
 
     A pass covers the content only when the receipt agrees on all three of target, merge base, and
     digest. The target is checked because a review against one branch says nothing about the same
-    branch retargeted at another, where the diff under review is a different one. It is compared
-    as the ref it resolves to, so naming the same branch two ways is not a mismatch.
+    branch retargeted at another, where the diff under review is a different one. Its optional
+    `origin/` prefix is ignored, so naming the same branch two ways is not a mismatch.
     """
     if not receipt:
         return []
-    if target_ref(str(receipt.get("target"))) != target_ref(target):
+    if not same_target(str(receipt.get("target")), target):
         return []
     if receipt.get("mergeBase") != base or receipt.get("contentDigest") != digest:
         return []
@@ -872,7 +898,14 @@ def main(argv: list[str] | None = None) -> int:
     p_run.set_defaults(func=cmd_run)
 
     for p in (p_status, p_record, p_check, p_run):
-        p.add_argument("--target", default=None, help=f"target branch (default {DEFAULT_TARGET})")
+        p.add_argument(
+            "--target",
+            default=None,
+            help=(
+                f"target branch (default {DEFAULT_TARGET}). Resolved as origin/<value> where that"
+                " exists, else as written, so another remote's branch can be named directly"
+            ),
+        )
 
     args = parser.parse_args(argv)
     # Holds the boundary code until a verdict is actually produced.
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index b0a82bdd..8600d917 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -209,6 +209,40 @@ def test_an_unchanged_path_is_not_in_the_key(self) -> None:
         (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")
         self.assertEqual(sorted(self.marks()), ["new.py"])
 
+    def test_a_slashed_target_prefers_the_remote_over_a_local_branch(self) -> None:
+        """A branch such as release/v1 is an ordinary name, not a fully qualified ref.
+
+        Treating any slash as "already a ref" measures against the local branch of that name, and
+        a local branch that has moved on then defines the review scope with no error at all, which
+        is a silent wrong answer rather than a failure.
+        """
+        run(self.tmp, "update-ref", "refs/remotes/origin/release/v1", "HEAD")
+        run(self.tmp, "checkout", "-b", "release/v1")
+        (self.tmp / "moved.txt").write_text("local moved on\n", encoding="utf-8")
+        run(self.tmp, "add", "moved.txt")
+        run(self.tmp, "commit", "-m", "local only")
+        run(self.tmp, "checkout", "task")
+        remote = run(self.tmp, "rev-parse", "origin/release/v1").strip()
+        local = run(self.tmp, "rev-parse", "release/v1").strip()
+        self.assertNotEqual(remote, local, "fixture does not distinguish the two")
+        self.assertEqual(local_review.target_ref("release/v1", self.tmp), "origin/release/v1")
+        self.assertEqual(local_review.merge_base("release/v1", self.tmp), remote)
+
+    def test_a_target_that_only_exists_on_another_remote_is_used_as_written(self) -> None:
+        """This is what lets a fork-based flow name an upstream branch directly."""
+        run(self.tmp, "update-ref", "refs/remotes/upstream/main", "HEAD")
+        self.assertEqual(local_review.target_ref("upstream/main", self.tmp), "upstream/main")
+
+    def test_a_target_resolving_nowhere_is_a_boundary(self) -> None:
+        with self.assertRaises(local_review.CannotRun):
+            local_review.target_ref("no-such-branch-anywhere", self.tmp)
+
+    def test_the_origin_prefix_is_optional_when_matching_a_receipt(self) -> None:
+        """A skill step and a hook spelling the target differently must not deadlock."""
+        self.assertTrue(local_review.same_target("develop", "origin/develop"))
+        self.assertTrue(local_review.same_target("origin/develop", "develop"))
+        self.assertFalse(local_review.same_target("main", "upstream/main"))
+
     def test_current_state_uses_the_real_merge_base(self) -> None:
         """One half of the claim: the base in the key is the fork point git computes."""
         (self.tmp / "new.py").write_text("print('x')\n", encoding="utf-8")

From 11daa23cee11e9a77662fc986a55268ba19c382b Mon Sep 17 00:00:00 2001
From: Pieter Viljoen <ptr727@users.noreply.github.com>
Date: Sun, 30 Aug 2026 07:16:52 -0700
Subject: [PATCH 12/12] Absorb a Closed Reader Where the Writing Happens, Not
 at the Top Level

A reviewer found that a BrokenPipeError raised inside a subcommand's own print
leaves the exit code at the boundary value, so `status | head -1` reported 2 for
a run that had completed. The diagnosis is right. The proposed remedy, treating
a pipe failure with no computed verdict as success, is not, and the case that
rules it out is `check`: its diagnostics go to stderr before it returns, so a
reader closing there would report exit 0, meaning covered, for a branch that is
not. That is the one wrong answer a gate must never give.

So the error is absorbed where the writing happens instead. Every subcommand does
its work and then reports, so a reader exiting first has not made the command
fail, and each verdict stays its own rather than collapsing into a blanket
success. The top-level handler stays as a backstop for anything not reporting
through that path, and silences both streams rather than only stdout.

Measured before and after, since two of the three were wrong in different ways:

  status | head -0      1  ->  0
  check 2>&1 | head -0  120 ->  1
  status | head -1      1  ->  0

The three cases driving this run the CLI through a real shell pipeline rather
than in process, because the failure is a genuine EPIPE on a descriptor and the
120 came from the interpreter's own shutdown flush, which no in-process case can
reach.

Worth stating: no single-line revert makes those cases fail, because the two
guards are deliberately redundant. Reverting both together does, and that is the
combination checked.

Part of #1104 and #1083.
---
 scripts/local_review.py            | 116 ++++++++++++++++++-----------
 scripts/tests/test_local_review.py |  52 ++++++++-----
 2 files changed, 107 insertions(+), 61 deletions(-)

diff --git a/scripts/local_review.py b/scripts/local_review.py
index 8fa44460..baf5e621 100755
--- a/scripts/local_review.py
+++ b/scripts/local_review.py
@@ -116,6 +116,50 @@
 }
 
 
+def silence(stream: object) -> None:
+    """Point a stream's descriptor at the null device, so a later flush cannot raise.
+
+    Guarded because a stream is not always backed by a descriptor: embedded in another process, or
+    under a test that replaces it, there is nothing to redirect, and the guard against a second
+    failure would raise one itself. The descriptor is closed once the duplicate is installed,
+    since dup2 leaves the original open and this module is importable into a long-lived process.
+    """
+    try:
+        fd = os.open(os.devnull, os.O_WRONLY)
+        try:
+            # Suppressing attr-defined here: the parameter is deliberately typed as object.
+            # The guard below is what handles a stream carrying no descriptor.
+            os.dup2(fd, stream.fileno())  # type: ignore[attr-defined]
+        finally:
+            os.close(fd)
+    except (OSError, ValueError, AttributeError):
+        pass
+
+
+def emit(text: str, stream: object = None) -> None:
+    """Write a line, tolerating a reader that closed the pipe before it arrived.
+
+    Every subcommand does its work and then reports, so a reader exiting first has not made the
+    command fail. Letting the error propagate would replace a real verdict with a boundary, or,
+    once the exit code is already decided, escape as a shutdown flush that exits 120.
+
+    Absorbing it here rather than at the top level is what keeps each verdict its own. A blanket
+    "no verdict yet, so report success" would turn a broken pipe on the diagnostics `check` writes
+    into an exit 0, which reads as covered, for a branch that is not.
+    """
+    target = sys.stdout if stream is None else stream
+    try:
+        # Suppressing arg-type and union-attr here for the same reason.
+        # The stream is typed as object so any writable stand-in is accepted.
+        # The guards below are what cover a stand-in that is not writable.
+        print(text, file=target)  # type: ignore[arg-type]
+        target.flush()  # type: ignore[union-attr]
+    except BrokenPipeError:
+        silence(target)
+    except (OSError, ValueError):
+        pass
+
+
 class CannotRun(Exception):
     """The check could not be performed, as distinct from being performed and failing.
 
@@ -729,7 +773,7 @@ def cmd_status(args: argparse.Namespace) -> int:
     base, digest, changed = current_state(target, root)
     receipt, problems = read_receipt(root)
     passes = covering_passes(receipt, base, digest, target)
-    print(
+    emit(
         json.dumps(
             {
                 "target": target,
@@ -751,17 +795,17 @@ def cmd_record(args: argparse.Namespace) -> int:
     backend = BACKENDS.get(args.reviewer)
     if backend is None:
         known = ", ".join(sorted(BACKENDS))
-        print(f"unknown reviewer '{args.reviewer}'. Known backends: {known}", file=sys.stderr)
+        emit(f"unknown reviewer '{args.reviewer}'. Known backends: {known}", sys.stderr)
         return EXIT_CANNOT_RUN
     if backend["headless"]:
-        print(
+        emit(
             f"'{args.reviewer}' is headless, so it is recorded by running it rather than by hand."
             f" Use: run --backend {args.reviewer}",
-            file=sys.stderr,
+            sys.stderr,
         )
         return EXIT_CANNOT_RUN
     if args.findings is not None and args.findings < 0:
-        print(f"--findings cannot be negative, got {args.findings}", file=sys.stderr)
+        emit(f"--findings cannot be negative, got {args.findings}", sys.stderr)
         return EXIT_CANNOT_RUN
     root = repo_root()
     target = resolve_target(args.target)
@@ -774,17 +818,17 @@ def cmd_record(args: argparse.Namespace) -> int:
     # The flag is required rather than optional, since an optional guard is the one a caller omits.
     # That omission looks exactly like a pass that was properly bound to its review.
     if args.expect_digest != digest:
-        print(
+        emit(
             "the content changed between the review and this record, so nothing was recorded.\n"
             f"  reviewed  {args.expect_digest[:12]}\n"
             f"  now       {digest[:12]}\n"
             "Re-run the review over the current content.",
-            file=sys.stderr,
+            sys.stderr,
         )
         return EXIT_CANNOT_RUN
     out = write_pass(root, target, base, digest, args.reviewer, args.findings)
     reviewers = ", ".join(p["reviewer"] for p in out["passes"])
-    print(
+    emit(
         f"recorded {args.reviewer} over {changed} changed path(s) against {target}"
         f" at {digest[:12]}. Passes now: {reviewers}"
     )
@@ -798,21 +842,21 @@ def cmd_check(args: argparse.Namespace) -> int:
     receipt, problems = read_receipt(root)
     if covering_passes(receipt, base, digest, target):
         return EXIT_COVERED
-    print(
+    emit(
         "No local review covers this branch's current content.\n"
         f"  target        {target}\n"
         f"  changed paths {changed}\n"
         f"  digest        {digest[:12]}",
-        file=sys.stderr,
+        sys.stderr,
     )
     if receipt:
-        print(
+        emit(
             f"  receipt       covers {receipt['target']} at {receipt['contentDigest'][:12]},"
             " which is not this content",
-            file=sys.stderr,
+            sys.stderr,
         )
     for problem in problems:
-        print(f"  receipt       {problem}", file=sys.stderr)
+        emit(f"  receipt       {problem}", sys.stderr)
     # Printed so it runs exactly as it stands.
     # That means the full digest rather than the abbreviated form above, the required
     # --expect-digest present, and no placeholder left to substitute.
@@ -821,12 +865,12 @@ def cmd_check(args: argparse.Namespace) -> int:
     # The target is quoted because git permits a ref name to hold shell metacharacters.
     # Confirmed against a real repository for a semicolon, a command substitution, and an ampersand.
     # Interpolating one raw into a line the reader is invited to paste would let a branch name run commands.
-    print(
+    emit(
         "\nRun the local-strict-review pass over this diff, then record it:\n"
         f"  python3 scripts/local_review.py record --reviewer agent-skill"
         f" --target {shlex.quote(target)} --expect-digest {shlex.quote(digest)}\n"
         "Add --findings <count> to record how many it raised.",
-        file=sys.stderr,
+        sys.stderr,
     )
     return EXIT_NOT_COVERED
 
@@ -835,13 +879,13 @@ def cmd_run(args: argparse.Namespace) -> int:
     backend = BACKENDS.get(args.backend)
     if backend is None:
         known = ", ".join(sorted(k for k, v in BACKENDS.items() if v["headless"]))
-        print(f"unknown backend '{args.backend}'. Headless backends: {known}", file=sys.stderr)
+        emit(f"unknown backend '{args.backend}'. Headless backends: {known}", sys.stderr)
         return EXIT_CANNOT_RUN
     if not backend["headless"]:
-        print(
+        emit(
             f"'{args.backend}' is {backend['why']}, which only a live agent session can run."
             " Run the pass, then record it with the record subcommand.",
-            file=sys.stderr,
+            sys.stderr,
         )
         return EXIT_CANNOT_RUN
     root = repo_root()
@@ -849,22 +893,22 @@ def cmd_run(args: argparse.Namespace) -> int:
     base, digest, _ = current_state(target, root)
     findings, error = run_coderabbit(base, root)
     if error:
-        print(f"{args.backend} did not complete, so no pass was recorded: {error}", file=sys.stderr)
+        emit(f"{args.backend} did not complete, so no pass was recorded: {error}", sys.stderr)
         return EXIT_CANNOT_RUN
     # Re-read the content the backend actually finished against.
     # An edit during a run that takes minutes would otherwise be recorded as reviewed.
     after_base, after_digest, _ = current_state(target, root)
     if (after_base, after_digest) != (base, digest):
-        print(
+        emit(
             f"{args.backend} finished but the content changed during the run,"
             " so no pass was recorded. Re-run it.",
-            file=sys.stderr,
+            sys.stderr,
         )
         # The check ran and the honest answer is that nothing covers the content now.
         # Reporting the boundary code would let a gate that warns and continues on 2 wave it through.
         return EXIT_NOT_COVERED
     write_pass(root, target, base, digest, args.backend, findings)
-    print(
+    emit(
         f"recorded {args.backend} against {target} at {digest[:12]}, {findings} finding(s)."
         " A pass records that a review ran, never that the content is clean."
     )
@@ -920,34 +964,22 @@ def main(argv: list[str] | None = None) -> int:
         sys.stdout.flush()
         return code
     except CannotRun as e:
-        print(f"local_review: {e}", file=sys.stderr)
+        emit(f"local_review: {e}", sys.stderr)
         return EXIT_CANNOT_RUN
     # A reader that closes early, `| head -1` being the ordinary case, otherwise raises here.
     # It then raises again during the interpreter's own shutdown flush, which exits 120, outside the contract.
     except BrokenPipeError:
-        # Redirected so the interpreter's own shutdown flush has somewhere to go.
-        # Letting it raise again is what turns this into an exit outside the contract.
-        # The descriptor is closed after the duplicate is installed, since dup2 leaves it open.
-        # This module is importable into a long-lived process, where that would accumulate.
-        # Guarded because stdout is not always a real file.
-        # Embedded in another process, or under a test that replaces it, there is no descriptor.
-        # The redirect that exists to stop a second failure would then raise one itself.
-        try:
-            devnull = os.open(os.devnull, os.O_WRONLY)
-            try:
-                os.dup2(devnull, sys.stdout.fileno())
-            finally:
-                os.close(devnull)
-        except (OSError, ValueError):
-            pass
-        # A reader closing the pipe is not this check failing to run.
-        # Where a verdict was already produced, that verdict is what gets reported.
-        # Where the failure came first, `code` still holds the boundary it started as.
+        # A backstop only, since `emit` absorbs a closed reader at the point of writing.
+        # Reaching here means the failure came from somewhere that does not report through it.
+        # Whatever verdict was reached is still the honest answer.
+        # Both streams are silenced so the shutdown flush cannot raise on top of it.
+        silence(sys.stdout)
+        silence(sys.stderr)
         return code
     # A crash is the check not having run, so it reports the boundary code.
     # Falling through to the interpreter's own exit 1 would read as the not-covered verdict.
     except Exception as e:  # noqa: BLE001
-        print(f"local_review: unexpected failure ({type(e).__name__}: {e})", file=sys.stderr)
+        emit(f"local_review: unexpected failure ({type(e).__name__}: {e})", sys.stderr)
         return EXIT_CANNOT_RUN
 
 
diff --git a/scripts/tests/test_local_review.py b/scripts/tests/test_local_review.py
index 8600d917..e85bbf5b 100755
--- a/scripts/tests/test_local_review.py
+++ b/scripts/tests/test_local_review.py
@@ -27,7 +27,8 @@
 import unittest.mock
 from pathlib import Path
 
-sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+SCRIPT = Path(__file__).resolve().parent.parent / "local_review.py"
+sys.path.insert(0, str(SCRIPT.parent))
 import local_review
 
 
@@ -639,29 +640,42 @@ def test_the_remedy_quotes_a_target_holding_shell_metacharacters(self) -> None:
         self.assertIn(shlex.quote(name), line, f"the target was interpolated raw: {line}")
         self.assertIn(name, shlex.split(line), "the target did not survive as one argument")
 
-    def test_a_closed_reader_reports_the_verdict_it_reached(self) -> None:
-        """A reader that exits first is not this check failing to run.
+    def run_piped(self, args: list[str], shell: str) -> int:
+        """Run the CLI for real with its output piped into a reader that exits first.
 
-        The command produced an answer, so that answer is what gets reported rather than the
-        boundary code, which a gate would read as tooling trouble.
+        Driven through a shell rather than in process, because the failure being tested is a
+        genuine EPIPE on a descriptor, and the exit code being tested includes the one the
+        interpreter produces during its own shutdown flush, which no in-process case can reach.
         """
+        script = f"python3 {SCRIPT} {' '.join(args)} {shell}"
+        proc = subprocess.run(
+            ["bash", "-c", f"{script}; exit ${{PIPESTATUS[0]}}"],
+            cwd=str(self.tmp),
+            capture_output=True,
+            text=True,
+            check=False,
+        )
+        return proc.returncode
+
+    def test_a_closed_stdout_reader_does_not_break_the_exit_contract(self) -> None:
+        """`status | head -1` has to stay inside 0, 1, 2 rather than exiting 120."""
         (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        self.assertEqual(self.run_piped(["status", "--target", self.target], "| head -1"), 0)
 
-        class ClosedPipe(io.StringIO):
-            def flush(self) -> None:
-                raise BrokenPipeError(32, "Broken pipe")
+    def test_a_reader_that_takes_nothing_still_reports_the_verdict(self) -> None:
+        """The work completed, so a reader exiting before any of it arrived is not a failure."""
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        self.assertEqual(self.run_piped(["status", "--target", self.target], "| head -0"), 0)
 
-        saved = os.dup(1)
-        try:
-            with (
-                contextlib.redirect_stdout(ClosedPipe()),
-                contextlib.redirect_stderr(io.StringIO()),
-            ):
-                code = local_review.main(["status", "--target", self.target])
-        finally:
-            os.dup2(saved, 1)
-            os.close(saved)
-        self.assertEqual(code, 0, "a closed reader turned a completed status into a boundary")
+    def test_a_closed_stderr_reader_still_reports_not_covered(self) -> None:
+        """The case that rules out reporting a blanket success when no verdict was returned yet.
+
+        `check` writes its diagnostics to stderr before returning, so a pipe closing there would
+        leave the verdict unreturned. Treating that as success reports covered for a branch that
+        is not, which is the one wrong answer a gate must never give.
+        """
+        (self.tmp / "a.py").write_text("x\n", encoding="utf-8")
+        self.assertEqual(self.run_piped(["check", "--target", self.target], "2>&1 | head -0"), 1)
 
     def test_record_requires_the_digest_the_review_saw(self) -> None:
         """An optional guard is the one a caller in a hurry omits.