# M5c-1 task 03: `gatewayd` sets a turn's speaker **Branch:** `m5c1` (run `git switch m5c1`; `git status --short` must be empty, otherwise stop) **Commit subject:** `gatewayd: a turn's speaker, the username of the one person who wrote its burst` **Log row:** the Task cell is `M5c1/03-gatewayd-speaker`. ## Goal Task 01 gave `Turn` a field `speaker: Option`, and task 02 made `loopd` log it for `memoryd`, which will send a turn to Honcho only when it has a speaker (M5c spec, sections 4 and 11). `gatewayd` still sends `speaker: None` on every turn. This task sets it. `gatewayd` joins the messages that arrive while a turn runs into one turn, a **burst**: their texts joined with a blank line. The speaker is the **Mattermost username** of the person who wrote the turn, and only when one person wrote all of it: | The turn's messages | `Turn.speaker` | |---|---| | one message, from Kyle (username `kyle`) | `Some("kyle")` | | a burst, every message from Kyle | `Some("kyle")` | | a burst from Eve and Kyle | `None`: two people | | Kyle's messages, but Mattermost does not answer the username lookup | `None`, and a log line | `None` is the safe side: a turn without a speaker is never sent to Honcho (the owner's rule: no speaker, or incomplete speaker information, is not synced). **Never** put anything else there: not the user id, not the first message's author, not a name from an earlier turn. 1. **Each message knows its author.** `sessions::Message` gains `user`, the post's `user_id`, set by `Router::route`. 2. **Each batch knows the author of each of its messages.** `Batch` gains `senders`, one user id per message, in order; `Queues` keeps them beside the waiting texts. `Batch::sender` gives the one user who wrote them all, or `None`. 3. **The username.** `gatewayd` already looks usernames up for approvals: `Client::username`, `GET /api/v4/users/{user}`. The event loop now keeps each name it found in a map, so a user is looked up once; a failure is logged and not kept. 4. **The turn.** `deliver` gets the speaker as a new argument and puts it on the `Turn`. **The turn's text does not change**: the join stays exactly as it is, and the speaker never goes into the text. ## Every place that changes This is the complete list. | Place | What happens there | |---|---| | `crates/gatewayd/src/sessions.rs`, `Message` and `Router::route` | new field `user`, set on both `Route::Queue` paths (edits S1 to S3) | | `crates/gatewayd/src/sessions.rs`, `Batch`, `Pending`, `Queues` | new `senders`, `Batch::sender` (edits S4 to S8) | | `crates/gatewayd/src/deliver.rs`, `one_turn`, `run_turn`, `deliver` | a new parameter `speaker: Option<&str>`, put on the `Turn` (edits D1 to D5) | | `crates/gatewayd/src/serve/mod.rs`, `Gateway` | new field `usernames` (edits G1, G2) | | `crates/gatewayd/src/serve/handle.rs`, `start` | new method `speaker`; passes it to `deliver` (edits H1, H2) | | `crates/gatewayd/src/serve/approvals.rs` | **no change**: it keeps its own lookups | | `crates/gatewayd/tests/sessions_mention.rs` | reads `text` only; no change | | tests that build a `Batch` or a `Message`, or call `deliver` | new versions are given | ## Files - Copy, from `docs/plans/M5c1/files/` (the same path under it): - new: `crates/gatewayd/tests/deliver_speaker.rs`, `crates/gatewayd/tests/serve_speaker.rs` - **new versions** of (they replace the files in the tree): `crates/gatewayd/tests/sessions.rs`, `deliver.rs`, `deliver_links.rs`, `deliver_notice.rs` and `crates/gatewayd/tests/support/fake_mm.rs` (it gains `refuse_users`). - Modify: `crates/gatewayd/src/sessions.rs`, `crates/gatewayd/src/deliver.rs`, `crates/gatewayd/src/serve/mod.rs`, `crates/gatewayd/src/serve/handle.rs`, `docs/implementer-log.md`. - Do not touch: any other source file (`serve/approvals.rs` and `mm/rest.rs` in particular), any file of another crate, any `Cargo.toml`. ## Facts you need - **Where the author is known.** A Mattermost post carries `user_id`, a 26-character id, and `Router::route` sees the whole `Post`. `route` makes a `Route::Queue(Message { .. })` in two places: for a message that starts with `!!`, and for any other message. Both get `user`. - **Where the texts are joined.** `Queues::push` starts a turn at once for an idle session: the batch holds that one message, so its `senders` is that message's author alone. While a turn runs, each new message waits in `Pending::waiting`; its author goes into `Pending::senders`, at the same place. A message refused by a full queue is in neither. `Queues::finish` makes the next batch from all that waited, and `std::mem::take` leaves `senders` empty for the batch after it, as `waiting.clear()` does for the texts. - **The username.** `Client::username` (in `mm/rest.rs`, unchanged) sends `GET /api/v4/users/{user}` and checks what comes back: the id must be a Mattermost id and the username 1 to 64 characters of `a-z`, `0-9`, `.`, `-` and `_`; anything else is an error. Pass the name on **as it comes**: `memoryd` turns it into a Honcho peer name in a later task (a `.` becomes `_` there, not here). - **The map.** `Gateway` belongs to the event loop's thread, and `start` runs on it, so a plain `HashMap` needs no lock. The lookup is a REST call on that thread before the turn's own thread starts, as the approvals' `place` does; with the map it is made once per user. A failed lookup is not stored, so the next turn of that user looks again. - **The log line** for a failed lookup is exactly this, and holds no text of the message: ```text gatewayd: no username for : ; the turn has no speaker ``` With the fake Mattermost refusing the lookup, the test expects this one line (`MmError`'s `Display` quotes the body): ```text gatewayd: no username for k0000000000000000000000000: status 404: "{\"message\":\"not found\"}"; the turn has no speaker ``` - **The fake Mattermost** answers `GET /api/v4/users/` with the username `kyle` for Kyle's id and `user-` plus the id's first four characters for anyone else (`user-e000` for Eve). The new `refuse_users(true)` makes it answer 404 instead; `users/me` is not affected. - **The speaker never reaches the model.** `loopd` logs `Turn.speaker` on the turn's `User` record and builds the request from the text alone (task 02). So the text `gatewayd` sends is the batch's `text` as before, with or without a speaker. - The calls the edits use: | Call | Signature, and what it gives | |---|---| | `slice::first` | `self.senders.first() -> Option<&String>` | | `Iterator::all` | `self.senders.iter().all(\|user\| user == first) -> bool` | | `std::mem::take` | `std::mem::take(&mut pending.senders) -> Vec`, leaving an empty `Vec` | | `HashMap::get`, `insert` | `self.usernames.get(user) -> Option<&String>`; `insert(String, String)` | | `Client::username` | `pub fn username(&self, user: &str) -> Result` (unchanged) | | `Option::map` | `speaker.map(str::to_string)`: `Option<&str>` to `Option` | | `Option::as_deref` | `speaker.as_deref()`: `Option` to `Option<&str>` | | `Batch::sender` | `pub fn sender(&self) -> Option<&str>` (new, S4) | | `Gateway::speaker` | `fn speaker(&mut self, batch: &Batch) -> Option` (new, H2) | | `run_turn` | `pub fn run_turn(socket: &Path, batch: &Batch, speaker: Option<&str>, on_event: &mut dyn FnMut(&TurnEvent)) -> Outcome` (D3) | | `deliver` | `pub fn deliver(poster: &dyn Poster, socket: &Path, batch: &Batch, speaker: Option<&str>, log: &dyn Fn(&str), notice: &Notice)` (D4) | ## The edits, exactly Nothing else in these files changes. Each quoted "before" block occurs exactly once in its file. ### `crates/gatewayd/src/sessions.rs` **S1.** In `struct Message`, the lines ```rust /// A thread in a channel or group message that this Boxmaker now takes part in. pub joins_thread: bool, } ``` become ```rust /// A thread in a channel or group message that this Boxmaker now takes part in. pub joins_thread: bool, /// Who wrote it: the post's `user_id`, a Mattermost user id. pub user: String, } ``` **S2.** In `Router::route`, the lines ```rust text: format!("!{rest}"), joins_thread: channel_type != "D", }); ``` become ```rust text: format!("!{rest}"), joins_thread: channel_type != "D", user: post.user_id.clone(), }); ``` **S3.** At the end of `Router::route`, the lines ```rust text: message.to_string(), joins_thread: channel_type != "D", }) ``` become ```rust text: message.to_string(), joins_thread: channel_type != "D", user: post.user_id.clone(), }) ``` **S4.** The lines ```rust pub struct Batch { pub session: SessionId, pub thread: Thread, pub resume: bool, pub text: String, } ``` become ```rust pub struct Batch { pub session: SessionId, pub thread: Thread, pub resume: bool, pub text: String, /// Who wrote each message of `text`, in order: Mattermost user ids. pub senders: Vec, } impl Batch { /// The one user who wrote every message of the batch. `None` when two or more people did, or /// when it names nobody: a burst joined from several people has no speaker (M5c spec, /// section 4). pub fn sender(&self) -> Option<&str> { let first = self.senders.first()?; if self.senders.iter().all(|user| user == first) { Some(first.as_str()) } else { None } } } ``` **S5.** The lines ```rust struct Pending { thread: Thread, waiting: Vec, } ``` become ```rust struct Pending { thread: Thread, waiting: Vec, /// The author of each message in `waiting`, in the same order. senders: Vec, } ``` **S6.** In `Queues::push`, the lines ```rust pending.waiting.push(message.text.clone()); return Pushed::Waiting; ``` become ```rust pending.waiting.push(message.text.clone()); pending.senders.push(message.user.clone()); return Pushed::Waiting; ``` **S7.** In `Queues::push`, the lines ```rust text: message.text.clone(), }; self.running.insert( message.session, Pending { thread: message.thread.clone(), waiting: Vec::new(), }, ); ``` become ```rust text: message.text.clone(), senders: vec![message.user.clone()], }; self.running.insert( message.session, Pending { thread: message.thread.clone(), waiting: Vec::new(), senders: Vec::new(), }, ); ``` **S8.** In `Queues::finish`, the lines ```rust text: pending.waiting.join("\n\n"), }; ``` become ```rust text: pending.waiting.join("\n\n"), senders: std::mem::take(&mut pending.senders), }; ``` ### `crates/gatewayd/src/deliver.rs` **D1.** The lines ```rust batch: &Batch, resume: bool, on_event: &mut dyn FnMut(&TurnEvent), ) -> Outcome { ``` become ```rust batch: &Batch, resume: bool, speaker: Option<&str>, on_event: &mut dyn FnMut(&TurnEvent), ) -> Outcome { ``` **D2.** In `one_turn`, the lines ```rust // Set in M5c-1 task 03. Until then no turn has a speaker, and none is synced. speaker: None, ``` become ```rust // On the turn only, never in its text: `memoryd` reads it from the log, the model never. speaker: speaker.map(str::to_string), ``` **D3.** The lines ```rust /// A turn for a batch. A reply in a thread `loopd` does not know creates the session, as /// `bxctl chat --session` does. pub fn run_turn(socket: &Path, batch: &Batch, on_event: &mut dyn FnMut(&TurnEvent)) -> Outcome { // `one_turn` with batch.resume. When that is Refused with NoSuchSession and batch.resume was // true, one more `one_turn` with resume false. Otherwise the first outcome. match one_turn(socket, batch, batch.resume, on_event) { Outcome::Refused(err) if err.code == ErrorCode::NoSuchSession && batch.resume => { one_turn(socket, batch, false, on_event) } ``` become ```rust /// A turn for a batch, spoken by `speaker` (a Mattermost username) when one person wrote all of /// it. A reply in a thread `loopd` does not know creates the session, as `bxctl chat --session` /// does. pub fn run_turn( socket: &Path, batch: &Batch, speaker: Option<&str>, on_event: &mut dyn FnMut(&TurnEvent), ) -> Outcome { // `one_turn` with batch.resume. When that is Refused with NoSuchSession and batch.resume was // true, one more `one_turn` with resume false. Otherwise the first outcome. match one_turn(socket, batch, batch.resume, speaker, on_event) { Outcome::Refused(err) if err.code == ErrorCode::NoSuchSession && batch.resume => { one_turn(socket, batch, false, speaker, on_event) } ``` **D4.** The lines ```rust /// Run a batch's turn and post what comes of it in its thread. A post that fails is logged. pub fn deliver( poster: &dyn Poster, socket: &Path, batch: &Batch, log: &dyn Fn(&str), ``` become ```rust /// Run a batch's turn, spoken by `speaker`, and post what comes of it in its thread. A post that /// fails is logged. pub fn deliver( poster: &dyn Poster, socket: &Path, batch: &Batch, speaker: Option<&str>, log: &dyn Fn(&str), ``` **D5.** In `deliver`, the line ```rust let outcome = run_turn(socket, batch, &mut |event| { ``` becomes ```rust let outcome = run_turn(socket, batch, speaker, &mut |event| { ``` ### `crates/gatewayd/src/serve/mod.rs` **G1.** At the end of `struct Gateway`, the lines ```rust /// How long a turn waits for its approval to come through `gateway.sock` before the notice. notice: Duration, } ``` become ```rust /// How long a turn waits for its approval to come through `gateway.sock` before the notice. notice: Duration, /// Mattermost usernames by user id, each looked up once (M5c spec, section 11). usernames: HashMap, } ``` **G2.** In `Gateway::new`, the lines ```rust askers: HashMap::new(), notice, } ``` become ```rust askers: HashMap::new(), notice, usernames: HashMap::new(), } ``` `HashMap` is imported at the top of the file already. ### `crates/gatewayd/src/serve/handle.rs` **H1.** In `start`, the lines ```rust let (session, thread) = (batch.session.clone(), batch.thread.clone()); let spawned = std::thread::Builder::new().spawn(move || { let posted = |id: u64| registry.was_posted(id); let notice = Notice { posted: &posted, wait, }; deliver(client.as_ref(), &socket, &batch, &|line| log(line), ¬ice); ``` become ```rust let (session, thread) = (batch.session.clone(), batch.thread.clone()); let speaker = self.speaker(&batch); let spawned = std::thread::Builder::new().spawn(move || { let posted = |id: u64| registry.was_posted(id); let notice = Notice { posted: &posted, wait, }; deliver( client.as_ref(), &socket, &batch, speaker.as_deref(), &|line| log(line), ¬ice, ); ``` **H2.** The lines ```rust /// Turns that ended: out of flight, who asked forgotten, and the next batch of each session /// started. Never blocks. ``` become ```rust /// The turn's speaker: the username of the one person who wrote all of the batch, as /// Mattermost gives it (M5c spec, section 11). `None` when two or more people did, or when /// the name cannot be found: never the user id, never a guess. A name found is kept; a /// failure is logged and not kept, so the next turn looks again. fn speaker(&mut self, batch: &Batch) -> Option { let user = batch.sender()?; if let Some(name) = self.usernames.get(user) { return Some(name.clone()); } match self.client.username(user) { Ok(name) => { self.usernames.insert(user.to_string(), name.clone()); Some(name) } Err(e) => { (self.log)(&format!( "gatewayd: no username for {user}: {e}; the turn has no speaker" )); None } } } /// Turns that ended: out of flight, who asked forgotten, and the next batch of each session /// started. Never blocks. ``` `Batch` is imported at the top of the file already. ## Steps - [ ] **1. Copy the given files.** From the repository root, seven commands: ```sh cp docs/plans/M5c1/files/crates/gatewayd/tests/deliver_speaker.rs crates/gatewayd/tests/ cp docs/plans/M5c1/files/crates/gatewayd/tests/serve_speaker.rs crates/gatewayd/tests/ cp docs/plans/M5c1/files/crates/gatewayd/tests/sessions.rs crates/gatewayd/tests/ cp docs/plans/M5c1/files/crates/gatewayd/tests/deliver.rs crates/gatewayd/tests/ cp docs/plans/M5c1/files/crates/gatewayd/tests/deliver_links.rs crates/gatewayd/tests/ cp docs/plans/M5c1/files/crates/gatewayd/tests/deliver_notice.rs crates/gatewayd/tests/ cp docs/plans/M5c1/files/crates/gatewayd/tests/support/fake_mm.rs \ crates/gatewayd/tests/support/ ``` Then `git status --short` lists 5 modified files and 2 new ones, all under `crates/gatewayd/tests/`. - [ ] **2. See them fail.** Two commands: - `cargo build --tests --keep-going -p gatewayd`: five test targets do not compile: `deliver`, `deliver_links`, `deliver_notice`, `deliver_speaker` and `sessions`. Every error is about a name this task adds or changes: `E0560` (``struct `Batch` has no field named `senders` `` or ``struct `gatewayd::sessions::Message` has no field named `user` ``), `E0609` (``no field `user` on type `gatewayd::sessions::Message` `` or ``no field `senders` on type `&Batch` ``), `E0599` (``no method named `sender` found for struct `Batch` ``), `E0061` (``this function takes 5 arguments but 6 arguments were supplied``, for `deliver`) or, once, `E0282` (``type annotations needed``, in `deliver.rs`, on the closure passed to `deliver`). - `cargo test -p gatewayd --test serve_speaker`: it compiles, and its four tests fail. Three fail on an assertion that finds `None` where a speaker was expected (`one_persons_message_has_their_username`, `a_username_is_looked_up_once`, `bursts_have_a_speaker_only_when_one_person_wrote_them`); the fourth, `a_name_that_cannot_be_found_leaves_the_turn_without_a_speaker`, with `no log line with "no username for"`. - [ ] **3. `sessions.rs`.** Make edits S1 to S8. `cargo check -p gatewayd`: no error. Then `cargo test -p gatewayd --test sessions`: 11 passed. - [ ] **4. `deliver.rs`.** Make edits D1 to D5. `cargo check -p gatewayd` now fails in `serve/handle.rs` only, at the call of `deliver`: `E0061` (``this function takes 6 arguments but 5 arguments were supplied``) and `E0277` (``expected an `Fn(&str)` closure, found `Notice<'_>` ``). The notes under them also point into `deliver.rs`, at `deliver`'s definition; that is where the call is compared, not a second error. - [ ] **5. `serve`.** Make edits G1, G2, H1 and H2. `cargo check -p gatewayd`: no error and no warning. - [ ] **6. See them pass.** `cargo test -p gatewayd --test deliver_speaker --test serve_speaker`: 3 and 4 passed. Then `cargo test -p gatewayd`: every suite passes. The new tests: | Test | What it checks | |---|---| | `a_queued_message_names_its_author` (`sessions.rs`) | S2, S3: `user` on both kinds of queued message | | `a_batch_carries_the_author_of_each_message` (`sessions.rs`) | S4 to S8: one author, two authors, and nothing carried to the next batch | | `a_batch_with_no_authors_has_no_sender` (`sessions.rs`) | S4: no authors, and a last one that differs | | `the_speaker_goes_on_the_turn_and_not_into_its_text` | D1, D2, D4, D5: the speaker on the `Turn`, the text unchanged | | `without_a_speaker_the_turn_has_none` | D2: `None` stays `None` | | `the_turn_sent_again_for_an_unknown_session_keeps_the_speaker` | D3: both turns carry it | | `one_persons_message_has_their_username` | H1, H2: `kyle`, not the id | | `a_username_is_looked_up_once` | G1, H2: one `GET /api/v4/users/…` for two turns | | `bursts_have_a_speaker_only_when_one_person_wrote_them` | end to end: `kyle`, then `None`, then `user-e000` | | `a_name_that_cannot_be_found_leaves_the_turn_without_a_speaker` | H2: `None`, the log line, and a later lookup | `a_full_queue_drops_the_message` in `sessions.rs` also checks that a refused message's author is not kept. - [ ] **7. Check.** Three commands: - `grep -rn 'speaker: None' crates/gatewayd/src` prints nothing: the placeholder is gone. - `git diff --stat -- crates/gatewayd/src` lists exactly `deliver.rs`, `serve/handle.rs`, `serve/mod.rs` and `sessions.rs`. - `git diff --stat -- crates/bxcore crates/loopd crates/memoryd crates/brokerd crates/bxctl` prints nothing. - [ ] **8. Run the gate.** `cargo fmt --all`, then `make gate`, with a shell timeout of 600000 ms. Expected last line: `gate: ok`, with 1148 tests: 1138 before this task, plus 10 new ones (three in `sessions.rs`, three in `deliver_speaker.rs`, four in `serve_speaker.rs`; the new versions of the other files keep their counts). - [ ] **9. Log and commit.** Add your row to `docs/implementer-log.md`; its Task cell is `M5c1/03-gatewayd-speaker`. `git status --short` lists the four sources, the five replaced test files and your log as modified, and `deliver_speaker.rs` and `serve_speaker.rs` as new. Stage with `git add crates/gatewayd docs/implementer-log.md Cargo.lock`, then commit as `AGENTS.md` says. `Cargo.lock` does not change in this task; the `git add` is harmless. ## Done when - `make gate` prints `gate: ok` with 1148 tests. - `git diff HEAD~1 --stat` lists only the four source files of "Modify", the seven copied files and `docs/implementer-log.md`. ## Stop and report if - A "before" block of any edit is not in its file exactly as shown, or is there more than once. - Step 2 shows a different result: other test targets, an error of another kind, or other failures in `serve_speaker`. - Step 4's `cargo check` fails anywhere other than the call of `deliver` in `serve/handle.rs`, or with other errors. - After step 3, 5 or 6, a test fails. Do not change a test: stop, and give the failing tests' names and their messages. - The fix seems to need a change anywhere else: in `serve/approvals.rs`, `mm/rest.rs`, another crate, a change to how the texts are joined, or a speaker when the name cannot be found. It does not; stop and say why you think so. - A test in `inferproxy`, `toolkit` or `memoryd` fails in the gate: check once whether it fails without your change, then stop and report both results. - The gate passes with a count other than 1148. - A comment in an edit does not fit the code under it. Quote it and stop.