Workspace and checkout¶

How a pull request's code gets onto disk, why none of it is ever run, and what is left behind afterwards. Implemented in src/pr_review_agent/workspace/.

📦 What it does¶

Given the facts about one pull request, it produces a directory containing that pull request's code at an exact commit, plus the diff against the merge base, and removes the directory afterwards.

It does nothing else. No review, no prompt, no publishing, and no tokens. Its one caller is the review worker, which checks a pull request out between the governor's admission and the engine's run; it landed before that caller existed, exactly as the queue and the governor did.

🚫 Why a checkout rather than the API diff¶

GET /pulls/{n}/files paginates and truncates on a large pull request; git diff does not. A local diff costs no request, so it does not compete with the poller for the rate-limit budget POLLER.md depends on. And a reviewer needs surrounding code, sibling files and AGENTS.md — context that only exists in a checkout.

DESIGN.md already listed github.com among the egress prerequisites. The design assumed a checkout from the beginning and never wrote it down.

🪞 One mirror, a worktree per run¶

cache_dir/
├── <owner>__<name>.git/        one bare mirror, incrementally fetched
└── runs/<run-id>/              per-run worktree, removed on teardown

The mirror carries the full commit graph, so git merge-base is always answerable and the second review of the day fetches almost nothing.

The run directories are siblings of the mirror, not children. $GIT_DIR/worktrees/ is where git keeps each worktree's own HEAD, index and commondir; a working tree placed there serves both roles at once and reports git's administrative files as untracked.

cache_dir is made absolute before anything uses it. Every git command here is git -C <mirror>, which is a chdir: a relative path on that argv is resolved against the mirror, not against the daemon's working directory. A relative cache_dir therefore put each run directory inside $GIT_DIR while Checkout.path still named a location the engine could not find, and every review failed at the subprocess launch. Resolving at the boundary is what keeps the two readings the same one.

The only shared mutable state is the mirror's ref namespace, and one asyncio.Lock serialises every write to it — the fetch and the teardown's ref deletion, because update-ref -d and a concurrent fetch contend for packed-refs.lock. One process-wide lock suffices because the daemon is a single process, the same assumption the queue's per-PR lease already rests on.

A fork is not a special case anywhere in this: refs/pull/{n}/head lives in the base repository for forks and branches alike.

Both refspecs are forced. Without the + on the base branch, a force-push upstream makes the fetch fail non-fast-forward — and then every review of every pull request on that base fails until an operator intervenes.

🛡 The tree is untrusted¶

A fork's pull request head is attacker-controlled content being written to the host filesystem and then handed to an agent. DESIGN.md already refuses the self-hosted-runner design for the neighbouring reason.

The environment is the control, not the flags. Every invocation goes through one runner in gitcmd.py, which builds the child environment from nothing rather than inheriting it. GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM at os.devnull are the load-bearing pair: the host's gitconfig is where core.hooksPath, core.fsmonitor, diff.external, credential helpers and smudge filters are all defined, so nulling that one file disables every one of them at once rather than needing a flag each.

GIT_ALLOW_PROTOCOL=https is a whitelist that overrides all protocol.* config. It is a constant with no parameter, config key or override. The obvious alternative, -c protocol.allow=never, is defeated by a single protocol.ext.allow=always anywhere in a config the nulling missed — and an ext:: submodule URL is the shortest path from "checked out untrusted code" to "ran it".

The per-invocation flags are redundancy, with two exceptions that stop things config-nulling does not:

Flag	What it stops
`core.symlinks=false`	A symlink resolving outside the worktree.
`transfer.fsckObjects=true`	A malicious pack — a tree containing `.GIT/`, a malformed `.gitmodules` — accepted at `index-pack`.
`core.hooksPath=<devnull>`	A hook planted in the mirror's own `hooks/`.
`submodule.recurse=false`	Recursion into a submodule.
`filter.lfs.*`	A filter named `lfs`, and only that one.

Symlinks are a leak, not an execution bug¶

This is the one worth stating plainly, because "nothing is executed" does not cover it. A fork can replace AGENTS.md — a file this design says the reviewer reads — with a symlink to ~/.claude/.credentials.json. Nothing runs; the reviewer simply reads the target and can quote it into a public review comment.

core.symlinks=false makes git write a plain file containing the link target instead. It has to be decided here, because no later layer can undo a symlink that already exists.

The diff is computed in the mirror¶

git diff honours the .gitattributes of the tree it runs in. A pull request that adds *.py -diff renders its own Python changes as Binary files ... differ, with --numstat reporting - -: the reviewer sees nothing, while the API's additions count looks entirely normal.

So the diff runs in the bare mirror, which does not read in-tree attributes, with --no-ext-diff. The worktree still exists for the engine to read surrounding files; it is simply not where the diff is produced.

The fetch is anonymous¶

No credential reaches git: not on the argv, not in the remote URL, not in .git/config. The target repository is public, so the poller's token buys nothing here, and not passing it is one fewer way to leak it to disk. A private repository fails the fetch loudly, which is the correct outcome for a capability that has not been designed.

tests/test_workspace.py::test_no_credential_reaches_the_wire asserts this against the requests the test double actually received, rather than against the command line.

What is not covered¶

Path traversal (.git, .GIT, git~1) is refused by git itself, with nothing written. The deployment target is Linux with a case-sensitive filesystem, which this design assumes.

Sandboxing the engine — containers, seccomp, a read-only tool set — is DESIGN.md's concern, not this module's. What this module guarantees is that the checkout executes nothing and that the tree cannot reach outside itself.

🔁 A later round sees only what changed¶

Every round used to diff merge_base..head, so round five re-read what rounds one to four had already read. Now a round after a completed one is handed only what changed since the head that round reviewed. The worker passes that head in as since_sha, and workspace/since.py decides where the diff starts. Checkout.since_sha records the answer: the previous head when the diff is incremental, None when it covers the whole pull request.

Trees are compared, not history. The reviewer never read the commits. It read the old head's content, and git diff <old head> <new head> compares two contents whatever the graph between them looks like. So a force-push is the ordinary case, not a reason to start over:

What the contributor did	What the round is shown
Pushed a fixup commit	The fixup
Amended, squashed, reworded or reordered	What the rewrite changed, often nothing
Rebased onto a newer base	What changed beyond the rebase
Merged the base branch in	What changed beyond the merge

When the base moved, the old content is replayed first. A rebase or a merge from the base carries upstream commits into the new head, and a plain diff against the old head would show them as the pull request's own. When the old head's merge base differs from the new one, the old head's changes are replayed onto the new merge base with git merge-tree --write-tree, and the diff starts from that tree. This cannot hide anything the contributor wrote: a line that reads the same in the replayed tree and the new head is either content the previous round reviewed, or the base branch's, which is not the pull request's.

Everything else is a full round, logged with the reason. None of these is refused, and none diffs against nothing. Any other git failure while placing the old head raises, like every other step of the checkout.

the first round on a pull request;
a head that has not moved, such as a mention asking for a re-review;
an old head the mirror no longer holds, or one sharing no history with the base;
a replay that conflicts, or that fails outright, which is also logged at ERROR so a stuck replay cannot stop a pull request being reviewed;
a git older than 2.38, which has no --write-tree. It says so once at WARNING, and every rebased pull request is reviewed in full. Fixups and amends are still incremental. The floor stays 2.32.
fewer new commits than budget.incremental_min_commits, or a previous round more recent than budget.incremental_min_seconds. Both default to 0, which disables them.

The tree on disk is always the whole head. Only the diff narrows. The previous round's findings still reach the prompt in full, and the prompt says the diff is incremental and that an earlier finding may sit on lines outside it, so "still present" stays checkable.

Like every diff here, the replay runs in the bare mirror, so no in-tree .gitattributes or merge driver is consulted, and the config that could name a driver is nulled.

💰 The caps are layer 2 of the budget¶

budget:
  max_changed_files: 100
  max_changed_lines: 5000
  excluded_paths: ['**/*.snap']   # added to the built-in list

A pull request over either cap raises PullRequestTooLarge before a worktree exists and before any diff can reach an engine. The exception carries a notice: the same refusal addressed to the contributor rather than to the journal, which the worker posts on the pull request through Publisher.notify. Both read the same four fields, so the number an operator sees and the number the pull request is told cannot drift apart.

The caps are measured on git diff --numstat with excluded_paths applied, not on the API's additions / deletions / changed_files. That is why the gate runs after the fetch rather than before it: those three are aggregates with no per-path breakdown, and a lockfile cannot be subtracted from an integer. The trade is set out in BUDGET.md — a fetch costs bandwidth, and these caps bound tokens.

The same pathspec arguments produce Checkout.diff, so the engine is shown exactly what the caps counted. On an incremental round both measure the same narrower range, from one variable, so a small fixup on a large pull request is no longer refused for the size of the whole — while a fixup that is itself over a cap still is. Checkout.reviewed carries those two numbers for the pre-flight estimate.

Checkout.omitted is the other side of the same range: the changed paths the pathspec kept out, so the posted review can say what it did not read. It costs two more local git diff --name-only -z calls, one with the pathspec and one without, and only when the effective exclusion list — the built-ins plus excluded_paths — is non-empty. Both lists are needed because the grouping never names a directory that holds a reviewed file: web/node_modules/ collapses to one entry, while three lockfiles scattered beside reviewed code stay three entries.

A refusal now happens between the fetch and the worktree, where a run-scoped ref already exists, so the ref is deleted on the way out. Before exclusions the gate fired before the fetch and there was nothing to clean up; leaving it would leak one ref per refused pull request until the next startup sweep.

They live in budget rather than in workspace so that every spending cap stays in one specification and one section — see BUDGET.md, layer 2. That also makes them reloadable: budget is the only section SIGHUP hot-swaps. The caps are therefore passed per checkout rather than captured when the Workspace is built, because a workspace holding a snapshot would silently ignore a tightened cap, which is the exact failure the reload mechanism exists to prevent.

Both exist because neither bounds the other: two thousand one-line files pass a line cap and still bury the engine; one fifty-thousand-line generated file passes a file cap.

They bound what the engine reads, not the disk. A fetch pulls every object reachable from the head, so a commit that adds a 100 MB blob and a later one that deletes it reports additions: 0 and still downloads 100 MB. Disk is bounded in practice by GitHub's own repository limits and by watching cache_dir. fetch --filter=blob:limit=N is the git-native knob if a real byte bound is ever wanted.

🧹 The startup sweep¶

A crash between worktree add and teardown leaves a worktree and a run-scoped ref behind forever. Workspace.sweep() prunes stale worktrees, deletes any leftover refs/run/*, and removes the runs/ directory.

It also clears stale *.lock files, and startup is the only safe moment to do that: mid-flight a lock may belong to a live process, while at startup no git of ours is running. Those locks come from the one case a timeout cannot avoid — Process.kill() is SIGKILL, and git removes its own locks on SIGTERM but cannot on SIGKILL. The runner therefore terminates with a grace period before killing, and the sweep catches whatever still survives.

🔭 What the workspace does not do¶

It does not write head_sha back to the queue row. It returns the resolved value; the worker that claimed the row is what knows the row.
It does not decide whether the head is still current. The fetched sha is what gets checked out and reported — the head can move between the API read and the fetch, and a review has to name the commit it actually read. Re-checking against the live head immediately before posting belongs to the publisher, as QUEUE.md sets out.
It does not read the tree. It puts it there.