stack/agents/darkwing/work/queue-93/r1-design.md

# Row 93 (#1587), review loop: design proposal before building (Darkwing)

Read-only prep. Nothing built or run. Brief blob `dff83c09`, "A review loop through the bus".

## The problem with the brief's wording

The brief says that for `push_working` "the host ... pushes one branch of the caller's clone", and that for `fetch_review` "the host fetches the requested commit into the caller's clone". Both mean the host running git inside a clone whose session is live. Row 91's `update()` is safe because it runs only when no session of the instance runs, so nothing changes the clone between the `unsafe()` checks and the git call. Here the session is running and can write its clone at any moment. That includes the window between a check and the git call, where it can add a config key, an attribute filter, a gitlink, a hook or a symlinked `.git` piece. Row 91's round 2 escape went through a single git child the checks didn't cover. A live clone gives the session a race against every check.

## What I propose

The host never runs git in a session's clone, and never opens a path inside a session's workspace. Commits move as git bundles over a host socket.

**push_working.**

1. The runner (inside the sandbox, as the session) runs `git bundle create - <branch> --not <the working ref's tip, or main>` in its own clone and streams the bytes to the host.
2. The host caps the size and writes the bytes to a file under `bus-host/` (0700).
3. In the bare working repository it runs `git fetch <bundle> refs/heads/<branch>:refs/heads/work/<task ref>` with `transfer.fsckObjects=true`, the SAFE `-c` flags and no `+`. Git then refuses anything but a fast-forward of that one ref.
4. The host builds the ref name from a task ref it has validated. The branch name only selects a ref inside the bundle.
5. The host records the new tip in a `git.push.working` event on the task.

Git on the host reads only a pack and the bundle header, as in a fetch from an untrusted remote, which git already hardens with fsck. No session config, attribute, hook or filter is in play.

**fetch_review.**

1. The host looks up the commit from the cited `review.request` event.
2. It runs `git bundle create - work/<task ref> --not main` in the working repository and streams the bundle back.
3. The runner writes it to `/tmp` and runs `git fetch <file> work/<ref>:refs/remotes/working/<ref>` in the reviewer's clone, inside the sandbox, under that session's own config.

The reviewer ends up with the same remote-tracking ref the brief asks for. The difference is that the session's own git writes it, not the host's.

**The working repository.** `<dataRoot>/working/<business>/<project>.git`, bare, 0700. The host creates it and seeds it with `main` fetched from the project root, which the host trusts. No sandbox binds it. One lock per repository serializes pushes and fetches.

**Transport.** This follows the launch socket's pattern (`launcher.mjs` `fromSession`). The runner sends its cap, and the host resolves the business, role and run with `host.op identity`. It finds the workspace from its own launch records, never from the request. Authorization goes through the broker, as `authorizeLaunch` does today. The bundles need a length-prefixed body: the launch socket reads one JSON line under `LINE_MAX`, and the bus client caps a request at 64 KiB. So either a second socket, `bus-host/git.sock`, or an `op` field on the launch socket with a framed body. I lean toward a second socket bound only for roles that hold `git.push.working` or `review.verdict`. The launch socket today goes only to the role that may launch.

**request_review.** The broker checks the commit against the task's recorded `git.push.working` events. It needs no git and no host call. A commit no push on that task named refuses as `unknown-commit`.

**review_verdict and task_close.** The verdict cites a `review.request` event on the same task. My proposal for the citation question the brief leaves open: when `task_close` cites a verdict, the broker checks that it is a `review.verdict` on the same task with `approve`, and that its request named the task's latest pushed commit. A close with no code needs no verdict, so the broker doesn't require one on every close. A stale approval, given before a later push, refuses.

## What changes against the brief

- `checkout.mjs`: no host-side push or fetch is needed there. The git calls live in a new host module next to the launcher, for example `packages/seat/src/working.mjs` or the launcher's file. I'd ask to own one new file in place of edits to `checkout.mjs`.
- `fetch_review` runs git inside the reviewer's sandbox, not on the host.
- `sandbox.mjs` binds the new socket. The brief doesn't list it.

The outcome the brief sets is unchanged: the coder pushes, the reviewer fetches and tests in its own workspace, the PM closes citing a verdict, and the root stays unchanged.

## Tests I'd add beyond the brief's list

- A clone rigged during the push (a filter, a hook, a gitlink, a symlinked `.git/config`) leaves no marker on the host. That holds because the host never looks in the clone.
- A bundle with a broken object or a missing prerequisite, and an oversized bundle, each refuse with no ref moved.
- Every case leaves both the working repository's other refs and the project root unchanged.
- A mutant per guard: no fsck, `+` in the refspec, the ref name taken from the request, and the size cap off.

## Open points for Sage

1. Is the deviation from "the host fetches into the caller's clone" acceptable? If not, I don't know how to make the host's git safe against a live session, and I'd rather say that now than after a round.
2. A second socket, or the launch socket with an `op` field?
3. The new host file in place of `checkout.mjs` edits, which changes the row's file list.

## Sage's answers (decision 135, f3b175e1, 2026-10-13)

1. The deviation is accepted. The host never runs git in a live clone and never opens a path in a session workspace.
2. Use a second socket, `bus-host/git.sock`, bound only for roles that hold `git.push.working` or `review.verdict`. The launch socket stays bound to the launching role alone.
3. A new host file, not `checkout.mjs` edits. Name it in the packet's file list with the `sandbox.mjs` bind. Dewey reviews the bind under decision 130.

Additions to build and test:

- `task_close`: if the task has any `git.push.working` event, the close must cite an `approve` verdict on that task whose request named the latest pushed commit. Only a task with no push closes without a verdict. Tests: a pushed task closed with no citation refuses, and one closed citing a stale approval refuses.
- The first push must descend from the working repo's `main`. Check it in the bare repo after the fetch: either roll the ref back, or fetch into a temp ref and move it only after the check. Test an unrelated-history bundle.
- Re-seed the working repo's `main` from the project root, under the lock, before each push. A missing prerequisite refuses with a named reason. Test it.
- Host git in the bare repo runs with no system or global config (`GIT_CONFIG_NOSYSTEM=1`, `GIT_CONFIG_GLOBAL=/dev/null`) and `core.hooksPath=/dev/null`, plus the SAFE flags. A test must fail if fsck isn't really applied to bundle fetches; the broken-object case does that if it runs against the actual flag. Git is 2.55.0.
- Delete the bundle file on every path, refusals included. Test that `bus-host/` holds no leftover bundle after each case.

The base is HEAD once row 91 lands, not 2b2a9e14. Don't build until Sage switches the row. The design file rides in the round 1 packet; don't commit it separately.