boxmaker/docs/plans/M3a/HANDOFF.md

# M3a plan: handoff, 2026-09-18

The M3a plan is half built. This file says what exists, where, and what is left. Delete it when
the plan is complete and `README.md` replaces it.

Read first: `CLAUDE.md`, `docs/specs/2026-09-18-m3a-decision-path.md` (revised after review; the
"Spec review of M3a" rows in `docs/decisions.md` say what changed and why), the decision of
2026-09-18 on reference implementations (top row of `docs/decisions.md`, tip T17 in
`docs/implementer-lessons.md`), and `docs/plans/M2b/` as the model for format
(`README.md`, `07-loopd-channel.md`).

## How the plan is being built

The given tests are checked before hand-over at one of three levels: a full reference
implementation (audit chain, approval handoff), the oracle inside the property test (policy), or
a skeleton (real signatures, `todo!()` bodies, tests must compile and be clippy-clean). Each area
records what its check exposed in `docs/plans/M3a/checks-<x>.md`; the final `README.md` folds
those into one table per task. That record decides what M3b gets.

Work happens in git worktrees on unpushed branches, never in `/tmp`:

| Worktree | Branch | Holds |
|---|---|---|
| `~/src/boxmaker-ref` | `m3a-ref` | Integration. Commit `3ba5f83`: the `proto` contract (new audit types, admin wire messages, `chain.rs` skeleton) and an empty file for every new module, so that areas never share a file. |
| `~/src/boxmaker-ref-a` | `m3a-ref-a` | Area A, audit: tasks 01, 03, 09, 19 |
| `~/src/boxmaker-ref-b` | `m3a-ref-b` | Area B, policy side: tasks 02, 04 to 08 |
| `~/src/boxmaker-ref-c` | `m3a-ref-c` | Area C, `loopd`: tasks 16, 17 |
| `~/src/boxmaker-ref-d` | `m3a-ref-d` | Area D, `bxctl` and the runbook script: tasks 18, 20, 21 |

Each area branch holds, under its worktree: the tests in place under `crates/`, the reference or
skeleton source, copies of everything the implementer must copy under
`docs/plans/M3a/files/<same path>`, its task files `docs/plans/M3a/NN-name.md`, and
`docs/plans/M3a/checks-<x>.md`.

## Task numbering (fixed; task files use these names)

| # | Task | Area | Check |
|---|---|---|---|
| 01 | `proto-audit-types` | A | reference (types; generates fixtures) |
| 02 | `proto-admin-wire` | B | reference (types) |
| 03 | `proto-chain-verifier` | A | reference |
| 04 | `brokerd-config` | B | minimal reference (needed under E) |
| 05 | `brokerd-args` | B | minimal reference |
| 06 | `brokerd-grants` | B | minimal reference |
| 07 | `brokerd-policy` | B | oracle, plus minimal reference |
| 08 | `brokerd-state` | B | minimal reference |
| 09 | `brokerd-audit-writer` | A | reference |
| 10 | `brokerd-runner` | E | reference |
| 11 | `brokerd-approvals` | E | reference |
| 12 | `brokerd-ledger` | E | reference |
| 13 | `brokerd-broker` | E | reference |
| 14 | `brokerd-admin` | E | reference |
| 15 | `brokerd-serve` | E | reference |
| 16 | `loopd-tools` | C | skeleton |
| 17 | `loopd-broker-port` | C | skeleton |
| 18 | `bxctl-admin` | D | skeleton |
| 19 | `bxctl-audit-verify` | A | reference |
| 20 | `bxctl-chat-approvals` | D | skeleton |
| 21 | `runbook-check` | D | run for real |
| 22 | `end-to-end` | E | compiles only, unless C's `BrokerPort` gets a body |

## Area E: not started

Tasks 10 to 15 and 22: `runner`, `approvals`, `ledger`, `broker`, `admin`, `serve`, the
two-process end-to-end test, and the scripted check on straylight. It builds on A's
`brokerd::audit::Writer` and B's `config`, `args`, `grants`, `policy`, `state`, so it starts from
a merge of those two branches. The design it must follow, beyond the spec:

- `ledger.rs`: `pub trait AuditSink: Send { fn append(&mut self, time: Timestamp, event: AuditEvent) -> Result<u64, AuditError>; }`
  implemented by `audit::Writer`, so tests can wrap a real writer in one that fails on demand.
  `Ledger { audit: Box<dyn AuditSink>, state: StateStore }` behind one `Mutex`, with the three
  locked steps as functions: decide and record; re-decide and record the approval; raise state
  and record the result. A poisoned ledger lock or any failed append means `audit_unavailable`
  for every later call.
- `approvals.rs`: `Verdict { Run(Decision), Denied(DenyReason) }`; `Table` over
  `Mutex<BTreeMap<u64, Entry>>` with `insert(info, ask) -> Receiver<Verdict>`, `take(id)`,
  `take_expired(now)`, `list()`. Whoever takes the entry answers it.
- `broker.rs`: `pub struct Broker { cfg, ledger, table, runtime }`, `handle(stream, &Broker)`.
  While pending: `recv_timeout(1 s)`, then a `read` with a 10 ms read timeout (`Ok(0)` is gone,
  `WouldBlock`/`TimedOut` is alive, bytes are a protocol error). One more check before running.
- `runner.rs`: spec section 7, with `Refusing` as the production runtime and a recording fake in
  the tests. `RunError` text is fixed text.
- `serve.rs`: the audit lock first, then directories (0700 whether made or found), stale socket
  removal, bind, 0600; one thread per connection; the expiry thread.
- The state-write failure test uses a read-only directory; the audit failure test uses the
  `AuditSink` seam.
- The end-to-end test lives in `loopd`, is `#[ignore]`d without `BOXMAKER_BROKERD`, and
  `make gate` builds the workspace and runs it with the variable set.
- The straylight script checks `GET /slots?model=ornith-1.5-35b-a3b` and uses slot 0 only.

The prompts given to areas A to D are the model for E's: they are in this session's transcript;
their substance is the numbered conventions below.

## Conventions every area follows

1. Tests carry "Do not edit." Deterministic, robust under CPU load, no `tempfile` crate (temp
   directories under `std::env::temp_dir()` with pid and a counter, as `crates/loopd/tests/support/`).
   `cargo fmt --all`; `cargo clippy --workspace --all-targets --offline -- -D warnings` clean.
2. A reference follows the code rules in `AGENTS.md`.
3. Everything the implementer copies is also under `docs/plans/M3a/files/`. Reference source is not.
4. Task files follow `docs/plans/M2b/07-loopd-channel.md`: branch `m3a`, at most about 3,000
   tokens, rules that walk every exit (T14, I11, I12), verified API snippets, expected test counts.
5. A `checks-<x>.md` per area, honest about what ran green and what only compiled.

## Left to do, in order

1. Read each area's final report (below) and its `checks-<x>.md`. Resolve contract changes they
   asked for. `crates/proto/tests/strict.rs` may have been edited by both A and B.
2. Merge `m3a-ref-a` and `m3a-ref-b` into `m3a-ref`; run the whole gate there.
3. Build area E on top. Merge C and D. Add the `audit verify` arm to `bxctl`'s `main.rs` (D owns
   the file, A specified the arm). Add the end-to-end line to the `Makefile` in `files/`.
4. In `m3a-ref`: `make gate` as far as skeletons allow; every test compiles; reference-backed
   suites pass ten times under load.
5. Copy `docs/plans/M3a/` (task files and `files/`) from `m3a-ref` to `master`. Write
   `docs/plans/M3a/README.md` in the M2b README's shape, with the per-task check table and what
   each check exposed. Fold any spec defects the areas found into the spec and `decisions.md`.
6. Update `CLAUDE.md`'s "Current state" and `docs/milestones.md` if needed. Delete this file.
7. Then the owner runs `tools/run-plan.sh docs/plans/M3a` with Ornith, on branch `m3a`.

Already known spec defects found while planning (both fixed in the spec, commit `66fa143`): the
one-process end-to-end test would have broken the crate-dependency gate; the locked steps needed a
module of their own (`ledger`). One finding about the process itself: the approval-handoff
reference cannot run without working `config`, `args`, `grants`, `policy` and `state` under it, so
those got a minimal reference too, and the saving from skeletons is confined to `loopd`, `bxctl`
and the gate script.

## Area reports

(Each area's own record is `docs/plans/M3a/checks-<x>.md` on its branch.)

### Area C, `loopd`: done, commit `56319ab` on `m3a-ref-c`

- Tasks 16 and 17 written. Task 16's changes are real code and its tests ran green (`tools` 10,
  `turn` 6, `turn_broker` 5, `config` 11, `pointers` 7, rest of the suite unchanged).
  `broker_port.rs` is a skeleton: only `BrokerPort::call` is `todo!()`, so 14 of 15 tests in
  `broker_port` and both in `broker_port_bad` compile and have never run.
- API settled: `ToolPort::call(&self, &ToolRequest, &mut dyn FnMut(&Pending)) -> ToolResponse`;
  `Pending { approval: u64, expires: Timestamp }`; `Registry::m3a()` (and `m2b()` kept as the
  test registry); `denial_text`; `BrokerPort::new(socket, timeout)` and `with_log`; `NoBroker`;
  `config::Broker { socket: Option<PathBuf>, timeout_ms }`; the envelope `id` is `request.call.0`
  (area E's `brokerd` must echo it).
- Spec defects it found, with wording in `checks-c.md`, NOT yet applied to the spec: the section 8
  timeout must be a deadline per frame wait, not a per-read socket timeout; `tool_denied.name` and
  `approval_pending.tool` carry the target tool, not `call_tool` (task 20 must know); "`echo` stays
  in `FakeTools` only" is not enough, because recorded conversations find it through the registry;
  five silent cases resolved and tested (oversized request, far-future `expires`, envelope id,
  `timeout_ms = 0`, a pending frame as the final answer).
- **Needs a run on straylight before hand-over:** `make verify-device` breaks at task 16 because
  `device.rs` asks Ornith to use `echo`. Task 17 ships a changed `device.rs` that compiles but has
  never run on the device and depends on how the model behaves.

### Area A, audit: done, commits `e3065e1`, `89f696e`, `7da817d` on `m3a-ref-a`

- Tasks 01, 03, 09, 19 written, all with a reference. Passing against it: `records` 3;
  `chain` 13 with 30 fixture logs under `crates/proto/tests/fixtures/audit/`; `brokerd` `audit` 9
  and `audit_startup` 7 (five runs); `bxctl` `verify` 6. Eight deliberate mutations of the
  reference each broke a given test. 66 files under `files/`, byte-identical to the tree. The
  fixture generator `crates/proto/tests/gen_audit_fixtures.rs` (`#[ignore]`d) stays in the
  reference tree only.
- In that worktree one `strict.rs` test fails and `proto`'s `wire.rs` test does not compile: both
  wait on area B's wire fixtures and should clear at the merge.
- **Contract changes other areas must absorb:**
  1. `DecisionRecord::Allowed {}` and `Ask {}` are empty struct variants, not unit variants: serde
     ignores `deny_unknown_fields` on unit variants of an internally tagged enum. JSON unchanged;
     every construction and match needs the braces (areas B, D, E). Check the other fieldless
     variants in internally tagged enums in `wire.rs` for the same hole.
  2. `strict.rs` changed in the audit test only (expected object count); B edits the envelope list.
  3. `ChainFailure` gained `last_good`, `break_prev`, `break_seq`, `tail_torn`;
     `ChainVerifier::feed(name, content)` added and used by both `bxctl` and `brokerd`.
  4. Writer API as specified, plus `RECOVERED_NOTICE` and `AuditError::Broken(Box<ChainFailure>)`.
  5. Tasks 09 and 19 say "Create" the module file and add the `pub mod` line (the stub files exist
     only in the reference tree). Every area's task files need the same check.
  6. Task 09 assumes task 04 adds `serde_json` to `brokerd`'s `Cargo.toml`. Check task 04 does.
- **Spec defects it found, wording in its report and `checks-a.md`, NOT yet applied to the spec:**
  the resumed verifier's break exception must also hold inside a failed region; startup step 2
  should fall back to verifying the whole log when the previous file's last line does not parse
  (otherwise an accepted break there deadlocks); the tampering suite fails at `brokerd`'s startup
  only when the damaged file is the latest; an unpaired `Recovery` is a failure; `unfinished`
  reports the decision's `seq` for approved calls too; `--accept-break` with a torn tail and no
  failure writes nothing and exits 2; section 9's verify output lines are fixed in task 19,
  including one for a torn tail.
- Left open: task 19 describes the `audit verify` arm in prose; reconcile with D's `main.rs`.

### Area D, `bxctl` and the runbook script: done, commits `292780f`, `5f27808`, `39f3cc7` on `m3a-ref-d`

- Tasks 18, 20, 21 written. Tests under `crates/bxctl/tests/`: `escape` 8, `cli` 12, `admin` 21,
  `chat_print` 9, `chat_approvals` 20, existing `chat` 12 untouched. `check-runbook.sh` runs for
  real; its self-test passes and fails under each of five mutations.
- At the skeleton state (`5f27808`) 38 tests only compiled. `cli.rs`, the `Printer` changes,
  `stream_turn` and `main.rs` had to be real code, because existing tests drive the binary. The
  area then wrote the remaining bodies (about 150 lines, `admin.rs` and `handle_pending`) as a
  measurement: all 38 passed first time, six more runs under load clean. **For this area the
  reference found nothing the skeleton and a desk-check had missed.** The bodies stay on the
  branch (the straylight check needs a working `bxctl approve`) and are not handed over.
- API settled: `escape::{escape_json_text, escape_model_text}`; `cli::{USAGE, ChatOptions, Command,
  UsageError, parse(args, home)}` (a new module: `main.rs` was near the line limit);
  `admin::{AdminError, reason_name, request, list, write_block, cmd_approvals, cmd_approve,
  cmd_refuse, cmd_grants_check}`, each `cmd_*` returning `Result<bool, AdminError>`;
  `chat::{OnPending, Approvals, TurnIo, handle_pending, stream_turn}`;
  `verify::run(home, out) -> io::Result<bool>`.
- **To act on when integrating:**
  1. `check-runbook.sh` fails on any pointer under `crates/**/*.rs` whose anchor is not written
     out literally (`format!("…#{anchor}")`, a `#<anchor>` placeholder in a comment), and if it
     finds no pointer at all. Run it on the merged tree; areas A, B, C, E may trip it. Proposed
     for spec section 11: "An anchor must be written out in the source; a pointer whose anchor the
     script cannot read is an error."
  2. **Conflict with area A:** task 18 creates `verify.rs` with a placeholder body and adds all
     four `pub mod` lines to `lib.rs`, so task 19 must MODIFY `verify.rs` and touch neither
     `lib.rs` nor `main.rs`. Area A wrote task 19 as "Create". Fix task 19.
  3. `GrantProblem.problem` can be multi-line because `toml` errors are. Task 06 should produce
     one-line problems at the source; check area B did.
  4. Spec section 9's example block shows spaced JSON; section 6 says `serde_json` output, which
     is compact. The tests use compact. Fix the example.
  5. "`--say` and `--json` print the event only" was ambiguous. Chosen: `--say` shows the block
     from `brokerd` and asks nothing; `--json` prints the event's JSON line and never contacts
     `brokerd`. Put that in the spec.
  6. Escaping now covers tool names and the answer on stdout too. Spec section 9 should say
     "everything the model wrote".
  7. `main.rs` must hold one `BufReader` on stdin for chat lines and approval answers alike; a
     binary-level test enforces it.
  8. Details the tests now fix that the spec leaves open: time spans and rounding; `expired` once
     `now >= expires`; a session id over 10 characters shown as 9 plus `…`; `no pending approvals`;
     an `ask` re-decision printed as `runs`; an approval id is ASCII digits only
     (`"+41".parse::<u64>()` succeeds); end of input at the prompt refuses; `brokerd` unreachable
     or an approval expired before the answer is one line and the turn goes on.
  9. `tests/admin.rs` is 497 lines and `tests/chat_approvals.rs` 487: no room to grow.
  10. `DecisionRecord::Allowed {}` / `Ask {}` (area A's contract change) must be applied to this
      branch's code and tests at the merge.
- The `Makefile` under `files/` has the `check-runbook.sh` line and no end-to-end line yet.

### Area B, policy side: done, commit `e89e2ce` on `m3a-ref-b`

- Tasks 02, 04 to 08 written, all with a minimal reference. Passing: proto `wire` 10, `turn_wire` 5,
  `admin_wire` 10 (new), 17 wire fixtures; `brokerd` 81 tests (`config` 7, `args` 13, `grants` 17,
  `policy` 7, `policy_matching` 10, `policy_redecide` 7, `policy_property` 4, `state` 9, doctests
  7), three runs and one under load. Support files are pulled in with `#[path]`
  (`tests/support/{tmp,build,oracle}.rs`), so there is no `support/mod.rs` for area E to collide
  with (area A added `tests/support/audit_dir.rs`; check how it is included).
- **Oracle teeth:** six mutants of the reference policy were each caught by the oracle alone.
  **The policy reference caught nothing the oracle would not have.** Second data point for the
  decision on references.
- API for area E, in full in its report and `checks-b.md`: `Config` with `broker_socket()`,
  `admin_socket()`, `audit_dir()`, `state_dir()`; `args::{ToolName, ToolArgs, ArgsError, parse,
  valid_path, inside, valid_host, valid_host_pattern, host_matches, url_host}` and
  `ToolArgs::canonical_json()`; `grants::{LoadedGrant, GrantSet, from_grants, load, render,
  valid_id, RUNBOOK}`; `policy::{SessionState, Label, Denial, Decision, Ask, Outcome, decide,
  redecide -> Result<Decision, Denial>}` (`SessionState` and `Label` live in `policy`);
  `state::{StateStore, StateError, RUNBOOK}` with `read` and `raise`.
- **CONFLICT with area A, to resolve first:** both found that `DecisionRecord` accepted unknown
  fields and fixed it differently. A made `Allowed {}` and `Ask {}` empty struct variants (call
  sites need braces). B kept unit variants and decodes through a private `RawDecision` with
  `#[serde(try_from = "RawDecision")]` (public type and JSON unchanged, no call-site change;
  `{"outcome":"allowed","reason":null}` is still accepted). Task 02 greps for `RawDecision` and
  stops without it. Pick one. B's keeps the public type as the spec wrote it and touches no other
  area; A's is stricter about `reason: null`. Whichever wins goes into task 01, and the loser's
  tests and task text change. Then check the other internally tagged enums (`TurnEvent`,
  `ToolResponse`, `AuditEvent`, `LogRecord`) for unit variants with the same hole.
- `strict.rs` is edited by tasks 01 and 02. The copy handed over with task 02 must carry A's audit
  object-count change as well as B's.
- Area E's tests must include: one invalid grant file denying a call a valid file would allow
  (`grants` only proves the set does not load), and section 12's sequence properties (taint never
  down; every `Result` follows a `Decision`).
- **Spec defects it found, NOT yet applied to the spec:**
  1. IPv4 literals pass the host grammar (`127.0.0.1`, `127.1`, `10.0.0.0x1`), so "no IP literals"
     was false. Rule added and tested; proposed wording: "...and the last label starts with a
     letter, which excludes every spelling of an IPv4 address."
  2. `write_file` with overlapping grant paths `/s` and `/s/out` and a write to `/s/out`: a grant
     path equal to the argument does not count, and the longest path that does count is matched.
  3. `redecide` returns `Result<Decision, Denial>`, not `DenyReason` (the spec's signature is wrong).
  4. Gaps settled in the tasks: an empty socket path means "under home"; a missing grants directory
     is a problem, not an empty set; a state file that says `public` is damaged; an absent `cwd` is
     left out of the canonical JSON.
- Other: serde's derived structs accept the JSON array form, so `"body":[]` decodes as `Empty`;
  true across `proto` since M1, not fixed, worth a decision. `docs/dependencies.md`'s "Used by"
  column is stale; task 04 has the implementer fix the `serde` rows. The owner's global gitignore
  swallows `*.toml~`, so that fixture is made in a temp dir.

## Tool warning for whoever continues

The Write and Edit tools in this environment decode a JSON escape such as backslash-u-002f into
the character itself when it appears in the text being written. Build such strings from pieces or
patch them in with a script, and check the bytes on disk afterwards.

## What the checks have shown so far (for the README and for M3b)

| Area | Check | Exposed |
|---|---|---|
| A, audit | reference | Five spec defects in section 5, two of which would have stopped `brokerd` starting; the serde unit-variant hole. The reference earned its cost. |
| B, policy | oracle, plus a minimal reference | One spec falsehood (IPv4), one unspecified case, one wrong signature, the same serde hole. The reference added nothing over the oracle. |
| C, `loopd` | skeleton | A timeout that had to be a deadline, five silent cases, a broken `verify-device`. 16 tests never run. |
| D, `bxctl` | skeleton, then bodies as a measurement | Spec ambiguities and an example that contradicted the text. The bodies found nothing the skeleton had not. |
| Planning itself | none | The one-process end-to-end test against the crate gate; the missing `ledger` module. |