Files
boxmaker/docs/implementer-log.md
T
2026-09-17 09:29:48 -07:00

61 lines
6.3 KiB
Markdown

# Implementer log
Kept by the implementing model, one row per task. The column meanings are in `AGENTS.md`. The
reviewer adds findings under "Reviews" once per milestone.
| Task | Date | Status | Gate runs | First gate | Deviations | Notes |
|---|---|---|---|---|---|---|
| M1/01-workspace-and-gate | 2026-09-17 | done | 1 | pass | none | Crate skeletons, Cargo files and the given Makefile/deny.toml/test-gate-scripts.sh were already present untracked from a prior attempt; I verified them against the plan and created only the missing gate scripts, dependencies.md, egress.md and this log row. |
| M1/02-proto-values | 2026-09-17 | done | 1 | pass | none | Implemented ValueError, SessionId, Epoch, CallId, Hash32 and Timestamp in crates/proto/src/ids.rs and DataClass in class.rs, using serde try_from/into for string-backed JSON validation, a hand-written hex encoder and humantime for RFC 3339 parsing with canonical re-serialization. |
| M1/03-proto-wire | 2026-09-17 | done | 2 | pass | none | Added Envelope, Message, WireError, ErrorCode, ToolRequest, ToolResponse and DenyReason in crates/proto/src/wire.rs, re-exported from lib.rs; all 9 fixture tests pass and `make gate` prints `gate: ok`. |
| M1/04-proto-frame | 2026-09-17 | done | 2 | fail | none | Added crates/proto/src/frame.rs (MAX_FRAME, FrameError, write_frame, read_frame) re-exported from lib.rs; 13 fixture tests pass. Two compile fixes: mapped read_bytes io::Error to FrameError::Io and annotated serde_json::from_slice::<Envelope>; cargo-fmt reordered the lib.rs re-export lines; `make gate` prints `gate: ok`. |
| M1/05-proto-grant | 2026-09-17 | done | 2 | fail | none | Added crates/proto/src/grant.rs (Mode, Constraints with Default, Grant with serde defaults + deny_unknown_fields) re-exported from lib.rs and toml 1.1.6 as a proto dev-dependency (workspace dep + dependencies.md row); 4 fixture tests pass. cargo-fmt reordered the lib.rs re-exports before the gate. |
| M1/06-proto-records | 2026-09-17 | done | 2 | fail | none | Added crates/proto/src/audit.rs (DecisionRecord, AuditRecord) and crates/proto/src/log.rs (ToolCall, LogRecord) re-exported from lib.rs; 3 fixture tests pass, 40 total across the five proto test files. cargo-fmt reordered the lib.rs re-exports before the gate. |
| M1/07-brokerd-decision | 2026-09-17 | done | 1 | pass | none | Added Decision (Debug only, private fields), decide (Err(NoGrant) until M3) and the run stub (ToolResponse::Failed) in crates/brokerd; Decision::new carries expect(dead_code). 2 unit + 3 doctests (2 compile_fail) pass; verified the compile_fail guards by temporarily making new pub. `make gate` prints `gate: ok`. |
| M1/08-proto-strictness | 2026-09-17 | done | 1 | pass | none | Added deny_unknown_fields to AuditRecord and ToolCall in crates/proto; bounded Timestamp (MAX const, from_unix_millis -> Result, parse bounds via from_unix_millis, now clamps to MAX) in ids.rs. 45 proto tests pass; `cargo fmt --all` and `make gate` print `gate: ok`. |
## Reviews
### M1, tasks 01 to 07 — reviewed 2026-09-17 by the design model (Claude)
**Verdict: accepted, with two follow-up tasks (08, 09).** Nothing has to be redone.
Checklist from `docs/plans/M1/README.md`:
| Check | Result |
|---|---|
| Seven commits on `m1`, one per task, each with the `Implemented-By` trailer | pass |
| All 23 copied files (tests, fixtures, `Makefile`, `deny.toml`, self-test) byte-identical to the plan | pass |
| No change to the brief, specs, plans, `AGENTS.md`, `CLAUDE.md`; working tree clean; nothing pushed | pass |
| `make gate` | `gate: ok`, 45 tests |
| `make audit` | `advisories ok` |
| No `unwrap`, `expect`, `panic!`, `#[allow]` or `unsafe` in library code; no dependency the tasks did not name | pass |
| Field order, derives and signatures match the tasks | pass |
Process: first gate run passed in 4 of 7 tasks. The three failures were two compile fixes (task 04)
and rustfmt reordering `lib.rs` re-exports (tasks 04 to 06). Commits run from 06:45 to 09:05.
Findings. "Implementer" means the task said it and the code missed it. "Task" means the task or its
tests, written by the reviewer, were wrong or silent; the reference implementation had the same
defect in both such cases.
| # | Severity | Owner | Finding | Fix |
|---|---|---|---|---|
| 1 | medium | implementer, and a gap in the given tests | `AuditRecord` and `ToolCall` lack `deny_unknown_fields`. An audit line with an extra `"forged":true` field decodes. The given tests only checked the enums. | Task 08; new `tests/strict.rs` checks every object at every depth |
| 2 | medium | task | `Timestamp::from_unix_millis` accepts any `u64`, but `to_rfc3339` and serialization panic above year 9999, because `humantime`'s `Display` returns an error and `to_string()` panics on that. | Task 08 |
| 3 | medium | task | `[dependencies.brokerd]` with `workspace = true` lets a role depend on another role while both dependency scripts pass. Dotted `brokerd.path = …` is also missed. The self-test had no such case. | Task 09 |
| 4 | low | implementer | All three scripts pass when `ROOT/crates` is missing, and hide tool errors with `2>/dev/null`. A gate check that cannot look must fail. | Task 09 |
| 5 | low | implementer | `check-lines.sh` does not print the line count. | Task 09 |
| 6 | nit | implementer | `frame.rs` uses bounded `as` casts where `try_from` would say the same without a second look. `read_bytes` has two match arms that do the same thing. `Constraints` has a needless `rename_all`. `ids.rs`, `class.rs`, `wire.rs` have no module doc comment. | Not worth a task; fix when next touched |
| 7 | low | task | The tasks told the implementer how to order `lib.rs` lines, and rustfmt disagreed, which cost three gate runs. | `AGENTS.md` now says to run `cargo fmt --all` before the gate |
Open question for the owner: the task 01 row says the skeleton files were "already present
untracked from a prior attempt". The log has no row for that attempt, so its gate runs and the
reason it ended are not recorded.
On the experiment (review once per milestone): it held up for M1. None of the defects was built on
by a later task, and all were found by reading the branch and probing it from outside. M1 is the
easy case, though: types pinned by byte-exact fixtures. M2 has behaviour that fixtures cannot pin
as tightly (a streaming HTTP client, the turn loop), so an early mistake there is more likely to
be built on.