diff --git a/docs/implementer-lessons.md b/docs/implementer-lessons.md index ec84b1c..e199ea1 100644 --- a/docs/implementer-lessons.md +++ b/docs/implementer-lessons.md @@ -67,6 +67,8 @@ How it is used: | T29 | Check a skeleton's comments, not only its code. Print every `todo!()` beside its signature and read each pair before hand-over. Filling the bodies from your own code proves the split can pass; it cannot show that a comment sits on the wrong function. Edit comments by function, never by "the next comment after this name". And tell the implementer to stop and quote a comment that does not fit. | M4a task 14, second session: `post`'s comment was `now_ms`'s text, put there by a script; Ornith saw the contradiction and deliberated over it until the turn ran out. M4a task 16: a comment naming the function it replaced made the task's own `grep` fail; and a given test copied into the plan before `cargo fmt` came back reformatted. Run the task's checks, and `cargo fmt`, on the end state before hand-over. | +| T30 | Every behaviour a task writes must be reachable by that task's own tests. When a function's effect shows only after a later task (here: a refusal's reason, which only task 15 could send), give a unit test in the first task, or write that function in the later one. And never let one name mean two things in a comment: say which `reason`. | M4b task 14 dropped a refusal's reason, reading `Refused -> Ended::Refused { by, reason }` as the `DenyReason` in scope; its tests passed, and task 15's caught it, in a file task 15 could not touch. | + ## What worked and should be kept - Byte-exact fixtures, compared in both directions. No wire-format defect reached review. diff --git a/docs/plans/M4b/15-gatewayd-answers.md b/docs/plans/M4b/15-gatewayd-answers.md index 9b6ec88..757402d 100644 --- a/docs/plans/M4b/15-gatewayd-answers.md +++ b/docs/plans/M4b/15-gatewayd-answers.md @@ -24,7 +24,8 @@ The last three pieces (M4b spec, section 7): pending here"), `crates/gatewayd/tests/support/gateway.rs` (`Tuning` has `notice`) - Modify: `crates/gatewayd/src/approvals/mod.rs`, `crates/gatewayd/src/deliver.rs`, `crates/gatewayd/src/serve/mod.rs`, `crates/gatewayd/src/serve/handle.rs`, - `crates/gatewayd/src/sessions.rs`, `docs/implementer-log.md` + `crates/gatewayd/src/serve/approvals.rs` (one fix, below), `crates/gatewayd/src/sessions.rs`, + `docs/implementer-log.md` ## The changes, exactly @@ -128,6 +129,32 @@ parameter of `Gateway::new`, which `run` passes as `tuning.notice`. **`sessions.rs`**: delete `pub const M4B_COMMAND`. +**`serve/approvals.rs`, a fix to task 14's `ended`.** `ended` has two different values that were +both called `reason`: the refusal's text, in `Ending::Answered { reason, .. }`, and the +`DenyReason` in `GatewayOutcome::Denied { reason }`. Task 14's comment did not say which went +where, and its tests could not send a refusal, so the refusal's text was dropped (`reason: _`, and +`Ended::Refused { by, reason: None }`). This task's `refused_with_a_reason_the_post_says_so` shows +it. Name the refusal's text `refusal`: + +```rust + Ending::Answered { + by, + reason: refusal, + outcome, + } => { +``` + +and in the `match outcome` inside it, the `Refused` arm becomes: + +```rust + GatewayOutcome::Refused {} => Ended::Refused { + by, + reason: refusal, + }, +``` + +The `Denied { reason }` arm stays as it is: that `reason` is the `DenyReason`. + ## Steps - [ ] **1. Copy.** `git switch m4b`, then diff --git a/docs/plans/M4b/README.md b/docs/plans/M4b/README.md index 21db49f..cefbb52 100644 --- a/docs/plans/M4b/README.md +++ b/docs/plans/M4b/README.md @@ -65,6 +65,20 @@ signature (tip T29). At the end of task 15: about 845 tests (768 before task 01). +## Changes during the run + +- 2026-09-24, task 15: tasks 01 to 14 committed, each on its first session. Task 15's session made + every change the task asks for, and one given test still failed: a refusal's reason was missing + from the post. It traced the cause to task 14's `ended` in `serve/approvals.rs`, a file task 15 + did not list, and stopped instead of editing it, which was right. The fault was the plan's: in + `ended`, the refusal's text and a `DenyReason` were both called `reason`, and the comment said + `Refused -> Ended::Refused { by, reason }` without saying which; and task 14's own tests could + not send a refusal, so nothing there could catch it. Task 14 took the `DenyReason` reading, then + silenced the unused-variable warning on the other with `reason: _` (and said so in its log). + Task 15 now lists `serve/approvals.rs` with the exact fix. Checked in a scratch copy: the saved + attempt (`.state/runs/M4b/15-first-attempt*`) plus that fix passes every suite three times and + the gate, 845 tests. Resume from task 15. + ## Running it ```sh