diff --git a/hive-matrix-mcp/src/accounts.rs b/hive-matrix-mcp/src/accounts.rs index f6be4ca6..9b2a2bf0 100644 --- a/hive-matrix-mcp/src/accounts.rs +++ b/hive-matrix-mcp/src/accounts.rs @@ -11,9 +11,11 @@ //! `hyperhive.matrixAccounts`) and are appended after `main`. //! //! So a single-account agent (no `HIVE_MATRIX_ACCOUNTS`) gets exactly -//! `main` — zero config, same behaviour as before. The **primary** is -//! `main`: it is selected when a tool call omits `account`, so callers -//! never specify one for the hive account. +//! `main` — zero config, same behaviour as before: omitting `account` on +//! a tool call resolves to `main`. When **multiple** accounts are +//! configured, omitting `account` is rejected with the list of names (see +//! `Registry::resolve` / `pick_name`) — the agent can't tell more than one +//! account exists, so it must choose explicitly. use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -202,6 +204,30 @@ pub struct Registry { by_name: HashMap>, } +/// Pick which account name a request resolves to, or an error. Pure (no +/// `Client`) so the ambiguity rule is unit-testable. `names` is the sorted +/// set of configured account names. +/// +/// - `Some(name)` → that name (membership is checked by the caller). +/// - `None` with a single account → the primary (zero-config default). +/// - `None` with multiple accounts → an error: the agent can't tell more +/// than one account exists, so force an explicit `account` and list the +/// choices. +fn pick_name<'a>( + account: Option<&'a str>, + primary: &'a str, + names: &[&str], +) -> Result<&'a str, String> { + match account { + Some(name) => Ok(name), + None if names.len() <= 1 => Ok(primary), + None => Err(format!( + "multiple matrix accounts configured; pass `account` explicitly — available: [{}]", + names.join(", ") + )), + } +} + impl Registry { /// Build an empty registry whose primary is `primary`. Clients are /// added with [`Registry::insert`] as each account restores. @@ -250,23 +276,28 @@ impl Registry { out } - /// Resolve a request's `account` to a client. `None` → the primary - /// account. Returns a human-readable error (listing known accounts) - /// when the name is unknown — surfaced to the agent as a tool error. + /// Resolve a request's `account` to a client. `None` selects the + /// primary account *only when it's unambiguous* — a single configured + /// account. With multiple accounts, omitting `account` is an error + /// (see `pick_name`): the agent can't tell more than one exists, so we + /// force an explicit choice and list the names. Returns a + /// human-readable error (listing known accounts) surfaced to the agent + /// as a tool error. /// /// # Errors /// - /// Errors when the named account (or the primary, if `None`) has no - /// live client — e.g. an unknown name, or the primary failed to + /// Errors when `account` is omitted but multiple accounts are + /// configured, or when the named account (or the primary, if `None`) + /// has no live client — e.g. an unknown name, or the primary failed to /// restore at startup. pub fn resolve(&self, account: Option<&str>) -> Result<&Arc, String> { - let name = account.unwrap_or(&self.primary); + let mut names: Vec<&str> = self.by_name.keys().map(String::as_str).collect(); + names.sort_unstable(); + let name = pick_name(account, &self.primary, &names)?; self.by_name.get(name).ok_or_else(|| { - let mut known: Vec<&str> = self.by_name.keys().map(String::as_str).collect(); - known.sort_unstable(); format!( "unknown matrix account {name:?}; available accounts: [{}]", - known.join(", ") + names.join(", ") ) }) } @@ -361,7 +392,28 @@ fn write_accounts_snapshot_inner( #[cfg(test)] mod tests { - use super::{AccountStatus, heartbeat_accounts_snapshot, write_accounts_snapshot}; + use super::{AccountStatus, heartbeat_accounts_snapshot, pick_name, write_accounts_snapshot}; + + #[test] + fn pick_name_single_account_defaults_to_primary() { + assert_eq!(pick_name(None, "main", &["main"]).unwrap(), "main"); + } + + #[test] + fn pick_name_multi_account_omitted_errors_with_menu() { + let err = pick_name(None, "main", &["main", "public"]).unwrap_err(); + assert!(err.contains("pass `account` explicitly")); + assert!(err.contains("main")); + assert!(err.contains("public")); + } + + #[test] + fn pick_name_explicit_passes_through_even_with_multiple() { + assert_eq!( + pick_name(Some("public"), "main", &["main", "public"]).unwrap(), + "public" + ); + } fn sample() -> Vec { vec![