diff --git a/desktop/src-tauri/src/commands/personas/mod.rs b/desktop/src-tauri/src/commands/personas/mod.rs index 6437d4b1a8..934d32c45c 100644 --- a/desktop/src-tauri/src/commands/personas/mod.rs +++ b/desktop/src-tauri/src/commands/personas/mod.rs @@ -15,6 +15,9 @@ use crate::{ util::now_iso, }; +mod propagate; +use propagate::{propagate_persona_name_rename, propagate_persona_respond_to}; + fn trim_required(value: &str, label: &str) -> Result { let trimmed = value.trim(); if trimmed.is_empty() { @@ -117,32 +120,6 @@ pub struct UpdatePersonaResult { persona: AgentDefinition, } -/// Propagate a persona definition's display_name rename to linked agent instances. -/// Only instances whose current `name` equals `old_display_name` are updated; -/// pool-named instances (e.g. "Birch", "Compass") keep their individualised name. -/// Updates both `record.name` (relay display name) and `record.display_name`. -/// Returns the pubkeys of the records that were renamed. -fn propagate_persona_name_rename( - records: &mut [ManagedAgentRecord], - persona_id: &str, - old_display_name: &str, - new_display_name: &str, -) -> Vec { - let mut renamed = Vec::new(); - for record in records.iter_mut() { - if record.persona_id.as_deref() != Some(persona_id) { - continue; - } - if record.name != old_display_name { - continue; // pool-named instance — keep its individualised name - } - record.name = new_display_name.to_string(); - record.display_name = Some(new_display_name.to_string()); - renamed.push(record.pubkey.clone()); - } - renamed -} - #[tauri::command] pub async fn update_persona( input: UpdatePersonaRequest, @@ -196,6 +173,7 @@ pub async fn update_persona( crate::managed_agents::validate_user_env_keys(&env_vars)?; persona.env_vars = env_vars; } + let behavior_present = input.behavior.is_some(); apply_persona_behavior(persona, input.behavior)?; persona.updated_at = now_iso(); @@ -205,14 +183,25 @@ pub async fn update_persona( retain_persona_pending(&app, &state, &result); try_regenerate_nest(&app); - // If the avatar or display_name changed, propagate to linked agent - // records and collect relay profile sync params for the async phase. - let sync_params: ProfileSyncParams = if avatar_changed || name_changed { + // Propagate definition edits that linked instances must mirror + // (avatar, display name, respond-to gate) and collect relay profile + // sync params for the async phase. + let needs_instance_sync = avatar_changed || name_changed || behavior_present; + let sync_params: ProfileSyncParams = if needs_instance_sync { let mut records = load_managed_agents(&app)?; let mut params: ProfileSyncParams = Vec::new(); let mut agents_modified = false; let workspace_relay = crate::relay::relay_ws_url_with_override(&state); + // The respond-to gate is copied onto instances at mint time only; + // without this, "Who can talk to this agent" edits never reach the + // harness and the agent stays owner-only (#2501). + if behavior_present + && propagate_persona_respond_to(&mut records, &result.id, &result)? > 0 + { + agents_modified = true; + } + // Propagate the display_name rename to instances that still // carry the old definition display_name (pool-named instances // keep their individualised name) in one pass; the loop below @@ -316,6 +305,8 @@ mod delete_cascade_tests; mod inbound_tests; #[cfg(test)] mod name_propagation_tests; +#[cfg(test)] +mod respond_to_propagation_tests; /// Return pubkeys of every managed agent whose definition is the given persona. /// diff --git a/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs b/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs index ba855ccbd6..4cef70a176 100644 --- a/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs +++ b/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs @@ -1,7 +1,8 @@ //! Tests for `propagate_persona_name_rename` — the helper that propagates a //! persona definition's display_name change to linked agent instances. -use super::*; +use super::propagate::propagate_persona_name_rename; +use crate::managed_agents::ManagedAgentRecord; fn agent(persona_id: &str, name: &str, display_name: Option<&str>) -> ManagedAgentRecord { ManagedAgentRecord { diff --git a/desktop/src-tauri/src/commands/personas/propagate.rs b/desktop/src-tauri/src/commands/personas/propagate.rs new file mode 100644 index 0000000000..18f8f6eb67 --- /dev/null +++ b/desktop/src-tauri/src/commands/personas/propagate.rs @@ -0,0 +1,91 @@ +//! Shared persona → instance field propagation helpers. +//! +//! A persona definition and the managed-agent instances linked to it are +//! separate records. Some definition edits must reach the running instances +//! (the harness only ever reads the instance record), and each helper here +//! covers one such field. + +use crate::managed_agents::{ + validate_respond_to_allowlist, AgentDefinition, ManagedAgentRecord, RespondTo, +}; + +/// Propagate a persona definition's display_name rename to linked agent instances. +/// Only instances whose current `name` equals `old_display_name` are updated; +/// pool-named instances (e.g. "Birch", "Compass") keep their individualised name. +/// Updates both `record.name` (relay display name) and `record.display_name`. +/// Returns the pubkeys of the records that were renamed. +pub(super) fn propagate_persona_name_rename( + records: &mut [ManagedAgentRecord], + persona_id: &str, + old_display_name: &str, + new_display_name: &str, +) -> Vec { + let mut renamed = Vec::new(); + for record in records.iter_mut() { + if record.persona_id.as_deref() != Some(persona_id) { + continue; + } + if record.name != old_display_name { + continue; // pool-named instance — keep its individualised name + } + record.name = new_display_name.to_string(); + record.display_name = Some(new_display_name.to_string()); + renamed.push(record.pubkey.clone()); + } + renamed +} + +/// Propagate a definition's respond-to gate onto its linked running instances. +/// +/// The "Who can talk to this agent" control edits the *definition*, writing +/// `definition_respond_to`. But the definition's gate is copied onto an +/// instance's `respond_to` at mint time only, and the harness (`buzz-acp`) +/// spawns from that instance field. Without this propagation a post-mint edit +/// never reaches the harness, so the agent keeps booting `owner-only` and no +/// one but the owner can talk to it (#2501). +/// +/// The definition gate is authoritative for every linked instance: on a +/// definition edit all instances converge to it. The allowlist is replaced +/// only when the definition mode is `allowlist` — for other modes the +/// instance's stored allowlist is left untouched so a later toggle back to +/// allowlist doesn't lose the entries (mirrors the preserve-across-toggle +/// semantics documented on `ManagedAgentRecord::respond_to_allowlist`). +/// +/// Key-less definition rows (empty `pubkey`) are skipped — they are not +/// running instances. Returns the number of instance records actually changed. +pub(super) fn propagate_persona_respond_to( + records: &mut [ManagedAgentRecord], + persona_id: &str, + definition: &AgentDefinition, +) -> Result { + let mode = match definition.respond_to.as_deref() { + Some(wire) => RespondTo::parse_wire(wire)?, + None => RespondTo::default(), + }; + let allowlist = if mode == RespondTo::Allowlist { + validate_respond_to_allowlist(&definition.respond_to_allowlist)? + } else { + Vec::new() + }; + + let mut updated = 0; + for record in records.iter_mut() { + if record.persona_id.as_deref() != Some(persona_id) { + continue; + } + if record.pubkey.is_empty() { + continue; // key-less definition row, not a running instance + } + + let mut changed = record.respond_to != mode; + record.respond_to = mode; + if mode == RespondTo::Allowlist && record.respond_to_allowlist != allowlist { + record.respond_to_allowlist = allowlist.clone(); + changed = true; + } + if changed { + updated += 1; + } + } + Ok(updated) +} diff --git a/desktop/src-tauri/src/commands/personas/respond_to_propagation_tests.rs b/desktop/src-tauri/src/commands/personas/respond_to_propagation_tests.rs new file mode 100644 index 0000000000..68e6d0fe01 --- /dev/null +++ b/desktop/src-tauri/src/commands/personas/respond_to_propagation_tests.rs @@ -0,0 +1,189 @@ +//! Tests for `propagate_persona_respond_to` — a definition's respond-to gate +//! edit must reach the linked instance records the harness actually reads +//! (#2501: edits stopped at the definition, so agents stayed owner-only). + +use super::propagate::propagate_persona_respond_to; +use crate::managed_agents::{AgentDefinition, ManagedAgentRecord, RespondTo}; + +fn agent(persona_id: &str, name: &str) -> ManagedAgentRecord { + ManagedAgentRecord { + pubkey: format!("pubkey-{name}"), + name: name.to_string(), + persona_id: Some(persona_id.to_string()), + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: String::new(), + agent_command: String::new(), + agent_command_override: None, + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 0, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars: std::collections::BTreeMap::new(), + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_binary_path: None, + team_id: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: RespondTo::OwnerOnly, + respond_to_allowlist: vec![], + display_name: Some(name.to_string()), + slug: None, + runtime: None, + name_pool: vec![], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: vec![], + definition_parallelism: None, + relay_mesh: None, + } +} + +fn definition(respond_to: Option<&str>, allowlist: &[&str]) -> AgentDefinition { + AgentDefinition { + id: "persona-1".to_string(), + display_name: "Scout".to_string(), + avatar_url: None, + system_prompt: String::new(), + runtime: None, + model: None, + provider: None, + name_pool: vec![], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + env_vars: Default::default(), + respond_to: respond_to.map(str::to_string), + respond_to_allowlist: allowlist.iter().map(|s| s.to_string()).collect(), + parallelism: None, + created_at: String::new(), + updated_at: String::new(), + } +} + +#[test] +fn anyone_propagates_to_linked_instances_only() { + let mut records = vec![agent("persona-1", "Scout"), agent("persona-2", "Other")]; + + let updated = + propagate_persona_respond_to(&mut records, "persona-1", &definition(Some("anyone"), &[])) + .expect("propagate"); + + assert_eq!(updated, 1); + assert_eq!(records[0].respond_to, RespondTo::Anyone); + assert!(records[0].respond_to_allowlist.is_empty()); + // A different persona's instance is untouched. + assert_eq!(records[1].respond_to, RespondTo::OwnerOnly); +} + +#[test] +fn allowlist_propagates_mode_and_entries() { + let allow = "a".repeat(64); + let mut records = vec![agent("persona-1", "Scout")]; + + let updated = propagate_persona_respond_to( + &mut records, + "persona-1", + &definition(Some("allowlist"), &[&allow]), + ) + .expect("propagate"); + + assert_eq!(updated, 1); + assert_eq!(records[0].respond_to, RespondTo::Allowlist); + assert_eq!(records[0].respond_to_allowlist, vec![allow]); +} + +#[test] +fn non_allowlist_mode_preserves_stored_instance_allowlist() { + // Toggling away from allowlist keeps the entries so a later toggle back + // doesn't lose them (preserve-across-toggle semantics). + let existing = "b".repeat(64); + let mut records = vec![agent("persona-1", "Scout")]; + records[0].respond_to = RespondTo::Allowlist; + records[0].respond_to_allowlist = vec![existing.clone()]; + + let updated = + propagate_persona_respond_to(&mut records, "persona-1", &definition(Some("anyone"), &[])) + .expect("propagate"); + + assert_eq!(updated, 1); + assert_eq!(records[0].respond_to, RespondTo::Anyone); + assert_eq!(records[0].respond_to_allowlist, vec![existing]); +} + +#[test] +fn absent_definition_gate_resolves_to_owner_only() { + let mut records = vec![agent("persona-1", "Scout")]; + records[0].respond_to = RespondTo::Anyone; + + let updated = propagate_persona_respond_to(&mut records, "persona-1", &definition(None, &[])) + .expect("propagate"); + + assert_eq!(updated, 1); + assert_eq!(records[0].respond_to, RespondTo::OwnerOnly); +} + +#[test] +fn no_change_reports_zero_updates() { + let mut records = vec![agent("persona-1", "Scout")]; // already owner-only + + let updated = propagate_persona_respond_to( + &mut records, + "persona-1", + &definition(Some("owner-only"), &[]), + ) + .expect("propagate"); + + assert_eq!(updated, 0); + assert_eq!(records[0].respond_to, RespondTo::OwnerOnly); +} + +#[test] +fn key_less_definition_rows_are_skipped() { + let mut records = vec![agent("persona-1", "Scout")]; + records[0].pubkey.clear(); // un-minted definition row, not a running instance + + let updated = + propagate_persona_respond_to(&mut records, "persona-1", &definition(Some("anyone"), &[])) + .expect("propagate"); + + assert_eq!(updated, 0); +} + +#[test] +fn invalid_definition_allowlist_is_rejected() { + let mut records = vec![agent("persona-1", "Scout")]; + + let result = propagate_persona_respond_to( + &mut records, + "persona-1", + &definition(Some("allowlist"), &["not-a-valid-pubkey"]), + ); + + assert!(result.is_err()); + // Records are left untouched on validation failure. + assert_eq!(records[0].respond_to, RespondTo::OwnerOnly); +}