From 6745a85b2486a93cf98b1c117ba2c1f8afc3cd87 Mon Sep 17 00:00:00 2001 From: James Maes Date: Wed, 29 Jul 2026 21:42:54 -0500 Subject: [PATCH] feat(mail): select an explicit account for SendMessage SendMessage previously always sent from the first configured account with no way to choose, so it silently used the wrong identity the moment a second account existed. Add an optional account_id field to SendMessageRequest; when set, resolve that specific account and fail with a new AccountNotFound error if it does not exist, rather than falling back to a different account. Omitting account_id preserves the prior first-account behavior. Thread the selector through ComposeRequest, resolve_sending_account, the CLI's mail send --account flag, and regenerate the nuncio.v1 descriptor golden. Refs #196, #170 --- crates/nuncio-cli/src/args.rs | 4 + crates/nuncio-cli/src/runner.rs | 42 +++++- .../proto/nuncio/v1/descriptor.bin | Bin 12734 -> 12785 bytes .../nuncio-proto/proto/nuncio/v1/nuncio.proto | 10 +- crates/nunciod/src/grpc.rs | 4 + crates/nunciod/src/send.rs | 128 +++++++++++++++--- crates/nunciod/tests/spine_e2e_test.rs | 1 + 7 files changed, 170 insertions(+), 19 deletions(-) diff --git a/crates/nuncio-cli/src/args.rs b/crates/nuncio-cli/src/args.rs index 96bba71..77a3314 100644 --- a/crates/nuncio-cli/src/args.rs +++ b/crates/nuncio-cli/src/args.rs @@ -308,6 +308,10 @@ pub enum MailSubcommand { /// Message body text content. #[arg(short, long, help = "Message body text content")] body: String, + /// Identifier of the account to send from (defaults to the first + /// configured account when omitted). + #[arg(short, long, help = "Identifier of the account to send from")] + account: Option, }, /// Full-text search across all cached email messages. Search { diff --git a/crates/nuncio-cli/src/runner.rs b/crates/nuncio-cli/src/runner.rs index 68c34f3..e1fbb93 100644 --- a/crates/nuncio-cli/src/runner.rs +++ b/crates/nuncio-cli/src/runner.rs @@ -322,8 +322,14 @@ impl HeadlessRunner { MailSubcommand::Sync => self.handle_sync(json_mode).await, MailSubcommand::List { folder } => self.handle_list_folder(folder, json_mode).await, MailSubcommand::Read { id } => self.handle_read_message(id, json_mode).await, - MailSubcommand::Send { to, subject, body } => { - self.handle_send_email(to, subject, body, json_mode).await + MailSubcommand::Send { + to, + subject, + body, + account, + } => { + self.handle_send_email(to, subject, body, account.as_deref(), json_mode) + .await } MailSubcommand::Search { query } => self.handle_search(query, json_mode).await, MailSubcommand::Mark { id, read, unread } => { @@ -695,6 +701,7 @@ impl HeadlessRunner { to: &str, subject: &str, body: &str, + account: Option<&str>, json_mode: bool, ) -> String { let mut client = match self.connect_mail_client().await { @@ -710,6 +717,7 @@ impl HeadlessRunner { body_text: body.to_string(), body_html: None, attachments: Vec::new(), + account_id: account.map(str::to_string), }) .await { @@ -3165,6 +3173,7 @@ mod tests { to: "alice@nuncio.mx".to_string(), subject: "Quarterly Roadmap".to_string(), body: "Let's discuss the roadmap.".to_string(), + account: None, }, }, true, @@ -3182,6 +3191,35 @@ mod tests { assert_eq!(recorded_send.to, "alice@nuncio.mx"); assert_eq!(recorded_send.subject, "Quarterly Roadmap"); assert_eq!(recorded_send.body_text, "Let's discuss the roadmap."); + assert_eq!(recorded_send.account_id, None); + + // `mail send --account ` threads the explicit selector through + // to the daemon over the wire, proving the CLI flag is not merely + // parsed but actually reaches `SendMessageRequest.account_id`. + let mail_send_with_account = runner + .execute_command( + &Commands::Mail { + action: MailSubcommand::Send { + to: "alice@nuncio.mx".to_string(), + subject: "Quarterly Roadmap".to_string(), + body: "Let's discuss the roadmap.".to_string(), + account: Some("acct-work".to_string()), + }, + }, + true, + ) + .await; + assert!(mail_send_with_account.contains(r#""sent":true"#)); + + let recorded_send_with_account = send_probe + .lock() + .unwrap_or_else(|e| e.into_inner()) + .clone() + .expect("stub daemon received a second send_message request"); + assert_eq!( + recorded_send_with_account.account_id, + Some("acct-work".to_string()) + ); } /// Reference-client proof: boots a stub `nuncio.v1.Calendar` gRPC server diff --git a/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin b/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin index 230409000f736815e6c458655140d33b16e9f95d..609afaa324768328d9cf368d61bb8edd7bc550b8 100644 GIT binary patch delta 96 zcmdm&{4tq{>)l4CJbtFhOp}ZFPkRU{ad9OkC+C;umBeSJNU$p~YH)fm1#v>fJX1Ot n8J$?UnB$X^op`vo;*;`ID&sRsa&w$`xwzvIIyVOi, /// File attachments. pub attachments: Vec, + /// Identifier of the account to send from. `None` preserves the prior + /// "first configured account" behavior. + pub account_id: Option, } /// Send `message` through `sender`. Pure seam shared by production and @@ -121,14 +131,24 @@ fn build_smtp_sender( } /// Resolves the account to send mail from. See the module-level "Account -/// selection" doc above for why this is currently the first configured -/// account rather than an explicit per-request choice. -async fn resolve_sending_account(db: &DatabaseEngine) -> Result { +/// selection" doc above: an explicit `account_id` must match a configured +/// account or the request fails; `None` preserves the "first configured +/// account" behavior. +async fn resolve_sending_account( + db: &DatabaseEngine, + account_id: Option<&str>, +) -> Result { let accounts = db.list_accounts().await?; - accounts - .into_iter() - .next() - .ok_or(SendError::NoAccountConfigured) + match account_id { + Some(id) => accounts + .into_iter() + .find(|a| a.id == id) + .ok_or_else(|| SendError::AccountNotFound(id.to_string())), + None => accounts + .into_iter() + .next() + .ok_or(SendError::NoAccountConfigured), + } } /// Generates a daemon-local identifier for a sent message, for reference in @@ -154,7 +174,7 @@ pub async fn send_message_for_account( secrets: &SecretManager, request: ComposeRequest, ) -> Result { - let config = resolve_sending_account(db).await?; + let config = resolve_sending_account(db, request.account_id.as_deref()).await?; let password = secrets.get_secret(&config.keyring_secret_key)?; let sender = build_smtp_sender(&config, &password)?; @@ -186,7 +206,7 @@ pub async fn send_message_with_injected_sender( sender: &dyn MessageSender, request: ComposeRequest, ) -> Result { - let config = resolve_sending_account(db).await?; + let config = resolve_sending_account(db, request.account_id.as_deref()).await?; let outbound = OutboundMessage { from: config.email_address, @@ -286,6 +306,7 @@ mod tests { body_text: Some("Body".to_string()), body_html: None, attachments: Vec::new(), + account_id: None, }, ) .await @@ -314,6 +335,7 @@ mod tests { body_text: Some("Body".to_string()), body_html: None, attachments: Vec::new(), + account_id: None, }, ) .await @@ -352,6 +374,7 @@ mod tests { body_text: Some("Body".to_string()), body_html: None, attachments: Vec::new(), + account_id: None, }, ) .await @@ -392,6 +415,7 @@ mod tests { body_text: Some("Body via injected sender.".to_string()), body_html: None, attachments: Vec::new(), + account_id: None, }, ) .await @@ -423,6 +447,7 @@ mod tests { body_text: Some("Body".to_string()), body_html: None, attachments: Vec::new(), + account_id: None, }, ) .await @@ -430,4 +455,75 @@ mod tests { assert!(matches!(err, SendError::NoAccountConfigured)); assert!(mock.sent_messages().is_empty()); } + + /// Proves that with multiple configured accounts, an explicit + /// `account_id` selects that SPECIFIC account as the sender -- not + /// whichever account happens to be first -- by asserting the captured + /// outbound message's `From:` matches the requested account. + #[tokio::test] + async fn send_message_with_injected_sender_uses_the_explicitly_selected_account() { + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("ephemeral db"); + + let first = sample_account("acct-first"); + let second = sample_account("acct-second"); + db.save_account(&first).await.expect("save first account"); + db.save_account(&second).await.expect("save second account"); + + let mock = MockMessageSender::new(); + send_message_with_injected_sender( + &db, + &mock, + ComposeRequest { + to: "bob@nuncio.mx".to_string(), + cc: None, + subject: "Hi".to_string(), + body_text: Some("Body".to_string()), + body_html: None, + attachments: Vec::new(), + account_id: Some(second.id.clone()), + }, + ) + .await + .expect("send succeeds via explicitly selected account"); + + let sent = mock.sent_messages(); + assert_eq!(sent.len(), 1); + assert_eq!(sent[0].from, second.email_address); + assert_ne!(sent[0].from, first.email_address); + } + + /// An `account_id` naming an account that does not exist must fail with + /// the specific [`SendError::AccountNotFound`] variant -- never a silent + /// fallback to the first configured account, and never confusable with + /// [`SendError::NoAccountConfigured`] (which means zero accounts exist). + #[tokio::test] + async fn send_message_with_injected_sender_rejects_unknown_account_id() { + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("ephemeral db"); + + let config = sample_account("acct-only"); + db.save_account(&config).await.expect("save account"); + + let mock = MockMessageSender::new(); + let err = send_message_with_injected_sender( + &db, + &mock, + ComposeRequest { + to: "bob@nuncio.mx".to_string(), + cc: None, + subject: "Hi".to_string(), + body_text: Some("Body".to_string()), + body_html: None, + attachments: Vec::new(), + account_id: Some("acct-does-not-exist".to_string()), + }, + ) + .await + .expect_err("unknown account id must fail"); + assert!(matches!(err, SendError::AccountNotFound(id) if id == "acct-does-not-exist")); + assert!(mock.sent_messages().is_empty()); + } } diff --git a/crates/nunciod/tests/spine_e2e_test.rs b/crates/nunciod/tests/spine_e2e_test.rs index f4d6253..4a2c844 100644 --- a/crates/nunciod/tests/spine_e2e_test.rs +++ b/crates/nunciod/tests/spine_e2e_test.rs @@ -305,6 +305,7 @@ async fn full_mail_spine_round_trips_over_authenticated_grpc_with_mocked_protoco body_text: "Let's discuss the roadmap.".to_string(), body_html: None, attachments: Vec::new(), + account_id: None, }) .await .expect("send_message succeeds") -- 2.54.0