Files
boxmaker/docs/plans/M2b/11-review-fixes.md
kyleandClaude Fable 5.1 2be8581a0c Review M2b: accept with one follow-up task; record lessons
All ten tasks pass the checklist, the gate, the audit and the device
checks, including a four-turn conversation with a loopd restart and no
cache loss. Reading and probing found four low defects: the busy guard
is released before the final frame on the main path but not on the
three error paths, its Drop skips a poisoned lock, an unreadable
core.md is treated as missing, and bxctl's interactive loop exits on a
failed turn. Task 11 carries the fixes with two new tests, checked
against a fixed copy of the branch.

The Model column is filled in (all Ornith) and one malformed row is
repaired. Two rules are promoted to AGENTS.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-18 20:47:03 -07:00

3.7 KiB

M2b task 11: four small fixes (review follow-up)

Branch: m2b (run git switch m2b; git status --short must be empty, otherwise stop) Commit subject: Fix four review findings: busy release, poison recovery, core.md errors, chat loop

Goal

The M2b review found four small defects. Two have a test to copy in; two are rules to apply. None changes an interface.

Files

  • Copy (replacing the old ones): crates/loopd/tests/baseline.rs, crates/bxctl/tests/chat.rs
  • Modify: crates/loopd/src/channel.rs, crates/loopd/src/baseline.rs, crates/bxctl/src/chat.rs, crates/bxctl/src/main.rs, docs/implementer-log.md

The four fixes

  1. channel.rs: release the session before every final frame. Task 07 said to drop the busy guard before turn_done or the turn's error, and that is done. But the error frames for a session that cannot be opened or created (no_such_session, session_exists, internal) are still sent while the session is marked busy. bxctl chat --session <new id> reads no_such_session and at once sends the same turn again with resume: false, so it can be answered session_busy by a server thread that has not returned yet. Drop the guard before each of those three writes. The rule: the last frame of a connection is never sent while the session is busy.
  2. channel.rs: a poisoned lock must not leave a session busy forever. The guard's Drop does if let Ok(mut busy) = self.ctx.busy.lock(), which skips the removal when the lock is poisoned. Recover it with unwrap_or_else(|p| p.into_inner()), as handle already does.
  3. baseline.rs: a memory/core.md that exists but cannot be read is an error. Today if let Ok(text) = read_to_string(..) treats an unreadable file like a missing one, so a session starts without the memory the owner curated, and nothing says so. If the file exists, a read failure is BaselineError::Read(path, e). A missing file is still fine.
  4. bxctl: the interactive loop goes on after a failed turn, and new_session_id has no expect. The spec says "A failed turn prints bxctl: <error> and the loop goes on"; today it exits 1. Report the error and continue; the session exists, so the next line resumes it. And AGENTS.md forbids expect in library code: replace the two in new_session_id with unwrap_or_default() for the clock and unwrap_or_else with a fixed valid id for the SessionId (it cannot fail, but the type must not be forced).

Steps

  • 1. Copy.
git switch m2b
cp docs/plans/M2b/files/crates/loopd/tests/baseline.rs crates/loopd/tests/
cp docs/plans/M2b/files/crates/bxctl/tests/chat.rs crates/bxctl/tests/
  • 2. See them fail. cargo test -p loopd --test baseline: 1 of 7 fails, an_unreadable_core_memory_file_is_an_error. cargo test -p bxctl --test chat: 1 of 12 fails, interactive_mode_survives_a_failed_turn.
  • 3. Make the four fixes. Run cargo fmt --all.
  • 4. See them pass. cargo test -p loopd --test baseline --test channel -p bxctl. Expected: 7, 6 and 12 passed. Run the channel tests ten times in a row.
  • 5. Run the gate. make gate. Expected last line: gate: ok.
  • 6. Log and commit.
git add crates/loopd crates/bxctl docs/implementer-log.md
git commit

Done when

  • make gate prints gate: ok with 219 tests.
  • grep -n "expect(" crates/bxctl/src/chat.rs prints nothing.
  • grep -c "drop(held)" crates/loopd/src/channel.rs prints 4 (or the guard is scoped so that every final frame is written after it is gone).

Stop and report if

  • interactive_mode_survives_a_failed_turn cannot pass without changing what --say does.