M5: SendMessage selects an explicit account instead of the first configured #196

Closed
opened 2026-07-28 14:04:38 +00:00 by KofTwentyTwo · 1 comment
KofTwentyTwo commented 2026-07-28 14:04:38 +00:00 (Migrated from github.com)

Context

Story for milestone M5. Read docs/STORY-WORKFLOW.md first — it defines the process, gates, and patterns; this issue defines only the specific work.

nuncio.v1.Mail.SendMessage (crates/nuncio-proto/proto/nuncio/v1/nuncio.proto:384-397) has no account_id field on SendMessageRequest at all. Server-side, nunciod::send::resolve_sending_account (crates/nunciod/src/send.rs:122-131) always does db.list_accounts().await?.into_iter().next() — it sends from whichever account happens to be first in list_accounts' result, with no way for a caller to choose. This is explicitly documented as a known limitation in the module doc comment (crates/nunciod/src/send.rs:13-21, "Account selection" section: "SendMessage sends from the first account returned by DatabaseEngine::list_accounts... Supporting an explicit multi-account choice is future work"). This is fine with exactly one configured account, but silently wrong (sends from the wrong identity) the moment a second account exists. Fixes #170.

Goal

Add an explicit account selector to SendMessage so a caller with multiple configured accounts can choose which one to send from, and make the daemon return an honest, specific error (not a silent wrong-account send, not a generic error that could be confused with "no account configured") when the requested account does not exist. Preserve today's behavior (send from the sole/first account) only when no account is specified and exactly one account is very much still the deliberate default for backward compatibility — but the request must be able to disambiguate when more than one account exists.

Files to touch / patterns to mirror

  • crates/nuncio-proto/proto/nuncio/v1/nuncio.proto — add optional string account_id = 7; to SendMessageRequest (line 384-397; next unused field number after attachments = 6). Update the SendMessage RPC's doc comment (line 246-254) to describe account selection. Regen the descriptor golden per the recipe in crates/nuncio-proto/src/lib.rs.
  • crates/nunciod/src/send.rs — change resolve_sending_account (line 122-131) to resolve_sending_account(db: &DatabaseEngine, account_id: Option<&str>) -> Result<AccountConfig, SendError>: when Some(id), look up that specific account (mirror nunciod::sync::find_account, crates/nunciod/src/sync.rs:110-119) and return a new SendError variant (e.g. AccountNotFound(String), distinct from the existing NoAccountConfigured at line 61-63) if it does not exist; when None, keep today's "first configured account" behavior for backward compatibility. Thread the new parameter through ComposeRequest (line 79-93) or as a separate argument to send_message_for_account/send_message_with_injected_sender (line 151-172, 183-202) — whichever keeps ComposeRequest's existing shape (it currently models exactly the RPC payload minus From:) most honest; since account_id becomes part of the RPC payload, adding it to ComposeRequest is the more consistent choice.
  • crates/nunciod/src/grpc.rs — MailGrpcService::send_message (~line 608-644) passes req.account_id through to the updated ComposeRequest/send_message_for_account call.
  • crates/nuncio-cli/src/args.rs — MailSubcommand::Send (line 269-279) gains an optional --account <id> flag.
  • crates/nuncio-cli/src/runner.rs — handle_send_email (line 764-810ish) passes the new flag through to SendMessageRequest.account_id.

Implementation steps

  1. Add account_id to SendMessageRequest; regen the golden; confirm contract_stability passes.
  2. Add the AccountNotFound (or similarly named) variant to nunciod::send::SendError; update resolve_sending_account to accept Option<&str> and look up a specific account when given one, returning the new error variant (not NoAccountConfigured, which should remain reserved for "zero accounts exist at all") when the id does not match any configured account.
  3. Thread the selector through ComposeRequest, send_message_for_account, and send_message_with_injected_sender; update every existing call site and test in crates/nunciod/src/send.rs's #[cfg(test)] mod tests (line 204+) to pass account_id: None (preserving current behavior) except new tests exercising explicit selection.
  4. Update MailGrpcService::send_message in grpc.rs to pass req.account_id through; add/adjust tests in crates/nunciod/src/grpc.rs's test module (search for valid_send_message_request, ~line 1650-2320) covering: sending with an explicit account_id that matches a second configured account (proving the right account's From:/SMTP endpoint was used, e.g. via MailEngineOverrides::message_sender and MockMessageSender as in send_message_with_injected_sender_captures_exact_outbound_message, line 2444+); sending with an account_id that does not exist (must fail with the new honest error, not silently fall back to the first account).
  5. Add the --account CLI flag; update handle_send_email and its tests.

Acceptance criteria

  • SendMessageRequest gains optional string account_id; contract golden regenerated; nuncio-proto's contract_stability test passes.
  • With two configured accounts and an explicit account_id, SendMessage sends from that account (proven via MockMessageSender/MailEngineOverrides::message_sender, asserting the captured outbound message's From: matches the requested account's email_address).
  • With an account_id naming an account that does not exist, SendMessage fails with an honest, specific error distinct from "no account configured" — never a silent fallback to the first account.
  • With account_id omitted and exactly one account configured, behavior is unchanged from today (sends from that account) — existing tests covering this path still pass.
  • Mail service remains mounted behind its own BearerAuthInterceptor (no service-mounting change in this story, but confirm no regression).
  • nuncio-cli mail send --account <id> ... exercises the new selector end to end against a mock daemon.
  • Offline E2E (wiremock / Mock*Backend) proves the capability end to end; no live network.
  • cargo verify green (fmt + clippy -D warnings all-targets + test-all), 0 ignored.

Definition of done & process

Follow docs/STORY-WORKFLOW.md: no fabricated data; comment convention (intent-only, no story/GH/phase refs in .rs/.proto); mock-only tests. Branch story/m5-s3-send-account-selection off dev; PR to dev with the gate output pasted as evidence; the PO reviews before the issue is closed.

Dependencies / notes

[PROTO] — touches nuncio.proto (additive-only: one new optional field on an existing message). Confirm with the PO that no other proto-touching story (e.g. M5-S1) is in flight before starting; if both are queued, whichever lands first should regenerate the golden and the second rebases on top of it rather than both racing to regenerate independently.

No relevant prior-art branch for this specific story. Rough size: S.

Fixes #170.

## Context Story for milestone M5. **Read [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md) first** — it defines the process, gates, and patterns; this issue defines only the specific work. `nuncio.v1.Mail.SendMessage` (`crates/nuncio-proto/proto/nuncio/v1/nuncio.proto:384-397`) has no `account_id` field on `SendMessageRequest` at all. Server-side, `nunciod::send::resolve_sending_account` (`crates/nunciod/src/send.rs:122-131`) always does `db.list_accounts().await?.into_iter().next()` — it sends from whichever account happens to be first in `list_accounts`' result, with no way for a caller to choose. This is explicitly documented as a known limitation in the module doc comment (`crates/nunciod/src/send.rs:13-21`, "Account selection" section: "`SendMessage` sends from the first account returned by `DatabaseEngine::list_accounts`... Supporting an explicit multi-account choice is future work"). This is fine with exactly one configured account, but silently wrong (sends from the wrong identity) the moment a second account exists. Fixes #170. ## Goal Add an explicit account selector to `SendMessage` so a caller with multiple configured accounts can choose which one to send from, and make the daemon return an honest, specific error (not a silent wrong-account send, not a generic error that could be confused with "no account configured") when the requested account does not exist. Preserve today's behavior (send from the sole/first account) only when no account is specified and exactly one account is very much still the deliberate default for backward compatibility — but the request must be able to disambiguate when more than one account exists. ## Files to touch / patterns to mirror - `crates/nuncio-proto/proto/nuncio/v1/nuncio.proto` — add `optional string account_id = 7;` to `SendMessageRequest` (line 384-397; next unused field number after `attachments = 6`). Update the `SendMessage` RPC's doc comment (line 246-254) to describe account selection. Regen the descriptor golden per the recipe in `crates/nuncio-proto/src/lib.rs`. - `crates/nunciod/src/send.rs` — change `resolve_sending_account` (line 122-131) to `resolve_sending_account(db: &DatabaseEngine, account_id: Option<&str>) -> Result<AccountConfig, SendError>`: when `Some(id)`, look up that specific account (mirror `nunciod::sync::find_account`, `crates/nunciod/src/sync.rs:110-119`) and return a new `SendError` variant (e.g. `AccountNotFound(String)`, distinct from the existing `NoAccountConfigured` at line 61-63) if it does not exist; when `None`, keep today's "first configured account" behavior for backward compatibility. Thread the new parameter through `ComposeRequest` (line 79-93) or as a separate argument to `send_message_for_account`/`send_message_with_injected_sender` (line 151-172, 183-202) — whichever keeps `ComposeRequest`'s existing shape (it currently models exactly the RPC payload minus `From:`) most honest; since `account_id` becomes part of the RPC payload, adding it to `ComposeRequest` is the more consistent choice. - `crates/nunciod/src/grpc.rs` — `MailGrpcService::send_message` (~line 608-644) passes `req.account_id` through to the updated `ComposeRequest`/`send_message_for_account` call. - `crates/nuncio-cli/src/args.rs` — `MailSubcommand::Send` (line 269-279) gains an optional `--account <id>` flag. - `crates/nuncio-cli/src/runner.rs` — `handle_send_email` (line 764-810ish) passes the new flag through to `SendMessageRequest.account_id`. ## Implementation steps 1. Add `account_id` to `SendMessageRequest`; regen the golden; confirm `contract_stability` passes. 2. Add the `AccountNotFound` (or similarly named) variant to `nunciod::send::SendError`; update `resolve_sending_account` to accept `Option<&str>` and look up a specific account when given one, returning the new error variant (not `NoAccountConfigured`, which should remain reserved for "zero accounts exist at all") when the id does not match any configured account. 3. Thread the selector through `ComposeRequest`, `send_message_for_account`, and `send_message_with_injected_sender`; update every existing call site and test in `crates/nunciod/src/send.rs`'s `#[cfg(test)] mod tests` (line 204+) to pass `account_id: None` (preserving current behavior) except new tests exercising explicit selection. 4. Update `MailGrpcService::send_message` in `grpc.rs` to pass `req.account_id` through; add/adjust tests in `crates/nunciod/src/grpc.rs`'s test module (search for `valid_send_message_request`, ~line 1650-2320) covering: sending with an explicit `account_id` that matches a second configured account (proving the right account's `From:`/SMTP endpoint was used, e.g. via `MailEngineOverrides::message_sender` and `MockMessageSender` as in `send_message_with_injected_sender_captures_exact_outbound_message`, line 2444+); sending with an `account_id` that does not exist (must fail with the new honest error, not silently fall back to the first account). 5. Add the `--account` CLI flag; update `handle_send_email` and its tests. ## Acceptance criteria - [ ] `SendMessageRequest` gains `optional string account_id`; contract golden regenerated; `nuncio-proto`'s `contract_stability` test passes. - [ ] With two configured accounts and an explicit `account_id`, `SendMessage` sends from that account (proven via `MockMessageSender`/`MailEngineOverrides::message_sender`, asserting the captured outbound message's `From:` matches the requested account's `email_address`). - [ ] With an `account_id` naming an account that does not exist, `SendMessage` fails with an honest, specific error distinct from "no account configured" — never a silent fallback to the first account. - [ ] With `account_id` omitted and exactly one account configured, behavior is unchanged from today (sends from that account) — existing tests covering this path still pass. - [ ] `Mail` service remains mounted behind its own `BearerAuthInterceptor` (no service-mounting change in this story, but confirm no regression). - [ ] `nuncio-cli mail send --account <id> ...` exercises the new selector end to end against a mock daemon. - [ ] Offline E2E (wiremock / Mock*Backend) proves the capability end to end; no live network. - [ ] `cargo verify` green (fmt + clippy `-D warnings` all-targets + test-all), 0 ignored. ## Definition of done & process Follow [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md): no fabricated data; comment convention (intent-only, no story/GH/phase refs in `.rs`/`.proto`); mock-only tests. Branch `story/m5-s3-send-account-selection` off `dev`; PR to `dev` with the gate output pasted as evidence; the PO reviews before the issue is closed. ## Dependencies / notes **[PROTO]** — touches `nuncio.proto` (additive-only: one new `optional` field on an existing message). Confirm with the PO that no other proto-touching story (e.g. M5-S1) is in flight before starting; if both are queued, whichever lands first should regenerate the golden and the second rebases on top of it rather than both racing to regenerate independently. No relevant prior-art branch for this specific story. Rough size: S. Fixes #170.
KofTwentyTwo commented 2026-07-30 02:45:18 +00:00 (Migrated from github.com)

Delivered via PR #232 (squash-merged to dev). SendMessageRequest gains optional account_id (additive; golden regenerated); resolve_sending_account uses it to pick the specific account, returning the new honest SendError::AccountNotFound (distinct from NoAccountConfigured) when it doesn't exist — never a silent fallback to the first account. account_id omitted preserves prior first-account behavior. CLI mail send --account . Fixes #170. Closes out milestone M5.

Delivered via PR #232 (squash-merged to dev). SendMessageRequest gains optional account_id (additive; golden regenerated); resolve_sending_account uses it to pick the specific account, returning the new honest SendError::AccountNotFound (distinct from NoAccountConfigured) when it doesn't exist — never a silent fallback to the first account. account_id omitted preserves prior first-account behavior. CLI mail send --account <id>. Fixes #170. Closes out milestone M5.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#196