diff --git a/AGENTS.md b/AGENTS.md index adf40b3..96cb136 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,7 +18,9 @@ Unix sockets. You are implementing it one task at a time. row, because the record of the stop is part of the log. 5. Work only from files inside this repository. Never read another checkout or worktree (such as `~/src/boxmaker-ref`), and never search the file system for code (`find /`, `locate`). If you - are stuck, stop as in point 4; copying code from elsewhere is not finishing the task. + are stuck, stop as in point 4; copying code from elsewhere is not finishing the task. A + crate's source in the Cargo registry (`~/.cargo/registry`) is outside the repository too: the + task's table of its API is all you need. To try something out (a small program, a script, a copy of a file), work under `.state/scratch/`, which git ignores, and delete it before you commit. Never use `/tmp` or any other directory outside the repository. @@ -83,9 +85,14 @@ 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, - outside `.state/scratch/`. +- When a test outside your task fails, check once whether it fails without your change + (`git stash -u`, run, `git stash pop`), then stop and report both results. Do not keep + experimenting, and never add a file the task does not list, outside `.state/scratch/`. The one + exception is a test the plan's README lists as flaky: if it passes on one rerun and the gate + passes, name it in your row's Notes and go on. +- An edit replaces exactly the task's "before" block and nothing around it. When a "before" block + does not match, look at `git diff ` for your own earlier edit, then stop and report; never + anchor the edit on another line. - Write your log row from `git diff` after staging, naming only what the diff shows. ## The gate @@ -95,7 +102,9 @@ it takes longer than the default 2 minutes, and a gate cut off by the timeout is your code. It runs offline: rustfmt, clippy with warnings denied, all tests, cargo-deny, and the scripts in `scripts/`. Run `cargo fmt --all` before the gate; rustfmt decides the order of `mod` and `use` lines, not you. After you add a dependency, -run `cargo build` once so that `Cargo.lock` is updated, then run the gate. +run `cargo build` once so that `Cargo.lock` is updated, then run the gate. A test count in a task +is the passed total of `cargo test --workspace` plus one, for the ignored `end_to_end` test that +the gate runs on its own; never run the gate again only to count. Useful while working: `cargo test -p --test ` runs one test file, and `cargo test -p ` runs tests whose name contains ``. diff --git a/docs/implementer-lessons.md b/docs/implementer-lessons.md index c3c4f6e..ba13e48 100644 --- a/docs/implementer-lessons.md +++ b/docs/implementer-lessons.md @@ -31,8 +31,8 @@ How it is used: | 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 | 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 | | +| 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. | ## Tips for writing tasks @@ -87,6 +87,15 @@ How it is used: | T47 | Name the log row in the task ("the row's Task cell is `M5b/14-runbook`"). The driver checks the row by the file's name; the implementer takes it from the task's heading. | M5b task 14: the row was `M5b/14-runbook-sanitizer-rules`, from the heading "the runbook entry `sanitizer-rules`", and `run-plan.sh` stopped on a finished task. | | T48 | Run the implementer with no plugins and no borrowed rules: `opencode run --pure` and `OPENCODE_DISABLE_CLAUDE_CODE=1`. Global plugins load into every session, and a plugin that rewrites the system prompt forces a full re-read of the conversation on a model that cannot rewind its cache. | M5b run (2026-09-26): a short-term-memory plugin rewrote the system prompt every fourth request (a 35,633-token re-read in task 10), 22 plugin tools rode in every request (about 2,400 tokens), and in tasks 01 to 10 prompt reads of 10,000 tokens or more took 76 of 111 minutes of prompt processing: 42 of them, of which only about 11 were session starts (about 15,000 tokens each). Tasks 11 to 14, run after the plugins were removed, had 5 such reads (4 session starts), 4 minutes in all. | | T49 | When a task adds requests on a path through another role, count them against that role's limits (`inferproxy`: 8 connections at once, a burst of 10, then 2 a second), and put the real proxy in at least one given test. A fake server reached directly cannot show a limit on the way to it. | M5b task 08: 66 `/tokenize` requests at startup passed every test and failed on straylight's first start, refused by `inferproxy`'s token bucket. | +| T50 | When a task gives a gate test count, give the command that produces it and say what it includes (the passed total of `cargo test --workspace` plus the ignored `end_to_end` test the gate runs on its own). Otherwise, do not make the count a stop condition. | M5c-1 tasks 01, 03, 13 and 15: each session found itself one short ("1129 passed — expected 1130") and rediscovered the rule. The count drove extra gate runs, scratch files in `/tmp` (02, 03, 05, 10, 13) and `git stash` baselines (03, 15); about a quarter of the transcripts of 03, 13 and 15 went on it. | +| T51 | Never tell the implementer to run a command that the harness denies or that AGENTS.md forbids. Write the branch line as a check ("`git branch --show-current` prints `m5c1`"), not as `git switch`. | M5c-1: all fifteen transcripts start with a refused `git switch m5c1`; AGENTS.md forbids switching branch. | +| T52 | List the known flaky tests in the plan README, with what to do (a test outside the task that fails once, then passes on a rerun and in the gate, is named in the log row's Notes and the task goes on). Before hand-over, stress the whole workspace under load, not only the new tests (T32). | M5c-1 task 01: gatewayd's `brokerd_closing_first_ends_it_as_no_longer_pending` (2 of 10 runs fail on a loaded talos with no M5c-1 change); task 15: loopd's `pointers`. Both older tests; Laguna went on in both cases, and task 01's row calls a different test "the only flake". | +| T53 | Give the baseline in the task so the implementer never has to measure one. Where a check without the change is needed, the stash is `git stash -u`: a plain stash leaves the new untracked tests behind, and they no longer compile. | M5c-1 task 15: a baseline run printed an empty total; task 03 moved the new tests aside by hand before stashing. | +| T54 | Give an import change as a single added `use` line, never as a rewritten import block: the implementer's Edit retypes the lines around it, and the next task's "before" block stops matching. | M5c-1 task 12: its W1 edit rewrote the import block in another order, W2's "before" block then failed to match, and Laguna anchored on its own line instead of stopping (logged as "Deviations: none", I17). Task 06: an M2 replacement landed in the wrong `match`. | +| T55 | Put "its spaces are checked" on every edit to a literal whose whitespace matters, not only the first. | M5c-1: task 04's C1 had the note and was right first time; task 14's U1 did not, and got one extra leading space (caught by a test; the row blames the task). | +| T56 | When a task uses an external crate, say that the task's table of its API is complete and that the crate's source (the Cargo registry) is outside the repository and off limits; better, put the signatures needed into `files/`. | M5c-1 tasks 10, 11 and 13 each tried to read `roncho`'s source in `~/.cargo/registry` (`find / … roncho`); only the harness stopped them. | +| T57 | Split a task that combines skeleton fills with more than about fifteen before/after edits over four or more files. | M5c-1: tasks of one skeleton or only listed edits took 6 to 14 minutes; 09 (18), 15 (20, 17 test files and about 30 edits) and 13 (25, two skeletons and ten edits) were the slow ones. | +| T58 | For a class ceiling or trust rule in a spec, list every path that writes to the egress it guards (turns, mirrored memory writes, undo, review accept, notes) and give each path a test at the forbidden class. A rule written for one path is not checked on the others by default. | M5c-1 review finding 1: the `secret` ceiling was built and tested for turns only; memory writes from a `secret` session were mirrored to the trusted and fleet workspaces (the reference had it too). Finding 2 is the same shape for undo. | ## What worked and should be kept @@ -142,3 +151,10 @@ wrote about the code, not the code: two log rows describe work not in the diff, 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). + +M5c-1 was Laguna (Ornith-1.5-35B-A3B) through the driver, fifteen tasks in about 3.5 hours with no +intervention, every task ending cleanly. Its code matched the reference in all but six files, each +difference an equivalent rewrite. Its friction was the plan's: an undefined test count (T50), a +forbidden `git switch` in every task (T51), and reaching for a crate's source the task did not +give (T56). The milestone's serious defect was the spec's reach, not the code's: a class ceiling +applied to turns and not to memory writes (T58), in the reference too. diff --git a/docs/implementer-log.md b/docs/implementer-log.md index e275061..885c51a 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -876,3 +876,39 @@ fourteen tasks, one of them 23 edits long. It wrote 16 small helpers of a Markdo their comments; the one real slip (finding 1) is a rule the tests did not pin. Its own `Out::wrap` wrote the `\n` of a `\r\n` twice; it fixed that when the resume prompt named it. Task 07's session proved a failing `brokerd` test flaky before re-running the gate instead of changing it. + +### M5c-1, tasks 01 to 15 — reviewed 2026-09-28 by the design model (Claude) + +Accepted, with fixes made in review (below). Tasks 01 to 15 by Ornith-1.5-35B-A3B through +`tools/run-plan.sh`, run on talos with no intervention, 2026-09-28 10:11 to 13:42 (about 3.5 +hours). The branch differs from the reference (`~/boxmaker-refs/m5c1-ref.bundle` on straylight, +`c1345e4`) in six files of `memoryd::honcho` only (`client.rs`, `config.rs`, `ledger.rs`, +`mirror.rs`, `report.rs`, `worker.rs`), each difference an equivalent rewrite; `config.rs`'s +`check_url` and `check_keys` were checked by hand on an empty port, a second colon and a key that +is neither an own nor a read key. The device check (spec, section 13, the M5c-1 part) is still to +do. + +| Check | Result | +|---|---| +| 15 task commits, each with `Co-Authored-By: ornith-1.5-35b-a3b ` | pass | +| Runbook and dependency files identical to the reference | pass | +| Log rows | 15, all `done`, first gate pass in every row; counts in rows 10 and 12 lower than the runs in the transcripts (I17) | +| `make gate` on talos (rustc 1.98.1) | `gate: ok`, 1345 tests at task 15, the plan's count; 1364 after the review's six fixes | +| Independent review of `git diff master m5c1 -- crates` against the spec | six findings, below; sound: the turn decision, the ledger, the lock order, keys and bodies never logged, `state.sock`'s reach, the crate and file-size rules | + +| # | Severity | Where | Finding | Resolution | +|---|---|---|---|---| +| 1 | high | plan (tasks 12, 13; the reference too) | Memory writes from a `secret` session were mirrored: `brokerd`'s built-in memory grant allows writes at `secret`, and neither the notes (`Noted.class`) nor the journal (no class at all) were checked, so a clean secret session's note became a trusted conclusion and an untrusted one a fleet message. Breaks spec section 3 ("nothing above `private` is written"). | Review `b392af9`: the journal records each call's class; a secret write sends only its deletions and one Skip; `tests/honcho_mirror_secret.rs`; tip T58 | +| 2 | medium | plan (task 12) | Undoing a `consolidate` concluded each moved entry again, so a later removal deleted the new conclusion and left the first in Honcho for good. | Review `7c207f6`: an undo concludes again only an entry whose latest decided conclusion was deleted (`honcho/concluded.rs`), which also keeps an undo from concluding a secret write's entries; `tests/honcho_mirror_undo.rs` | +| 3 | low | plan (task 12) | A memory write never woke the worker, so an undo or accept waited out the 30 s idle sleep; the spec says "at once". | Review `3dd95c5`: `tests/honcho_wake_serve.rs` | +| 4 | low | plan (tasks 11, 14) | `bxctl memory sync` showed neither a refused key nor an outage, which spec sections 10 and 13 require. | Review `3d266df`: a shared leaf-locked `Status` (refused keys, Honcho outage, `brokerd` unanswered, log unreadable) in the report; every Honcho runbook entry now says what sync shows; `tests/honcho_status.rs` | +| 5 | low | plan (task 08) | One invisible character inside a key split it for the secret rules (masked to a space), so the turn was sent. | Review `bd4cf9f`: the rules also run on the text with those characters deleted; `tests/honcho_decide_hidden.rs` | +| 6 | low | plan (task 11) | Any error reading a session log marked the turn failed for good, a passing I/O error included. | Review `5e57cd6`: an error other than NotFound is waited out with its own backoff and status line; a missing or damaged log still fails; `tests/honcho_worker_log.rs` | +| 7 | low | tests (M4a) | gatewayd's `brokerd_closing_first_ends_it_as_no_longer_pending` fails 2 of 10 runs on a loaded talos without any M5c-1 change: the handler can hold the answer's receiver after `Ended`. loopd's `pointers` failed once under load in task 15. | Open; listed in the plan README; tip T52 | +| 8 | low | Ornith (task 12) | A "before" block did not match after its own earlier edit reordered imports; it anchored on its own line and logged no deviation. | Noted; AGENTS.md rule; tip T54 | +| 9 | nit | plan (every task) | Every task said to run `git switch m5c1`, which AGENTS.md forbids and the harness refused. The gate count in every task's stop list did not say it includes the ignored `end_to_end` test, which cost tasks 03, 13 and 15 much of their time. | Tips T50, T51; AGENTS.md now defines the count | + +What was good: every given file and every whole edit arrived exactly as written, fifteen tasks in +a row with no stop, including task 15's thirty edits over seventeen test files. Where Ornith filled +bodies from comments, the code agreed with the reference in behaviour everywhere checked. All six +findings were in the plan, and the reference had each of them.