Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 20 additions & 29 deletions desktop/src-tauri/src/commands/personas/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> {
let trimmed = value.trim();
if trimmed.is_empty() {
Expand Down Expand Up @@ -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<String> {
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,
Expand Down Expand Up @@ -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();

Expand All @@ -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
Expand Down Expand Up @@ -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.
///
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
91 changes: 91 additions & 0 deletions desktop/src-tauri/src/commands/personas/propagate.rs
Original file line number Diff line number Diff line change
@@ -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<String> {
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<usize, String> {
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)
}
Original file line number Diff line number Diff line change
@@ -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);
}