M5a review: tasks 01 to 22 and the device check; tips I16, I17, T34 to T40
Two review entries in the implementer log: tasks 01 to 15 (the header forgery, review ids, the runbook entries, log rows written from memory) and tasks 16 to 22 with the device check on straylight (the ledger raising state by the grant's label, the note answers, the stale target/ that stalled task 21, the clippy 1.95 stop, the fake podman race). Two short rules go into AGENTS.md: stop after one check when a test outside the task fails, and write the log row from the diff. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+86
-2
@@ -6,9 +6,9 @@ reviewer adds findings under "Reviews" once per milestone.
|
||||
|
||||
| Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model |
|
||||
|---|---|---|---|---|---|---|---|
|
||||
| M5a/22-plain-note-answers | 2026-09-25 | done | 1 | pass | none | The edits are all in `crates/memoryd/src/edit.rs` and match the task exactly. Replaced the `SAVED_AS_NOTE` const with a `saved_as_note(file)` fn ("not added to {file.name()}: this session has read untrusted content, so it was saved in notes.md as an untrusted note") plus the `NOTED` const "saved in notes.md" and `NOTED_UNTRUSTED` const "saved in notes.md, marked untrusted: this session has read untrusted content"; the `format!` string stays one line past 100 chars and `cargo fmt` left it so. Updated the comment in `apply` ("replaced by saved_as_note(file)") and the `answer:` line (`saved_as_note(file)`). In `noted` replaced the two string literals with `NOTED_UNTRUSTED`/`NOTED`. Copied the new `crates/memoryd/tests/edit.rs` and `serve.rs` from `docs/plans/M5a/files/` (not the `-task06`/`-task10` versions). Step 2 showed exactly the four named tests failing (`a_note_is_labelled_by_the_session` and `an_untrusted_session_can_take_notes_but_not_change_identity` in edit, `an_untrusted_session_writes_notes_not_identity` and `review_holds_identity_writes_but_not_notes` in serve); after the edits the step-2 command and `cargo test -p memoryd` every suite pass; `make gate` printed `gate: ok` with 944 tests, unchanged; `grep -rn SAVED_AS_NOTE crates/` prints nothing. No new dependency, so `Cargo.lock` is unchanged. | ? |
|
||||
| M5a/22-plain-note-answers | 2026-09-25 | done | 1 | pass | none | The edits are all in `crates/memoryd/src/edit.rs` and match the task exactly. Replaced the `SAVED_AS_NOTE` const with a `saved_as_note(file)` fn ("not added to {file.name()}: this session has read untrusted content, so it was saved in notes.md as an untrusted note") plus the `NOTED` const "saved in notes.md" and `NOTED_UNTRUSTED` const "saved in notes.md, marked untrusted: this session has read untrusted content"; the `format!` string stays one line past 100 chars and `cargo fmt` left it so. Updated the comment in `apply` ("replaced by saved_as_note(file)") and the `answer:` line (`saved_as_note(file)`). In `noted` replaced the two string literals with `NOTED_UNTRUSTED`/`NOTED`. Copied the new `crates/memoryd/tests/edit.rs` and `serve.rs` from `docs/plans/M5a/files/` (not the `-task06`/`-task10` versions). Step 2 showed exactly the four named tests failing (`a_note_is_labelled_by_the_session` and `an_untrusted_session_can_take_notes_but_not_change_identity` in edit, `an_untrusted_session_writes_notes_not_identity` and `review_holds_identity_writes_but_not_notes` in serve); after the edits the step-2 command and `cargo test -p memoryd` every suite pass; `make gate` printed `gate: ok` with 944 tests, unchanged; `grep -rn SAVED_AS_NOTE crates/` prints nothing. No new dependency, so `Cargo.lock` is unchanged. | Ornith-1.5-35B-A3B |
|
||||
| M5a/21-session-follows-result | 2026-09-25 | done (by the design model) | many | fail | The session added and then deleted diagnostic test files (`crates/bxctl/tests/zz_diag.rs`, `zz_diag2.rs`) and ran about 80 minutes of experiments instead of stopping when a test outside the task failed, as the task says. | The session's edits match the task file exactly and the copied `ledger_label.rs` is identical to the plan's; step 2 showed the two named tests failing, and after the edits `ledger_label`, `cargo test -p brokerd` and `-p bxctl` passed. The gate then failed in `bxctl`'s `args_os` with `NotFound`. The cause was the design model's: its reference worktree for tasks 21 and 22 shared this `target/` through a symlink, so test binaries built there had the worktree's absolute paths compiled in (`CARGO_BIN_EXE_*`, `CARGO_MANIFEST_DIR`), and Cargo, whose freshness check does not include the workspace's absolute path, kept them after the worktree was deleted. After `cargo clean`, `make gate` printed `gate: ok`, 944 tests. The owner stopped the run. | Ornith-1.5-35B-A3B (the session); design model (the commit) |
|
||||
| M5a/20-review-ids | 2026-09-25 | done | 1 | pass | none | The two edits exactly as the task gives, both in `submit` in `crates/memoryd/src/review.rs`: the comment block "how many Waiting lines have an id starting format!(\"p-{day}-\"). The id is format!(\"p-{day}-{:04}\", taken + 1)." became "how many Waiting lines of `lines(store)?`, done or not, have an id starting format!(\"p-{day}-\"): an id is never used twice. The id is format!(\"p-{day}-{:04}\", taken + 1)."; and `let taken = waiting(store)?` became `let taken = lines(store)?`, the `.iter()`/`.filter(..)`/`.count()` after it unchanged. `waiting` untouched, since `list`/`accept`/`reject` still use it. Copied `crates/memoryd/tests/review_ids.rs` from `docs/plans/M5a/files/`. `cargo fmt --all`; step 2 showed exactly the two named tests failing (`a_write_staged_after_an_accepted_one_gets_a_new_id_and_can_be_accepted` expected `p-2026-09-24-0002` got `p-2026-09-24-0001`; `ids_count_every_write_staged_that_day_waiting_or_done` expected `p-2026-09-24-0004` got `p-2026-09-24-0002`); after the edits both pass and `cargo test -p memoryd` every suite passes; `make gate` printed `gate: ok`. `cargo test --workspace` reports 940 passed, plus the gate's separate loopd `end_to_end` ignored test = 941 total. No new dependency, so `Cargo.lock` is unchanged. | ? |
|
||||
| M5a/20-review-ids | 2026-09-25 | done | 1 | pass | none | The two edits exactly as the task gives, both in `submit` in `crates/memoryd/src/review.rs`: the comment block "how many Waiting lines have an id starting format!(\"p-{day}-\"). The id is format!(\"p-{day}-{:04}\", taken + 1)." became "how many Waiting lines of `lines(store)?`, done or not, have an id starting format!(\"p-{day}-\"): an id is never used twice. The id is format!(\"p-{day}-{:04}\", taken + 1)."; and `let taken = waiting(store)?` became `let taken = lines(store)?`, the `.iter()`/`.filter(..)`/`.count()` after it unchanged. `waiting` untouched, since `list`/`accept`/`reject` still use it. Copied `crates/memoryd/tests/review_ids.rs` from `docs/plans/M5a/files/`. `cargo fmt --all`; step 2 showed exactly the two named tests failing (`a_write_staged_after_an_accepted_one_gets_a_new_id_and_can_be_accepted` expected `p-2026-09-24-0002` got `p-2026-09-24-0001`; `ids_count_every_write_staged_that_day_waiting_or_done` expected `p-2026-09-24-0004` got `p-2026-09-24-0002`); after the edits both pass and `cargo test -p memoryd` every suite passes; `make gate` printed `gate: ok`. `cargo test --workspace` reports 940 passed, plus the gate's separate loopd `end_to_end` ignored test = 941 total. No new dependency, so `Cargo.lock` is unchanged. | Ornith-1.5-35B-A3B |
|
||||
| M5a/19-no-forged-headers | 2026-09-25 | done | 2 | timed out (shell 2 min) | none | Two edits exactly as the task gives. `crates/proto/src/memory.rs`: the doc comment above `pub fn entry_problem` (the two lines "holding a line that is only the separator") became the three lines explaining a line starting with the separator, spaces aside, would read as a header in notes.md and a header says where an entry came from; and in the body, `if line.trim() == SEPARATOR { return Some(format!("an entry cannot hold a line that is only {SEPARATOR}")); }` became `if line.trim_start().starts_with(SEPARATOR) { return Some(format!("an entry cannot hold a line that starts with {SEPARATOR}")); }`, the for loop and closing braces unchanged. `crates/memoryd/src/notes.rs` `parse_header`: the `untrusted` match `Some(s) => *s == "untrusted"` became `Some(&"untrusted") => true, Some(_) => return None`, so a `noted` header whose last part is anything other than `untrusted` is not a header. Copied three test files from `docs/plans/M5a/files/`: `crates/proto/tests/memory_args.rs` and `crates/memoryd/tests/notes.rs` (new versions) and the new `crates/memoryd/tests/forgery.rs` (not the `-task02`/`-task06` files). `cargo fmt --all`. Step 2 showed exactly the three named tests failing (`entries_are_not_empty_not_too_long_and_hold_no_separator_line`, `a_noted_header_with_an_unknown_last_part_is_not_a_header`, `a_note_holding_a_header_line_is_refused_and_nothing_is_written`); the fourth, `recall_finds_no_identity_in_what_an_untrusted_session_wrote`, already passed. After the edits the step-2 command passed and `cargo test -p proto -p memoryd -p brokerd` every suite passed. `make gate` printed `gate: ok`; `cargo test --workspace` reported 938 passed and the gate's separate loopd `end_to_end` ignored test added 1, for 939 total. First `make gate` run hit the shell's 120 s timeout before finishing (the loopd daemon test needs more than 2 min); the second run completed with `gate: ok`. No new dependency, so `Cargo.lock` is unchanged. | Ornith-1.5-35B-A3B |
|
||||
| M5a/18-repeated-code | 2026-09-25 | done | 2 | pass | none | The four edits exactly as the task gives: `edit.rs` added the private `claim(file, before, &mut named, old)` one blank line above `consolidate`, and replaced the four identical `Keep`/`Rewrite`/`Move`/`Drop` blocks (the `position` check plus the `named.insert` duplicate guard) with `claim(file, before, &mut named, &old)?;`, leaving the `New` arm untouched; `proto/src/memory.rs` added the private `entry(text, max)` one blank line above `impl RecallArgs`, and rewrote `MemoryArgs::check` to call `entry` with `?` instead of the nine `if let Some(problem) = entry_problem(..) { return Err(problem); }` blocks (the `Consolidate` arm's `map_err(|problem| problem.to_string())` became `map_err(str::to_string)`), `RecallArgs::check` unchanged; `brokerd/src/builtin.rs` replaced the two `GrantProblem {..}` inline closures in `grants` with a single `let problem = |error: String| GrantProblem {..}` and `map_err(|e| problem(e.to_string()))`; `bxctl/src/memory.rs` added `let unreachable = |e: &dyn std::fmt::Display| format!("cannot reach memoryd at {}: {e}", socket.display())` and used it for connect, the two `set_*_timeout` sets and the `write_frame` (the `memoryd did not answer` line was left as its own sentence). `cargo fmt --all`; the step-2 greps print 1, 0, 2, 1 in order; `cargo test -p memoryd -p proto -p brokerd -p bxctl` every suite passes; `make gate` printed `gate: ok` with 936 tests, unchanged. No new dependency, so `Cargo.lock` is unchanged. | Ornith-1.5-35B-A3B |
|
||||
| M5a/17-memoryd-no-slicing | 2026-09-25 | done | 1 | pass | none | The five slice-to-`get` edits exactly as the task gives: `notes.rs` `minute` two lines -> `format!("{}Z", s.get(..16).unwrap_or(&s))`; `journal.rs` `append` the five-line `day` -> `rfc.get(..10).unwrap_or("0000-00-00")` and `&day[..7]` -> `day.get(..7).unwrap_or("0000-00")`; `review.rs` `submit` `&rfc[..10]` -> `rfc.get(..10).unwrap_or("0000-00-00")`; `recall.rs` `recall` `&rfc[..7]` -> `rfc.get(..7).unwrap_or("0000-00")`. Left `&all[all.len().saturating_sub(count)..]` in `log` as instructed. `cargo fmt --all`; the step-2 grep for `\[..[0-9]+\]` prints nothing; `cargo test -p memoryd` every suite passes; `make gate` printed `gate: ok` on the first run, 936 tests unchanged. | Ornith-1.5-35B-A3B |
|
||||
@@ -706,3 +706,87 @@ message to read a file that the `ask` grant covers:
|
||||
| Task 16's sentence | the call was refused with "no tool runner is configured: brokerd.toml has no [runner] section", and the model passed it on to the asker |
|
||||
|
||||
Finding 1 is closed.
|
||||
|
||||
### M5a, tasks 01 to 15 — reviewed 2026-09-24 by the design model (Claude)
|
||||
|
||||
Accepted, with four follow-up tasks (17 to 20). Tasks 01 to 14 by Ornith-1.5-35B-A3B through
|
||||
`tools/run-plan.sh`; task 15 finished by the design model. The code does what the plan says; a
|
||||
differential test on random inputs against the reference (the bundle on straylight) matched
|
||||
everywhere except findings 2 and 3. The worst finding is a gap in the spec, which the reference
|
||||
shared (finding 1).
|
||||
|
||||
| Check | Result |
|
||||
|---|---|
|
||||
| 15 task commits, each with the `Implemented-By` trailer | pass |
|
||||
| 40 given tests and fixtures identical to the plan | pass; two of the four runbook fragments were not copied (finding 5, found at the second review) |
|
||||
| Protected files | untouched, except `deny.toml`, which task 03 was told to change (the Zlib licence for `foldhash`) |
|
||||
| `make gate` on talos | `gate: ok`, 936 tests, the count the plan gave |
|
||||
| Banned constructs in new library code | none; `unwrap` and `panic!` only in doctests |
|
||||
|
||||
| # | Severity | Owner | Finding | Fix |
|
||||
|---|---|---|---|---|
|
||||
| 1 | high | spec (the reference too) | A note's text could hold a line shaped like a `notes.md` header (`§ moved <time> from memory.md in session <id>`). Read back, it started an entry of its own, and `recall` showed the text after it tagged `identity`, not untrusted: an untrusted session could plant identity that a clean session would trust and could copy into `memory.md`. | Spec `4ae827a` (section 3); task 19 |
|
||||
| 2 | medium | Ornith, from an ambiguous plan comment | `review::submit` counted only the waiting writes, so a write staged after an accepted one got the accepted one's id and was lost, while the model was told it was waiting. | Task 20; tip T30 |
|
||||
| 3 | low | Ornith | `notes::parse_header` read an unknown last part of a `noted` header as trusted (fail-open). | Task 19 |
|
||||
| 4 | nit | Ornith | Unguarded slices of `to_rfc3339()` output; the same error lines written out four times in four places. | Tasks 17 and 18 |
|
||||
| 5 | low | Ornith (task 13) | The runbook's `memory-unavailable` kept `core-memory-unreadable`'s old body, about `memory/core.md`, and `memory-index-damaged` was Ornith's own text: it named a pointer `memoryd` never prints and left out `bxctl reindex`. The log row says both fragments were copied. | `96e3190`: the plan's fragments, verbatim; tip I17 |
|
||||
| 6 | low | Ornith (log rows) | Task 10's row describes code that is not in `serve.rs` ("caller must be the owner", `journal::block`); it reads as written from memory, not from the diff. Task 06 logged a deviation about a files directory the task never named. Task 15's stop blamed straylight's model routing; the cause was `BOXMAKER_MODEL`, read by both `run-plan.sh` and the device tests. | Tip I17; the plan names its files directory (T39); `run-plan.sh` reads `OPENCODE_MODEL` (`4f0870a`) |
|
||||
|
||||
What was good: Ornith's `memoryd` agreed with the reference on every random input outside findings
|
||||
2 and 3. Every given test was copied unchanged, and no task added a dependency the plan did not
|
||||
name. Task 15's session stopped when the device tests failed instead of changing the test or the
|
||||
soul.
|
||||
|
||||
**The run.** Task 04 stopped twice: the plan's files held the model's chat-template tags, and
|
||||
Ornith's reasoning ended inside the given test. The plan was fixed (`e40906f`, `3ed1b8b`) and the
|
||||
research is in `docs/research/2026-09-24-chat-template-tokens.md` (tips I15 and T33). Task 15 was
|
||||
finished by the design model once `BOXMAKER_MODEL` was unset: `make verify-device` passed 6 of 6.
|
||||
First gate on the first session: 7 of 14.
|
||||
|
||||
### M5a, tasks 16 to 22 and the device check — reviewed 2026-09-25 by the design model (Claude)
|
||||
|
||||
Accepted, with two follow-up tasks from the device check (21 and 22), both run and accepted. The
|
||||
work moved from talos to titan (macOS, Rust 1.95, then 1.98.1) before task 16. Every task's code is
|
||||
the code its task file gives, byte for byte: 17 to 20 and 22 by comparison with the task files, 21
|
||||
by the reference (`m5a-ref21.bundle` on straylight). `make gate` on titan: `gate: ok`, 944 tests,
|
||||
the count the plan gave.
|
||||
|
||||
| Task | Result |
|
||||
|---|---|
|
||||
| 16, the gate on macOS | Ornith's edits exact; it stopped on a clippy 1.95 lint in `memoryd` that talos's clippy never flagged, which was right (finding 6). Committed by the design model after the lint fix. |
|
||||
| 17, 18, 19, 20 | exact; 19's first gate run hit the shell's 2-minute limit (finding 9) |
|
||||
| 21, the session follows the result | Ornith's edits exact, the two named tests failed first; then the gate failed for the design model's reason (finding 4) and Ornith did not stop (finding 5). Committed by the design model. |
|
||||
| 22, plain note answers | exact; `edit.rs` is the reference's blob; the four named tests failed first |
|
||||
|
||||
**On the device** (2026-09-25, straylight: `m5a` built by the new flake, every role running,
|
||||
`brokerd` with the Podman runner and a rebuilt tool image, `loopd` on Ornith slot 0, `gatewayd`
|
||||
against the owner's Mattermost; the owner sent the messages). The checks of spec section 14:
|
||||
|
||||
| Claim | Seen |
|
||||
|---|---|
|
||||
| The baseline with the starter soul stays under 3,000 tokens | 881 tokens; 2,432 with every file at its limit (`make verify-device`, 6 of 6, in the flake's dev shell) |
|
||||
| A fact told in one thread is recalled in another | `recall` found the parking spot in the other thread's log, tagged `memory` |
|
||||
| An `add`, made with no question and listed by `bxctl memory log`, appears in a new thread and not in the one that wrote it | `0001 add user.md`: absent from the writer's baseline, present in the next thread's, which answered without a tool call |
|
||||
| A `consolidate` that moves an entry, which `recall` then finds | `user.md` emptied, the `§ moved … from user.md` note written, `recall` ranked it first, tagged `identity` |
|
||||
| A write after a fetched page becomes an untrusted note | `http_fetch` in a real container through the egress proxy; the session untrusted; an `add` to `memory.md` became `add notes.md untrusted`, `memory.md` unchanged |
|
||||
| `bxctl memory undo` of the latest edit | `0002` reverted and journalled; undoing a note, or an edit that is no longer the latest, refused with the reason |
|
||||
| After tasks 21 and 22: a clean session that recalls a tainted note | the session became untrusted; its `add` answered `not added to memory.md: …`, and the model told the owner so |
|
||||
|
||||
The audit log verified after each part (`bxctl audit verify`, 16 and then 27 records).
|
||||
|
||||
| # | Severity | Owner | Finding | Fix |
|
||||
|---|---|---|---|---|
|
||||
| 1 | high | spec (the reference too) | The ledger raised a session's state by the grant's label, never the result's. M5a lets `memoryd` make a result stricter, and the spec said the state used the combined label "as today", which was not what the code did. So a clean session that recalled a `memory-tainted` note stayed trusted and wrote the note into `memory.md`: on the device, the path from a fetched page into identity, which P21 exists to close. The tests checked the response and the audit record, never the state. | Spec `0e5b1d2` (section 8); task 21; tips T36 and T37 |
|
||||
| 2 | medium | spec | Twice the model told the owner "Added to memory.md" for a write that had become an untrusted note; the answers (`noted (untrusted)`, `saved as a note (untrusted): …`) did not say what had not happened. | Spec `0e5b1d2` (section 3); task 22; tip T38. On the re-check the model reported it correctly. |
|
||||
| 3 | low | spec (M3a) | An invalid-arguments refusal gives no reason. The model sent `note` with a `file` twice in two sessions and found the right form by trial. | Open in `docs/decisions.md`, for M5b: it needs a detail in `ToolResponse::Denied`, a wire change |
|
||||
| 4 | high (for the run) | design model | Task 21's gate failed in `bxctl`'s `args_os` with `NotFound`. The design model's reference worktree for tasks 21 and 22 shared this `target/` through a symlink; test binaries built there had its absolute paths compiled in (`CARGO_BIN_EXE_*`, `CARGO_MANIFEST_DIR`), Cargo kept them because its freshness check ignores the workspace's absolute path, and the worktree was then deleted. | `cargo clean`; tip T35 |
|
||||
| 5 | medium | Ornith (task 21) | The task said to stop when a test outside it failed. The session stashed its change and saw the failure remain, which was right, then spent about 80 minutes on experiments, wrote two diagnostic test files into `crates/bxctl/tests/` and deleted them, until the owner stopped the run. | Tip I16 |
|
||||
| 6 | low | plan | Task 16 was replayed on talos, whose clippy did not flag `collapsible_match` in `memoryd`'s `index.rs`; titan's 1.95, the workspace's `rust-version`, did. | `d47fb32`; tip T34 |
|
||||
| 7 | low | tests (M3b) | On titan, loaded by serving Ornith, the fake `podman`'s time-limit test failed about one run in five: the fake wrote each argument with its own `printf`, and a `kill` recorded mid-`run` landed inside `run`'s lines. | `1adce5c`: each call written by one `cat`, and the time-limit tests check the calls other than `run`; 20 of 20 |
|
||||
| 8 | nit | Ornith (20, 22) | No `Implemented-By` trailer; the Model cell `?`. | Model filled in here |
|
||||
| 9 | low | plan | On titan the gate takes more than the OpenCode shell's 2 minutes; tasks 18 and 19 each ran it twice. | Tasks 21 and 22 give a 600000 ms timeout; tip T40 |
|
||||
|
||||
What was good: every task's code was exact. Task 16's session found a failure in a crate it did not
|
||||
touch, proved it happens without its change, committed only its log row and stopped, which is
|
||||
what the task asked. Tasks 17, 18, 20 and 22 passed the gate on the first run. The device check
|
||||
found the milestone's worst defect, which review, reference and tests had all missed.
|
||||
|
||||
Reference in New Issue
Block a user