review: M6a subagents

docs/implementer-lessons.md

@@ -33,9 +33,9 @@ How it is used:
 | I15 | Never write the model's chat-template tags out whole (the think and tool-call tags, the angle-bar turn markers) in code, comments, commands or the log. Build them from pieces (`concat!("<", "think")`) and name them in words. Text holding them turns into the model's own control tokens when it reads it. | M5a task 04: the given test held the tags; Ornith's reasoning ended at the test file's closing think tag, and it lost track of the file and made no change. `docs/research/2026-09-24-chat-template-tokens.md`. | yes | M5a tasks 05 to 22: not seen again. |
 | I16 | When a test outside your task fails, check once whether it fails without your change (stash, run, unstash), then stop and report both results. Do not keep experimenting, and never add a file the task does not list, not even a temporary one. | M5a task 21: the session did the check, then spent about 80 minutes on experiments and wrote two diagnostic test files into `crates/bxctl/tests/` until the owner stopped the run; the cause was outside the repository (review finding 4). M5a task 16 did it right: one check, its log row, a stop. | yes | M5c-1: the stash left the new untracked tests behind and a baseline printed no total (task 15; now `git stash -u` in AGENTS.md, T53); the two flaky tests were rerun and named, not stopped on (now an exception for a test the README lists, T52). |
 | I17 | Write your log row from `git diff` after staging, naming only what the diff shows. A row written from memory describes what you meant to do. | M5a task 10's row describes checks that are not in `serve.rs`; task 13's says two runbook fragments were copied, and neither was (review findings 5 and 6 of 2026-09-24). | yes | M5c-1: task 10 logged one gate run and ran three, task 12 logged three and ran four and "Deviations: none" after anchoring an edit on its own line, and task 14's row blames the task for its own spacing error. M5c-2: every row's first gate is `pass`; task 01 still ran the gate twice, and every session ran `cargo test --workspace` after `gate: ok` only to count (T59). |
-| I18 | After `gate: ok`, do not run `cargo test --workspace` (or the gate) only to count tests. The gate's total is the count. If HEAD already has this task's commit and a `done` row, stop; do not start the task again. | M5c-2: every session re-ran the workspace tests after a passing gate (T50 still driving extra work). Task 05's run log is a resume after HEAD was already at task 12; Laguna did not re-commit, and the original transcript was truncated. | yes | M5d-1: tasks 03, 07 and 08 ran the gate two or three times with first-gate pass. M5d-2: task 04 ran the gate twice with first-gate pass. |
+| I18 | After `gate: ok`, do not run `cargo test --workspace` (or the gate) only to count tests. The gate's total is the count. If HEAD already has this task's commit and a `done` row, stop; do not start the task again. | M5c-2: every session re-ran the workspace tests after a passing gate (T50 still driving extra work). Task 05's run log is a resume after HEAD was already at task 12; Laguna did not re-commit, and the original transcript was truncated. | yes | M5d-1: tasks 03, 07 and 08 ran the gate two or three times with first-gate pass. M5d-2: task 04 ran the gate twice with first-gate pass. M6a: no; five tasks, one gate each, first-gate pass. |
 | I19 | Add the log row immediately under the table header (newest first). Do not insert it by date among older rows. | M5d-1 task 06: the row was inserted after M4b/03. | yes | M5d-2: task 04's commit duplicated 03's identical `done` row. |
-| I20 | After a turn, do every bookkeeping step the task lists, including when the outcome is `Err`, unless the task says only on Ok. Gating `last_heartbeat` on success is a judgement the task did not ask for. | M5d-2 finding 1: heartbeat sets `last_heartbeat` and pushes `Heartbeat` only on `outcome.is_ok()`. | yes | |
+| I20 | After a turn, do every bookkeeping step the task lists, including when the outcome is `Err`, unless the task says only on Ok. Gating `last_heartbeat` on success is a judgement the task did not ask for. | M5d-2 finding 1: heartbeat sets `last_heartbeat` and pushes `Heartbeat` only on `outcome.is_ok()`. | yes | M6a: not exercised. |
 
 ## Tips for writing tasks
 

docs/implementer-log.md

@@ -1152,3 +1152,41 @@ the prefix.
 The listing in the job prompt was the `bxctl memory tiers` text (loopd does not special-case
 tiering). A leftover `started` for `job-tier-20261004t0145` is from `* * * * *` firing the next
 minute; the stack was stopped there. No Mattermost.
+
+### M6a, tasks 01 to 05 — reviewed 2026-10-03 by the design model (Grok 4.6)
+
+Accepted. No product finding. Ornith-1.5-35B-A3B through `tools/run-plan.sh` on straylight, branch
+`m6a`. The branch differs from the reference (`~/boxmaker-refs/m6a-ref.bundle`, `87c3fa7`) only in
+`crates/loopd/src/subagent.rs`: unused `Dispatch` import dropped, types written as `bxcore::…` /
+`crate::tools::…`, a doc comment on `RUNBOOK`, rustfmt wrapping. Same behaviour. Log rows are the
+other delta. Spec 7 device check on straylight is not in this review; it is next.
+
+| Check | Result |
+|---|---|
+| 5 done task commits, each with `Co-Authored-By: ornith-1.5-35b-a3b <agent@wntrmute.dev>` | pass (plus the plan commit `25278b4`) |
+| Given tests byte-identical to `docs/plans/M6a/files/` (the runbook fragment is appended into `docs/runbook.md`) | pass |
+| Protected files (`docs/design.md`, `docs/decisions.md`, `docs/inference-contract.md`, `docs/specs/`, `docs/plans/`, `AGENTS.md`, `CLAUDE.md`) untouched by Ornith | pass |
+| Log rows | 5 `done`, newest first, first-gate pass, one gate each, deviations none. Rows match the diffs. |
+| No `unwrap`, `expect`, `panic!`, `#[allow]` or `unsafe` in new library code; every source file ≤ 500 lines; no new crate | pass |
+| `make gate` on this host (rustc 1.98.1 / clippy 0.1.98) | `gate: ok`, 1590 tests, the plan's count |
+| Independent review of `git diff 25278b4 HEAD -- crates docs/runbook.md` against spec sections 3, 5, 6, 7 (M6a) and 8 | `subagent` is not core; `call_tool` does not send it to the port; overflow names `subagent-id` before hold; child uses `slots.background` and the time line; speaker `None`; thinking is not in the summary; `task` is capped; nested `sub-sub-…` returns through the middle `ToolResult`; a child that ends on a tool call is `failed`; heartbeat and cron skip while any `sub-*` is held; missing `sub-*` state inherits via `raise`; a plain `chat-*` read still creates nothing. |
+
+No numbered product finding. Spec 3.3's five-level background order is still SlotGate FIFO plus the
+two scheduler skips (heartbeat after due, cron after heartbeat returns), the same shape as M5d-2.
+A tool-output summary already waiting on the gate can still run before a child that starts waiting
+later. `holds_prefix("sub-")` is any held id starting `sub-`, including a live `--session sub-foo`
+turn. `run` rebuilds `sub-<parent>-<n>` instead of calling `child_id`. None of those is a miss
+against the tasks; they match the reference.
+
+Device check, spec 7 for M6a, is not in this review. It needs straylight, slot 0 live / slot 2
+background, slot 1 left busy, a private `$HOME`, and production daemons untouched: a `bxctl`
+session calls `subagent` with a short task; the child log is on slot 2; the parent's next turn has
+`cache_n` near its previous total. Spec 7's taint-copy case (untrusted parent, child's identity
+`add` lands in `notes.md`) is the same setup.
+
+What was good: five tasks, five first-gate passes, no stops, given tests unedited, every
+review-focus case in the plan is covered by a given test and holds, `subagent-id` has a pointer
+and a runbook entry, no new crate.
+
+Known M6a limits, left as the README lists them: no Mattermost delivery of the child's log; one
+background slot (nested children serialise); `brokerd` does not see the spawn; `consult` is M6b.