10 KiB
Row 100 (#1594) round 1: git socket hardening and refusal tests
Brief: docs/plans/2026-10-12_pm-test-fixes.md, section "Row 93 follow-up: git socket hardening and refusal tests" (commit 8fda9611, blob 017ac786).
BASE: 8e0fbca420. Candidate: build.patch against BASE; candidate-manifest.sha256 lists the files.
What changed
- packages/seat/src/working.mjs. A git past GIT_TIMEOUT_MS gets SIGTERM, then SIGKILL after GIT_GRACE_MS (5 s). git removes its lockfiles on SIGTERM. A rollback whose update-ref fails refuses rollback-failed, which outranks record's own error. push and review take
limitsso the tests can shorten both timers. - packages/cli/src/git-socket.mjs.
- readBody has a total deadline,
bodyDeadline(size) = 30000 + ceil(size / 256)ms (161 s at the 32 MiB cap), beside the 30 s idle timeout. Both refuse body-timeout. - The bundle send to a reviewer is a loop,
send(). It used to be pipeline(), whose ERR_STREAM_PREMATURE_CLOSE became git-failed. A close while chunks are still to go is fetch-aborted, and so is a close after a write of a queued chunk failed (EPIPE). A close after every chunk left with no failed write is a served fetch, because the runner reads exactlysizebytes and hangs up, often before the host's loop sees the file's end or before 'finish'. - git-failed and rollback-failed go to the log as
git socket: <code>: <message>. - close() answers host-closing to a connection still reading its request line, and drops the rest as before.
maxActiveandbodyDeadlineare options for the tests.
- readBody has a total deadline,
- packages/harness/src/git-tools.mjs. fetch_review's
havelists every clone's working refs, newest first, before any of origin's, and stops at HAVE_MAX. - packages/seat/README.md: rollback-failed, the SIGTERM/SIGKILL timing, and the lock's one-launcher-per-dataRoot assumption.
- packages/cli/README.md: body-timeout, what nothing-to-fetch, unknown-project and fetch-aborted mean to a reviewer, the log lines, and host-closing.
Tests
| Brief item | Test |
|---|---|
| not-a-role | git-socket.test: no role, then another business |
| host-closing | git-socket.test: close() while a request line is half sent |
| git-busy | git-socket.test: maxActive 2, a third connection |
| root-unreadable | working.test: a root that doesn't exist |
| unknown-project | git-socket.test: a review naming a project the business lacks |
| timed-out push leaves no lock | working.test: a hook holds the ref lock, SIGTERM, no .lock, next push lands |
| SIGKILL escalation | working.test: a git that ignores SIGTERM |
| rollback-failed | working.test (delete and previous-ref cases) and git-socket.test (logged, trail) |
| trickling client | git-socket.test: body-timeout within the deadline, slot freed |
| fetch-aborted | git-socket.test: a reviewer leaves mid-bundle; no git-failed, no log |
| a served fetch isn't fetch-aborted | git-socket.test: the review loop now closes the socket and checks action.refused is empty; a raw reviewer takes a 1 MiB bundle and hangs up at once, ten times; send() against two stand-in sockets, one that never finishes and one whose queued write fails EPIPE |
| have over 32 refs | harness git-tools.test: 53 refs over two clones |
Mutants
16 mutants, each in agents/darkwing/work/queue-100/mutants.sh, run with mut.mjs (one exact match, else exit 3) and run.py (own process group, killpg at 240 s). All 16 killed:
sigkill-first: killed (ℹ fail 1, ✖ a git past its time gets SIGTERM, leaves no ref lock, and the next push lands (10291.116)
no-escalation: killed (TIMEOUT after 240 s, ✖ a git that ignores SIGTERM gets SIGKILL once the grace period is over (20098.619138ms))
no-rollback-check: killed (ℹ fail 2, ✖ a record that fails and can't be undone refuses rollback-failed, which the host logs (39)
rollback-not-logged: killed (ℹ fail 1, ✖ a record that fails and can't be undone refuses rollback-failed, which the host logs (41)
no-root-check: killed (ℹ fail 2, ✖ a root that can't be read refuses root-unreadable (35.076601ms))
no-role-check: killed (ℹ fail 1, ✖ an identity with no role, or of another business, refuses not-a-role and moves no ref (2)
no-business-check: killed (ℹ fail 1, ✖ an identity with no role, or of another business, refuses not-a-role and moves no ref (2)
no-project-check: killed (ℹ fail 1, ✖ a review of a project the business no longer has refuses unknown-project (259.074398ms))
no-busy-check: killed (ℹ fail 2, ✖ past maxActive connections the socket refuses git-busy, and a hung-up connection frees i)
no-closing-answer: killed (ℹ fail 1, ✖ close() answers host-closing to a connection still sending its request line (180.301962m)
no-body-deadline: killed (ℹ fail 1, ✖ a bundle that trickles in refuses body-timeout at its deadline and frees its slot (5212.)
abort-as-git-failed: killed (ℹ fail 2, ✖ a reviewer gone before its bundle was sent is filed fetch-aborted, not as the host's fau)
close-is-aborted: killed (ℹ fail 2, ✖ a reviewer that hangs up the moment it has the whole bundle was served, not aborted (508)
errored-ignored: killed (ℹ fail 1, ✖ send: a close after every byte left is served; one that fails a queued write is fetch-ab)
have-origin-first: killed (ℹ fail 1, ✖ fetch_review's have lists the working refs before origin's, and no more than HAVE_MAX (1)
have-uncapped: killed (ℹ fail 1, ✖ fetch_review's have lists the working refs before origin's, and no more than HAVE_MAX (1)
no-escalation hits the 240 s cap: its test fails at its own 20 s timeout, and the wrapper git that ignores SIGTERM keeps node --test alive until run.py kills the group. The sweep after each run found none of my processes left.
What the mutants found
The first cut of send() waited for 'finish' and filed any earlier close as fetch-aborted. abort-as-git-failed showed it: the review loop test logged git-failed: the reviewer left before the bundle was sent for both of its fetches, so with the real code each successful fetch was on the trail as reviewer fetch-aborted. The review loop test checked the log but not action.refused, so it passed. Instrumenting showed the close usually comes before the loop reads the file's end, not only before 'finish'. The fix is above; the review loop now checks action.refused after close(), and the three tests in the table cover the race. A mutant that counted bytes (sent === size) survived as an equivalent: the loop's destroyed check already refuses a close with chunks to go, so I dropped the counter.
Gate
Run in a clean worktree at BASE with the candidate applied and node_modules symlinked, 16:49:50Z to 16:54:16Z on 2026-10-13, by agents/darkwing/work/queue-100/gate.sh (test-task under DOCKER_HOST=unix:///nonexistent). Every suite exited 0 with no failures:
bus rc=0 16:49:50-16:49:54 ℹ pass 127 ℹ fail 0 ℹ skipped 0 0 marks
business rc=0 16:49:54-16:49:55 ℹ pass 67 ℹ fail 0 ℹ skipped 0 0 marks
cli rc=0 16:49:55-16:50:12 ℹ pass 146 ℹ fail 0 ℹ skipped 0 0 marks
control-board rc=0 16:50:12-16:50:13 ℹ pass 124 ℹ fail 0 ℹ skipped 0 0 marks
conversation rc=0 16:50:13-16:51:01 ℹ pass 182 ℹ fail 0 ℹ skipped 0 0 marks
discord rc=0 16:51:01-16:51:03 ℹ pass 178 ℹ fail 0 ℹ skipped 0 0 marks
harness rc=0 16:51:03-16:51:14 ℹ pass 70 ℹ fail 0 ℹ skipped 0 0 marks
mosaic rc=0 16:51:14-16:51:16 ℹ pass 69 ℹ fail 0 ℹ skipped 0 0 marks
queue rc=0 16:51:16-16:51:45 ℹ pass 150 ℹ fail 0 ℹ skipped 0 0 marks
runs rc=0 16:51:45-16:51:45 ℹ pass 43 ℹ fail 0 ℹ skipped 0 0 marks
seat rc=0 16:51:45-16:51:50 ℹ pass 109 ℹ fail 0 ℹ skipped 0 0 marks
tasks rc=0 16:51:50-16:51:56 ℹ pass 90 ℹ fail 0 ℹ skipped 0 0 marks
webui rc=0 16:51:56-16:52:42 ℹ pass 107 ℹ fail 0 ℹ skipped 0 0 marks
test-auth rc=0 16:52:42-16:52:42 selftest: 15 passed, 0 failed
test-conductor rc=0 16:52:42-16:52:47 selftest: 17 passed, 0 failed
test-config rc=0 16:52:47-16:52:48 selftest: 24 passed, 0 failed
test-discord rc=0 16:52:48-16:52:55 discord suite: 66 passed, 0 failed
test-extension-package rc=0 16:52:55-16:52:56 extension package selftest: 18 passed, 0 failed
test-foundation rc=0 16:52:56-16:53:32 selftest: 44 passed, 0 failed
test-queue rc=0 16:53:32-16:54:03 queue suite: 27 passed, 0 failed
test-release rc=0 16:54:03-16:54:14 selftest: 14 passed, 0 failed
test-task rc=0 16:54:14-16:54:16 selftest: 26 passed, 0 failed
ledger rc=0 run separately: pass 78, fail 0
packages/ledger has tests but no package.json, so gate.sh prints a MODULE_NOT_FOUND line and "ledger: no test script" for it. I ran node --test packages/ledger/tests/ on its own; 78 passed. The ledger tests don't touch the row's files.
The cli suite also passed eight runs in a row after the send() fix (five before the counter came out, three on the final code).
BASE..origin/refactor (a50c9d23a) touches none of the eight manifest paths. build.patch applied to a fresh BASE checkout reproduces candidate-manifest.sha256.
Choices
- host-closing comes from the waiting map in close(). The
if (closed)check after readHeader stays as a guard; with the map it is unreachable in practice. - body-timeout is a new code for both timeouts. The idle timeout refused invalid-request before. broker.refuse takes any
/^[a-z-]{1,64}$/code, so there is no schema change. The 30 s idle path has no test of its own; the deadline test uses the same done(). - fetch-aborted isn't logged: a reviewer leaving is not a host fault.
havesorts by committer date, newest first, inside each remote.
Not owned, for the reviewers
roles/reviewer.md and the harness README and tools.mjs are what a reviewer reads, and row 100 doesn't own them. The nothing-to-fetch text is in packages/cli/README.md. Pointing roles/reviewer.md at it is a follow-up.
Follow-ups (not in this row)
- packages/seat/src/checkout.mjs git() and the harness runner's git() still use spawn's timeout with SIGKILL.
- The kill goes to the direct git child, not its process group; a hook or helper git spawns may outlive it.
- startHost's host.json single-host check isn't atomic (the README says so).