boxmaker/docs/plans/M5c1/03-gatewayd-speaker.md

# 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<String>`, 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 <user id>: <the MmError>; 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/<id>` 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<String>`, 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<String, MmError>` (unchanged) |
  | `Option::map` | `speaker.map(str::to_string)`: `Option<&str>` to `Option<String>` |
  | `Option::as_deref` | `speaker.as_deref()`: `Option<String>` to `Option<&str>` |
  | `Batch::sender` | `pub fn sender(&self) -> Option<&str>` (new, S4) |
  | `Gateway::speaker` | `fn speaker(&mut self, batch: &Batch) -> Option<String>` (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<String>,
}

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<String>,
}
```

become

```rust
struct Pending {
    thread: Thread,
    waiting: Vec<String>,
    /// The author of each message in `waiting`, in the same order.
    senders: Vec<String>,
}
```

**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<String, String>,
}
```

**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), &notice);
```

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),
                &notice,
            );
```

**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<String> {
        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.