review: M6b consult

CLAUDE.md

@@ -43,7 +43,10 @@ promoted only a trusted note). M6 (subagents and cloud consult) is approved
 `docs/specs/2026-10-04-m6-subagents-consult.md` sections 3, 5, 6, 7 M6a and 8; plan
 `docs/plans/M6a/`) is done, reviewed in `docs/implementer-log.md`; no finding is open; spec 7
 device-checked on straylight (child `sub-m6a-live-2` on slot 2; following slot-0 turn `cache_n`
-1787 of 1788). M6b (consult) is later.
+1787 of 1788). M6b (`consult` behind `brokerd`, allowlisted loopback HTTP, Decision `body`; spec
+`docs/specs/2026-10-04-m6-subagents-consult.md` sections 4, 5, 6, 7 M6b and 8; plan
+`docs/plans/M6b/`) is done, reviewed in `docs/implementer-log.md`; no finding is open; spec 7
+device check (loopback dummy on straylight, no internet) is later.
 P22 (proposed, not applied) makes Boxmaker a fleet of
 independent agents, one per host, deployed with Nix in a last milestone, M8; `flake.nix` builds
 every binary, the tool image and a dev shell. Work happens on titan (macOS, no Nix); build and run

docs/implementer-lessons.md

@@ -19,7 +19,7 @@ How it is used:
 | I1 | When a rule says "every" or "everywhere", finish by listing each place it could apply (every struct and enum in the file, every script, every branch) and check them one by one. An example in the task shows one place; the rule covers all of them. | M1 finding 1: `deny_unknown_fields` was put on both enums in task 06, where the task's example showed it, and on neither struct. In task 03, where the task said "works on structs and on enums", every type got it. | yes |  M2a: no. All six config structs have it; none of the server-format structs do. M2b: partly. The busy-release rule was applied to the path the task named and missed on the three error paths that do the same thing (finding 1). |
 | I2 | A check must fail when it cannot do its job: missing input, unreadable file, a tool that errors. Never throw errors away (`2>/dev/null`, `\|\| true`, an ignored `Result`). | M1 finding 4: all three gate scripts passed when `crates/` did not exist, and hid `find` and `awk` errors. | yes |  M2a: no recurrence. M2b: partly. An unreadable core.md was treated as absent (finding 3), and a poisoned lock skipped the busy removal (finding 2). |
 | I3 | Report every problem found, not only the first. | M1 finding 8: `check-lines.sh` exits at the first long file. | yes | M2a: not exercised. |
-| I4 | Run the formatter before the gate, and let it decide ordering. | M1 finding 7: three first-gate failures were rustfmt reordering `lib.rs`. | yes | M2a: no fmt failures; the first-gate failures were clippy. M5d-2: first-gate clippy on 05 (`collapsible-if`), 06 (`single_match`), and a USAGE indent on 10. |
+| I4 | Run the formatter before the gate, and let it decide ordering. | M1 finding 7: three first-gate failures were rustfmt reordering `lib.rs`. | yes | M2a: no fmt failures; the first-gate failures were clippy. M5d-2: first-gate clippy on 05 (`collapsible-if`), 06 (`single_match`), and a USAGE indent on 10. M6b: rustfmt moved the consult `use` in `args.rs` above the bxcore imports versus the reference; first-gate still pass. |
 | I5 | Log every attempt, including one you abandon. | M1 review: the task 01 row mentions files "from a prior attempt" that has no row of its own. | yes |  M2a: partly. Task 11's `stopped` row was overwritten by its later `done` row. M2b: no recurrence; all ten rows correct, one with a stray pipe. |
 | I6 | Prefer a conversion that can fail (`u32::try_from`) to a cast that is only safe because of a check somewhere else (`as u32`). | M1 finding 6: bounded `as` casts in `frame.rs`. Correct today, but the safety depends on a line ten lines away. | no (already implied by the code rules) | M2a: no `as` casts at all. M4a: yes, bounded casts in `http.rs` and `handshake.rs`, where the task named no conversion; none where a skeleton comment gave the `try_from`. |
 | I7 | A `read` returns as soon as it has any data to give. It blocks only when it has none. Never keep reading to fill the caller's buffer. | M2a finding 2: the chunked reader delivered a whole stream at its end. | yes | M2b: no recurrence. |
@@ -32,9 +32,9 @@ How it is used:
 | I14 | Recovering from a crash is a case to write a test for, not only to reason about. A file the daemon itself can leave behind (created, not yet written) must load like an empty one. | M3a finding 1: one zero-length log file made `brokerd` panic at startup, where the same directory verified as `ok, 0 records`. |
 | 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. 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. |
+| 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). M6b: task 05's row says `NOT_CONFIGURED` was inserted before `SECRET_POINTER`; the file has it after. Same strings, same behaviour. |
+| 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. M6b: no; six 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. M6b: held (06 then 05 through 01 under the header). |
 | 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
@@ -194,3 +194,8 @@ code difference from the reference is cosmetic in `subagent.rs`. Device check pa
 child `sub-m6a-live-2` on slot 2, then a following slot-0 turn with `cache_n` 1787 of 1788. The
 same-turn continuation after a busy-slot wait on 2 lost the prefix (`CacheLoss` 0 of 1672),
 which is P9; the immediate next turn held it.
+
+M6b was Ornith through the driver, six tasks, first-gate pass in every row, no stop. The product
+files match the reference in behaviour (import order, const order, two blanks). Task 03 wrote the
+given tests in place and they `cmp` identical. Task 05's log row says `NOT_CONFIGURED` was
+inserted before `SECRET_POINTER`; the file has it after (I17). Device check is later.

docs/implementer-log.md

@@ -1210,3 +1210,38 @@ 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.
+
+### M6b, tasks 01 to 06 — reviewed 2026-10-04 by the design model (Grok 4.6)
+
+Accepted. No product finding. Ornith-1.5-35B-A3B through `tools/run-plan.sh` on straylight, branch
+`m6b`. The product files match the reference (`~/boxmaker-refs/m6b-ref.bundle`, `5fb05f0`) in
+behaviour. The remaining delta is rustfmt import order in `args.rs`, the order of
+`SECRET_POINTER` and `NOT_CONFIGURED` in `consult.rs`, a blank line in `main.rs`, and a missing
+blank line before `## consult-not-configured` in `docs/runbook.md`. Log rows are the other delta.
+The two owner docs commits ahead of the plan (`docs/decisions.md` cloud-led row, `docs/egress.md`
+loopback dummy) sit on the branch and were outside the reference. Spec 7 device check is later
+(loopback dummy on straylight, no internet).
+
+| Check | Result |
+|---|---|
+| 6 done task commits, each with `Co-Authored-By: ornith-1.5-35b-a3b <agent@wntrmute.dev>` | pass (plus the plan commit `50e9cc2` and the two section-10 docs commits) |
+| Given tests byte-identical to `docs/plans/M6b/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 | 6 `done`, newest first, first-gate pass, one gate each. Task 03 notes the given tests were written in place; they `cmp` identical. Task 05's row says `NOT_CONFIGURED` was inserted before `SECRET_POINTER`; the file has it after. Same strings. |
+| 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`, 1611 tests, the plan's count |
+| Independent review of `git diff 50e9cc2 HEAD -- crates docs/runbook.md` against spec sections 4, 5, 6, 7 (M6b) and 8 | `consult` is not core; `m5c()` pushes it after `subagent`; the epoch tools array is `core_schemas()`; `timeout_for("consult")` is `max` with `CONSULT_WAIT` 180 s and `[broker] timeout_ms` still 120_000; `[consult]` denies unknown keys, stores temperature as `temperature_milli` (default 600, ceiling 0.6), and accepts only `http://127.0.0.1` or `http://localhost`; `load_secret` runs only when the table is present, role `"brokerd"`, and a missing or too-open secret names `consult-secret-unreadable`; `ToolName::Consult` is parsed and is outside `ALL` and `is_memory`; grants reject consult paths and hosts; `covers` returns `Some(None)`; `Podman` refuses with `IN_CONSULT`; `AuditEvent::Decision` carries optional `body` and `Approval` does not; no `[consult]` is `decide_forced` `InvalidArguments` with `consult-not-configured` and still records the body (empty model, 0.6); a matching grant goes through `decide_with_body`; `wire_body` is model, temperature, one user message, no tools; `HttpTransport` is loopback TCP HTTP/1.1; `ConsultStack` wraps `Routed`; the answer is tag-neutralised and capped; posted bytes equal the audit body; `untrusted` default true and `result_class` default `private`; both runbook headings exist. |
+
+No numbered product finding. Task 03's in-place tests and task 05's const-order log line are
+process notes. `rule_3`'s unknown-tool sentence still names the four container tools.
+
+Device check is owner-side after this review: a loopback OpenAI-compatible dummy the check starts
+on straylight; a clean session's audit body matches a captured request; a session that first
+recalled an untrusted note is refused. No internet consult. Production daemons stay untouched.
+
+What was good: six tasks, six first-gate passes, no stops, given tests unedited, every
+review-focus case in the plan is covered by a given test and holds, both consult runbook entries
+have pointers, no new crate, no vendor names in code.
+
+Known M6b limits, left as the README lists them: no internet consult, no HTTPS, cloud-led
+sessions later.