docs: the M5c-1 review, tips T50 to T58, and AGENTS.md rules from the run
The review of tasks 01 to 15 in the implementer log, with the six findings fixed in review. Nine tips for writing tasks from the run's transcripts and the review, new evidence for I16 and I17, and four rules in AGENTS.md: the Cargo registry is outside the repository, `git stash -u` and the README's flaky tests, an edit never anchors on another line, and what a test count includes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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 <agent@wntrmute.dev>` | 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.
|
||||
|
||||
Reference in New Issue
Block a user