Plan M4b: approvals over Mattermost in 15 tasks
Skeletons for new files only; existing files change by exact edits, so no task overwrites an earlier one's work. Each task's tests were run against a reference built in order from master, with the gate at each step and mutations checked; each skeleton compiles and fails its tests, and every todo!() comment was read beside its signature. The reference is kept off this machine. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,133 @@
|
||||
# M4b task 06: the courier
|
||||
|
||||
**Branch:** `m4b` (run `git switch m4b`; `git status --short` must be empty, otherwise stop)
|
||||
**Commit subject:** `brokerd: courier, carrying pending approvals to gatewayd and answers back`
|
||||
|
||||
## Goal
|
||||
|
||||
When an `ask` call enters the pending table and `[gateway]` is set, a **courier** thread carries
|
||||
it to `gatewayd` over `gateway.sock` (task 02's four frames) and brings the answer back (M4b spec,
|
||||
section 5):
|
||||
|
||||
- It stops as soon as the table no longer holds the approval (answered in `bxctl`, expired, the
|
||||
requester gone), closing the connection; `gatewayd` then marks the post "no longer pending".
|
||||
- If `gatewayd` cannot be reached or cannot post, it logs **once** for this approval, with the
|
||||
pointer `see docs/runbook.md#gateway-unreachable`, and tries again every 5 seconds; when a later
|
||||
try posts it, it logs `brokerd: approval <id> posted in Mattermost`.
|
||||
- If `gatewayd` goes away after posting, it posts it again.
|
||||
- On an answer, it takes the entry out of the table and answers it with `admin::answer`, exactly
|
||||
as `bxctl approve` and `refuse` do, with the Mattermost user and post (task 04); then sends what
|
||||
came of it. If the entry was already gone, the outcome is `Gone` and nothing is written.
|
||||
|
||||
The courier runs **beside** the connection thread that waits for the verdict: a scoped thread
|
||||
(`std::thread::Builder::spawn_scoped`), stopped by a flag when the wait ends. So `ask`'s wait moves
|
||||
into a function of its own, called inside `courier::alongside`.
|
||||
|
||||
## Files
|
||||
|
||||
- Copy: the skeleton `crates/brokerd/src/courier.rs`, `crates/brokerd/tests/courier.rs`,
|
||||
`crates/brokerd/tests/support/fake_gateway.rs`
|
||||
- Append: `docs/plans/M4b/files/runbook-gateway-unreachable.md` to the end of `docs/runbook.md`
|
||||
- Modify: `crates/brokerd/src/lib.rs` (`pub mod courier;` after `pub mod config;`),
|
||||
`crates/brokerd/src/broker.rs`, `docs/implementer-log.md`
|
||||
|
||||
## The skeleton
|
||||
|
||||
Written, because they are the protocol and its glue: `RUNBOOK`, `Timing` (5 s retry, 1 s tick),
|
||||
`Attempt`, **`alongside`**, **`carry`**, **`attempt`**, **`exchange`** and `lost`. Read them first:
|
||||
they call everything you write. To fill, each a few lines with the code in its comment:
|
||||
`finished`, `pause`, `frames`, `answer`.
|
||||
|
||||
`frames` reads `gatewayd`'s frames whole on a thread of its own and hands them over a channel, so
|
||||
the courier can wait with a timeout without losing half a frame (`UnixStream::peek` is not stable).
|
||||
|
||||
## `broker.rs`, exactly
|
||||
|
||||
1. `use std::sync::mpsc::RecvTimeoutError;` becomes
|
||||
`use std::sync::mpsc::{Receiver, RecvTimeoutError};`, and add `use crate::courier;` after
|
||||
`use crate::config::Config;`.
|
||||
2. In `pending`, keep a copy of what the table lists, just before it is inserted:
|
||||
`let shown = info.clone();` on the line before `let verdict = broker.table().insert(info, ask);`.
|
||||
3. Replace step 4 of `pending`, from `// 4. Wait for the verdict, or for the peer to leave.` down
|
||||
to the end of that `let outcome = loop { … };`, with:
|
||||
|
||||
```rust
|
||||
// 4. Wait for the verdict, or for the peer to leave, with a courier carrying the approval to
|
||||
// Mattermost meanwhile when `[gateway]` is set.
|
||||
let waited = courier::alongside(broker, &shown, || {
|
||||
wait_for_verdict(broker, stream, &verdict, seq)
|
||||
});
|
||||
let outcome = match waited {
|
||||
Waited::Verdict(outcome) => outcome,
|
||||
Waited::PeerLeft => return None, // 4. the peer left while we waited
|
||||
};
|
||||
```
|
||||
|
||||
4. Add, before `pub fn alive`, the old step 4 as a function (the same branches, with `return`s
|
||||
instead of `break`s):
|
||||
|
||||
```rust
|
||||
/// What waiting for the verdict came to.
|
||||
enum Waited {
|
||||
Verdict(Result<Verdict, DenyReason>),
|
||||
PeerLeft,
|
||||
}
|
||||
|
||||
/// Wait for the verdict, or for the peer to leave. If the peer leaves and the entry is still in
|
||||
/// the table, taking it ends the approval unanswered; if someone already took it, their verdict
|
||||
/// is awaited.
|
||||
fn wait_for_verdict(
|
||||
broker: &Broker,
|
||||
stream: &UnixStream,
|
||||
verdict: &Receiver<Verdict>,
|
||||
seq: u64,
|
||||
) -> Waited {
|
||||
loop {
|
||||
match verdict.recv_timeout(Duration::from_secs(1)) {
|
||||
Ok(verdict) => return Waited::Verdict(Ok(verdict)),
|
||||
Err(RecvTimeoutError::Timeout) => {
|
||||
if alive(stream) {
|
||||
continue;
|
||||
}
|
||||
match broker.table().take(seq) {
|
||||
Some(_) => return Waited::PeerLeft,
|
||||
None => match verdict.recv() {
|
||||
Ok(verdict) => return Waited::Verdict(Ok(verdict)),
|
||||
Err(_) => return Waited::Verdict(Err(DenyReason::AuditUnavailable)),
|
||||
},
|
||||
}
|
||||
}
|
||||
// The taker dropped it unanswered.
|
||||
Err(RecvTimeoutError::Disconnected) => {
|
||||
return Waited::Verdict(Err(DenyReason::AuditUnavailable));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Steps
|
||||
|
||||
- [ ] **1. Copy.** `git switch m4b`, then
|
||||
`cp docs/plans/M4b/files/crates/brokerd/src/courier.rs crates/brokerd/src/ && cp docs/plans/M4b/files/crates/brokerd/tests/courier.rs crates/brokerd/tests/ && cp docs/plans/M4b/files/crates/brokerd/tests/support/fake_gateway.rs crates/brokerd/tests/support/`
|
||||
and `cat docs/plans/M4b/files/runbook-gateway-unreachable.md >> docs/runbook.md`. Add
|
||||
`pub mod courier;` to `lib.rs`.
|
||||
- [ ] **2. Make the four `broker.rs` changes.** `cargo build -p brokerd` must succeed.
|
||||
- [ ] **3. See it fail.** `cargo test -p brokerd --test courier`. Expected: 9 fail, 1 passes
|
||||
(`without_a_gateway_nothing_is_carried`), in about 10 s.
|
||||
- [ ] **4. Fill `finished`, `pause`, `frames`, `answer`**, `cargo check -p brokerd` after each.
|
||||
- [ ] **5. See it pass.** `cargo test -p brokerd --test courier`, three times: 10 passed each
|
||||
time, in about 10 s (two tests wait out the 5 s retry on purpose). Then `cargo test -p brokerd`:
|
||||
every suite passes.
|
||||
- [ ] **6. Run the gate.** `cargo fmt --all`, then `make gate`. Expected last line: `gate: ok`.
|
||||
- [ ] **7. Log and commit.** `git add crates/brokerd docs/runbook.md docs/implementer-log.md Cargo.lock && git commit`
|
||||
|
||||
## Done when
|
||||
|
||||
- The courier suite passes three times running; `make gate` prints `gate: ok`.
|
||||
|
||||
## Stop and report if
|
||||
|
||||
- A comment above a `todo!()` does not fit its function, or contradicts a test: quote it.
|
||||
- A written function (`alongside`, `carry`, `attempt`, `exchange`) seems to need a change.
|
||||
- A test hangs or takes much longer than stated.
|
||||
Reference in New Issue
Block a user