boxmaker/docs/plans/M3a/checks-c.md

# M3a checks, fork C: `loopd` (tasks 16 and 17)

Reference tree: `~/src/boxmaker-ref-c`, branch `m3a-ref-c`. The rest of the workspace does not
build its tests there (`proto`'s `records.rs` and `wire.rs` belong to other forks), so every
command below is `-p loopd`. `cargo clippy -p loopd -p bxctl --all-targets -- -D warnings` is
clean, and `scripts/check-lines.sh` passes.

## Task 16: the tool port, the registry and denials

**Check: in effect a reference.** The task changes working code, and the smallest change that lets
the updated tests compile is the whole change (about 150 lines in `tools.rs`, 30 in `turn.rs`). A
`todo!()` skeleton was not possible: the existing turn-loop tests run through this code.

Run to green, three times: `tools` (10), `turn` (6), `turn_broker` (5, new), and unchanged
`limits` (9), `channel` (6), `session` (7), `baseline` (7), `serve` (3) and the rest of the suite.

Defects the check exposed:

1. **Spec, section 8.** "`echo` stays in `FakeTools` for tests only" is not enough: two recorded
   conversations (`find_tool.http`, `call_tool.http`) and four test files discover `echo`
   through the registry. Removing `Registry::m2b()` would have meant new copies of `baseline.rs`,
   `channel.rs` and `support/turn.rs` for a one-word change each. Resolved: `m2b()` stays as the
   test registry, `m3a()` is what `loopd serve` uses. Proposed wording for section 8: "`echo`
   stays in `FakeTools` and in the test registry `Registry::m2b()`; `loopd serve` uses
   `Registry::m3a()`."
2. **Spec, section 8, unclear.** `tool_denied { name, reason }` does not say which name. For a
   `call_tool` call, `call.name` is `call_tool`, which tells the owner nothing. Resolved: both new
   events carry the tool `brokerd` decides on (`request.tool`); `tool_call_started` and
   `tool_result` keep `call.name`. A test pins it (`a_denied_call_tool_names_the_target_tool`).
   Task 20 (`bxctl chat`) should know that `ApprovalPending.tool` is the target tool.
3. **Spec, silent.** What `run_call` does if a port returns `PendingApproval` as its answer. It
   cannot be removed (the `match` must be exhaustive while `ToolPort` returns `ToolResponse`).
   Resolved: a plain failure, "The tool failed: the tool broker gave no final answer", no event.
   It is a failure of the port, not a decision about the call.
4. **Tests.** `turn.rs` grew past 500 lines; the new tests moved to `turn_broker.rs`.
5. **Verified claim.** "The registry change affects only new `find_tool` results; the tools array
   is unchanged" is true: `Baseline::assemble` takes only `registry.core_schemas()`, which is
   `clock`, `find_tool`, `call_tool` with identical schemas for `m2b()` and `m3a()`; resumed
   sessions use their snapshot. `the_m3a_registry_has_the_same_core…` asserts equality.
6. **Plan, outside this fork: `make verify-device` breaks at task 16.** `device.rs` asks Ornith to
   "use the echo tool" through `loopd serve`, and asserts that `find_tool` and `call_tool` were
   called. With `m3a()` there is no `echo` to find. Task 17's files carry a changed `device.rs`
   (find a file-reading tool, call it on `/etc/hostname`, which fails with "no tool broker is
   configured"; the restart check asks for the first turn's words instead of the echoed word). It
   compiles and is clippy-clean. **It has not been run on straylight** (this fork was told not to
   contact it), and it depends on model behaviour, so the owner should run it once before the
   plan is handed over. Between tasks 16 and 17 `verify-device` is broken either way.

## Task 17: `BrokerPort`, `[broker]` and the pointers

**Check: skeleton** for `BrokerPort::call` (`todo!()`); everything else in the task is small
enough that the skeleton is the code (`config::Broker`, `NoBroker`, the two line functions, the
`main.rs` wiring, the three pointers).

Run to green: `config` (11, two new), `pointers` (7, new; three of them run the `loopd` binary),
`broker_port::without_a_broker…` (1).
**Compiled only, never run to green:** the other 14 tests of `broker_port` and both tests of
`broker_port_bad`. They were desk-checked against the exit list in the task, not executed. The
likeliest faults, if any: a timing bound too tight under load (`a_broker_that_never_answers…`
allows 250 to 1,100 ms for a 300 ms timeout; `a_pending_call_nobody_answers…` allows 550 to
1,700 ms), and a fake-broker thread that panics noisily, but harmlessly, when the port closes
without sending.

Defects the check exposed:

1. **Spec, section 8, ambiguous.** "The read timeout is `timeout_ms` until the first frame" can be
   read as a per-read socket timeout. That lets a peer that trickles bytes hold a turn for ever,
   and it cannot express "until `expires` plus `timeout_ms`" at all. Resolved as a deadline per
   frame wait, with a test whose answer (not its timing) tells the two apart. Note this is a total
   deadline, unlike the inference path's liveness rule; the spec already implies it ("M3b must
   keep its tool time limit under `timeout_ms`"). Proposed wording: "`BrokerPort` waits for the
   first frame until `timeout_ms` after the call began, and after a pending frame until the
   frame's `expires` plus `timeout_ms`. These are deadlines, not per-read timeouts."
2. **Spec, silent: a request too large for a frame.** `arguments` near 1 MiB makes `write_frame`
   fail with `TooLarge`. Calling that "the tool broker is unavailable" with a runbook pointer
   would send the owner looking for an outage. Resolved: its own failure text,
   "the request is too large for the tool broker", nothing printed.
3. **Spec, silent: `expires` is a peer's number.** A far-future `expires` overflows
   `Instant + Duration`, which panics. Resolved: `checked_add`, and `None` is exit 10.
4. **Spec, silent: the envelope `id`.** Resolved: `request.call.0`; every answer must carry it.
5. **Spec, silent: `timeout_ms = 0`.** `set_write_timeout(Some(0))` is an error in std. Resolved:
   no config validation; the call fails closed at exit 3, and a test checks there is no panic.
6. **Spec: `core-memory-unreadable` needs a new error variant.** `BaselineError::Read` serves both
   `system.md` and `core.md`; the pointer belongs to one. Added `BaselineError::Core`. The M2b
   test that checks the unreadable `core.md` only looks for "core.md", so it still passes.
7. **Testability.** "`loopd` prints …" cannot be asserted in-process through `eprintln!`.
   Resolved with `BrokerPort::with_log` and the two `*_line()` functions; the startup and
   self-test lines are asserted by running the binary.
8. **Task-writing.** The line-limit hit again: `broker_port.rs` was 546 lines. Split into
   `broker_port.rs`, `broker_port_bad.rs` and `support/broker.rs`.

## For the record of the experiment

What skeleton-only cost here: 16 of about 40 new or changed tests in this area were never run. What
it saved: about 110 lines of `BrokerPort`. The five spec gaps under task 17 (1 to 5) were all found
while writing the exit list and the tests, not by running anything, so a reference would not have
found them sooner; what a reference would add is only confidence in the timing bounds.

## Run on straylight (2026-09-18, at the merge)

Task 17's changed `device.rs` was run with `make verify-device` on the merged reference tree
(`f9195ec`: areas A to D, `loopd` at the task 17 state with no broker configured) against Ornith
on straylight, slot 0, after checking `/slots` showed both slots idle. It passed twice in a row,
6 of 6 each time (43 s and 39 s). What the test asserts is that Ornith called `find_tool` and
then `call_tool`, that `clock`, `find_tool` and `call_tool` each left a result, and that the
restart check passed; which tool it found and what the call returned are not asserted. Between
tasks 16 and 17 `verify-device` is still broken, as noted above; the owner should not run it there.