feat(#2039): require explicit account when multiple matrix accounts configured
This commit is contained in:
parent
e349416d1a
commit
f477a2f901
1 changed files with 65 additions and 13 deletions
|
|
@ -11,9 +11,11 @@
|
||||||
//! `hyperhive.matrixAccounts`) and are appended after `main`.
|
//! `hyperhive.matrixAccounts`) and are appended after `main`.
|
||||||
//!
|
//!
|
||||||
//! So a single-account agent (no `HIVE_MATRIX_ACCOUNTS`) gets exactly
|
//! So a single-account agent (no `HIVE_MATRIX_ACCOUNTS`) gets exactly
|
||||||
//! `main` — zero config, same behaviour as before. The **primary** is
|
//! `main` — zero config, same behaviour as before: omitting `account` on
|
||||||
//! `main`: it is selected when a tool call omits `account`, so callers
|
//! a tool call resolves to `main`. When **multiple** accounts are
|
||||||
//! never specify one for the hive account.
|
//! 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::collections::HashMap;
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
|
|
@ -202,6 +204,30 @@ pub struct Registry {
|
||||||
by_name: HashMap<String, Arc<Client>>,
|
by_name: HashMap<String, Arc<Client>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// 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 {
|
impl Registry {
|
||||||
/// Build an empty registry whose primary is `primary`. Clients are
|
/// Build an empty registry whose primary is `primary`. Clients are
|
||||||
/// added with [`Registry::insert`] as each account restores.
|
/// added with [`Registry::insert`] as each account restores.
|
||||||
|
|
@ -250,23 +276,28 @@ impl Registry {
|
||||||
out
|
out
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Resolve a request's `account` to a client. `None` → the primary
|
/// Resolve a request's `account` to a client. `None` selects the
|
||||||
/// account. Returns a human-readable error (listing known accounts)
|
/// primary account *only when it's unambiguous* — a single configured
|
||||||
/// when the name is unknown — surfaced to the agent as a tool error.
|
/// 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
|
||||||
///
|
///
|
||||||
/// Errors when the named account (or the primary, if `None`) has no
|
/// Errors when `account` is omitted but multiple accounts are
|
||||||
/// live client — e.g. an unknown name, or the primary failed to
|
/// 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.
|
/// restore at startup.
|
||||||
pub fn resolve(&self, account: Option<&str>) -> Result<&Arc<Client>, String> {
|
pub fn resolve(&self, account: Option<&str>) -> Result<&Arc<Client>, 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(|| {
|
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!(
|
format!(
|
||||||
"unknown matrix account {name:?}; available accounts: [{}]",
|
"unknown matrix account {name:?}; available accounts: [{}]",
|
||||||
known.join(", ")
|
names.join(", ")
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
@ -361,7 +392,28 @@ fn write_accounts_snapshot_inner(
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
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<AccountStatus> {
|
fn sample() -> Vec<AccountStatus> {
|
||||||
vec![
|
vec![
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue