diff --git a/desktop/src-tauri/src/broker/desktop_agents.rs b/desktop/src-tauri/src/broker/desktop_agents.rs new file mode 100644 index 0000000000..e0169d2622 --- /dev/null +++ b/desktop/src-tauri/src/broker/desktop_agents.rs @@ -0,0 +1,390 @@ +//! The Desktop-backed [`AgentService`] and [`AgentRoster`]. +//! +//! This is the only place in the broker that knows about Tauri. It translates a +//! validated capability request into the same commands the UI calls — +//! `create_managed_agent`, `update_managed_agent`, `delete_managed_agent` — so a +//! brokered mutation and a hand-driven one take one code path. Reimplementing +//! the mutation here would mean two agent-creation routines drifting apart, and +//! the broker's would be the one nobody looks at. +//! +//! # Where the credential boundary sits +//! +//! Nothing here returns secret material. `create_managed_agent` hands back a +//! `CreateManagedAgentResponse` that *does* contain the minted +//! `private_key_nsec`; [`DesktopAgentService::create`] reads only the summary +//! and drops the response, and [`AgentsCreateOutcome`] has no field that could +//! hold it. That is the narrowing point: the secret exists on one stack frame +//! inside this file and never reaches the pipeline, the store, or the wire. +//! +//! # Why the request translation is a separate pure function +//! +//! [`create_request`] and [`update_request`] are pure and tested directly. +//! Everything they decide — which optional fields become `None`, that a +//! brokered create never spawns a process — is policy worth pinning in a test, +//! and none of it needs an `AppHandle`. + +use buzz_sdk_pkg::broker::{ + AgentTarget, AgentsCreateArgs, AgentsCreateOutcome, AgentsDeleteArgs, AgentsDeleteOutcome, + AgentsUpdateArgs, AgentsUpdateOutcome, +}; +use tauri::{AppHandle, Manager}; + +use crate::app_state::AppState; +use crate::managed_agents::{ + load_managed_agents, CreateManagedAgentRequest, ManagedAgentRecord, RespondTo, + UpdateManagedAgentRequest, DEFAULT_ACP_COMMAND, +}; + +use super::agents_policy::AgentRoster; +use super::handlers::{AgentService, ServiceError}; +use super::pipeline::BoxFuture; + +/// Build the create request the Desktop command expects. +/// +/// A brokered create is deliberately *not* spawned (`spawn_after_create: +/// false`) and does not auto-start (`start_on_app_launch: false`). A remote +/// requester asking for an agent to exist has not asked for a local process to +/// be running, and starting one would consume host resources on a schedule +/// nobody at the keyboard chose. The owner starts it from the UI. +/// +/// `persona_id` and `team_id` stay `None`: linking a definition pins a snapshot +/// of someone else's config onto the new agent, which is a second decision the +/// request never made. +fn create_request(args: &AgentsCreateArgs) -> Result { + let respond_to = args + .respond_to + .as_deref() + .map(RespondTo::parse_wire) + .transpose()?; + + Ok(CreateManagedAgentRequest { + name: args.display_name.clone(), + persona_id: None, + team_id: None, + relay_url: None, + acp_command: Some(DEFAULT_ACP_COMMAND.to_string()), + // `agent_command` is the harness pin. A requested runtime is resolved to + // its command here rather than passed through raw, so an arbitrary + // string can never become an executed command. + agent_command: args + .runtime + .as_deref() + .map(resolve_runtime_command) + .transpose()?, + harness_override: args.runtime.is_some(), + agent_args: Vec::new(), + mcp_command: None, + turn_timeout_seconds: None, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: None, + system_prompt: Some(args.system_prompt.clone()), + avatar_url: None, + model: args.model.clone(), + provider: args.provider.clone(), + env_vars: std::collections::BTreeMap::new(), + spawn_after_create: false, + start_on_app_launch: false, + backend: crate::managed_agents::BackendKind::Local, + respond_to, + respond_to_allowlist: Vec::new(), + relay_mesh: None, + }) +} + +/// Resolve a requested runtime id to its harness command. +/// +/// Refuses an id the catalog does not know instead of storing it: an +/// unresolvable pin produces an agent that cannot start, and the failure would +/// surface much later as a spawn error with no connection to this request. +fn resolve_runtime_command(runtime: &str) -> Result { + crate::managed_agents::command_for_runtime_id(runtime) + .ok_or_else(|| format!("unknown runtime \"{runtime}\"")) +} + +/// Build the update request, and report which fields it will change. +/// +/// The field list is derived from the request rather than from a diff of the +/// record, because the requester asked for these fields specifically. The +/// outcome names exactly what was asked for and applied. +fn update_request( + pubkey: &str, + args: &AgentsUpdateArgs, +) -> Result<(UpdateManagedAgentRequest, Vec), String> { + let mut changed = Vec::new(); + if args.display_name.is_some() { + changed.push("displayName".to_string()); + } + if args.system_prompt.is_some() { + changed.push("systemPrompt".to_string()); + } + if args.runtime.is_some() { + changed.push("runtime".to_string()); + } + if args.provider.is_some() { + changed.push("provider".to_string()); + } + if args.model.is_some() { + changed.push("model".to_string()); + } + if args.respond_to.is_some() { + changed.push("respondTo".to_string()); + } + changed.sort(); + + let respond_to = args + .respond_to + .as_deref() + .map(RespondTo::parse_wire) + .transpose()?; + + let request = UpdateManagedAgentRequest { + pubkey: pubkey.to_string(), + name: args.display_name.clone(), + // `Option>`: absent leaves the stored value alone. The broker + // has no "clear to default" verb, so it never sends `Some(None)` — a + // requester cannot blank a field it did not name. + model: args.model.clone().map(Some), + system_prompt: args.system_prompt.clone().map(Some), + env_vars: None, + parallelism: None, + turn_timeout_seconds: None, + relay_url: None, + acp_command: None, + agent_command: args + .runtime + .as_deref() + .map(resolve_runtime_command) + .transpose()?, + harness_override: args.runtime.is_some(), + agent_args: None, + mcp_command: None, + provider: args.provider.clone().map(Some), + respond_to, + // Allowlist mode is not reachable through this capability (the SDK + // rejects the mode), so there is never a list to replace. + respond_to_allowlist: None, + }; + Ok((request, changed)) +} + +/// Add `agent_pubkey` to `channel_id` as a bot member. +/// +/// The same owner-signed kind-9000 event `add_channel_members` publishes. It is +/// spelled out here rather than routed through that command because the command +/// swallows per-pubkey failures into a result array, and a broker create must +/// treat a failed attachment as a reportable outcome, not a field in a summary. +async fn attach_to_channel( + app: &AppHandle, + channel_id: &str, + agent_pubkey: &str, +) -> Result<(), String> { + let uuid = uuid::Uuid::parse_str(channel_id) + .map_err(|_| format!("invalid channel UUID: {channel_id}"))?; + let builder = crate::events::build_add_member(uuid, agent_pubkey, Some("bot"))?; + let state = app.state::(); + crate::relay::submit_event(builder, &state).await?; + Ok(()) +} + +/// Resolve a target selector against the owner's roster. +/// +/// Name matching is case-insensitive and must be unambiguous: two agents +/// sharing a name is a real state, and picking one arbitrarily would delete or +/// rewrite the wrong agent. +fn resolve_target( + records: &[ManagedAgentRecord], + target: &AgentTarget, +) -> Result { + match target { + AgentTarget::Pubkey(pubkey) => records + .iter() + .find(|record| record.pubkey.eq_ignore_ascii_case(pubkey)) + .cloned() + .ok_or_else(|| format!("no managed agent with pubkey {pubkey}")), + AgentTarget::Name(name) => { + let mut matches = records + .iter() + .filter(|record| record.name.eq_ignore_ascii_case(name)); + let first = matches + .next() + .cloned() + .ok_or_else(|| format!("no managed agent named \"{name}\""))?; + if matches.next().is_some() { + return Err(format!( + "more than one managed agent is named \"{name}\"; target it by pubkey" + )); + } + Ok(first) + } + } +} + +/// [`AgentService`] over the Desktop agent commands. +pub struct DesktopAgentService { + app: AppHandle, +} + +impl DesktopAgentService { + /// Bind the service to a running app. + pub fn new(app: AppHandle) -> Self { + Self { app } + } + + /// Snapshot the owner's agent records. + fn records(&self) -> Result, String> { + let state = self.app.state::(); + let _guard = state + .managed_agents_store_lock + .lock() + .map_err(|error| error.to_string())?; + load_managed_agents(&self.app) + } +} + +impl AgentService for DesktopAgentService { + fn create<'a>( + &'a self, + _owner_pubkey: &'a str, + args: &'a AgentsCreateArgs, + ) -> BoxFuture<'a, Result> { + Box::pin(async move { + let request = create_request(args).map_err(ServiceError::Failed)?; + let response = crate::commands::create_managed_agent( + request, + self.app.clone(), + self.app.state::(), + ) + .await + .map_err(ServiceError::Failed)?; + + let agent_pubkey = response.agent.pubkey.clone(); + let display_name = response.agent.name.clone(); + // The minted nsec dies with `response` at the end of this scope. + drop(response); + + // Attachment is a second, separately failable step. The agent + // already exists at this point, so a failure here is + // `Unknown`, not `Failed`: reporting "failed" would tell the + // requester nothing happened while an agent sits in the roster. + attach_to_channel(&self.app, &args.channel_id, &agent_pubkey) + .await + .map_err(|error| { + ServiceError::Unknown(format!( + "agent {agent_pubkey} was created but could not be added to channel \ + {}: {error}", + args.channel_id + )) + })?; + + Ok(AgentsCreateOutcome { + agent_pubkey, + display_name, + channel_id: args.channel_id.clone(), + }) + }) + } + + fn update<'a>( + &'a self, + _owner_pubkey: &'a str, + args: &'a AgentsUpdateArgs, + ) -> BoxFuture<'a, Result> { + Box::pin(async move { + let records = self.records().map_err(ServiceError::Failed)?; + let record = resolve_target(&records, &args.target).map_err(ServiceError::Failed)?; + let (request, updated_fields) = + update_request(&record.pubkey, args).map_err(ServiceError::Failed)?; + + let response = crate::commands::update_managed_agent( + request, + self.app.clone(), + self.app.state::(), + ) + .await + .map_err(ServiceError::Failed)?; + + Ok(AgentsUpdateOutcome { + agent_pubkey: response.agent.pubkey, + display_name: response.agent.name, + updated_fields, + }) + }) + } + + fn delete<'a>( + &'a self, + _owner_pubkey: &'a str, + args: &'a AgentsDeleteArgs, + ) -> BoxFuture<'a, Result> { + Box::pin(async move { + let records = self.records().map_err(ServiceError::Failed)?; + let record = resolve_target(&records, &args.target).map_err(ServiceError::Failed)?; + let agent_pubkey = record.pubkey.clone(); + let display_name = record.name.clone(); + + // `force_remote_delete: None` keeps the deployed-remote guard in + // force. Orphaning provisioned remote infrastructure is a decision + // for a human looking at the warning, not something a brokered + // request may force. + crate::commands::delete_managed_agent(agent_pubkey.clone(), None, self.app.clone()) + .await + .map_err(ServiceError::Failed)?; + + Ok(AgentsDeleteOutcome { + agent_pubkey, + display_name, + }) + }) + } +} + +/// [`AgentRoster`] reading the authoritative Desktop agent store. +pub struct DesktopAgentRoster { + app: AppHandle, +} + +impl DesktopAgentRoster { + /// Bind the roster to a running app. + pub fn new(app: AppHandle) -> Self { + Self { app } + } +} + +impl AgentRoster for DesktopAgentRoster { + fn agent_pubkeys<'a>( + &'a self, + owner_pubkey: &'a str, + ) -> BoxFuture<'a, Result, String>> { + Box::pin(async move { + // The store holds exactly one owner's agents — the owner whose keys + // this app instance holds. A request naming a different owner + // cannot be answered from here, and answering it from this roster + // anyway would authorize against the wrong set. + let state = self.app.state::(); + let host_owner = state + .keys + .lock() + .map_err(|error| error.to_string())? + .public_key() + .to_hex(); + if !host_owner.eq_ignore_ascii_case(owner_pubkey) { + return Err(format!( + "this host holds credentials for {host_owner}, not {owner_pubkey}" + )); + } + + let _guard = state + .managed_agents_store_lock + .lock() + .map_err(|error| error.to_string())?; + Ok(load_managed_agents(&self.app)? + .into_iter() + .map(|record| record.pubkey) + .collect()) + }) + } +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/broker/desktop_agents/tests.rs b/desktop/src-tauri/src/broker/desktop_agents/tests.rs new file mode 100644 index 0000000000..a826e2f9cc --- /dev/null +++ b/desktop/src-tauri/src/broker/desktop_agents/tests.rs @@ -0,0 +1,176 @@ +//! Tests for request translation and target resolution. +//! +//! These cover the decisions this module makes on its own — which fields a +//! brokered mutation may touch, and which agent a selector resolves to. The +//! Tauri-calling parts are not covered here: they hold no logic beyond +//! forwarding and the `Failed`/`Unknown` split, and testing them would mean +//! booting an app to observe a call it already delegates. + +use buzz_sdk_pkg::broker::AgentTarget; + +use super::*; + +const CHANNEL: &str = "b2c38ca8-9ec3-411e-bab5-f9deab34d52e"; + +fn create_args() -> AgentsCreateArgs { + AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "Helper".into(), + system_prompt: "Help with things.".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + } +} + +fn update_args() -> AgentsUpdateArgs { + AgentsUpdateArgs { + target: AgentTarget::Name("helper".into()), + display_name: None, + system_prompt: None, + runtime: None, + provider: None, + model: None, + respond_to: None, + } +} + +/// Records are built from JSON, matching `managed_agents::reconcile::tests` — +/// `ManagedAgentRecord` has no `Default`, and enumerating its fifty-odd fields +/// here would break on every unrelated schema addition. +fn record(pubkey: &str, name: &str) -> ManagedAgentRecord { + serde_json::from_str(&format!( + r#"{{ + "pubkey": "{pubkey}", + "name": "{name}", + "relay_url": "wss://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": "You are a test agent.", + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + }}"# + )) + .unwrap() +} + +/// A remote request for an agent to exist is not a request to run a process on +/// the owner's machine, now or at every launch. +#[test] +fn a_brokered_create_neither_spawns_nor_auto_starts() { + let request = create_request(&create_args()).unwrap(); + assert!(!request.spawn_after_create); + assert!(!request.start_on_app_launch); +} + +/// Linking a definition would pin someone else's config snapshot onto the new +/// agent — a decision the request never expressed. +#[test] +fn a_brokered_create_links_no_definition_or_team() { + let request = create_request(&create_args()).unwrap(); + assert!(request.persona_id.is_none()); + assert!(request.team_id.is_none()); + assert!(request.env_vars.is_empty()); + assert_eq!(request.backend, crate::managed_agents::BackendKind::Local); +} + +#[test] +fn create_carries_the_requested_name_and_prompt() { + let request = create_request(&create_args()).unwrap(); + assert_eq!(request.name, "Helper"); + assert_eq!(request.system_prompt.as_deref(), Some("Help with things.")); +} + +#[test] +fn create_translates_a_respond_to_mode() { + let mut args = create_args(); + args.respond_to = Some("anyone".into()); + let request = create_request(&args).unwrap(); + assert_eq!(request.respond_to, Some(RespondTo::Anyone)); +} + +/// An unresolvable runtime must fail here, not later as a spawn error with no +/// connection back to the request that caused it. +#[test] +fn create_refuses_a_runtime_the_catalog_does_not_know() { + let mut args = create_args(); + args.runtime = Some("not-a-real-runtime".into()); + let error = create_request(&args).unwrap_err(); + assert!(error.contains("unknown runtime"), "unexpected: {error}"); +} + +/// The broker has no "clear this field" verb, so an unnamed field must stay +/// absent rather than becoming an explicit null that wipes stored config. +#[test] +fn update_leaves_unrequested_fields_untouched() { + let (request, changed) = update_request("abc", &update_args()).unwrap(); + assert!(changed.is_empty()); + assert!(request.name.is_none()); + assert!(request.model.is_none()); + assert!(request.system_prompt.is_none()); + assert!(request.provider.is_none()); + assert!(request.respond_to.is_none()); + // Fields the capability does not expose at all. + assert!(request.env_vars.is_none()); + assert!(request.relay_url.is_none()); + assert!(request.agent_args.is_none()); + assert!(request.respond_to_allowlist.is_none()); +} + +#[test] +fn update_sets_only_the_named_fields_and_reports_them_sorted() { + let mut args = update_args(); + args.model = Some("some-model".into()); + args.display_name = Some("Renamed".into()); + let (request, changed) = update_request("abc", &args).unwrap(); + + assert_eq!(changed, vec!["displayName", "model"]); + assert_eq!(request.name.as_deref(), Some("Renamed")); + assert_eq!(request.model, Some(Some("some-model".to_string()))); + assert!(request.system_prompt.is_none()); +} + +#[test] +fn update_resolves_a_runtime_to_a_command_and_marks_it_a_pin() { + let mut args = update_args(); + args.runtime = Some("buzz-agent".into()); + let (request, changed) = update_request("abc", &args).unwrap(); + assert_eq!(changed, vec!["runtime"]); + assert!(request.agent_command.is_some()); + assert!(request.harness_override); +} + +#[test] +fn a_pubkey_target_resolves_case_insensitively() { + let records = vec![record("aabb", "helper")]; + let resolved = resolve_target(&records, &AgentTarget::Pubkey("AABB".into())).unwrap(); + assert_eq!(resolved.pubkey, "aabb"); +} + +#[test] +fn a_name_target_resolves_case_insensitively() { + let records = vec![record("aabb", "Helper")]; + let resolved = resolve_target(&records, &AgentTarget::Name("helper".into())).unwrap(); + assert_eq!(resolved.pubkey, "aabb"); +} + +/// Two agents can legitimately share a name. Picking one would mutate or delete +/// an agent the requester did not identify. +#[test] +fn an_ambiguous_name_is_refused_rather_than_guessed() { + let records = vec![record("aabb", "helper"), record("ccdd", "Helper")]; + let error = resolve_target(&records, &AgentTarget::Name("helper".into())).unwrap_err(); + assert!(error.contains("more than one"), "unexpected: {error}"); + assert!(error.contains("by pubkey"), "unexpected: {error}"); +} + +#[test] +fn a_missing_target_is_an_error() { + let records = vec![record("aabb", "helper")]; + assert!(resolve_target(&records, &AgentTarget::Name("ghost".into())).is_err()); + assert!(resolve_target(&records, &AgentTarget::Pubkey("ffff".into())).is_err()); +} diff --git a/desktop/src-tauri/src/broker/mod.rs b/desktop/src-tauri/src/broker/mod.rs index af99b3659b..b5748d9d81 100644 --- a/desktop/src-tauri/src/broker/mod.rs +++ b/desktop/src-tauri/src/broker/mod.rs @@ -33,6 +33,7 @@ use sha2::{Digest, Sha256}; pub mod agents_policy; +pub mod desktop_agents; pub mod handlers; pub mod pipeline; pub mod store;