diff --git a/AGENTS.md b/AGENTS.md index 8383963..6a50044 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -77,6 +77,10 @@ These come from defects found in review. The evidence is in `docs/implementer-le angle-bar turn markers), in code, comments, commands or your log. Build them from pieces (`concat!("<", "think")`) and name them in words: text holding them becomes your own control tokens when you read it. +- When a test outside your task fails, check once whether it fails without your change, 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. +- Write your log row from `git diff` after staging, naming only what the diff shows. ## The gate diff --git a/docs/implementer-lessons.md b/docs/implementer-lessons.md index 962fef2..37cf299 100644 --- a/docs/implementer-lessons.md +++ b/docs/implementer-lessons.md @@ -30,7 +30,9 @@ How it is used: | I12 | A file that exists but cannot be read is an error. Only a file that does not exist may be treated as absent. | M2b finding 3. | yes | | | I13 | Before calling a failure "environmental", measure the thing you blame, and name the machine, OS and file system the evidence came from. A sample that shows a thread inside a system call shows where the time goes, not that the call is stuck. | M3a stop before task 20: an fsync "stall" was blamed from `__fcntl` frames that were ordinary 4 ms `F_FULLFSYNC` calls on the Mac, and the real cause, a macOS socket rule, went unfixed. | no | | | 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 | | +| 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 | | +| 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 | | ## Tips for writing tasks @@ -69,6 +71,13 @@ How it is used: | T31 | In a spec, for every post or message the harness sends, name who can read it and whether that reader can act on it. Walk each kind of sender through each kind of channel (the owner, an allowed non-approver, a stranger; direct, group, channel). | M4b: an approval asked by a non-approver in a direct message went into that direct message, where no approver could see it. Every test passed, because the tests used the spec's cases; the device check with a second account found it (review finding 1, fixed by tasks 17 and 18). | | T32 | A given test must not assert the order of two things that run on their own threads. Check the order where it is decided (here, the sort in the catch-up parser), and compare what the threads deliver as a set. Before hand-over, stress each test with concurrency under load (`yes > /dev/null` beside a loop of 60 runs). | M4a's `serve_restart` expected two caught-up sessions' turns to reach `loopd` in order. They run on separate threads, and under load 1 run in 60 failed. M4b task 18's session hit it, proved it happens without its change, and reported it instead of editing the test. | | T33 | A file the implementer reads must never hold the model's chat-template tags literally: not in skeletons, given tests, fixtures, task text or specs it may open. Build them from pieces and name them in words. grep the plan for the tags before hand-over. | M5a task 04 (above); the measurements in `docs/research/2026-09-24-chat-template-tokens.md`: the think and tool-call tags are one token whatever the tokenizer's flag, and inside a chat message the turn markers become control tokens. | +| T34 | Replay the plan with the toolchain of the host that will run it, and write the `rustc` and `clippy` versions in the plan. Each clippy release adds lints, and the workspace's `rust-version` is only a floor. | M5a task 16 was replayed on talos; on titan, clippy 1.95 failed the gate on `collapsible_match` in code from task 08, and the session stopped (review finding 6 of 2026-09-25). | +| T35 | A scratch worktree for a reference or a replay builds into its own `target/`, never the main repository's. Test binaries carry absolute paths (`CARGO_BIN_EXE_*`, `CARGO_MANIFEST_DIR`), and Cargo's freshness check ignores the workspace's path, so the main tree reuses them. If one ever shared it, run `cargo clean` before the implementer starts. | M5a task 21: the gate failed with `NotFound` in `args_os`, and in most suites with `--no-fail-fast`, after the reference worktree was deleted; the run stalled (review finding 4). | +| T36 | When a milestone makes a value vary that used to be fixed, list every place that reads it and give each a test with the new variation. A spec's "as today" about existing code is a claim to check against the code, not a reassurance. | M5a: results gained labels of their own. The runner and the audit record used them; the ledger kept raising the session's state by the grant's label, as it always had, and the spec said "as today". The tests checked the response and the audit record, not the state (device finding 1). | +| T37 | For every trust boundary in a spec, write two attacks as tests: untrusted text shaped like trusted structure, and a trusted reader that takes in an untrusted result and keeps its trust. Both M5a highs were one of these, and the reference had both. | M5a: a note forging a `notes.md` header (review finding 1 of 2026-09-24); a clean session recalling a tainted note and writing it into identity (device finding 1). | +| T38 | A tool's answer that the model relays must say the outcome in plain words, including what did not happen ("not added to memory.md: …"). A label in parentheses reads as success. | M5a device check: the model twice told the owner "Added to memory.md" for a write that became an untrusted note. After task 22 it reported it correctly. | +| T39 | Name the source directory of copied files in every task's Files section. | M5a task 06 logged a deviation about a files directory the task never named. | +| T40 | Give the gate's shell timeout in the task (600000 ms). On a machine that also serves the model, the gate takes longer than the implementer's default of 2 minutes, and a timed-out gate looks like a failure. | M5a tasks 18 and 19 each ran the gate twice on titan; tasks 21 and 22, which gave the timeout, ran it once. | ## What worked and should be kept @@ -81,6 +90,9 @@ How it is used: edited tests and no touched design documents across nine tasks. - One task, one fresh session, one commit, with only listed paths staged. The history reads as the plan. +- A device check against the real model and the owner's server before landing. M4b's non-approver + approval and M5a's path from a fetched page into identity were found only there; tests, + reference and review had all passed them. ## What we have seen of the implementers so far @@ -113,3 +125,11 @@ above low. Given a skeleton whose comments name the calls and expressions, it wr and its code agreed with the reference on every fuzzed input. Given a `todo!()` with many branches, or a task that needs the whole crate's API, it plans in prose until the turn runs out; given a comment that contradicts its function, it notices but deliberates rather than stopping (T29). + +M5a was Ornith through the driver, twenty-two tasks over two machines: first gate 7 of 14 on the +main run, and four of the five follow-ups on their first run. Its code matched the reference on +every random input outside two findings, and every follow-up was exact. Its misses were in what it +wrote about the code, not the code: two log rows describe work not in the diff, and one runbook +change was its own text where the task gave a fragment (I17). It stopped correctly on a failure +outside its task once (task 16), and once kept experimenting for over an hour (task 21, I16). The +milestone's two serious defects were the spec's, and the reference had them too (T36, T37). diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 4ba5c85..b63147b 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -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