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

Row 93, a review loop through the bus, round 1 build (Darkwing)

Issue #1587. Brief blob dff83c09. Design and Sage's answers (decision 135): r1-design.md in this directory.

Base d240645d. build.patch is git diff --cached --binary d240645d of the 29 paths below. Apply it with git apply --index to a detached worktree of d240645d, and sha256sum -c candidate-manifest.sha256 then gives 29/29 OK.

What it does

A coder pushes a branch of its clone to a bare working repository on the host. It asks a reviewer to look at the commit, the reviewer fetches and tests it in its own clone and answers with a verdict, and the PM closes the task citing that verdict. The host never runs git in a session's clone and never opens a path in a session's workspace. Commits move as git bundles over a second host socket, bus-host/git.sock.

1. push_working (verb working.push). The runner, inside the sandbox, bundles the branch in its own clone against the work ref's tip (or main) and sends it over the git socket. The host checks the cap and the action with the broker, caps the size, writes the bundle under bus-host/, and lands it in <dataRoot>/working/<business>/<project>.git as refs/heads/work/vikunja-<project>-<task>. The broker records the push as the action.allowed for git.push.working on the task, naming the commit (see "Event kinds" below).
2. request_review (verb review.request). The broker checks that the commit is the task's latest push and that the named role holds review.verdict, records a review.requested event and sends that role a REVIEW-REQUEST message.
3. fetch_review (verb working.fetch). The host looks the commit up from the cited request, bundles it as refs/mosaic/review, leaving out the commits the reviewer says it has, and sends it back. The runner fetches it into refs/remotes/working/work/vikunja-… in the reviewer's clone.
4. review_verdict (verb review.verdict). One verdict per request, approve or changes, sent to the role that asked.
5. task_close on a task with a recorded push must cite an approve verdict on that task for its latest pushed commit. A task with no push closes as before.

Files

File	Change
packages/seat/src/working.mjs (new)	the bare working repository: workRef, validBranch, push, review, the per-repository lock, the SAFE git environment
packages/cli/src/git-socket.mjs (new)	the host's git socket: framing, limits, broker checks, bundle files under bus-host/
packages/harness/src/git-tools.mjs (new)	the runner's side: bundle the branch, send it, fetch a review bundle into the clone
packages/bus/src/broker.mjs	checkPush, recordPush, reviewFetch, review.request, review.verdict, the task.close check, refuse() subject
packages/bus/src/process.mjs, runtime.mjs	the new host ops (checkPush, recordPush, reviewFetch)
packages/cli/src/launcher.mjs	starts the git socket; binds it only for a role with git.push.working or review.verdict; session.json names it
packages/seat/src/sandbox.mjs, session.mjs	the read-only bind of bus-host/git.sock; git-tools.mjs joins RUNNER_FILES
packages/harness/src/tools.mjs, runner.mjs, bundle.mjs	the four tools, local routing of the two git verbs, prompt lines
packages/tasks/src/verbs.mjs	task.close passes its verdict to the broker's check
roles/coder.md, reviewer.md, pm.md	the loop from each role's side
packages/{bus,cli,harness,seat}/README.md	the git socket, the working repositories, the tools, the limits
packages/bus/tests/review.test.mjs (new), packages/cli/tests/git-socket.test.mjs (new), packages/seat/tests/working.test.mjs (new)	see Tests
packages/harness/tests/{bundle,tools}.test.mjs, packages/seat/tests/sandbox.test.mjs, packages/cli/tests/launcher.test.mjs, packages/tasks/tests/verbs.test.mjs	one test or a few assertions each
packages/bus/tests/broker.test.mjs	one assertion: its task-subject-required check used a review.verdict event through recordEvent, which is now reserved, so it uses action.refused

Event kinds: no schema change

The first gate run failed because packages/bus/schema.sql limits events.kind to a fixed list, and the build wrote two kinds not on it, git.push.working and review.request. Adding them would be the bus's first schema change. store.mjs pins the DDL's hash and compares a live database's schema with it, so the running bus would refuse its own database (schema-mismatch) until a migration rebuilt events, an append-only table, and replaced the schema_digest row in meta, which is append-only too. That is a breaking data change the row doesn't need, so the build uses kinds the schema already has:

• The request is a review.requested event. The schema has listed that kind since row 37, and the Console already shows it as "Review requested". The verb and the action stay review.request.
• The push is the action.allowed for git.push.working, written by recordPush once the ref moved, with {project, branch, ref, commit, previous} in its body next to the usual {action, class, target}. The git socket's first broker call, checkPush, now checks the authority without recording anything (a refusal is still filed). So each landed push has exactly one action.allowed, and a refused one has none. Decision 135's "a task with any git.push.working event" reads as "a task with an action.allowed for git.push.working that names a commit".
• recordEvent refuses review.requested, review.verdict, and an action.allowed whose action is git.push.working, so a trusted adapter can't forge a push or a verdict. Mutant push-forgeable drops the last check.

If Sage wants a dedicated git.push.working kind instead, that is a schema migration and a separate ruling.

The refuse() subject, a bus behaviour change

Sage asked for this as its own item (ruling 3).

Before: broker.refuse(cap, code) filed every host-decided refusal with no context, so its action.refused event had a null subject. After: refuse(cap, code, subject = null) files it under { args: { task_ref: subject } }. The refusal's subject is then the task ref when it is one, and null otherwise, by the same rule every other refusal already uses. The git socket passes the request's task_ref (process.mjs forwards m.subject), so a non-fast-forward refusal shows in that task's trail. The launcher's three existing refuse calls pass no subject and record what they did before. runtime.mjs's authorizeLaunch became a refusing() wrapper that the git ops share; launch refusals still pass no subject.

Test: review.test "a refusal the host decided is filed on the task it names, and only a task ref counts". TASK files on TASK; 'gitea:1' (not a vikunja ref) and no subject both file null. Mutant refuse-subject-null sets the subject back to null.

broker.mjs is a bus landing, so decision 109 notices go to Mos before and after it lands. Sage sends them; the bus restart stays with Mos.

Decision 135, item by item

1. The close needs an approve verdict for the latest push. authorize() runs #closeVerdict in the same transaction as the authority it consumes, so a refusal rolls the action.allowed back. Codes: verdict-not-found (no citation, or one that isn't a verdict on this task), verdict-not-approve, verdict-stale. Tests: review.test "task.close on a pushed task needs …", and tasks verbs.test "a task with pushed work closes only citing …", which also checks that no refused close reached the tracker.
2. Descends from main. The bundle's branch is fetched into a temp ref refs/mosaic/incoming/<hex>. merge-base <commit> main must succeed before the work ref moves, else unrelated-history. The temp ref is deleted on every path. The check runs on every push, not only the first. A later push that fast-forwards a descendant passes it anyway, and running it every time costs one git call. Test: working.test "a branch with no history in common with main refuses unrelated-history".
3. Re-seed main under the lock. Each push and each review fetches main from the project root into the bare repository first, under the per-repository lock. A bundle whose prerequisites the repository lacks refuses missing-prerequisite, read from git bundle verify's message (not -q, which drops it in 2.55). Tests: working.test "main is fetched from the root again before each push …", "a bundle that needs commits … missing-prerequisite", "a root moved back after the clone …".
4. Host git environment. Every host git call runs with GIT_CONFIG_NOSYSTEM=1, GIT_CONFIG_GLOBAL=/dev/null, core.hooksPath=/dev/null and the SAFE flags, and every bundle fetch with transfer.fsckObjects=true. Test: working.test "a commit with a broken object refuses invalid-bundle, and only because fsck is on" lands the same bundle with fsck: [], so the refusal is fsck's. Mutant fsck-off empties the real constant.
5. The bundle file is deleted on every path. The git socket removes it in a finally, and the runner removes its own copy in the run's /tmp. After every case, refusals included, the git-socket tests check that no .bundle file is left in bus-host/ or in either session's /tmp. Mutant bundle-kept drops the host's delete.

Tests

New files.

• packages/seat/tests/working.test.mjs, 15 tests: workRef and validBranch; a push and a fast-forward; non-fast-forward; unrelated history; missing prerequisite; re-seed; root moved back; broken object and fsck; only a v2 bundle with the named branch; a branch named main or for another task moves only this task's ref; a record that throws moves the ref back; a repository path that isn't a directory; the review bundle and have; review with no repository; the runner's limits match the host's.
• packages/cli/tests/git-socket.test.mjs, 6 tests: the whole loop between a fake coder and a fake reviewer through a real bus, ending in the PM's close and the task's trail; refusals (no action, non-fast- forward, unknown commit or request, another task's branch); the push size cap; the fetch size cap; no capability, an unknown one, a bad request line; a rigged clone (hooks, fsmonitor, a filter, a gitlink, a symlinked .git/config) leaves no marker, and a branch whose .gitmodules carries an update = !cmd line refuses invalid-bundle at the host's fsck with the work ref unmoved. Every case checks that the project root's refs and the working repository's other refs did not move.
• packages/bus/tests/review.test.mjs, 7 tests: checkPush records nothing, push recording, and the reserved kinds and forged push; review.request; its refusals (stale-commit, unknown-commit, unknown-role, self-review, not-a-reviewer, and decision-required for a role that holds it only cross-role); reviewFetch; one verdict per request; the close check; the refuse subject.

Additions. harness bundle.test (the prompt lines per role), tools.test (which role gets which tools), seat sandbox.test (the git socket bind is read-only and only when given), cli launcher.test (the PM's gitSocket is null, the socket is 0600, the coder is bound and the PM is not), tasks verbs.test (the verdict reaches the broker).

Gate

Run 2, 15:27:52Z to 15:31:00Z, on the 29-path candidate over d240645d, in a clean worktree with node_modules linked, under the busy check. Runner: ../gate.sh.

Suite	Result
seat (node --test tests/)	105 pass, 0 fail
cli	137 pass, 0 fail
harness	69 pass, 0 fail
bus	122 pass, 0 fail
tasks	90 pass, 0 fail
webui (node --test tests/)	104 pass, 0 fail
test-auth	15 passed, 0 failed
test-conductor	17 passed, 0 failed
test-config	24 passed, 0 failed
test-discord	66 passed, 0 failed
test-extension-package	18 passed, 0 failed
test-foundation	44 passed, 0 failed
test-queue	27 passed, 0 failed
test-release	14 passed, 0 failed
test-task (DOCKER_HOST=unix:///nonexistent)	26 passed, 0 failed

Run 1 (15:19Z) failed 14 node tests. Most came from the event kinds above. The rest were three test faults: a sandbox assertion compared a relative path with RUNNER_FILES' absolute ones, the junk-bundle case sent bytes git read as a bundle with no such branch, and the rigged clone's update = !cmd line was refused by the host's fsck before any clone could run it. That refusal is right, so it became its own case.

Mutants

../mutants.sh with ../mut.mjs, 15:31Z, each mutant against the named test files. All 15 killed; every file restored.

Mutant	Site	Killed by
fsck-off	FSCK = [] in working.mjs	git-socket rigged clone, working broken object
plus-refspec	+ before the refspec	working non-fast-forward
ref-from-request	ref name from the request's branch	6 tests
cap-off-push	push size check off	git-socket loop and size cap
cap-off-fetch	fetch size check off	git-socket fetch over the cap
refuse-subject-null	refuse() subject back to null	review.test refuse subject
no-close-check	task.close skips #closeVerdict	review.test close check
no-verdict-stale	stale verdict accepted	review.test close check
verbs-drop-verdict	task.close drops verdict	tasks verbs.test
push-forgeable	recordEvent takes a git.push.working action.allowed	review.test push record
no-request-commit-check	review.request takes any commit	review.test refusals
no-merge-base	no shared-history check	working unrelated history
no-reseed-on-push	main not fetched before a push	9 tests
bundle-kept	host keeps the bundle file	5 tests
git-socket-always	git socket bound for every role	launcher.test

The runner-side transfer.fsckObjects in git-tools.mjs has no mutant. The host repository fscks every bundle it takes, so a runner without it still can't land a broken object on the host. What it guards is the reviewer's own clone.

Deviations from the brief

• The owner is Darkwing, not Filbert (Sage's assignment).
• The host-side git lives in packages/seat/src/working.mjs and packages/cli/src/git-socket.mjs, not in checkout.mjs edits (decision 135, answer 3).
• fetch_review runs git in the reviewer's sandbox, not on the host (answer 1).
• Files outside the brief's list: packages/tasks/src/verbs.mjs (the close passes its verdict), packages/tasks/tests/verbs.test.mjs, and the cli README.
• The seat README said the runner files were "ten" at base, while RUNNER_FILES held 11. With git-tools.mjs it says twelve.
• The refuse() task-ref change in broker.mjs (and its runtime.mjs and process.mjs plumbing) is beyond the design. It has its own section above, a test, and mutant refuse-subject-null.
• packages/bus/tests/broker.test.mjs L199: its task-subject-required case wrote a review.verdict through recordEvent, which this build reserves, so it now writes action.refused. The case checks the same rule.
• The event kinds differ from the design's: review.requested and an action.allowed for git.push.working, not two new kinds. See "Event kinds: no schema change".

Limits and follow-ups

• Timeouts disagree. callTool gives up at 60 s and the tool socket at 150 s, while git-tools allows 300 s for git and its socket. A push slower than 60 s looks failed to the model while the runner still finishes it. A repeat then refuses nothing-to-push, and trail shows the push's action.allowed. The 32 MiB cap keeps this rare. Documented in the harness README's Limits.
• No assignment check. Any role holding git.push.working can push to any task's work ref, and the broker doesn't check that the task is assigned to it. That matches how the other task verbs treat authority today. A follow-up row could add it.
• --no-auto-maintenance on every host fetch, so no gc runs inside the lock.
• Row 80 overlap. Row 80 also edits packages/bus/src/broker.mjs; its diff adds a STRANDED const near L47, a stranded query in #agentState (about L1047 to L1052), and stranded in that return. None of it touches lines this build changes. The nearest hunk here starts at base L1055, about a dozen lines below #agentState's return, so a rebase should apply without a conflict. It is still the same file. Sage's ruling: row 93 lands first, and row 80 rebases onto it and gates again. If row 80 lands first, I rebase and gate again.
• Base drift. At 15:28Z, d240645d..11bf4cb0 touched none of the 29 paths. It did change packages/tasks/src/fake.mjs (row 71) and some webui tests, which the tasks and webui suites run, so the landing gate on the new base covers them. Rechecked at 15:33Z on 11bf4cb00: still none of the 29 paths.
• Out of scope, as the brief says: landing a work ref, git.push.protected, a forge, credentials.