benchd M0: the bench daemon skeleton, with the risky bets spiked first

[Wirasm]

The future-planning roadmap's M0, started on the operator's word: helm needs a headless daemon to grow into, and this adds its skeleton — a self-contained Rust workspace at `daemon/` with three crates (`benchd` the daemon, `bench` the CLI, `bench-wire` for every wire type and shared rule, spelled once so the binaries cannot drift).

What works today: a suite-aware record root (`BENCH_SUITE` isolates socket and state, `BENCH_DIR` overrides for tests; a name that cannot isolate refuses the launch instead of falling back to the live root), an append-only `events.jsonl` where every mutation is logged before the response reporting it, one unix socket answering `status`/`events`/`stop`, and helm's spool exit codes on the CLI. `daemon/direction.md` carries the spine — bench-visible means logged, one door, one spelling — plus the posture: agents are smart, expose capabilities, never parse prose.

Three spike verdicts ride along in `daemon/spikes/`, because the design's riskiest assumptions were measured before anything builds on them: a headless pty owner hosts full interactive claude/codex/pi sessions (replies in 8–10s, live resize survived); agent-to-agent mail works as files + a log + a `mail/received ⇒ agent/wake` reactor with the pty paste as the universal wake (a number passed around the three-runtime ring gained +1 per hop, 31.5s cycle); and a group room generalizes it with fan-out wakes (exactly the predicted 9 wakes, room settled on its own). `daemon/justfile` is the wanted surface as an executable spec — `just spec` prints which verbs the daemon answers today.

Validation: `bash daemon/test.sh` green — fmt, clippy `-D warnings`, build, 6 wire unit tests, 10 conformance tests running the real binaries across every status case, including the M0 prove line (a live and a suite instance sharing nothing, with the negative control that the shared root never appears). No Swift file is touched; the new CI job triggers only on `daemon/**` and the Swift jobs are unchanged in both directions.

[Wirasm]

---

pr: 340
base: development
head: feat/benchd-m0
reviewed: 2026-08-18T10:41:52Z
reviewed_head: 5c0ac88
verdict: NEEDS FIXES
open_findings: 7
scopes: [code, seams]
publication: #340 (comment)

PR Review: #340 — benchd M0: the bench daemon skeleton, with the risky bets spiked first

Signal

M0 does what it says: the gate is green, the isolation promise is proven with a real negative
control, and bench-wire is genuinely the single spelling everywhere the socket is involved.
Two reproduced defects hold it back, and they share one cause — the daemon has no bounded
recovery from a caller or a file that misbehaves. A single torn line at the tail of
events.jsonl refuses every future start on that root with no route out (R1), and a single
client that connects without finishing its line parks the only accept loop, so every other
agent hangs with no exit code at all (R2). Both were reproduced twice, once by the reviewer and
once independently here, against the real binaries. The remaining findings are the same shape
one milestone earlier: the pre-socket halves of contracts bench-wire owns — client-side exit
codes, the verb list, the event envelope — were hand-typed rather than derived, in the PR that
argues at length against exactly that. Nothing here is wrong today; all three become wrong the
first time somebody edits one copy.

• Blocking: 2
• Non-blocking: 5
• Resolved since previous review: Not applicable (first review)
• Tracked follow-ups: None
• Validation: bash daemon/test.sh PASS (16 tests); exit-code drift proven detectable by mutation; all four CI checks green on the merge commit

Findings

ID	Severity	Finding	State
R1	Important	A torn last line in events.jsonl refuses every subsequent start on that root, permanently, with no repair verb and no route named	OPEN
R2	Important	No read/write timeout on either side — one stalled client parks the daemon's only accept loop, and callers hang with no exit code	OPEN
R3	Suggestion	refuse()/fail() in both binaries return literal 3/4 instead of Status::exit_code(), and no test ties the two spellings together	OPEN
R4	Suggestion	events.jsonl carries no format/version marker although it is the record read outside the process	OPEN
R5	Suggestion	The verb set is spelled three times — dispatch arms, the refusal string, the justfile probe list — none derived from another	OPEN
R6	Suggestion	The new daemon CI job is not a required status check, so a red daemon gate does not block a merge	OPEN
R7	Suggestion	AGENTS.md:12 and docs/future-planning/README.md still say the folder describes something that does not exist — false the moment this merges	OPEN

Detailed Findings

R1 — a torn tail line bricks the daemon on that root, forever

Impact: After any interrupted append, benchd refuses to start on that record root on every
subsequent attempt, and bench <verb> reports no daemon (exit 2) indefinitely. There is no
repair verb, no --truncate-corrupt-tail, and no code path that advances past the bad line — the
only way back is hand-editing the JSONL outside the product. daemon/AGENTS.md requires that
"a refusal names the rule it applied and the route to use instead"; this refusal names the line
and no route, because none exists.

Evidence: daemon/crates/benchd/src/main.rs:151-174 — the boot integrity scan's only two
outcomes on a bad line are StartError::Failed (I/O) and StartError::Refused (parse); neither
recovers. append() at :361-377 is a bare write_all + flush, no fsync and no atomic
rename-into-place.

Reproduced independently here on the real binaries: a clean two-event log, one truncated line
appended, then two restart attempts —

benchd: refusing to start: event log …/events.jsonl line 3 is not a readable event
  (EOF while parsing a string at line 1 column 56) — refusing to append after history
  this daemon cannot read

identical on attempt 1 and attempt 2 (so it is permanent, not transient), with bench status
exit 2 afterwards. The reviewer reproduced the same sequence separately.

A crash is not the only way in, and this is a code-path reading rather than something reproduced:
write_all loops over short writes, so an ENOSPC part-way through one line returns Err with
the partial line already on disk — a full disk tears the tail with no crash involved.

Required outcome: Forgive a torn last line only — accept the log up to the last cleanly
parsed line and quarantine or drop the tail with a loud, distinct log line — or add the repair
route the refusal should be naming. A bad line in the middle should keep refusing: that one is
unexplained rather than an interrupted append.

Found by: prp-core:code-reviewer (reported Critical; recorded here as Important — the record
holds nothing that must survive at M0, no committed data is lost, and the refusal is loud rather
than silent, so the cost is service on that root rather than history. It still blocks: the fix is
small, and every later milestone inherits this file as the thing that "must survive")

Disposition: OPEN

R2 — one non-responsive client wedges the daemon, and the caller hangs with no exit code

Impact: serve() is a single-threaded accept() → handle() loop and handle() blocks in
read_line with no read timeout, so a client that connects and never sends a newline parks the
daemon for everybody. Every other caller then blocks in its own unbounded read_line on the
client side. The CLI's header promises "the exit code IS the outcome — an agent reads $?, not
prose"; a hung caller produces no exit code at all, which is the one failure shape an unattended
agent cannot act on.

Evidence: daemon/crates/benchd/src/main.rs:216-245 (accept loop and unbounded read; no
set_read_timeout anywhere in the workspace) and daemon/crates/bench/src/main.rs:114-118 (the
client's own unbounded read_line).

Reproduced independently here: baseline bench status exit 0; then one raw socket holding a
partial request line —

baseline status exit=0
status WHILE stalled client holds: exit=124 (hung, killed by timeout 5)
status AFTER stalled client closed: exit=0

Service returns the instant the stalled client closes and needs no restart, which isolates the
cause to the missing bound rather than to a crash. The reviewer reproduced the same.

This also contradicts the file's own claim at main.rs:18-20 — "a bounded, inspectable behavior
beats a concurrency story nothing needs yet". Serial handling is the bounded choice only if a
single connection is itself bounded, and nothing bounds it.

Required outcome: set_read_timeout/set_write_timeout on the accepted stream in handle(),
and the same on the client's stream, mapping a timeout onto the existing Refused /
EXIT_NO_DAEMON paths rather than blocking forever. Serial handling can stay.

Found by: prp-core:code-reviewer

Disposition: OPEN

R3 — the exit-code contract is hand-typed in both binaries, beside the crate that exists to spell it once

Impact: Status::exit_code() (bench-wire/src/lib.rs:158-166) maps 0/3/4, and every
socket-answered outcome goes through it. But the pre-socket refusals do not:
bench/src/main.rs:137-145 (refuse → 3, fail → 4) and benchd/src/main.rs:90-98
(refuse_start → 3, fail_start → 4) return bare integers, and bench does not even import
Status (main.rs:14-16). Change the enum and the socket-mediated half moves while the
bad---suite, bad-argument and cannot-bind half silently keeps the old code. There is no runtime
boundary here to earn the honest-duplicate carve-out — both binaries already compile against the
crate.

Evidence: Measured, not argued. Mutating Status::Refused => 3 to 5 in bench-wire and
rebuilding turns an_unknown_verb_is_refused_with_exit_3_naming_the_known_verbs red
(conformance.rs:168) — so the socket path is covered — while
a_suite_that_cannot_isolate_is_refused_client_side_before_any_socket stays green, because it
pins its own literal 3 against a code path that never consults the enum. The mutation was
reverted and the gate re-run green (16/16).

That green test under a mutated contract is the finding: this is the one seam the gate cannot see.

Required outcome: Return Status::Refused.exit_code() / Status::Error.exit_code() from all
four functions. One line each, no new type — the type is already there and already in scope on one
side.

Found by: prp-core:seam-analyzer, with the drift-detection gap measured during validation

Disposition: OPEN

R4 — the event log has no envelope version, though it is the record read from outside

Impact: Event (bench-wire/src/lib.rs:184-193) carries seq/at/kind/data and nothing
saying what envelope shape wrote it. AGENTS.md states the rule this repo already applies to the
analogous case: "Anything read outside the process says so in its header too: BenchSnapshot
carries format, version and writtenAt, so a reader that predates a change fails loudly
instead of misreading it." A shape-incompatible future line does fail loudly here; a
shape-compatible, semantics-changed one (gapped seq, a second daemon generation) decodes clean
and is misread silently.

Evidence: bench-wire/src/lib.rs:184-193 against AGENTS.md's discriminator rule, and
lib.rs:7-9, which names the second reader as the documented next step ("anything that later
reads these types from Swift gets a generated or conformance-pinned copy"). Note the file-level
option does not work for this file: read_events's since parameter
(benchd/src/main.rs:331-359) means a reader can legitimately start mid-stream and never see
seq: 0, so the marker belongs on the record.

Required outcome: One format/v field on Event, on the BenchSnapshot precedent — cheaper
now than as a migration once the Swift copy exists, which is AGENTS.md's own argument about
RequestID/Handle/TerminalID.

Found by: prp-core:seam-analyzer

Disposition: OPEN

R5 — the verb set is spelled three times, none derived

Impact: dispatch's match arms (benchd/src/main.rs:283-313) are the real set; the refusal
string fifteen lines below hand-types the same three names ("this daemon answers: status, events,
stop", :306); and daemon/justfile:91-92 hand-types a third list for just spec, whose header
calls itself "a live scoreboard of the surface". The scoreboard self-corrects a row marked "not
yet" once a verb lands, but it cannot report a verb nobody added to the array — it under-reports
with no error. Nothing in daemon/test.sh or the conformance suite exercises the justfile at all.

Evidence: benchd/src/main.rs:306 and daemon/justfile:91-92; conformance.rs:170 pins only
that the refusal string still contains the substring, rather than deriving it from the verb set.

Required outcome: At minimum a const KNOWN_VERBS that dispatch matches against and the
refusal string is built from. The justfile list is the lower-priority half — a caveat in its own
header is proportionate at three verbs; M1 is where it first gets tested.

Found by: prp-core:seam-analyzer

Disposition: OPEN

R6 — the daemon gate is advisory: it is not a required status check

Impact: Branch protection on development requires exactly three contexts — build · test · format, mailbox hooks · conformance, skill gates. The new job is named fmt · clippy · build · test and is not among them, so a PR whose daemon gate is red is still mergeable. The Swift half of
the estate is enforced; the Rust half is not.

Evidence: gh api repos/Wirasm/helm/branches/development/protection →
contexts: ["build · test · format", "mailbox hooks · conformance", "skill gates"], against
.github/workflows/daemon.yml:15 (name: fmt · clippy · build · test).

Worth knowing before reaching for the obvious fix: because daemon.yml is paths:-filtered, the
job does not run at all on a non-daemon PR, so simply adding it to the required list leaves every
other PR pending forever. Enforcing it means either dropping the filter (which is what
gate.yml's own comment argues for — "A paths: hooks/** filter would skip exactly the PR that
breaks it") or an always-running job that no-ops when daemon/ is untouched. Note the roadmap
invariant this PR cites pulls the other way ("its own gate, run only when touched"), so this is a
decision to make rather than a bug to fix.

Found by: validation

Disposition: OPEN

R7 — two steering documents assert the daemon does not exist

Impact: AGENTS.md:12 says of docs/future-planning/: "the rest describes something that does
not exist", and docs/future-planning/README.md opens "Nothing in this directory is built, and
nothing in it authorises building." Both are true until this merges and false after it — M0 is
built, and AGENTS.md's own new paragraph says so eleven lines further down ("M0 (skeleton and
suite isolation) is the part that exists"), which leaves the file contradicting itself. The README
is the first thing an agent sent to that folder reads.

Evidence: AGENTS.md:12 and the added block at AGENTS.md:172-181;
docs/future-planning/README.md:3. The README itself names this failure mode: "a document that
asserts something untrue is expensive whether the untruth is behind it or ahead of it" — and
records that this repo spent 2026-08-10 fixing seven such claims.

Required outcome: One sentence in each, saying M0 landed and the rest is still a proposal. The
"work starts when the operator names a milestone" rule is unaffected and should stay.

Found by: validation

Disposition: OPEN

Agent Coverage

Scope	Result
code	R1, R2
seams	R3, R4, R5
validation	R6, R7

Validation

Command	Result	Evidence
bash daemon/test.sh (fresh worktree of the PR head)	PASS	fmt-check, clippy -D warnings, build, 6 wire unit tests + 10 conformance tests, 8.8s; no orphaned benchd or cargo processes afterwards
Mutation: Status::Refused => 5, rebuild, cargo test --workspace	PASS (drift detected, one gap)	an_unknown_verb_is_refused_with_exit_3… red at conformance.rs:168; a_suite_that_cannot_isolate_is_refused_client_side… stayed green → R3. Reverted, tree clean, gate re-run 16/16 green
Negative control: ls ~/.bench after the full suite	PASS	No such file or directory — the shared root was never created on this machine
just spec (in daemon/)	PASS	Prints the live scoreboard, 3 ✓ / 14 ✗, and leaves no stray daemon or cargo process
cargo metadata at the repo root	PASS	could not find Cargo.toml — the deliberate loud failure the workspace comment promises
Torn-tail reproduction (bounded, disposable BENCH_DIR)	FAIL (R1)	Restart refused identically twice; bench status exit 2 afterwards
Stalled-client reproduction (bounded, disposable BENCH_DIR)	FAIL (R2)	bench status exit 124 while a partial-line client holds; exit 0 the moment it closes
CI on the merge commit	PASS	All four checks green: build · test · format, fmt · clippy · build · test, mailbox hooks · conformance, skill gates
swift build && swift test && make lint && xcodegen generate	NOT RUN	The diff touches zero Swift (git diff --stat …-- '*.swift' is empty), and CI ran the full Swift job green against the merge commit — stronger than a local run against the branch tip

Verdict

NEEDS FIXES

R1 and R2 are both reproduced, both small to fix, and both land on the property M0 exists to
establish — that this daemon is the thing which survives. R2 in particular is the failure shape an
unattended agent cannot recover from: no exit code, no timeout, no answer. Fix those two and the
verdict flips.

R3 is a one-line change in four places and the measurement is already in hand, so it is worth taking
in the same pass even though it blocks nothing. R4 and R5 are cheapest now and become migrations
later; R6 and R7 are decisions for the operator rather than code defects.

Everything else examined came back clean, and two things are worth recording as proven rather than
assumed: the suite-isolation promise has a real negative control that runs against real subprocess
binaries with HOME overridden, and stop really is logged before it is answered — verified from
the file rather than from the daemon.

[Wirasm]

Review addressed at aec13ca — every finding has a disposition, and the two blocking ones were fixed test-first (each new test watched red against the pre-fix binaries, then green).

ID	Disposition
R1	Fixed. A torn last line is quarantined to events.jsonl.torn-<ts>, the log truncates to its last whole event, and the repair is logged as log/repaired — the record's own surgery is in the record. A bad middle line still refuses naming the line. Appends now sync_data. Red first: the torn-tail test reproduced the permanent brick before the fix.
R2	Fixed. I/O bounds spelled once in bench-wire: the daemon gives a connection 5s and answers a refusal on timeout; the client waits 15s (strictly longer, so a daemon refusal outruns the client giving up) and maps a timeout to exit 2. Red first: the stalled-client test hung past 12s and the mute-daemon test past 25s on the old binaries.
R3	Fixed. All four pre-socket helpers derive from Status::exit_code(); bench now imports Status. The mutation gap closes because both spellings are one.
R4	Fixed. A fresh log opens with a log/format event (bench.events-log v0); conformance asserts it is line one.
R5	Fixed. KNOWN_VERBS + a parsed Verb enum in bench-wire: the dispatcher matches the enum (a new verb is a compile-forced decision), the refusal string derives from the list, and a conformance test pins the justfile's probe line to it by reading the justfile's own source.
R6	Operator's call. Making the daemon job a required status check is a branch-protection setting; happy to flip it on the word.
R7	Fixed. future-planning/README.md and the roadmap now say M0 landed (this PR); everything else there remains a proposal and the milestone-naming rule stands.

Gate after fixes: fmt, clippy -D warnings, 7 wire unit tests, 16 conformance tests — all green, no stray processes.