kyleandClaude Opus 5.5
af7137dafe
Task 15's session found the defect in serve/approvals.rs, a file it did not list, and stopped. The comment in task 14 used one name for two values; task 14's tests could not reach it. Lesson T30. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
90 lines
6.2 KiB
Markdown
90 lines
6.2 KiB
Markdown
# M4b implementation plan: approvals over Mattermost
|
|
|
|
> **For the implementing model:** do not work from this file. The owner gives you one task file at
|
|
> a time (`01-…` to `15-…`). This file is the index for the owner and the reviewer.
|
|
|
|
**Goal:** an `ask` call is posted in Mattermost, in the thread that asked for it (or, for a session
|
|
not in Mattermost, in the first approver's direct messages), and an approver answers it there with
|
|
`!approve <id>` or `!deny <id> [reason]`. The answer is recorded exactly as `bxctl`'s is, naming
|
|
the approver and the post; the post then shows what came of it. Nobody else can answer, and no
|
|
post is left looking answerable when it is not.
|
|
|
|
**Architecture:** `proto` gains the shared escaping (01) and the four `gateway.sock` messages (02);
|
|
`bxctl` gains `deny` (03). `brokerd` records the answering post (04), reads `[gateway]` (05), and
|
|
gets the **courier**, which carries each pending approval to `gatewayd` and its answer back (06).
|
|
`gatewayd` reads `[approvals]` (07), strips a leading mention of itself and recognises approval
|
|
commands (08), can edit a post and look up a name (09), renders an approval's posts (10), keeps
|
|
the approvals it posted and judges commands (11), records their posts in the state file (12),
|
|
listens on `gateway.sock` (13), posts and marks approvals from its serve loop (14), and wires the
|
|
commands, the typing pause and the fallback notice (15).
|
|
|
|
**Spec:** `docs/specs/2026-09-24-m4b-approvals.md` (approved; P16 applied to the brief). Runbook:
|
|
task 06 adds `gateway-unreachable`, task 07 widens `gatewayd-start-failed`.
|
|
|
|
**No task needs Mattermost, Podman or a network.** Everything runs against fakes: a fake `gatewayd`
|
|
for `brokerd`, and for `gatewayd` a fake Mattermost, a fake `loopd` and a fake `brokerd` client.
|
|
|
|
## Global constraints
|
|
|
|
- Everything in `AGENTS.md`, including "Lessons from earlier reviews".
|
|
- No new dependency.
|
|
- Branch `m4b`. One task, one fresh OpenCode session, one commit. Run `cargo fmt --all` before the
|
|
gate. Review happens once, after task 15.
|
|
- **New files** come as skeletons: types, constants and the glue written, `todo!()` bodies with the
|
|
code to write in the comment above. **Existing files** are changed by the exact edits in the
|
|
task, never replaced whole: a whole-file copy would overwrite an earlier task's work.
|
|
- Some tasks hand over **new versions of given test files**, because the change is in what the
|
|
test must see (a new field, a new text). Copy them over the old ones as the task says; they are
|
|
still not to be edited.
|
|
|
|
## Tasks
|
|
|
|
Every task had a reference implementation, built in this order from master with the gate at every
|
|
step (tip T26), its given tests run against it, and mutations of it caught by them (tip T28); then
|
|
the reference was moved off this machine (tip T18). Every skeleton was checked to compile at its
|
|
task's end state and fail its tests, and every comment above a `todo!()` was read beside its
|
|
signature (tip T29).
|
|
|
|
| # | File | Delivers | Tests | Check |
|
|
|---|---|---|---|---|
|
|
| 01 | `01-proto-escape.md` | `proto::escape`, moved from `bxctl` | `proto/tests/escape.rs` | reference; `bxctl` unchanged |
|
|
| 02 | `02-proto-gateway-wire.md` | the four `gateway.sock` messages | `gateway_wire.rs`, 8 fixtures | reference |
|
|
| 03 | `03-bxctl-deny.md` | `bxctl deny` | `cli_deny.rs` | reference |
|
|
| 04 | `04-brokerd-answer-post.md` | answers record the post; `Table::contains` | `ledger_post.rs`; 3 given tests updated | reference |
|
|
| 05 | `05-brokerd-config-gateway.md` | `[gateway]` | `config_gateway.rs`; `rig.rs` updated | reference |
|
|
| 06 | `06-brokerd-courier.md` | the courier | `courier.rs`, `support/fake_gateway.rs` | reference; 10 mutations caught; no test hangs |
|
|
| 07 | `07-gatewayd-config-approvals.md` | `[approvals]` | `config_approvals.rs` | reference |
|
|
| 08 | `08-gatewayd-mention-commands.md` | the leading mention; `Route::Approval` | `sessions_mention.rs`; `sessions.rs`, `serve.rs` updated | reference; 7 mutations, 5 caught, 2 that cannot change behaviour |
|
|
| 09 | `09-gatewayd-mm-edit.md` | `patch_post`, `username` | `mm_edit.rs` | reference; the bot's edit checked on the owner's server |
|
|
| 10 | `10-gatewayd-approval-text.md` | the posts' text | `approvals_text.rs` | reference; 7 mutations caught |
|
|
| 11 | `11-gatewayd-approvals-registry.md` | the registry, `judge` | `approvals_judge.rs` | reference; 6 mutations caught |
|
|
| 12 | `12-gatewayd-state-approvals.md` | approval posts in `state.json`; `turn_of` | `state_approvals.rs` | reference |
|
|
| 13 | `13-gatewayd-gateway-sock.md` | the listener and connection threads | `gateway_sock.rs` | reference; 6 mutations, 5 caught, 1 that cannot change behaviour |
|
|
| 14 | `14-gatewayd-serve-approvals.md` | posting, marking, the restart, the listener in `run` | `serve_approvals.rs`, `support/brokerd_client.rs`; `fake_mm.rs` updated | reference; 5 mutations caught |
|
|
| 15 | `15-gatewayd-answers.md` | commands judged, typing paused, the notice | `serve_answers.rs`, `deliver_notice.rs`; `deliver.rs`, `serve.rs`, `support/gateway.rs` updated | reference; 4 mutations caught |
|
|
|
|
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
|
|
git switch m4b
|
|
BOXMAKER_MODEL=straylight/ornith-1.5-35b-a3b tools/run-plan.sh docs/plans/M4b
|
|
```
|
|
|
|
Keep the OpenCode TUI closed while it runs.
|