From 92a57f6350a0d353358aacab6d29a1f843275ce1 Mon Sep 17 00:00:00 2001 From: "K. Isom" Date: Fri, 18 Sep 2026 21:10:15 -0700 Subject: [PATCH] Review M2b task 11: accept; M2b done Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 13 +++++++------ docs/implementer-lessons.md | 4 +++- docs/implementer-log.md | 22 +++++++++++++++++++++- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 11cbde9..5905fc6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -5,10 +5,10 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Current state Boxmaker is a sovereign personal agent harness written in Rust. Work proceeds one milestone at a -time (M0 to M7, table in `docs/milestones.md`). M0 (measurements) and M1 (workspace, `proto`, gate, -`Decision`) and M2a (the inference path: `inferproxy`, `loopd`'s HTTP, SSE and llama client, -the startup self-test) are done. M2b (sessions, turn loop, `bxctl chat`) is specified in -`docs/specs/2026-09-18-m2b-agent-loop.md` and planned in `docs/plans/M2b/`. Check `docs/implementer-log.md` for +time (M0 to M7, table in `docs/milestones.md`). M0 (measurements), M1 (workspace, `proto`, gate, +`Decision`), M2a (the inference path: `inferproxy`, `loopd`'s HTTP, SSE and llama client, the +startup self-test) and M2b (sessions, the turn loop, `loopd serve`, `bxctl chat`) are done. M3 +(`brokerd`) is next to design. Check `docs/implementer-log.md` for what is in flight. - `docs/design.md` is the binding design brief. If it looks wrong or conflicts with a measurement, @@ -43,8 +43,9 @@ requests, check `GET /slots?model=ornith-1.5-35b-a3b` so you do not evict someon - `make audit` runs `cargo deny check advisories`; it needs the network. - `make verify-device` (from M2a task 13) runs the ignored tests in `crates/loopd/tests/device.rs` against straylight through a private `inferproxy`. It uses Ornith slot 0. -- `bxctl` is the owner CLI (`bxctl chat` from M2, `bxctl reindex` from M5). Until then every binary - prints "not implemented" and exits 2. +- `bxctl` is the owner CLI: `bxctl chat` talks to a running `loopd serve --config `; + `bxctl reindex` comes in M5. Roles not yet built (`brokerd`, `gatewayd`, `toolkit`) print "not + implemented" and exit 2. ## Architecture in brief diff --git a/docs/implementer-lessons.md b/docs/implementer-lessons.md index ba77290..1493c4b 100644 --- a/docs/implementer-lessons.md +++ b/docs/implementer-lessons.md @@ -48,6 +48,7 @@ How it is used: | T13 | Set the sampling defaults on the server for coding agents (`temp`, `top-p`, `top-k`, `reasoning-budget`). OpenCode sends none, so the server's default temperature of 1.0 applies, and thinking has no cap. | Five Ornith turns ran 16k tokens of thinking to the output limit and produced nothing. | | T14 | When a rule has a "before the final frame" or "on every exit" shape, list the exits in the task, or say "including the early returns". The implementer follows the path the task walks through. | M2b finding 1: task 07 walked the success path and the turn's error; the open and create errors were early returns it did not mention. | | T15 | Running the whole plan through `tools/run-plan.sh` worked: ten tasks, one commit each, unattended, in about three hours. Keep the TUI closed while it runs; a second message into the driver's session starts a second agent on the same tree. | M2b run, 2026-09-18. | +| T16 | When a task prescribes the fix, check that it compiles against the types as they are, in the reference tree, before handing it over. A fix that cannot be written as described pushes the implementer outside the listed paths. | M2b task 11: "`unwrap_or_else` with a fixed valid id" had no non-panicking form outside `proto`; the implementer added `Default` to `SessionId` and reported it. | ## What worked and should be kept @@ -84,4 +85,5 @@ M1, and failed as a coordinator. First-gate pass rate for the milestone: 8 of 13 M2b was Ornith alone, driven by the shell script: ten tasks, ten commits, no intervention, first gate 4 of 10, and four low findings in review, two of which are the "everywhere" pattern again in a new shape (paths instead of types). Its code reads well and follows the numbered steps in the -tasks closely; where the task did not walk a path, the rule was not applied there. +tasks closely; where the task did not walk a path, the rule was not applied there. Task 11 fixed all four in one run and reported its one +deviation, which the task itself had caused. diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 74fe20a..eb9314a 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -40,7 +40,7 @@ reviewer adds findings under "Reviews" once per milestone. | M2b/08-loopd-serve | 2026-09-18 | done | 2 | fail | none | Rewrote crates/loopd/src/main.rs into two commands, `selftest` and `serve`, both sharing `run_selftest_check` so the self-test lines are identical. `serve` loads config (exit 1 on failure), removes an existing socket via `channel_socket()` before the self-test, runs the self-test and exits 1 without binding on failure, then creates the socket's parent dir, binds, sets mode 0600 with `std::fs::set_permissions`, prints `serving on`, and calls `channel::serve` with a `Context` from the config, client, `Box::new(FakeTools::new())` and `Registry::m2b()`. Anything else prints both usages and exits 2. The `serve_refuses...` test's "no socket left behind" holds because the socket is removed before the self-test and binding happens only after it passes. First gate run failed on two clippy `collapsible_if` lints; collapsed the two nested `if let` into edition-2024 let-chains and re-ran, which passed. `cargo test -p loopd --test serve` reports 3 passed. | Ornith-1.5-35B-A3B | | M2b/09-bxctl-chat | 2026-09-18 | done | 5 | fail | none | Wrote crates/bxctl/src/chat.rs: `run_turn` (open socket, one `write_frame` with id 1, loop `read_frame` asserting id 1, dispatch final `TurnDone`/`Error` and non-final `TurnEvent` to `on_event`, every other frame `Protocol`); `ChatError` (Connect/Frame/Refused/Protocol) with `source()` returning the io and FrameError; `new_session_id` = `chat--` via two `expect`s (the epoch check and a private-field construction that cannot fail); `Printer` with json mode (one serde_json line per event, no skipping, no escape codes), a dimmed reasoning block opened on the first `Reasoning` and closed on the next non-reasoning event or `end_reasoning`, and every other event kind named exactly. Registered `pub mod chat;` in lib.rs. Rewrote main.rs into a `chat` subcommand: usage + exit 2 for a wrong first arg or unknown flag/missing value/invalid id, `$BOXMAKER_HOME/run/loop/loop.sock` else `/var/lib/boxmaker/...`, `--say` (events to stderr, answer to stdout, resume=true then one retry with resume=false on no_such_session), interactive (create on first turn, resume on the rest, `/quit` stops, the created session id printed once to stdout), `--json` (events to stderr, the TurnDone also to stderr after them, plain answer to stdout). A `Sink` records the first write error so the `on_event` closure (which cannot return a Result) does not lose it. All 11 chat tests pass. Four gate runs before clean: clippy `io_other_error` (switched to `Error::other`), then `redundant_closure` twice (the `other` map and `get_or_insert_with`), then a rustfmt import-order diff./? | Ornith-1.5-35B-A3B | | M2b/10-verify-device | 2026-09-18 | done | 1 | pass | none | No library code. Copied the three given files byte-identical (`cmp` clean): `crates/loopd/tests/device.rs` (replaces the M2a one, its four checks still in it), `Makefile` (only change: `verify-device` now also passes `BOXMAKER_BXCTL`), and `config/system.md`. `make gate` printed `gate: ok` with device at `0 passed; 0 failed; 6 ignored`. `curl http://straylight:11434/health` returned `{"status":"ok"}`. `make verify-device` ran all six checks against the real server in 41.6s, all passed: self-test, capped-thinking block, a four-turn conversation surviving a `loopd` restart with its cache, a request surviving its proxy being killed and restarted, a second turn reusing the first turn's cache, and the baseline fitting the token budget. The baseline is 251 tokens (the brief allows 3000). Ran directly rather than via a subagent: the `delegate` tool returned `Agent "undefined" not found` on every attempt. | Ornith-1.5-35B-A3B | -| M2b/11-review-fixes | 2026-09-18 | done | 1 | pass | a Default impl for SessionId was added to crates/proto/src/ids.rs, which the task did not list | Copied the two given tests (loopd/baseline.rs, bxctl/chat.rs). In channel.rs the busy guard is now dropped before every final frame (the three open/create/assemble session errors, plus the existing turn_done/error path) and Held::drop recovers a poisoned lock with unwrap_or_else(|p| p.into_inner()). baseline::assemble treats a core.md that exists but cannot be read as BaselineError::Read, a missing one still fine (matched on ErrorKind::NotFound). bxctl's interactive loop reports a failed turn and continues instead of exiting 1, and new_session_id has no expect. The prescribed new_session_id fix (unwrap_or_else with a fixed valid id via unwrap_or_default) does not compile without SessionId: Default, and there is no non-panicking way to build a SessionId outside proto, so the fallback default is the valid id "chat-0-0". Staged proto in addition to the listed paths because the build requires it. Gate: ok, 219 tests. | ? | +| M2b/11-review-fixes | 2026-09-18 | done | 1 | pass | a Default impl for SessionId was added to crates/proto/src/ids.rs, which the task did not list | Copied the two given tests (loopd/baseline.rs, bxctl/chat.rs). In channel.rs the busy guard is now dropped before every final frame (the three open/create/assemble session errors, plus the existing turn_done/error path) and Held::drop recovers a poisoned lock with unwrap_or_else(|p| p.into_inner()). baseline::assemble treats a core.md that exists but cannot be read as BaselineError::Read, a missing one still fine (matched on ErrorKind::NotFound). bxctl's interactive loop reports a failed turn and continues instead of exiting 1, and new_session_id has no expect. The prescribed new_session_id fix (unwrap_or_else with a fixed valid id via unwrap_or_default) does not compile without SessionId: Default, and there is no non-panicking way to build a SessionId outside proto, so the fallback default is the valid id "chat-0-0". Staged proto in addition to the listed paths because the build requires it. Gate: ok, 219 tests. | Ornith | ## Reviews @@ -223,3 +223,23 @@ server releases the session before the final frame exactly as asked; the fixes t held (a `read` returns as soon as it has data; unknown fields rejected in ours, ignored in the server's). On straylight the model used `clock`, then `find_tool` and `call_tool` for `echo`, unprompted, and the log shows one `Usage` per completion and not one `CacheLoss`. + +### M2b, task 11 — reviewed 2026-09-18 by the design model (Claude) + +Accepted. M2b is done. + +| Check | Result | +|---|---| +| One commit with the trailer; both given tests identical to the plan | pass | +| The four fixes | all present: `drop(held)` before each of the three session errors and the existing final frames; `Drop` recovers a poisoned lock; an unreadable `core.md` is `BaselineError::Read` (missing is still fine, matched on `NotFound`); the interactive loop reports a failed turn and goes on | +| `make gate` | `gate: ok`, 219 tests | +| `channel` suite ten times in a row | no failure | +| `make verify-device` on straylight | 6 passed in 39 s | + +| # | Severity | Owner | Finding | Fix | +|---|---|---|---|---| +| 1 | nit | task | The task asked for "`unwrap_or_else` with a fixed valid id" in `new_session_id`, but outside `proto` there is no way to build a `SessionId` without a fallible call, so the instruction could not be followed as written. The implementer added `impl Default for SessionId` (`chat-0-0`) in `proto`, outside the listed paths, and said so in the log. It is correct and fails closed (a second session with the fallback id is refused with `session_exists`), but a `bxctl` choice now lives in `proto`. | When next touched: `new_session_id` returns a `Result`, and `Default` is removed | + +What was good: the deviation was reported in the right column with the reason, rather than worked +around silently or by stopping without a report. The task was the cause: an instruction that +names a fix must be checked to compile against the types as they are (tip T16).