External/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-08-18 06:50:31 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(desktop): propagate agent definition edits to instances and remove dead config knobs (#1715)

Browse Source
This commit is contained in:
Will Pfleger
2026-07-10 13:21:48 -04:00
committed by GitHub
parent 9b2f0c8cb5
commit 1f48860b7b
24 changed files with 1865 additions and 1278 deletions
Download Patch File Download Diff File Expand all files Collapse all files
desktop/scripts/check-file-sizes.mjs
+4 -1
View File
@@ -182,7 +182,10 @@ const overrides = new Map([
// path instead of being dropped back to inherit. Load-bearing, not debt.
// unified-agent-model 1A.1: inline test module moved to discovery/tests.rs,
// ratcheting 1259 -> 802 (under the 1000 default; entry kept as a ratchet).
["src-tauri/src/managed_agents/discovery.rs", 802],
// agent-config-propagation: the agent_command_override decision family
// (divergent / create-time / update-time / apply) moved to
// discovery/overrides.rs; ratcheting 802 -> 685 to bank the headroom.
["src-tauri/src/managed_agents/discovery.rs", 685],
// identity-import-keyring: the identity resolution state machine's behavioral
// matrix (46 tests over FakeIdentityStore — probe × marker × file cells,
// adoption / read-back-corruption / marker-failure arms, recovery-mode
desktop/src-tauri/src/commands/agent_models.rs
+21 -24
View File
@@ -839,9 +839,9 @@ pub async fn update_managed_agent(
if let Some(parallelism) = input.parallelism {
record.parallelism = parallelism;
}
if let Some(turn_timeout_seconds) = input.turn_timeout_seconds {
record.turn_timeout_seconds = turn_timeout_seconds;
}
// turn_timeout_seconds is intentionally not applied here —
// BUZZ_ACP_TURN_TIMEOUT is deprecated and ignored by the harness.
// Use idle_timeout_seconds or max_turn_duration_seconds instead.
// Store the relay override exactly as supplied (trimmed). An explicit
// value pins the agent; empty falls back to the workspace relay at
// read-time. A name-only edit (relay_url == None) leaves the pin intact.
@@ -851,33 +851,30 @@ pub async fn update_managed_agent(
if let Some(acp_command) = input.acp_command {
record.acp_command = acp_command;
}
// Harness edit: the persona's runtime is authoritative, so we persist an
// explicit `agent_command_override` ONLY when the user picks a command
// that diverges from the persona. An empty/whitespace value (the
// "Inherit from persona" sentinel) clears the pin back to `None`. A
// name-only edit (`agent_command == None`) leaves the pin intact.
//
// `harness_override` threads the user's explicit intent: when they pick
// a runtime/Custom command in the dialog it is a real pin even if it
// maps to the persona's own runtime, so a same-runtime pick is kept
// rather than dropped back to inherit (see
// `update_time_agent_command_override`).
// Harness edit: the persona's runtime is authoritative, so an explicit
// `agent_command_override` is persisted ONLY when the user picks a
// command that diverges from the persona, and the empty/whitespace
// "Inherit from persona" sentinel clears both the pin and the
// materialized record runtime. A name-only edit
// (`agent_command == None`) leaves the pin intact. `harness_override`
// threads the user's explicit intent — see `apply_agent_command_update`
// and `update_time_agent_command_override` for the full resolution
// rules.
if let Some(agent_command) = input.agent_command {
let personas = load_personas(&app).unwrap_or_default();
record.agent_command_override =
crate::managed_agents::update_time_agent_command_override(
record.persona_id.as_deref(),
&personas,
Some(&agent_command),
input.harness_override,
);
crate::managed_agents::apply_agent_command_update(
record,
&personas,
&agent_command,
input.harness_override,
);
}
if let Some(agent_args) = input.agent_args {
record.agent_args = agent_args;
}
if let Some(mcp_command) = input.mcp_command {
record.mcp_command = mcp_command;
}
// mcp_command is intentionally not applied here — the effective MCP
// command is always catalog-derived (known_acp_runtime at spawn time)
// and the per-record field is never read by the runtime.
if let Some(env_vars) = input.env_vars {
crate::managed_agents::validate_user_env_keys(&env_vars)?;
record.env_vars = env_vars;
desktop/src-tauri/src/commands/agent_models_tests.rs
+33
View File
@@ -219,6 +219,39 @@ fn saved_agent_model_discovery_uses_record_snapshot() {
// Parse/filter/pagination tests live in crates/buzz-agent/src/catalog.rs
// (they moved there with the Option C refactor).
// ---------------------------------------------------------------------------
// Dead-knob guards: mcp_command and turn_timeout_seconds
// ---------------------------------------------------------------------------
#[test]
fn update_request_mcp_command_parses_for_wire_compat() {
// UpdateManagedAgentRequest accepts mcpCommand for backward-compatibility
// with frontends that still send it: the deprecated field must keep
// parsing cleanly. Nothing consumes it — the patching loop in
// update_managed_agent has no mcp_command arm (the effective MCP command
// is always catalog-derived at spawn). That absent-arm invariant lives in
// the code, not in this test: it only guards the wire shape.
let req: crate::managed_agents::UpdateManagedAgentRequest =
serde_json::from_str(r#"{"pubkey": "abc", "mcpCommand": "user-override"}"#)
.expect("request with deprecated mcpCommand parses");
assert_eq!(req.mcp_command.as_deref(), Some("user-override"));
}
#[test]
fn update_request_turn_timeout_parses_for_wire_compat() {
// UpdateManagedAgentRequest accepts turnTimeoutSeconds for
// backward-compatibility with frontends that still send it: the deprecated
// field must keep parsing cleanly. Nothing consumes it — the patching loop
// in update_managed_agent has no turn_timeout_seconds arm
// (BUZZ_ACP_TURN_TIMEOUT is deprecated and ignored by the harness). That
// absent-arm invariant lives in the code, not in this test: it only
// guards the wire shape.
let req: crate::managed_agents::UpdateManagedAgentRequest =
serde_json::from_str(r#"{"pubkey": "abc", "turnTimeoutSeconds": 9999}"#)
.expect("request with deprecated turnTimeoutSeconds parses");
assert_eq!(req.turn_timeout_seconds, Some(9999));
}
#[test]
fn is_databricks_provider_matches_both_variants() {
assert!(is_databricks_provider(Some("databricks")));
desktop/src-tauri/src/commands/agents.rs
+17 -44
View File
@@ -254,37 +254,14 @@ async fn start_local_agent_with_preflight(
return Err(format!("agent {pubkey} is no longer a local agent"));
}
// Re-snapshot the persona onto the record at every spawn so the agent always
// starts with the current persona config (system_prompt, model, provider).
// This clears the "out of date" drift badge without requiring a
// delete+recreate. `record.env_vars` is NOT rewritten: it holds agent-level
// overrides only, and spawn merges the live persona env underneath at read
// time — so persona env edits refresh here too. When the persona leaves
// model or provider blank, the agent record's own configured values are
// preserved so a user-set model/provider is never clobbered by an
// unconfigured persona.
// starts with the current persona config (system_prompt, model, provider,
// runtime). This clears the "out of date" drift badge without requiring a
// delete+recreate. See `apply_persona_snapshot` for the precedence and
// env-override self-heal rules.
if let Some(persona_id) = record.persona_id.clone() {
let personas = load_personas(app).unwrap_or_default();
if let Some(persona) = personas.iter().find(|p| p.id == persona_id) {
let snapshot =
crate::managed_agents::persona_events::persona_snapshot_with_agent_config_fallback(
persona,
record.model.as_deref(), // fallback: record.model
record.provider.as_deref(), // fallback: record.provider
);
if let Some(prompt) = snapshot.system_prompt {
record.system_prompt = Some(prompt);
}
record.model = snapshot.model;
record.provider = snapshot.provider;
// Self-heal records written before env refresh: persona env used to
// be baked into `record.env_vars` at create/spawn, turning inherited
// values into pseudo-overrides that shadow later persona edits. An
// override equal to the persona's current value is indistinguishable
// from inheritance, so drop it and let the live merge supply it.
record
.env_vars
.retain(|k, v| persona.env_vars.get(k) != Some(v));
record.persona_source_version = Some(snapshot.source_version);
crate::managed_agents::persona_events::apply_persona_snapshot(record, persona);
record.updated_at = crate::util::now_iso();
}
}
@@ -594,18 +571,14 @@ pub async fn create_managed_agent(
.collect::<Vec<_>>(),
);
let mcp_command = input
.mcp_command
.as_deref()
.map(str::trim)
.filter(|value| !value.is_empty())
.map(str::to_string)
.unwrap_or_else(
|| match crate::managed_agents::known_acp_runtime(&agent_command) {
Some(p) => p.mcp_command.unwrap_or("").to_string(),
None => String::new(),
},
);
// Derive MCP command exclusively from the runtime catalog — the
// per-record field is never read at spawn time so user-supplied input
// is silently discarded. Always sourcing from the catalog ensures
// new agents pick up the correct value without any stored override.
let mcp_command = match crate::managed_agents::known_acp_runtime(&agent_command) {
Some(p) => p.mcp_command.unwrap_or("").to_string(),
None => String::new(),
};
// For pack-backed personas, resolve the installed pack path and the
// persona's internal name (slug). ACP's resolve_persona_by_name()
@@ -699,10 +672,10 @@ pub async fn create_managed_agent(
agent_command_override,
agent_args,
mcp_command,
turn_timeout_seconds: input
.turn_timeout_seconds
.filter(|seconds| *seconds > 0)
.unwrap_or(DEFAULT_AGENT_TURN_TIMEOUT_SECONDS),
// BUZZ_ACP_TURN_TIMEOUT is deprecated and ignored by the harness;
// store the schema default only. Use idle_timeout_seconds or
// max_turn_duration_seconds for actual turn-length control.
turn_timeout_seconds: DEFAULT_AGENT_TURN_TIMEOUT_SECONDS,
// 0 or None → harness uses its own default (320s idle, 3600s max), and the CLI also clamps 0 → minimum.
idle_timeout_seconds: input.idle_timeout_seconds.filter(|s| *s > 0),
max_turn_duration_seconds: input.max_turn_duration_seconds.filter(|s| *s > 0),
desktop/src-tauri/src/commands/personas/mod.rs
+112 -47
View File
@@ -111,20 +111,61 @@ pub async fn create_persona(
.map_err(|e| format!("spawn_blocking failed: {e}"))?
}
/// Return value of the `update_persona` command. Uses flatten so all
/// `PersonaRecord` fields appear at the top level of the JSON response,
/// alongside the optional `writeback_warning` field — backward-compatible with
/// callers that already destructure a raw persona object.
#[derive(Debug, serde::Serialize)]
pub struct UpdatePersonaResult {
#[serde(flatten)]
persona: PersonaRecord,
/// Non-`None` when the pack `.persona.md` write-back failed (non-fatal).
/// The in-app edit was already saved; the frontend can use this to surface
/// a "pack file diverged" indicator so the user knows to check the file.
#[serde(skip_serializing_if = "Option::is_none")]
pub writeback_warning: Option<String>,
}
/// 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,
app: AppHandle,
) -> Result<PersonaRecord, String> {
) -> Result<UpdatePersonaResult, String> {
use tauri::Manager;
/// Profile sync params collected under the store lock for async relay publish.
type ProfileSyncParams = Vec<(nostr::Keys, String, String, Option<String>, Option<String>)>;
// Phase 1: synchronous save (persona record + linked agent avatar updates)
let (result, profile_sync_params) = tokio::task::spawn_blocking({
let (result, profile_sync_params, writeback_warning) = tokio::task::spawn_blocking({
let app = app.clone();
move || -> Result<(PersonaRecord, ProfileSyncParams), String> {
move || -> Result<(PersonaRecord, ProfileSyncParams, Option<String>), String> {
let state = app.state::<AppState>();
let display_name = trim_required(&input.display_name, "Display name")?;
// Do not trim system_prompt: `compose_prompt` appends pack_instructions
@@ -151,8 +192,10 @@ pub async fn update_persona(
return Err("Built-in agents cannot be edited.".to_string());
}
// Track whether avatar changed so we can sync linked agents.
// Track what changed so we can propagate to linked agent records.
let avatar_changed = persona.avatar_url != avatar_url;
let name_changed = persona.display_name != display_name;
let old_display_name = persona.display_name.clone();
persona.display_name = display_name;
persona.avatar_url = avatar_url;
@@ -173,60 +216,79 @@ pub async fn update_persona(
apply_persona_behavior(persona, input.behavior)?;
persona.updated_at = now_iso();
let result = persona.clone();
save_personas(&app, &personas)?;
let result = personas
.into_iter()
.find(|record| record.id == input.id)
.ok_or_else(|| format!("agent {} disappeared unexpectedly", input.id))?;
// For pack-backed personas, also write the edit back to the source
// `.persona.md` so that launch sync (which reads the file) becomes a
// no-op rather than overwriting the record we just saved.
write_back_persona_md(&app, &result);
let writeback_warning = write_back_persona_md(&app, &result);
retain_persona_pending(&app, &state, &result);
try_regenerate_nest(&app);
// If the avatar changed, propagate to linked agent records and
// collect relay profile sync params for the async phase.
let sync_params: ProfileSyncParams = if avatar_changed {
// 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 {
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);
// 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
// only decides which records need a relay profile sync.
let renamed: Vec<String> = if name_changed {
propagate_persona_name_rename(
&mut records,
&result.id,
&old_display_name,
&result.display_name,
)
} else {
Vec::new()
};
for record in records.iter_mut() {
if record.persona_id.as_deref() != Some(&result.id) {
continue;
}
// Update the persisted avatar so reconciliation on next
// start agrees with what we're about to publish.
// When the persona avatar is cleared, fall back to the
// command-default icon so the record never stores `None`
// (which reconcile_agent_profile treats as "un-migrated").
let effective_cmd = effective_agent_command(
record.persona_id.as_deref(),
std::slice::from_ref(&result),
record.agent_command_override.as_deref(),
);
record.avatar_url = result
.avatar_url
.clone()
.or_else(|| managed_agent_avatar_url(&effective_cmd));
agents_modified = true;
let mut record_changed = renamed.contains(&record.pubkey);
if let Ok(agent_keys) = nostr::Keys::parse(&record.private_key_nsec) {
let relay_url = crate::relay::effective_agent_relay_url(
&record.relay_url,
&workspace_relay,
if avatar_changed {
// Update the persisted avatar so reconciliation on next
// start agrees with what we're about to publish.
// When the persona avatar is cleared, fall back to the
// command-default icon so the record never stores `None`
// (which reconcile_agent_profile treats as "un-migrated").
let effective_cmd = effective_agent_command(
record.persona_id.as_deref(),
std::slice::from_ref(&result),
record.agent_command_override.as_deref(),
);
params.push((
agent_keys,
relay_url,
record.name.clone(),
record.avatar_url.clone(),
record.auth_tag.clone(),
));
record.avatar_url = result
.avatar_url
.clone()
.or_else(|| managed_agent_avatar_url(&effective_cmd));
record_changed = true;
}
if record_changed {
agents_modified = true;
if let Ok(agent_keys) = nostr::Keys::parse(&record.private_key_nsec) {
let relay_url = crate::relay::effective_agent_relay_url(
&record.relay_url,
&workspace_relay,
);
params.push((
agent_keys,
relay_url,
record.name.clone(),
record.avatar_url.clone(),
record.auth_tag.clone(),
));
}
}
}
@@ -239,16 +301,16 @@ pub async fn update_persona(
Vec::new()
};
Ok((result, sync_params))
Ok((result, sync_params, writeback_warning))
}
})
.await
.map_err(|e| format!("spawn_blocking failed: {e}"))??;
// Phase 2: await relay profile sync for linked agents whose avatar was
// just updated. We await (rather than fire-and-forget) so the frontend
// cache invalidation that follows the mutation settlement sees the fresh
// relay profile. Best-effort — failures are logged, not surfaced.
// Phase 2: await relay profile sync for linked agents whose avatar or
// display_name was just updated. We await (rather than fire-and-forget)
// so the frontend cache invalidation that follows the mutation settlement
// sees the fresh relay profile. Best-effort — failures are logged, not surfaced.
if !profile_sync_params.is_empty() {
let state = app.state::<AppState>();
for (agent_keys, relay_url, display_name, avatar_url, auth_tag) in profile_sync_params {
@@ -262,14 +324,15 @@ pub async fn update_persona(
)
.await
{
eprintln!(
"buzz-desktop: relay profile sync failed after persona avatar update: {e}"
);
eprintln!("buzz-desktop: relay profile sync failed after persona update: {e}");
}
}
}
Ok(result)
Ok(UpdatePersonaResult {
persona: result,
writeback_warning,
})
}
mod writeback;
@@ -277,6 +340,8 @@ use writeback::write_back_persona_md;
#[cfg(test)]
mod inbound_tests;
#[cfg(test)]
mod name_propagation_tests;
#[tauri::command]
pub async fn delete_persona(id: String, app: AppHandle) -> Result<(), String> {
desktop/src-tauri/src/commands/personas/name_propagation_tests.rs
+159
View File
@@ -0,0 +1,159 @@
//! Tests for `propagate_persona_name_rename` — the helper that propagates a
//! persona definition's display_name change to linked agent instances.
use super::*;
fn agent(persona_id: &str, name: &str, display_name: Option<&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,
mcp_toolsets: 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,
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: Default::default(),
respond_to_allowlist: vec![],
display_name: display_name.map(str::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_mcp_toolsets: None,
definition_parallelism: None,
relay_mesh: None,
}
}
#[test]
fn test_rename_propagates_to_matching_instance() {
// An instance whose `name` equals the OLD persona display_name must get
// both `name` and `display_name` updated to the new value.
let mut records = vec![agent("persona-1", "Paul", Some("Paul"))];
let renamed = propagate_persona_name_rename(&mut records, "persona-1", "Paul", "Paul Atreides");
assert_eq!(
renamed,
vec!["pubkey-Paul".to_string()],
"must report the renamed record's pubkey"
);
assert_eq!(records[0].name, "Paul Atreides", "name must be updated");
assert_eq!(
records[0].display_name,
Some("Paul Atreides".to_string()),
"display_name must be updated"
);
// The relay-profile sync params use `record.name`; after rename it carries
// the new display_name, so the relay profile will be published with the correct name.
assert_eq!(records[0].name, "Paul Atreides");
}
#[test]
fn test_rename_skips_pool_named_instance() {
// A pool-named instance (e.g. "Birch") has a name DIFFERENT from the
// persona display_name. It must keep its individualised name.
let mut records = vec![agent("persona-1", "Birch", Some("Birch"))];
let renamed = propagate_persona_name_rename(&mut records, "persona-1", "Paul", "Paul Atreides");
assert!(
renamed.is_empty(),
"pool-named instance must not be reported as renamed"
);
assert_eq!(records[0].name, "Birch", "pool name must be preserved");
assert_eq!(
records[0].display_name,
Some("Birch".to_string()),
"pool display_name must be preserved"
);
}
#[test]
fn test_rename_propagates_both_name_and_display_name() {
// Explicit dual-field check: BOTH `name` and `display_name` must be
// updated so the relay profile and the local UI are consistent.
let mut records = vec![agent("persona-1", "OldName", None)];
propagate_persona_name_rename(&mut records, "persona-1", "OldName", "NewName");
assert_eq!(records[0].name, "NewName");
assert_eq!(records[0].display_name, Some("NewName".to_string()));
}
#[test]
fn test_rename_only_affects_linked_persona() {
// An instance linked to a DIFFERENT persona must not be touched, even
// if it happens to carry the same display_name.
let mut records = vec![
agent("persona-1", "Paul", Some("Paul")),
agent("persona-2", "Paul", Some("Paul")),
];
propagate_persona_name_rename(&mut records, "persona-1", "Paul", "Paul Atreides");
assert_eq!(records[0].name, "Paul Atreides", "linked instance renamed");
assert_eq!(
records[1].name, "Paul",
"unrelated persona's instance untouched"
);
}
#[test]
fn test_rename_renames_all_matching_instances_in_one_pass() {
// Several instances may carry the definition name (multi-instance deploys
// without a name pool): one call renames every match and reports each
// pubkey, which is what the relay profile sync collection keys on.
let mut records = vec![
agent("persona-1", "Paul", Some("Paul")),
agent("persona-1", "Paul", Some("Paul")),
agent("persona-1", "Birch", Some("Birch")),
];
records[1].pubkey = "pubkey-Paul-2".to_string();
let renamed = propagate_persona_name_rename(&mut records, "persona-1", "Paul", "Duncan Idaho");
assert_eq!(
renamed,
vec!["pubkey-Paul".to_string(), "pubkey-Paul-2".to_string()],
"every matching instance's pubkey must be reported"
);
assert_eq!(records[0].name, "Duncan Idaho");
assert_eq!(records[1].name, "Duncan Idaho");
assert_eq!(records[2].name, "Birch", "pool-named instance untouched");
}
desktop/src-tauri/src/commands/personas/writeback.rs
+119 -729
View File
@@ -23,6 +23,11 @@ fn find_team_for_persona_source<'a>(
/// logged and swallowed so that the in-app edit — already persisted to
/// `personas.json` — always lands.
///
/// Returns `Some(warning)` when the write-back failed (non-fatal), `None` on
/// success. The warning is forwarded to the `update_persona` command result so
/// the frontend can surface a "pack file diverged" indicator instead of
/// silently drifting.
///
/// Only the four fields that the UI can set and that live in frontmatter are
/// rewritten: `display_name`, `runtime`, `avatar`, and `model` (the combined
/// `"provider:model"` string used by the pack format). The markdown body is
@@ -42,65 +47,129 @@ fn find_team_for_persona_source<'a>(
/// same key as `sync_team_from_dir` (`team_persona_key`). This handles both
/// modern teams (where `team.id` equals the manifest id) and legacy/backfilled
/// teams (where `team.id` is a UUID and the manifest id lives in `source_dir`).
pub(super) fn write_back_persona_md(app: &AppHandle, persona: &PersonaRecord) {
pub(super) fn write_back_persona_md(app: &AppHandle, persona: &PersonaRecord) -> Option<String> {
// Only pack-backed personas have a source file to write back to.
persona.source_team.as_ref()?;
let result = load_teams(app).and_then(|teams| try_write_back_persona_md(&teams, persona));
match result {
Ok(()) => None,
Err(e) => {
eprintln!("buzz-desktop: persona-writeback: {e}");
Some(e)
}
}
}
/// Inner logic for write-back, extracted for testability. Takes resolved teams
/// rather than an `AppHandle` so tests can exercise the full path → symlink →
/// pack → rewrite flow without a running Tauri application.
///
/// Returns `Err` if write-back fails for any reason (non-fatal at the call
/// site); `Ok(())` if the persona has no pack source or the file was updated
/// (or was already current).
fn try_write_back_persona_md(teams: &[TeamRecord], persona: &PersonaRecord) -> Result<(), String> {
let Some(source_team_id) = &persona.source_team else {
return;
return Ok(()); // non-pack persona — nothing to write back
};
let Some(slug) = &persona.source_team_persona_slug else {
eprintln!(
"buzz-desktop: persona-writeback: persona {} has source_team but no slug; skipping",
return Err(format!(
"persona {} has source_team but no slug; cannot write back",
persona.id
);
return;
));
};
let result = (|| -> Result<(), String> {
let teams = load_teams(app)?;
let team = find_team_for_persona_source(&teams, source_team_id)
.ok_or_else(|| format!("team {source_team_id} not found"))?;
let source_dir = team
.source_dir
.as_ref()
.ok_or_else(|| "team has no source_dir (JSON-only team)".to_string())?;
let team = find_team_for_persona_source(teams, source_team_id)
.ok_or_else(|| format!("team {source_team_id} not found"))?;
let source_dir = team
.source_dir
.as_ref()
.ok_or_else(|| "team has no source_dir (JSON-only team)".to_string())?;
// Resolve the actual source file via the pack manifest, matching the
// same path the launch sync reads. `LoadedPersona.source_path` is the
// absolute path set by `safe_resolve` against the pack root, so it is
// correct regardless of the manifest layout.
let pack = buzz_persona_pkg::pack::load_pack(source_dir)
.map_err(|e| format!("load_pack {}: {e}", source_dir.display()))?;
let loaded = pack
.personas
.iter()
.find(|p| p.name == *slug)
.ok_or_else(|| {
format!(
"persona '{slug}' not found in pack at {}",
source_dir.display()
)
})?;
let path = &loaded.source_path;
// Resolve the actual source file via the pack manifest, matching the
// same path the launch sync reads. `LoadedPersona.source_path` is the
// absolute path set by `safe_resolve` against the pack root, so it is
// correct regardless of the manifest layout.
let pack = buzz_persona_pkg::pack::load_pack(source_dir)
.map_err(|e| format!("load_pack {}: {e}", source_dir.display()))?;
let loaded = pack
.personas
.iter()
.find(|p| p.name == *slug)
.ok_or_else(|| {
format!(
"persona '{slug}' not found in pack at {}",
source_dir.display()
)
})?;
let path = &loaded.source_path;
let content =
std::fs::read_to_string(path).map_err(|e| format!("read {}: {e}", path.display()))?;
let updated = rewrite_persona_md(
&content,
persona,
&loaded.prompt,
pack.pack_instructions.as_deref(),
)?;
if updated == content {
return Ok(());
}
std::fs::write(path, &updated).map_err(|e| format!("write {}: {e}", path.display()))?;
Ok(())
})();
if let Err(e) = result {
eprintln!("buzz-desktop: persona-writeback: {e}");
// Containment: the manifest-resolved file must stay inside the pack root.
// Both sides are canonicalized so a symlinked pack dir (the legacy deploy
// layout) compares on its resolved location — the check only rejects a
// manifest that redirects the write outside the pack.
let canonical_path = path
.canonicalize()
.map_err(|e| format!("resolve {}: {e}", path.display()))?;
let canonical_root = source_dir
.canonicalize()
.map_err(|e| format!("resolve {}: {e}", source_dir.display()))?;
if !canonical_path.starts_with(&canonical_root) {
return Err(format!(
"persona file {} resolves outside its pack root {}",
canonical_path.display(),
canonical_root.display()
));
}
let content = std::fs::read_to_string(&canonical_path)
.map_err(|e| format!("read {}: {e}", canonical_path.display()))?;
let updated = rewrite_persona_md(
&content,
persona,
&loaded.prompt,
pack.pack_instructions.as_deref(),
)?;
if updated == content {
return Ok(());
}
write_file_atomic(&canonical_path, &updated)
}
/// Replace `path`'s content atomically: write a sibling temp file, then rename
/// it over the target so a crash mid-write can never leave a truncated
/// `.persona.md`. The target's write permission is probed first because a
/// rename only needs directory permission — without the probe, an atomic
/// replace would silently defeat a deliberately read-only pack file that the
/// previous in-place write (and the surfaced warning) honored.
fn write_file_atomic(path: &std::path::Path, content: &str) -> Result<(), String> {
use std::io::Write;
std::fs::OpenOptions::new()
.write(true)
.open(path)
.map_err(|e| format!("write {}: {e}", path.display()))?;
let parent = path
.parent()
.ok_or_else(|| format!("no parent directory for {}", path.display()))?;
let mut tmp = tempfile::NamedTempFile::new_in(parent)
.map_err(|e| format!("create temp file in {}: {e}", parent.display()))?;
tmp.write_all(content.as_bytes())
.map_err(|e| format!("write {}: {e}", path.display()))?;
// NamedTempFile creates with 0600; carry the target's permissions over so
// the replacement doesn't change the file's mode.
let perms = std::fs::metadata(path)
.map_err(|e| format!("stat {}: {e}", path.display()))?
.permissions();
tmp.as_file()
.set_permissions(perms)
.map_err(|e| format!("set permissions on temp for {}: {e}", path.display()))?;
tmp.persist(path)
.map_err(|e| format!("write {}: {e}", path.display()))?;
Ok(())
}
/// Rewrite a `.persona.md` file with updated frontmatter fields and, when safe,
@@ -232,683 +301,4 @@ fn rewrite_persona_md(
}
#[cfg(test)]
mod writeback_tests {
use super::*;
use std::collections::BTreeMap;
/// Build a minimal PersonaRecord with the fields that `rewrite_persona_md` reads.
fn persona(
display_name: &str,
runtime: Option<&str>,
avatar_url: Option<&str>,
provider: Option<&str>,
model: Option<&str>,
) -> PersonaRecord {
// system_prompt matches the SAMPLE_MD body so the "no prompt edit" path
// is taken in rewrite_persona_md (body preserved byte-for-byte).
persona_with_prompt(
display_name,
runtime,
avatar_url,
provider,
model,
"You are Paul.\n",
)
}
/// Like `persona` but with an explicit system_prompt value.
fn persona_with_prompt(
display_name: &str,
runtime: Option<&str>,
avatar_url: Option<&str>,
provider: Option<&str>,
model: Option<&str>,
system_prompt: &str,
) -> PersonaRecord {
PersonaRecord {
id: "test-id".to_string(),
display_name: display_name.to_string(),
avatar_url: avatar_url.map(str::to_string),
system_prompt: system_prompt.to_string(),
runtime: runtime.map(str::to_string),
model: model.map(str::to_string),
provider: provider.map(str::to_string),
name_pool: vec![],
is_builtin: false,
is_active: true,
source_team: Some("team-1".to_string()),
source_team_persona_slug: Some("paul".to_string()),
env_vars: BTreeMap::new(),
respond_to: None,
respond_to_allowlist: Vec::new(),
mcp_toolsets: None,
parallelism: None,
created_at: "2025-01-01T00:00:00Z".to_string(),
updated_at: "2025-01-01T00:00:00Z".to_string(),
}
}
const SAMPLE_MD: &str = "\
---
name: paul
display_name: \"Paul\"
description: \"An orchestrator.\"
model: goose-claude-4-6-opus
runtime: goose
extra_key: keep-me
---
You are Paul.
";
// ── rewrite_persona_md unit tests ─────────────────────────────────────────
#[test]
fn test_rewrite_model_provider_joined_and_body_preserved() {
let p = persona(
"Paul",
Some("goose"),
None,
Some("databricks_v2"),
Some("goose-claude-opus-4-8"),
);
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
// Body is byte-preserved.
assert!(
result.ends_with("\nYou are Paul.\n"),
"body not preserved: {result:?}"
);
// model key is the joined form.
assert!(
result.contains("model: databricks_v2:goose-claude-opus-4-8"),
"joined model missing: {result}"
);
// No separate provider key.
assert!(
!result.contains("provider:"),
"separate provider key must not be emitted: {result}"
);
// Unrelated key preserved.
assert!(
result.contains("extra_key: keep-me"),
"extra key lost: {result}"
);
// Still valid frontmatter (parses cleanly).
assert!(result.starts_with("---\n"), "must start with ---");
}
#[test]
fn test_rewrite_bare_model_when_provider_none() {
let p = persona("Paul", Some("goose"), None, None, Some("bare-model-id"));
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
assert!(
result.contains("model: bare-model-id"),
"bare model missing: {result}"
);
assert!(
!result.contains("provider:"),
"provider key must not be emitted: {result}"
);
}
#[test]
fn test_rewrite_runtime_removed_when_none() {
let p = persona("Paul", None, None, None, Some("some-model"));
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
// runtime was in source but persona.runtime is None — key must be removed.
assert!(
!result.contains("runtime:"),
"runtime key should be removed when None: {result}"
);
}
#[test]
fn test_rewrite_preserves_description_and_name_and_extra_keys() {
let p = persona(
"Paul Updated",
Some("goose"),
None,
Some("anthropic"),
Some("claude-opus-4"),
);
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
// name and description are not persona record fields — must survive untouched.
assert!(result.contains("name: paul"), "name key lost: {result}");
assert!(
result.contains("description:"),
"description key lost: {result}"
);
assert!(
result.contains("extra_key: keep-me"),
"extra_key lost: {result}"
);
// display_name updated.
assert!(
result.contains("display_name: Paul Updated")
|| result.contains("display_name: \"Paul Updated\""),
"display_name not updated: {result}"
);
}
#[test]
fn test_rewrite_no_provider_no_model_removes_model_key() {
let p = persona("Paul", None, None, None, None);
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
// When both provider and model are cleared, the model key is removed.
assert!(
!result.contains("model:"),
"model key should be removed when both absent: {result}"
);
}
#[test]
fn test_rewrite_avatar_set_and_cleared() {
let with_avatar = persona(
"Paul",
Some("goose"),
Some("data:image/png;base64,abc"),
Some("openai"),
Some("gpt-4o"),
);
let result = rewrite_persona_md(SAMPLE_MD, &with_avatar, "You are Paul.\n", None).unwrap();
assert!(
result.contains("avatar:"),
"avatar key should be set: {result}"
);
let without_avatar = persona("Paul", Some("goose"), None, Some("openai"), Some("gpt-4o"));
let result =
rewrite_persona_md(SAMPLE_MD, &without_avatar, "You are Paul.\n", None).unwrap();
assert!(
!result.contains("avatar:"),
"avatar key should be absent when None: {result}"
);
}
#[test]
fn test_rewrite_body_not_replaced_by_system_prompt() {
// system_prompt on the PersonaRecord is the COMPOSED prompt (body + pack instructions).
// The body of the .persona.md must not be replaced with it.
let p = persona(
"Paul",
Some("goose"),
None,
Some("databricks_v2"),
Some("goose-claude-opus-4-8"),
);
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
// The raw body from the file ("You are Paul.") is preserved.
assert!(
result.ends_with("You are Paul.\n"),
"raw body must be preserved, not replaced by composed system_prompt: {result:?}"
);
// The composed instructions suffix must NOT appear in the file body.
assert!(
!result.contains("# Team Instructions"),
"composed system_prompt must not be written to body: {result}"
);
}
// ── body (system_prompt) write-back tests ─────────────────────────────────
//
// These tests exercise the compose_prompt inversion logic in rewrite_persona_md.
/// Frontmatter-only MD (no body to preserve) for prompt tests.
const PROMPT_MD: &str = "\
---
name: paul
display_name: \"Paul\"
model: goose-claude-4-6-opus
---
You are Paul.
";
#[test]
fn test_prompt_edited_with_pack_instructions_body_rewritten() {
// User edits the prompt. system_prompt = new_raw_body + separator + instructions.
// Body in file should be updated to new_raw_body; Team Instructions must NOT appear.
let instructions = "Follow the rules.";
let new_raw_body = "You are Paul, a wise orchestrator.";
let composed = format!("{new_raw_body}\n\n---\n# Team Instructions\n{instructions}");
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
p.system_prompt = composed;
let result =
rewrite_persona_md(PROMPT_MD, &p, "You are Paul.", Some(instructions)).unwrap();
assert!(
result.ends_with("You are Paul, a wise orchestrator."),
"new body not written: {result:?}"
);
assert!(
!result.contains("# Team Instructions"),
"Team Instructions must not appear in body: {result}"
);
}
#[test]
fn test_prompt_edited_no_pack_instructions_body_rewritten_verbatim() {
// No pack instructions: composed == raw. New body is system_prompt verbatim.
let new_raw_body = "You are Paul, updated.";
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
p.system_prompt = new_raw_body.to_string();
let result = rewrite_persona_md(PROMPT_MD, &p, "You are Paul.", None).unwrap();
assert!(
result.ends_with("You are Paul, updated."),
"body not updated: {result:?}"
);
}
#[test]
fn test_prompt_unedited_body_preserved() {
// system_prompt equals compose_prompt(current_raw_body, instructions) exactly.
// The body section must not change even though frontmatter may be rewritten.
let instructions = "Follow the rules.";
let raw_body = "You are Paul.";
let composed = format!("{raw_body}\n\n---\n# Team Instructions\n{instructions}");
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
// Frontmatter also unchanged (matches PROMPT_MD).
p.system_prompt = composed;
let result = rewrite_persona_md(PROMPT_MD, &p, raw_body, Some(instructions)).unwrap();
// Body must remain "You are Paul." (no prompt edit — body section preserved).
assert!(
result.ends_with("You are Paul.\n"),
"body must be unchanged: {result:?}"
);
// Team Instructions must not leak into the body.
assert!(
!result.contains("# Team Instructions"),
"Team Instructions must not appear: {result}"
);
}
#[test]
fn test_prompt_safety_guard_missing_suffix_preserves_body() {
// pack_instructions is non-empty but system_prompt does NOT end with the
// expected suffix (user edited inside the Team Instructions block, or the
// instructions drifted). The existing body must be preserved — no corruption.
let instructions = "Follow the rules.";
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
// system_prompt lacks the expected suffix entirely.
p.system_prompt = "Some rogue prompt with # Team Instructions in the middle".to_string();
let result =
rewrite_persona_md(PROMPT_MD, &p, "You are Paul.", Some(instructions)).unwrap();
// Body preserved from the file.
assert!(
result.ends_with("You are Paul.\n"),
"body must be preserved by safety guard: {result:?}"
);
}
#[test]
fn test_prompt_round_trip_no_double_append() {
// After write-back, running compose_prompt on the written body + instructions
// must reproduce the stored system_prompt exactly. This proves no double-append.
let instructions = "Follow the rules.";
let new_raw_body = "You are Paul, updated for the round-trip.";
let composed = format!("{new_raw_body}\n\n---\n# Team Instructions\n{instructions}");
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
p.system_prompt = composed.clone();
let result =
rewrite_persona_md(PROMPT_MD, &p, "You are Paul.", Some(instructions)).unwrap();
// Extract the written body (everything after the last "---\n").
let written_body = result.split("---\n").last().unwrap();
// Re-compose: body + instructions must equal the original composed prompt.
let recomposed = format!("{written_body}\n\n---\n# Team Instructions\n{instructions}");
assert_eq!(
recomposed, composed,
"round-trip failed — double-append would occur: recomposed={recomposed:?}"
);
}
#[test]
fn test_prompt_edited_with_trailing_newline_in_instructions() {
// Regression: normal instructions.md files have a trailing newline.
// compose_prompt includes it verbatim; system_prompt must NOT be trimmed
// in update_persona or the suffix-strip fails and the safety guard fires.
//
// This test verifies that pack_instructions with a trailing "\n" still
// decomposes correctly — i.e., the body is rewritten, not preserved.
let instructions = "Follow the rules.\n"; // trailing newline from file read
let new_raw_body = "You are Paul, rewritten.";
let composed = format!("{new_raw_body}\n\n---\n# Team Instructions\n{instructions}");
// system_prompt is NOT trimmed (update_persona must preserve it as-is).
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
p.system_prompt = composed;
let result =
rewrite_persona_md(PROMPT_MD, &p, "You are Paul.", Some(instructions)).unwrap();
assert!(
result.contains("You are Paul, rewritten."),
"body not rewritten with trailing-newline instructions: {result:?}"
);
assert!(
!result.contains("# Team Instructions"),
"Team Instructions must not appear in body: {result}"
);
}
// ── write_back_persona_md path-resolution tests ───────────────────────────
//
// These tests verify that write_back_persona_md resolves the source file via
// the pack manifest rather than by convention. This is the class of bug that
// Thufir's IMPORTANT finding caught: a pack whose manifest points at
// `personas/foo.persona.md` (not `agents/<slug>.persona.md`) must still be
// rewritten correctly.
use tempfile::TempDir;
/// Build a minimal pack on disk with the given personas layout.
///
/// `persona_entries` is a list of (manifest_rel_path, file_content) pairs.
/// The manifest lists the relative paths; the files are written verbatim.
fn make_temp_pack(persona_entries: &[(&str, &str)]) -> TempDir {
let dir = TempDir::new().expect("tempdir");
let root = dir.path();
std::fs::create_dir_all(root.join(".plugin")).unwrap();
let persona_paths: Vec<&str> = persona_entries.iter().map(|(p, _)| *p).collect();
let manifest = serde_json::json!({
"id": "test-team",
"name": "Test Team",
"version": "0.1.0",
"personas": persona_paths,
});
std::fs::write(
root.join(".plugin/plugin.json"),
serde_json::to_string_pretty(&manifest).unwrap(),
)
.unwrap();
for (rel_path, content) in persona_entries {
let abs = root.join(rel_path);
std::fs::create_dir_all(abs.parent().unwrap()).unwrap();
std::fs::write(&abs, content).unwrap();
}
dir
}
#[test]
fn test_writeback_uses_manifest_path_not_convention() {
// Pack uses `personas/paul.persona.md` layout (not `agents/`).
// Convention-based path would derive `agents/paul.persona.md` (wrong).
// Manifest-based resolution must write to the correct `personas/` path.
let persona_md = "\
---
name: paul
display_name: \"Paul\"
description: \"Orchestrator\"
model: goose-claude-4-6-opus
---
You are Paul.
";
let dir = make_temp_pack(&[("personas/paul.persona.md", persona_md)]);
let source_dir = dir.path().to_path_buf();
let pack = buzz_persona_pkg::pack::load_pack(&source_dir).unwrap();
assert_eq!(pack.personas[0].name, "paul");
let source_path = pack.personas[0].source_path.clone();
// File lives under personas/, not agents/. Assert on path components so
// the check is separator-agnostic (Windows uses `\`, not `/`).
assert_eq!(
source_path.file_name().and_then(|n| n.to_str()),
Some("paul.persona.md"),
"expected paul.persona.md: {}",
source_path.display()
);
assert_eq!(
source_path
.parent()
.and_then(|p| p.file_name())
.and_then(|n| n.to_str()),
Some("personas"),
"expected personas/ layout: {}",
source_path.display()
);
// Simulate what write_back_persona_md does after the fix:
// use source_path from pack, not source_dir/agents/<slug>.persona.md
let mut p = persona(
"Paul Updated",
Some("goose"),
None,
Some("databricks_v2"),
Some("goose-claude-opus-4-8"),
);
p.source_team_persona_slug = Some("paul".to_string());
let content = std::fs::read_to_string(&source_path).unwrap();
let updated = rewrite_persona_md(&content, &p, "You are Paul.\n", None).unwrap();
std::fs::write(&source_path, &updated).unwrap();
let after = std::fs::read_to_string(&source_path).unwrap();
assert!(
after.contains("databricks_v2:goose-claude-opus-4-8"),
"model not written: {after}"
);
assert!(
after.ends_with("You are Paul.\n"),
"body not preserved: {after:?}"
);
}
#[test]
fn test_writeback_name_differs_from_filename() {
// Pack file is `personas/orchestrator.persona.md` but `name: paul`.
// Slug matches `name:`, not the filename.
let persona_md = "\
---
name: paul
display_name: \"Paul\"
description: \"Orchestrator\"
model: old-model
---
You are Paul.
";
let dir = make_temp_pack(&[("personas/orchestrator.persona.md", persona_md)]);
let source_dir = dir.path().to_path_buf();
let pack = buzz_persona_pkg::pack::load_pack(&source_dir).unwrap();
let loaded = pack.personas.iter().find(|p| p.name == "paul").unwrap();
let source_path = loaded.source_path.clone();
// File basename is orchestrator, not paul
assert!(
source_path
.to_string_lossy()
.contains("orchestrator.persona.md"),
"expected orchestrator.persona.md: {}",
source_path.display()
);
let mut p = persona(
"Paul",
Some("goose"),
None,
Some("anthropic"),
Some("claude-4"),
);
p.source_team_persona_slug = Some("paul".to_string());
let content = std::fs::read_to_string(&source_path).unwrap();
let updated = rewrite_persona_md(&content, &p, "You are Paul.\n", None).unwrap();
std::fs::write(&source_path, &updated).unwrap();
let after = std::fs::read_to_string(&source_path).unwrap();
assert!(
after.contains("anthropic:claude-4"),
"model not written to orchestrator.persona.md: {after}"
);
// The wrong file (paul.persona.md in agents/) must NOT exist.
assert!(
!dir.path().join("agents/paul.persona.md").exists(),
"convention-based path must not be created"
);
}
// ── find_team_for_persona_source tests ────────────────────────────────────
//
// Verify that write_back_persona_md finds teams by team_persona_key, not
// team.id. Legacy/backfilled teams have a UUID `id` while PersonaRecord
// stores the manifest directory name in `source_team`; matching by `id`
// alone silently misses those teams.
fn make_team(id: &str, source_dir: Option<&str>) -> TeamRecord {
TeamRecord {
id: id.to_string(),
name: id.to_string(),
description: None,
persona_ids: vec![],
is_builtin: false,
source_dir: source_dir.map(std::path::PathBuf::from),
is_symlink: false,
symlink_target: None,
version: None,
created_at: "2026-01-01T00:00:00Z".to_string(),
updated_at: "2026-01-01T00:00:00Z".to_string(),
}
}
#[test]
fn test_find_team_legacy_uuid_id_matched_by_source_dir_name() {
// Legacy shape: team.id is a UUID; PersonaRecord.source_team is the
// manifest directory name. The old `team.id == source_team` predicate
// missed this — find_team_for_persona_source must match via source_dir.
let teams = vec![make_team("some-uuid-123", Some("/teams/com.test.pack"))];
let found = find_team_for_persona_source(&teams, "com.test.pack");
assert!(
found.is_some(),
"legacy team must be found by manifest dir name, not UUID"
);
assert_eq!(found.unwrap().id, "some-uuid-123");
}
#[test]
fn test_find_team_modern_id_matched_directly() {
// Modern shape: team.id equals the manifest directory name. Must match.
let teams = vec![make_team("com.test.pack", Some("/teams/com.test.pack"))];
let found = find_team_for_persona_source(&teams, "com.test.pack");
assert!(found.is_some(), "modern team must be found");
}
#[test]
fn test_find_team_manifest_dir_name_matches_regardless_of_uuid_id() {
// Regression: the old predicate `team.id == source_team` missed legacy
// teams when source_team holds the manifest dir name, not the UUID.
// This test confirms that searching by manifest dir name always works
// even when team.id is a UUID.
let teams = vec![make_team("some-uuid-123", Some("/teams/com.test.pack"))];
// source_team holds the manifest dir name "com.test.pack", not the UUID.
let by_dir = find_team_for_persona_source(&teams, "com.test.pack");
assert!(
by_dir.is_some(),
"manifest dir name must find the legacy team"
);
assert_eq!(by_dir.unwrap().id, "some-uuid-123");
}
#[test]
fn test_find_team_no_source_dir_falls_back_to_id() {
// JSON-only team: no source_dir, team_persona_key falls back to id.
let teams = vec![make_team("builtin-team:fizz", None)];
let found = find_team_for_persona_source(&teams, "builtin-team:fizz");
assert!(
found.is_some(),
"no source_dir: must match via team.id fallback"
);
}
#[test]
fn test_find_team_returns_none_when_no_match() {
let teams = vec![make_team("some-uuid-123", Some("/teams/com.test.pack"))];
let not_found = find_team_for_persona_source(&teams, "com.other.pack");
assert!(not_found.is_none(), "unrelated source_team must not match");
}
#[test]
fn test_multi_save_round_trip_short_circuit_holds_both_times() {
// Regression: the `==` short-circuit must hold on the second save too.
//
// Without the frontend fix (removing systemPrompt.trim()), the first
// save stores the trimmed composed prompt; on re-open the form emits the
// trimmed value, which != compose_prompt(body, instructions\n) — the `==`
// check fails, suffix-strip fails on the trimmed string, and the safety
// guard fires on every subsequent save. This test verifies the path stays
// clean across two consecutive no-edit saves.
let instructions = "Follow the rules.\n"; // realistic: trailing newline from file
// `split_frontmatter` strips the "\n" after the closing "---" delimiter
// but preserves the rest of the body verbatim. PROMPT_MD's body line ends
// with "\n", so loaded.prompt at runtime = "You are Paul.\n".
let (_, raw_body) = buzz_persona_pkg::persona::split_frontmatter(PROMPT_MD).unwrap();
// compose_prompt equivalent (must match the real impl exactly)
let composed = format!("{raw_body}\n\n---\n# Team Instructions\n{instructions}");
// ── Save 1: user opens dialog, saves without editing ─────────────────
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
p.system_prompt = composed.clone(); // no frontend trim — system_prompt is the full composed value
let written_1 = rewrite_persona_md(PROMPT_MD, &p, raw_body, Some(instructions)).unwrap();
// Short-circuit must have fired: body is preserved (still "You are Paul.\n").
assert!(
written_1.ends_with("You are Paul.\n"),
"save 1: body must be preserved by == short-circuit: {written_1:?}"
);
assert!(
!written_1.contains("# Team Instructions"),
"save 1: Team Instructions must not appear in file body: {written_1}"
);
// ── Save 2: user re-opens (reads written_1), saves again without editing
// `loaded.prompt` comes from split_frontmatter on the written file — same
// raw_body as before (the write-back preserved it byte-for-byte).
let (_, body_from_file) = buzz_persona_pkg::persona::split_frontmatter(&written_1).unwrap();
assert_eq!(
body_from_file, raw_body,
"split_frontmatter after save 1 must return the original raw_body"
);
// Compose again from the read-back body — must equal the stored system_prompt.
let recomposed = format!("{body_from_file}\n\n---\n# Team Instructions\n{instructions}");
assert_eq!(
recomposed, composed,
"save 2 precondition: recomposed must equal stored system_prompt so == holds"
);
// Now simulate save 2: system_prompt is the recomposed value (no trim).
p.system_prompt = recomposed;
let written_2 =
rewrite_persona_md(&written_1, &p, body_from_file, Some(instructions)).unwrap();
// Short-circuit must fire again: body still preserved, no corruption.
assert!(
written_2.ends_with("You are Paul.\n"),
"save 2: body must still be preserved by == short-circuit: {written_2:?}"
);
assert!(
!written_2.contains("# Team Instructions"),
"save 2: Team Instructions must not appear after second save: {written_2}"
);
// Both writes are byte-identical — no spurious mutations.
assert_eq!(
written_1, written_2,
"save 2: file content must be identical to save 1"
);
}
}
mod tests;
desktop/src-tauri/src/commands/personas/writeback/tests.rs
+847
View File
@@ -0,0 +1,847 @@
use super::*;
use std::collections::BTreeMap;
/// Build a minimal PersonaRecord with the fields that `rewrite_persona_md` reads.
fn persona(
display_name: &str,
runtime: Option<&str>,
avatar_url: Option<&str>,
provider: Option<&str>,
model: Option<&str>,
) -> PersonaRecord {
// system_prompt matches the SAMPLE_MD body so the "no prompt edit" path
// is taken in rewrite_persona_md (body preserved byte-for-byte).
persona_with_prompt(
display_name,
runtime,
avatar_url,
provider,
model,
"You are Paul.\n",
)
}
/// Like `persona` but with an explicit system_prompt value.
fn persona_with_prompt(
display_name: &str,
runtime: Option<&str>,
avatar_url: Option<&str>,
provider: Option<&str>,
model: Option<&str>,
system_prompt: &str,
) -> PersonaRecord {
PersonaRecord {
id: "test-id".to_string(),
display_name: display_name.to_string(),
avatar_url: avatar_url.map(str::to_string),
system_prompt: system_prompt.to_string(),
runtime: runtime.map(str::to_string),
model: model.map(str::to_string),
provider: provider.map(str::to_string),
name_pool: vec![],
is_builtin: false,
is_active: true,
source_team: Some("team-1".to_string()),
source_team_persona_slug: Some("paul".to_string()),
env_vars: BTreeMap::new(),
respond_to: None,
respond_to_allowlist: Vec::new(),
mcp_toolsets: None,
parallelism: None,
created_at: "2025-01-01T00:00:00Z".to_string(),
updated_at: "2025-01-01T00:00:00Z".to_string(),
}
}
const SAMPLE_MD: &str = "\
---
name: paul
display_name: \"Paul\"
description: \"An orchestrator.\"
model: goose-claude-4-6-opus
runtime: goose
extra_key: keep-me
---
You are Paul.
";
// ── rewrite_persona_md unit tests ─────────────────────────────────────────
#[test]
fn test_rewrite_model_provider_joined_and_body_preserved() {
let p = persona(
"Paul",
Some("goose"),
None,
Some("databricks_v2"),
Some("goose-claude-opus-4-8"),
);
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
// Body is byte-preserved.
assert!(
result.ends_with("\nYou are Paul.\n"),
"body not preserved: {result:?}"
);
// model key is the joined form.
assert!(
result.contains("model: databricks_v2:goose-claude-opus-4-8"),
"joined model missing: {result}"
);
// No separate provider key.
assert!(
!result.contains("provider:"),
"separate provider key must not be emitted: {result}"
);
// Unrelated key preserved.
assert!(
result.contains("extra_key: keep-me"),
"extra key lost: {result}"
);
// Still valid frontmatter (parses cleanly).
assert!(result.starts_with("---\n"), "must start with ---");
}
#[test]
fn test_rewrite_bare_model_when_provider_none() {
let p = persona("Paul", Some("goose"), None, None, Some("bare-model-id"));
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
assert!(
result.contains("model: bare-model-id"),
"bare model missing: {result}"
);
assert!(
!result.contains("provider:"),
"provider key must not be emitted: {result}"
);
}
#[test]
fn test_rewrite_runtime_removed_when_none() {
let p = persona("Paul", None, None, None, Some("some-model"));
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
// runtime was in source but persona.runtime is None — key must be removed.
assert!(
!result.contains("runtime:"),
"runtime key should be removed when None: {result}"
);
}
#[test]
fn test_rewrite_preserves_description_and_name_and_extra_keys() {
let p = persona(
"Paul Updated",
Some("goose"),
None,
Some("anthropic"),
Some("claude-opus-4"),
);
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
// name and description are not persona record fields — must survive untouched.
assert!(result.contains("name: paul"), "name key lost: {result}");
assert!(
result.contains("description:"),
"description key lost: {result}"
);
assert!(
result.contains("extra_key: keep-me"),
"extra_key lost: {result}"
);
// display_name updated.
assert!(
result.contains("display_name: Paul Updated")
|| result.contains("display_name: \"Paul Updated\""),
"display_name not updated: {result}"
);
}
#[test]
fn test_rewrite_no_provider_no_model_removes_model_key() {
let p = persona("Paul", None, None, None, None);
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
// When both provider and model are cleared, the model key is removed.
assert!(
!result.contains("model:"),
"model key should be removed when both absent: {result}"
);
}
#[test]
fn test_rewrite_avatar_set_and_cleared() {
let with_avatar = persona(
"Paul",
Some("goose"),
Some("data:image/png;base64,abc"),
Some("openai"),
Some("gpt-4o"),
);
let result = rewrite_persona_md(SAMPLE_MD, &with_avatar, "You are Paul.\n", None).unwrap();
assert!(
result.contains("avatar:"),
"avatar key should be set: {result}"
);
let without_avatar = persona("Paul", Some("goose"), None, Some("openai"), Some("gpt-4o"));
let result = rewrite_persona_md(SAMPLE_MD, &without_avatar, "You are Paul.\n", None).unwrap();
assert!(
!result.contains("avatar:"),
"avatar key should be absent when None: {result}"
);
}
#[test]
fn test_rewrite_body_not_replaced_by_system_prompt() {
// system_prompt on the PersonaRecord is the COMPOSED prompt (body + pack instructions).
// The body of the .persona.md must not be replaced with it.
let p = persona(
"Paul",
Some("goose"),
None,
Some("databricks_v2"),
Some("goose-claude-opus-4-8"),
);
let result = rewrite_persona_md(SAMPLE_MD, &p, "You are Paul.\n", None).unwrap();
// The raw body from the file ("You are Paul.") is preserved.
assert!(
result.ends_with("You are Paul.\n"),
"raw body must be preserved, not replaced by composed system_prompt: {result:?}"
);
// The composed instructions suffix must NOT appear in the file body.
assert!(
!result.contains("# Team Instructions"),
"composed system_prompt must not be written to body: {result}"
);
}
// ── body (system_prompt) write-back tests ─────────────────────────────────
//
// These tests exercise the compose_prompt inversion logic in rewrite_persona_md.
/// Frontmatter-only MD (no body to preserve) for prompt tests.
const PROMPT_MD: &str = "\
---
name: paul
display_name: \"Paul\"
model: goose-claude-4-6-opus
---
You are Paul.
";
#[test]
fn test_prompt_edited_with_pack_instructions_body_rewritten() {
// User edits the prompt. system_prompt = new_raw_body + separator + instructions.
// Body in file should be updated to new_raw_body; Team Instructions must NOT appear.
let instructions = "Follow the rules.";
let new_raw_body = "You are Paul, a wise orchestrator.";
let composed = format!("{new_raw_body}\n\n---\n# Team Instructions\n{instructions}");
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
p.system_prompt = composed;
let result = rewrite_persona_md(PROMPT_MD, &p, "You are Paul.", Some(instructions)).unwrap();
assert!(
result.ends_with("You are Paul, a wise orchestrator."),
"new body not written: {result:?}"
);
assert!(
!result.contains("# Team Instructions"),
"Team Instructions must not appear in body: {result}"
);
}
#[test]
fn test_prompt_edited_no_pack_instructions_body_rewritten_verbatim() {
// No pack instructions: composed == raw. New body is system_prompt verbatim.
let new_raw_body = "You are Paul, updated.";
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
p.system_prompt = new_raw_body.to_string();
let result = rewrite_persona_md(PROMPT_MD, &p, "You are Paul.", None).unwrap();
assert!(
result.ends_with("You are Paul, updated."),
"body not updated: {result:?}"
);
}
#[test]
fn test_prompt_unedited_body_preserved() {
// system_prompt equals compose_prompt(current_raw_body, instructions) exactly.
// The body section must not change even though frontmatter may be rewritten.
let instructions = "Follow the rules.";
let raw_body = "You are Paul.";
let composed = format!("{raw_body}\n\n---\n# Team Instructions\n{instructions}");
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
// Frontmatter also unchanged (matches PROMPT_MD).
p.system_prompt = composed;
let result = rewrite_persona_md(PROMPT_MD, &p, raw_body, Some(instructions)).unwrap();
// Body must remain "You are Paul." (no prompt edit — body section preserved).
assert!(
result.ends_with("You are Paul.\n"),
"body must be unchanged: {result:?}"
);
// Team Instructions must not leak into the body.
assert!(
!result.contains("# Team Instructions"),
"Team Instructions must not appear: {result}"
);
}
#[test]
fn test_prompt_safety_guard_missing_suffix_preserves_body() {
// pack_instructions is non-empty but system_prompt does NOT end with the
// expected suffix (user edited inside the Team Instructions block, or the
// instructions drifted). The existing body must be preserved — no corruption.
let instructions = "Follow the rules.";
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
// system_prompt lacks the expected suffix entirely.
p.system_prompt = "Some rogue prompt with # Team Instructions in the middle".to_string();
let result = rewrite_persona_md(PROMPT_MD, &p, "You are Paul.", Some(instructions)).unwrap();
// Body preserved from the file.
assert!(
result.ends_with("You are Paul.\n"),
"body must be preserved by safety guard: {result:?}"
);
}
#[test]
fn test_prompt_round_trip_no_double_append() {
// After write-back, running compose_prompt on the written body + instructions
// must reproduce the stored system_prompt exactly. This proves no double-append.
let instructions = "Follow the rules.";
let new_raw_body = "You are Paul, updated for the round-trip.";
let composed = format!("{new_raw_body}\n\n---\n# Team Instructions\n{instructions}");
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
p.system_prompt = composed.clone();
let result = rewrite_persona_md(PROMPT_MD, &p, "You are Paul.", Some(instructions)).unwrap();
// Extract the written body (everything after the last "---\n").
let written_body = result.split("---\n").last().unwrap();
// Re-compose: body + instructions must equal the original composed prompt.
let recomposed = format!("{written_body}\n\n---\n# Team Instructions\n{instructions}");
assert_eq!(
recomposed, composed,
"round-trip failed — double-append would occur: recomposed={recomposed:?}"
);
}
#[test]
fn test_prompt_edited_with_trailing_newline_in_instructions() {
// Regression: normal instructions.md files have a trailing newline.
// compose_prompt includes it verbatim; system_prompt must NOT be trimmed
// in update_persona or the suffix-strip fails and the safety guard fires.
//
// This test verifies that pack_instructions with a trailing "\n" still
// decomposes correctly — i.e., the body is rewritten, not preserved.
let instructions = "Follow the rules.\n"; // trailing newline from file read
let new_raw_body = "You are Paul, rewritten.";
let composed = format!("{new_raw_body}\n\n---\n# Team Instructions\n{instructions}");
// system_prompt is NOT trimmed (update_persona must preserve it as-is).
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
p.system_prompt = composed;
let result = rewrite_persona_md(PROMPT_MD, &p, "You are Paul.", Some(instructions)).unwrap();
assert!(
result.contains("You are Paul, rewritten."),
"body not rewritten with trailing-newline instructions: {result:?}"
);
assert!(
!result.contains("# Team Instructions"),
"Team Instructions must not appear in body: {result}"
);
}
// ── write_back_persona_md path-resolution tests ───────────────────────────
//
// These tests verify that write_back_persona_md resolves the source file via
// the pack manifest rather than by convention. This is the class of bug that
// Thufir's IMPORTANT finding caught: a pack whose manifest points at
// `personas/foo.persona.md` (not `agents/<slug>.persona.md`) must still be
// rewritten correctly.
use tempfile::TempDir;
/// Build a minimal pack on disk with the given personas layout.
///
/// `persona_entries` is a list of (manifest_rel_path, file_content) pairs.
/// The manifest lists the relative paths; the files are written verbatim.
fn make_temp_pack(persona_entries: &[(&str, &str)]) -> TempDir {
let dir = TempDir::new().expect("tempdir");
let root = dir.path();
std::fs::create_dir_all(root.join(".plugin")).unwrap();
let persona_paths: Vec<&str> = persona_entries.iter().map(|(p, _)| *p).collect();
let manifest = serde_json::json!({
"id": "test-team",
"name": "Test Team",
"version": "0.1.0",
"personas": persona_paths,
});
std::fs::write(
root.join(".plugin/plugin.json"),
serde_json::to_string_pretty(&manifest).unwrap(),
)
.unwrap();
for (rel_path, content) in persona_entries {
let abs = root.join(rel_path);
std::fs::create_dir_all(abs.parent().unwrap()).unwrap();
std::fs::write(&abs, content).unwrap();
}
dir
}
#[test]
fn test_writeback_uses_manifest_path_not_convention() {
// Pack uses `personas/paul.persona.md` layout (not `agents/`).
// Convention-based path would derive `agents/paul.persona.md` (wrong).
// Manifest-based resolution must write to the correct `personas/` path.
let persona_md = "\
---
name: paul
display_name: \"Paul\"
description: \"Orchestrator\"
model: goose-claude-4-6-opus
---
You are Paul.
";
let dir = make_temp_pack(&[("personas/paul.persona.md", persona_md)]);
let source_dir = dir.path().to_path_buf();
let pack = buzz_persona_pkg::pack::load_pack(&source_dir).unwrap();
assert_eq!(pack.personas[0].name, "paul");
let source_path = pack.personas[0].source_path.clone();
// File lives under personas/, not agents/. Assert on path components so
// the check is separator-agnostic (Windows uses `\`, not `/`).
assert_eq!(
source_path.file_name().and_then(|n| n.to_str()),
Some("paul.persona.md"),
"expected paul.persona.md: {}",
source_path.display()
);
assert_eq!(
source_path
.parent()
.and_then(|p| p.file_name())
.and_then(|n| n.to_str()),
Some("personas"),
"expected personas/ layout: {}",
source_path.display()
);
// Simulate what write_back_persona_md does after the fix:
// use source_path from pack, not source_dir/agents/<slug>.persona.md
let mut p = persona(
"Paul Updated",
Some("goose"),
None,
Some("databricks_v2"),
Some("goose-claude-opus-4-8"),
);
p.source_team_persona_slug = Some("paul".to_string());
let content = std::fs::read_to_string(&source_path).unwrap();
let updated = rewrite_persona_md(&content, &p, "You are Paul.\n", None).unwrap();
std::fs::write(&source_path, &updated).unwrap();
let after = std::fs::read_to_string(&source_path).unwrap();
assert!(
after.contains("databricks_v2:goose-claude-opus-4-8"),
"model not written: {after}"
);
assert!(
after.ends_with("You are Paul.\n"),
"body not preserved: {after:?}"
);
}
#[test]
fn test_writeback_name_differs_from_filename() {
// Pack file is `personas/orchestrator.persona.md` but `name: paul`.
// Slug matches `name:`, not the filename.
let persona_md = "\
---
name: paul
display_name: \"Paul\"
description: \"Orchestrator\"
model: old-model
---
You are Paul.
";
let dir = make_temp_pack(&[("personas/orchestrator.persona.md", persona_md)]);
let source_dir = dir.path().to_path_buf();
let pack = buzz_persona_pkg::pack::load_pack(&source_dir).unwrap();
let loaded = pack.personas.iter().find(|p| p.name == "paul").unwrap();
let source_path = loaded.source_path.clone();
// File basename is orchestrator, not paul
assert!(
source_path
.to_string_lossy()
.contains("orchestrator.persona.md"),
"expected orchestrator.persona.md: {}",
source_path.display()
);
let mut p = persona(
"Paul",
Some("goose"),
None,
Some("anthropic"),
Some("claude-4"),
);
p.source_team_persona_slug = Some("paul".to_string());
let content = std::fs::read_to_string(&source_path).unwrap();
let updated = rewrite_persona_md(&content, &p, "You are Paul.\n", None).unwrap();
std::fs::write(&source_path, &updated).unwrap();
let after = std::fs::read_to_string(&source_path).unwrap();
assert!(
after.contains("anthropic:claude-4"),
"model not written to orchestrator.persona.md: {after}"
);
// The wrong file (paul.persona.md in agents/) must NOT exist.
assert!(
!dir.path().join("agents/paul.persona.md").exists(),
"convention-based path must not be created"
);
}
// ── find_team_for_persona_source tests ────────────────────────────────────
//
// Verify that write_back_persona_md finds teams by team_persona_key, not
// team.id. Legacy/backfilled teams have a UUID `id` while PersonaRecord
// stores the manifest directory name in `source_team`; matching by `id`
// alone silently misses those teams.
fn make_team(id: &str, source_dir: Option<&str>) -> TeamRecord {
TeamRecord {
id: id.to_string(),
name: id.to_string(),
description: None,
persona_ids: vec![],
is_builtin: false,
source_dir: source_dir.map(std::path::PathBuf::from),
is_symlink: false,
symlink_target: None,
version: None,
created_at: "2026-01-01T00:00:00Z".to_string(),
updated_at: "2026-01-01T00:00:00Z".to_string(),
}
}
fn make_team_with_path(id: &str, source_dir: &std::path::Path) -> TeamRecord {
TeamRecord {
id: id.to_string(),
name: id.to_string(),
description: None,
persona_ids: vec![],
is_builtin: false,
source_dir: Some(source_dir.to_path_buf()),
is_symlink: false,
symlink_target: None,
version: None,
created_at: "2026-01-01T00:00:00Z".to_string(),
updated_at: "2026-01-01T00:00:00Z".to_string(),
}
}
#[test]
fn test_find_team_legacy_uuid_id_matched_by_source_dir_name() {
// Legacy shape: team.id is a UUID; PersonaRecord.source_team is the
// manifest directory name. The old `team.id == source_team` predicate
// missed this — find_team_for_persona_source must match via source_dir.
let teams = vec![make_team("some-uuid-123", Some("/teams/com.test.pack"))];
let found = find_team_for_persona_source(&teams, "com.test.pack");
assert!(
found.is_some(),
"legacy team must be found by manifest dir name, not UUID"
);
assert_eq!(found.unwrap().id, "some-uuid-123");
}
#[test]
fn test_find_team_modern_id_matched_directly() {
// Modern shape: team.id equals the manifest directory name. Must match.
let teams = vec![make_team("com.test.pack", Some("/teams/com.test.pack"))];
let found = find_team_for_persona_source(&teams, "com.test.pack");
assert!(found.is_some(), "modern team must be found");
}
#[test]
fn test_find_team_manifest_dir_name_matches_regardless_of_uuid_id() {
// Regression: the old predicate `team.id == source_team` missed legacy
// teams when source_team holds the manifest dir name, not the UUID.
// This test confirms that searching by manifest dir name always works
// even when team.id is a UUID.
let teams = vec![make_team("some-uuid-123", Some("/teams/com.test.pack"))];
// source_team holds the manifest dir name "com.test.pack", not the UUID.
let by_dir = find_team_for_persona_source(&teams, "com.test.pack");
assert!(
by_dir.is_some(),
"manifest dir name must find the legacy team"
);
assert_eq!(by_dir.unwrap().id, "some-uuid-123");
}
#[test]
fn test_find_team_no_source_dir_falls_back_to_id() {
// JSON-only team: no source_dir, team_persona_key falls back to id.
let teams = vec![make_team("builtin-team:fizz", None)];
let found = find_team_for_persona_source(&teams, "builtin-team:fizz");
assert!(
found.is_some(),
"no source_dir: must match via team.id fallback"
);
}
#[test]
fn test_find_team_returns_none_when_no_match() {
let teams = vec![make_team("some-uuid-123", Some("/teams/com.test.pack"))];
let not_found = find_team_for_persona_source(&teams, "com.other.pack");
assert!(not_found.is_none(), "unrelated source_team must not match");
}
// ── legacy symlink shape + missing-runtime insert ─────────────────────────
//
// Regression: write-back for a team whose `id` is a UUID (legacy backfill)
// and whose `source_dir` is a path whose FINAL component is the
// team_persona_key (e.g. `.../agents/teams/com.wpfleger.sietch-tabr`), with
// that path being a SYMLINK to the real pack directory. The persona's
// `source_team` stores the key, not the UUID, and the pack file has no
// `runtime:` frontmatter key (write-back must INSERT it).
/// Build a PersonaRecord matching the legacy incident shape.
fn legacy_persona(source_team: &str, runtime: Option<&str>) -> PersonaRecord {
PersonaRecord {
id: "ab5c038c-1b12-46e2-8283-d6f7c0606fce".to_string(),
display_name: "Paul Updated".to_string(),
avatar_url: None,
system_prompt: "You are Paul.\n".to_string(),
runtime: runtime.map(str::to_string),
model: Some("claude-opus-4-8".to_string()),
provider: Some("anthropic".to_string()),
name_pool: vec![],
is_builtin: false,
is_active: true,
source_team: Some(source_team.to_string()),
source_team_persona_slug: Some("paul".to_string()),
env_vars: BTreeMap::new(),
respond_to: None,
respond_to_allowlist: Vec::new(),
mcp_toolsets: None,
parallelism: None,
created_at: "2026-01-01T00:00:00Z".to_string(),
updated_at: "2026-01-01T00:00:00Z".to_string(),
}
}
#[test]
fn test_writeback_legacy_symlink_uuid_team_inserts_missing_runtime() {
// Real-world shape: pack lives in a real directory (pack_dir), and the
// team's source_dir is a SYMLINK at `<link_parent>/com.wpfleger.sietch-tabr`
// pointing to pack_dir. The team record has a UUID id; persona.source_team
// is the link basename (the team_persona_key), not the UUID.
let pack_dir = TempDir::new().expect("pack_dir tempdir");
let link_parent = TempDir::new().expect("link_parent tempdir");
// paul.persona.md has NO runtime: key (write-back must INSERT it).
let persona_md = "\
---
name: paul
display_name: \"Paul\"
description: \"Orchestrator\"
---
You are Paul.
";
// Build a minimal pack in pack_dir.
std::fs::create_dir_all(pack_dir.path().join(".plugin")).unwrap();
let manifest = serde_json::json!({
"id": "com.wpfleger.sietch-tabr",
"name": "Sietch Tabr",
"version": "0.1.0",
"personas": ["paul.persona.md"],
});
std::fs::write(
pack_dir.path().join(".plugin/plugin.json"),
serde_json::to_string_pretty(&manifest).unwrap(),
)
.unwrap();
std::fs::write(pack_dir.path().join("paul.persona.md"), persona_md).unwrap();
// Create the symlink: `<link_parent>/com.wpfleger.sietch-tabr` → pack_dir
let symlink_path = link_parent.path().join("com.wpfleger.sietch-tabr");
#[cfg(unix)]
std::os::unix::fs::symlink(pack_dir.path(), &symlink_path).expect("symlink");
#[cfg(not(unix))]
{
// On non-Unix, skip symlink traversal test (macOS-only scenario).
return;
}
// Team: UUID id, source_dir = symlink path (basename = team_persona_key).
let team = TeamRecord {
id: "ab5c038c-1b12-46e2-8283-d6f7c0606fce".to_string(),
name: "Sietch Tabr".to_string(),
description: None,
persona_ids: vec![],
is_builtin: false,
source_dir: Some(symlink_path.clone()),
is_symlink: true,
symlink_target: Some(pack_dir.path().to_string_lossy().to_string()),
version: None,
created_at: "2026-01-01T00:00:00Z".to_string(),
updated_at: "2026-01-01T00:00:00Z".to_string(),
};
let teams = vec![team];
// persona.source_team = the basename = team_persona_key
let persona = legacy_persona("com.wpfleger.sietch-tabr", Some("goose"));
let result = try_write_back_persona_md(&teams, &persona);
assert!(
result.is_ok(),
"write-back through symlink must succeed: {result:?}"
);
// Verify runtime was inserted into the pack file.
let written = std::fs::read_to_string(pack_dir.path().join("paul.persona.md")).unwrap();
assert!(
written.contains("runtime: goose"),
"runtime must be inserted into the file: {written}"
);
assert!(
written.contains("display_name:"),
"display_name must be present: {written}"
);
}
#[cfg(unix)]
#[test]
fn test_writeback_readonly_pack_file_returns_warning() {
use std::os::unix::fs::PermissionsExt;
// Pack with a persona that has a different display_name than what we'll
// request — this forces a write (not a no-op short-circuit).
let persona_md = "\
---
name: paul
display_name: \"Paul Old\"
description: \"Orchestrator\"
runtime: goose
---
You are Paul.
";
let dir = make_temp_pack(&[("paul.persona.md", persona_md)]);
let source_dir = dir.path().to_path_buf();
// Make the file read-only.
let paul_path = dir.path().join("paul.persona.md");
let mut perms = std::fs::metadata(&paul_path).unwrap().permissions();
perms.set_mode(0o444);
std::fs::set_permissions(&paul_path, perms).unwrap();
let team = make_team_with_path("test-team", &source_dir);
let teams = vec![team];
// Persona with updated display_name — write is needed but will be denied.
let mut persona = persona("Paul New", Some("goose"), None, None, Some("some-model"));
persona.source_team = Some("test-team".to_string());
persona.source_team_persona_slug = Some("paul".to_string());
let result = try_write_back_persona_md(&teams, &persona);
assert!(
result.is_err(),
"read-only pack file must return an error: {result:?}"
);
let warning = result.unwrap_err();
assert!(
warning.contains("write"),
"error must mention the write failure: {warning}"
);
}
#[test]
fn test_multi_save_round_trip_short_circuit_holds_both_times() {
// Regression: the `==` short-circuit must hold on the second save too.
//
// Without the frontend fix (removing systemPrompt.trim()), the first
// save stores the trimmed composed prompt; on re-open the form emits the
// trimmed value, which != compose_prompt(body, instructions\n) — the `==`
// check fails, suffix-strip fails on the trimmed string, and the safety
// guard fires on every subsequent save. This test verifies the path stays
// clean across two consecutive no-edit saves.
let instructions = "Follow the rules.\n"; // realistic: trailing newline from file
// `split_frontmatter` strips the "\n" after the closing "---" delimiter
// but preserves the rest of the body verbatim. PROMPT_MD's body line ends
// with "\n", so loaded.prompt at runtime = "You are Paul.\n".
let (_, raw_body) = buzz_persona_pkg::persona::split_frontmatter(PROMPT_MD).unwrap();
// compose_prompt equivalent (must match the real impl exactly)
let composed = format!("{raw_body}\n\n---\n# Team Instructions\n{instructions}");
// ── Save 1: user opens dialog, saves without editing ─────────────────
let mut p = persona("Paul", None, None, None, Some("goose-claude-4-6-opus"));
p.system_prompt = composed.clone(); // no frontend trim — system_prompt is the full composed value
let written_1 = rewrite_persona_md(PROMPT_MD, &p, raw_body, Some(instructions)).unwrap();
// Short-circuit must have fired: body is preserved (still "You are Paul.\n").
assert!(
written_1.ends_with("You are Paul.\n"),
"save 1: body must be preserved by == short-circuit: {written_1:?}"
);
assert!(
!written_1.contains("# Team Instructions"),
"save 1: Team Instructions must not appear in file body: {written_1}"
);
// ── Save 2: user re-opens (reads written_1), saves again without editing
// `loaded.prompt` comes from split_frontmatter on the written file — same
// raw_body as before (the write-back preserved it byte-for-byte).
let (_, body_from_file) = buzz_persona_pkg::persona::split_frontmatter(&written_1).unwrap();
assert_eq!(
body_from_file, raw_body,
"split_frontmatter after save 1 must return the original raw_body"
);
// Compose again from the read-back body — must equal the stored system_prompt.
let recomposed = format!("{body_from_file}\n\n---\n# Team Instructions\n{instructions}");
assert_eq!(
recomposed, composed,
"save 2 precondition: recomposed must equal stored system_prompt so == holds"
);
// Now simulate save 2: system_prompt is the recomposed value (no trim).
p.system_prompt = recomposed;
let written_2 = rewrite_persona_md(&written_1, &p, body_from_file, Some(instructions)).unwrap();
// Short-circuit must fire again: body still preserved, no corruption.
assert!(
written_2.ends_with("You are Paul.\n"),
"save 2: body must still be preserved by == short-circuit: {written_2:?}"
);
assert!(
!written_2.contains("# Team Instructions"),
"save 2: Team Instructions must not appear after second save: {written_2}"
);
// Both writes are byte-identical — no spurious mutations.
assert_eq!(
written_1, written_2,
"save 2: file content must be identical to save 1"
);
}
desktop/src-tauri/src/managed_agents/discovery.rs
+2 -119
View File
@@ -348,125 +348,8 @@ pub fn effective_agent_command(
.unwrap_or_else(default_agent_command)
}
/// Decide whether a user-picked harness command is an explicit per-instance
/// pin or merely the persona's own runtime restated. Returns the override to
/// persist: `Some(picked)` when it diverges from the persona, `None` when it
/// inherits.
///
/// Comparison is by RUNTIME IDENTITY, not raw string: a persona on the `claude`
/// runtime resolves to `claude-agent-acp`, but a client with only the
/// `claude-code-acp` adapter installed sends that command instead. Both map to
/// the same `claude` runtime, so neither is a real divergence — string equality
/// would wrongly bake a pin. An unknown/custom command (no matching runtime)
/// only inherits when it exactly equals the persona command.
pub fn divergent_agent_command_override(
persona_id: Option<&str>,
personas: &[crate::managed_agents::types::PersonaRecord],
picked_command: Option<&str>,
) -> Option<String> {
let picked = picked_command
.map(str::trim)
.filter(|value| !value.is_empty())?;
let persona_command = effective_agent_command(persona_id, personas, None);
let same_runtime = match (
known_acp_runtime(picked),
known_acp_runtime(&persona_command),
) {
(Some(a), Some(b)) => std::ptr::eq(a, b),
_ => picked == persona_command,
};
if same_runtime {
None
} else {
Some(picked.to_string())
}
}
/// Decide the `agent_command_override` to persist at AGENT UPDATE time.
///
/// The edit dialog sends `agent_command` as a tri-state string: the empty
/// "inherit from persona" sentinel (clear the pin), or a concrete command
/// (pin). Resolution:
///
/// - EMPTY / whitespace → the inherit sentinel: always `None` regardless of
/// `harness_override`, so toggling "Inherit runtime from persona" clears the
/// pin.
/// - DELIBERATE OVERRIDE (`harness_override` true, persona linked): the user
/// explicitly picked a runtime/Custom command in the dialog. This is a real
/// pin and is preserved VERBATIM — even when the picked command maps to, or
/// is byte-identical to, the persona's own runtime command. Selecting "Custom
/// command" and saving e.g. `goose` for a goose persona is a deliberate act
/// to freeze the harness against future persona runtime edits; dropping it
/// back to inherit (as [`divergent_agent_command_override`] would) defeats
/// that intent. Unlike the create-time path, there is no byte-identical
/// exception here: at create the command is machine-derived from the persona,
/// so equality means "no user divergence"; at update an equal command reached
/// the force branch only because the user picked Custom, which IS the
/// divergence.
/// - NO OVERRIDE INTENT (`harness_override` false) or NO PERSONA: defer to
/// [`divergent_agent_command_override`], which keeps the persona authoritative
/// and treats a same-runtime restatement as inherit.
pub fn update_time_agent_command_override(
persona_id: Option<&str>,
personas: &[crate::managed_agents::types::PersonaRecord],
picked_command: Option<&str>,
harness_override: bool,
) -> Option<String> {
let picked = picked_command
.map(str::trim)
.filter(|value| !value.is_empty())?;
if persona_id.is_some() && harness_override {
return Some(picked.to_string());
}
divergent_agent_command_override(persona_id, personas, Some(picked))
}
/// Decide the `agent_command_override` to persist at AGENT CREATE time.
///
/// A persona-backed create receives its harness command from
/// `resolvePersonaRuntime` (frontend), which produces a divergent command in two
/// distinct cases that the backend MUST tell apart:
///
/// - DELIBERATE OVERRIDE (`harness_override` true): the user explicitly picked a
/// runtime command in UI that exposes a runtime selector. This is a real pin
/// and is preserved when it differs from the command inheritance would spawn,
/// including installed aliases such as `claude-code-acp`.
/// - MISSING-RUNTIME FALLBACK (`harness_override` false): the persona's runtime
/// isn't installed locally, so `resolvePersonaRuntime` substitutes a fallback
/// default. This is NOT a pin — baking it would freeze the agent on the fallback
/// harness even after the persona's runtime is installed and the persona is
/// re-edited, the exact bug this resolver chain exists to prevent. Stores `None`
/// so the persona stays authoritative.
///
/// `isOverridden` from `resolvePersonaRuntime` cannot distinguish these — it is
/// `true` for BOTH — so the caller must thread the explicit user-intent bit.
///
/// Persona-less creates (`persona_id` is `None`, e.g. the standalone
/// CreateAgentDialog) have no persona to inherit, so the picked command is always a
/// real pin and is preserved via `divergent_agent_command_override` regardless of
/// `harness_override`.
pub fn create_time_agent_command_override(
persona_id: Option<&str>,
personas: &[crate::managed_agents::types::PersonaRecord],
picked_command: Option<&str>,
harness_override: bool,
) -> Option<String> {
if persona_id.is_some() && !harness_override {
return None;
}
if persona_id.is_some() && harness_override {
let picked = picked_command
.map(str::trim)
.filter(|value| !value.is_empty())?;
let inherited_command = effective_agent_command(persona_id, personas, None);
return (picked != inherited_command).then(|| picked.to_string());
}
divergent_agent_command_override(persona_id, personas, picked_command)
}
mod overrides;
pub use overrides::{apply_agent_command_update, create_time_agent_command_override};
fn default_agent_args(command: &str) -> Option<Vec<String>> {
match normalize_command_identity(command).as_str() {
desktop/src-tauri/src/managed_agents/discovery/overrides.rs
+153
View File
@@ -0,0 +1,153 @@
//! The `agent_command_override` decision family: divergence comparison,
//! create-time and update-time resolution, and the record apply. Child module
//! of `discovery` (split under the desktop file-size cap) so the override
//! decisions stay next to the resolution ladder they feed.
use super::{effective_agent_command, known_acp_runtime};
/// Decide whether a user-picked harness command is an explicit per-instance
/// pin or merely the persona's own runtime restated. Returns the override to
/// persist: `Some(picked)` when it diverges from the persona, `None` when it
/// inherits.
///
/// Comparison is by RUNTIME IDENTITY, not raw string: a persona on the `claude`
/// runtime resolves to `claude-agent-acp`, but a client with only the
/// `claude-code-acp` adapter installed sends that command instead. Both map to
/// the same `claude` runtime, so neither is a real divergence — string equality
/// would wrongly bake a pin. An unknown/custom command (no matching runtime)
/// only inherits when it exactly equals the persona command.
pub fn divergent_agent_command_override(
persona_id: Option<&str>,
personas: &[crate::managed_agents::types::PersonaRecord],
picked_command: Option<&str>,
) -> Option<String> {
let picked = picked_command
.map(str::trim)
.filter(|value| !value.is_empty())?;
let persona_command = effective_agent_command(persona_id, personas, None);
let same_runtime = match (
known_acp_runtime(picked),
known_acp_runtime(&persona_command),
) {
(Some(a), Some(b)) => std::ptr::eq(a, b),
_ => picked == persona_command,
};
if same_runtime {
None
} else {
Some(picked.to_string())
}
}
/// Decide the `agent_command_override` to persist at AGENT UPDATE time.
///
/// The edit dialog sends `agent_command` as a tri-state string: the empty
/// "inherit from persona" sentinel (clear the pin), or a concrete command
/// (pin). Resolution:
///
/// - EMPTY / whitespace → the inherit sentinel: always `None` regardless of
/// `harness_override`, so toggling "Inherit runtime from persona" clears the
/// pin.
/// - DELIBERATE OVERRIDE (`harness_override` true, persona linked): the user
/// explicitly picked a runtime/Custom command in the dialog. This is a real
/// pin and is preserved VERBATIM — even when the picked command maps to, or
/// is byte-identical to, the persona's own runtime command. Selecting "Custom
/// command" and saving e.g. `goose` for a goose persona is a deliberate act
/// to freeze the harness against future persona runtime edits; dropping it
/// back to inherit (as [`divergent_agent_command_override`] would) defeats
/// that intent. Unlike the create-time path, there is no byte-identical
/// exception here: at create the command is machine-derived from the persona,
/// so equality means "no user divergence"; at update an equal command reached
/// the force branch only because the user picked Custom, which IS the
/// divergence.
/// - NO OVERRIDE INTENT (`harness_override` false) or NO PERSONA: defer to
/// [`divergent_agent_command_override`], which keeps the persona authoritative
/// and treats a same-runtime restatement as inherit.
pub fn update_time_agent_command_override(
persona_id: Option<&str>,
personas: &[crate::managed_agents::types::PersonaRecord],
picked_command: Option<&str>,
harness_override: bool,
) -> Option<String> {
let picked = picked_command
.map(str::trim)
.filter(|value| !value.is_empty())?;
if persona_id.is_some() && harness_override {
return Some(picked.to_string());
}
divergent_agent_command_override(persona_id, personas, Some(picked))
}
/// Apply an explicit `agent_command` edit to `record`: persist the override
/// pin decided by [`update_time_agent_command_override`], and on the inherit
/// sentinel (empty/whitespace command) also clear the materialized
/// `record.runtime` so the resolution ladder falls through to the live
/// definition immediately instead of silently keeping the stale instance copy.
///
/// The runtime clear is guarded on a live persona link: for a definition-less
/// record the materialized runtime is the only harness source left after the
/// override clear, so a stray empty `agent_command` from a non-dialog caller
/// must not change what the agent runs.
pub fn apply_agent_command_update(
record: &mut crate::managed_agents::types::ManagedAgentRecord,
personas: &[crate::managed_agents::types::PersonaRecord],
agent_command: &str,
harness_override: bool,
) {
record.agent_command_override = update_time_agent_command_override(
record.persona_id.as_deref(),
personas,
Some(agent_command),
harness_override,
);
if agent_command.trim().is_empty() && record.persona_id.is_some() {
record.runtime = None;
}
}
/// Decide the `agent_command_override` to persist at AGENT CREATE time.
///
/// A persona-backed create receives its harness command from
/// `resolvePersonaRuntime` (frontend), which produces a divergent command in two
/// distinct cases that the backend MUST tell apart:
///
/// - DELIBERATE OVERRIDE (`harness_override` true): the user explicitly picked a
/// runtime command in UI that exposes a runtime selector. This is a real pin
/// and is preserved when it differs from the command inheritance would spawn,
/// including installed aliases such as `claude-code-acp`.
/// - MISSING-RUNTIME FALLBACK (`harness_override` false): the persona's runtime
/// isn't installed locally, so `resolvePersonaRuntime` substitutes a fallback
/// default. This is NOT a pin — baking it would freeze the agent on the fallback
/// harness even after the persona's runtime is installed and the persona is
/// re-edited, the exact bug this resolver chain exists to prevent. Stores `None`
/// so the persona stays authoritative.
///
/// `isOverridden` from `resolvePersonaRuntime` cannot distinguish these — it is
/// `true` for BOTH — so the caller must thread the explicit user-intent bit.
///
/// Persona-less creates (`persona_id` is `None`, e.g. the standalone
/// CreateAgentDialog) have no persona to inherit, so the picked command is always a
/// real pin and is preserved via `divergent_agent_command_override` regardless of
/// `harness_override`.
pub fn create_time_agent_command_override(
persona_id: Option<&str>,
personas: &[crate::managed_agents::types::PersonaRecord],
picked_command: Option<&str>,
harness_override: bool,
) -> Option<String> {
if persona_id.is_some() && !harness_override {
return None;
}
if persona_id.is_some() && harness_override {
let picked = picked_command
.map(str::trim)
.filter(|value| !value.is_empty())?;
let inherited_command = effective_agent_command(persona_id, personas, None);
return (picked != inherited_command).then(|| picked.to_string());
}
divergent_agent_command_override(persona_id, personas, picked_command)
}
desktop/src-tauri/src/managed_agents/discovery/tests.rs
+47 -4
View File
@@ -1,10 +1,10 @@
use std::path::PathBuf;
use super::overrides::{divergent_agent_command_override, update_time_agent_command_override};
use super::{
classify_runtime, create_time_agent_command_override, default_agent_command,
divergent_agent_command_override, effective_agent_command, find_via_login_shell,
managed_agent_avatar_url, normalize_agent_args, record_agent_command,
update_time_agent_command_override, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL,
apply_agent_command_update, classify_runtime, create_time_agent_command_override,
default_agent_command, effective_agent_command, find_via_login_shell, managed_agent_avatar_url,
normalize_agent_args, record_agent_command, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL,
CODEX_AVATAR_URL, GOOSE_AVATAR_URL,
};
use crate::managed_agents::AcpAvailabilityStatus;
@@ -562,3 +562,46 @@ fn update_time_override_preserves_pin_for_persona_less_agent() {
Some("codex-acp".to_string())
);
}
#[test]
fn apply_agent_command_update_inherit_sentinel_clears_pin_and_runtime() {
// Choosing Inherit on a persona-linked record clears BOTH the explicit
// pin and the materialized runtime, so resolution falls through to the
// live definition immediately — not on the next spawn.
let personas = vec![persona_with_runtime("p1", Some("goose"))];
let mut record = record_with(Some("claude"), Some("p1"), Some("codex-acp"));
apply_agent_command_update(&mut record, &personas, "", false);
assert_eq!(record.agent_command_override, None);
assert_eq!(record.runtime, None);
assert_eq!(record_agent_command(&record, &personas), "goose");
}
#[test]
fn apply_agent_command_update_sentinel_keeps_runtime_for_definition_less_record() {
// For a record with no persona link the materialized runtime is the only
// harness source left once the pin is cleared — a stray empty
// agent_command must not change what the agent runs.
let mut record = record_with(Some("claude"), None, Some("codex-acp"));
apply_agent_command_update(&mut record, &[], "", false);
assert_eq!(record.agent_command_override, None);
assert_eq!(record.runtime.as_deref(), Some("claude"));
assert_eq!(record_agent_command(&record, &[]), "claude-agent-acp");
}
#[test]
fn apply_agent_command_update_concrete_pin_keeps_materialized_runtime() {
// A concrete pick only sets the pin; the materialized runtime is left for
// the next snapshot apply. The pin shadows it in resolution either way.
let personas = vec![persona_with_runtime("p1", Some("goose"))];
let mut record = record_with(Some("claude"), Some("p1"), None);
apply_agent_command_update(&mut record, &personas, "codex-acp", true);
assert_eq!(record.agent_command_override.as_deref(), Some("codex-acp"));
assert_eq!(record.runtime.as_deref(), Some("claude"));
assert_eq!(record_agent_command(&record, &personas), "codex-acp");
}
desktop/src-tauri/src/managed_agents/persona_events.rs
+46 -1
View File
@@ -9,7 +9,7 @@ use buzz_core_pkg::kind::KIND_PERSONA;
use nostr::{EventBuilder, Kind, Tag};
use serde::{Deserialize, Serialize};
use super::PersonaRecord;
use super::{ManagedAgentRecord, PersonaRecord};
use crate::app_state::AppState;
/// The JSON body stored in a persona event's content field.
@@ -334,6 +334,12 @@ pub struct PersonaSnapshot {
pub system_prompt: Option<String>,
pub model: Option<String>,
pub provider: Option<String>,
/// Preferred ACP runtime ID, copied verbatim from the persona (including
/// `None`). Unlike `model`/`provider`, there is no record-fallback: the
/// materialized instance `runtime` must mirror the definition so that
/// definition edits propagate on the next spawn rather than being silently
/// shadowed by the stale materialized value.
pub runtime: Option<String>,
/// `persona_content_hash` of the persona at snapshot time; the drift basis.
pub source_version: String,
}
@@ -367,6 +373,7 @@ pub fn persona_snapshot(persona: &PersonaRecord) -> PersonaSnapshot {
system_prompt: Some(persona.system_prompt.clone()),
model: persona.model.clone(),
provider: persona.provider.clone(),
runtime: persona.runtime.clone(),
source_version: persona_content_hash(&persona_event_content(persona)),
}
}
@@ -410,5 +417,43 @@ pub fn persona_snapshot_with_agent_config_fallback(
..base
}
}
/// Re-pin `record` to `persona`: build the snapshot (via
/// [`persona_snapshot_with_agent_config_fallback`], so blank persona
/// `model`/`provider` preserve the record's own values) and mirror it onto the
/// record — the definition quad (`system_prompt`/`model`/`provider`/`runtime`),
/// the env-override self-heal, and the `persona_source_version` drift basis.
///
/// This is the single apply used by every snapshot-apply site: the spawn
/// re-pin (`start_local_agent_with_preflight`), the launch backfill and
/// restore re-snapshot (`restore.rs`), and the prospective re-snapshot inside
/// `spawn_config_hash` — so a future `PersonaSnapshot` field addition
/// propagates to all of them at once.
///
/// Deliberately does NOT touch `updated_at`: persistence stamps are the
/// caller's concern, and `spawn_config_hash` (which applies this to a clone)
/// must stay pure.
pub fn apply_persona_snapshot(record: &mut ManagedAgentRecord, persona: &PersonaRecord) {
let snapshot = persona_snapshot_with_agent_config_fallback(
persona,
record.model.as_deref(), // fallback: record.model
record.provider.as_deref(), // fallback: record.provider
);
if let Some(prompt) = snapshot.system_prompt {
record.system_prompt = Some(prompt);
}
record.model = snapshot.model;
record.provider = snapshot.provider;
record.runtime = snapshot.runtime;
// env_vars stay overrides-only. Self-heal records written before the env
// refresh: persona env used to be baked into `record.env_vars`, turning
// inherited values into pseudo-overrides that shadow later persona edits.
// An override equal to the persona's current value is indistinguishable
// from inheritance, so drop it and let the live merge supply it.
record
.env_vars
.retain(|k, v| persona.env_vars.get(k) != Some(v));
record.persona_source_version = Some(snapshot.source_version);
}
#[cfg(test)]
mod tests;
desktop/src-tauri/src/managed_agents/persona_events/tests.rs
+27
View File
@@ -484,6 +484,33 @@ fn field_fallback_record_blank_is_none() {
);
}
// ── PersonaSnapshot.runtime ───────────────────────────────────────────────
/// (b) The snapshot carries the persona's runtime VERBATIM — including None,
/// which clears a stale materialized value on the instance record. Unlike
/// model/provider, runtime does not fall back to the record's own value:
/// instances have no user-owned runtime, so the definition must stay
/// authoritative.
#[test]
fn snapshot_runtime_verbatim_from_persona() {
let persona = sample_persona(); // runtime = Some("goose")
let snap = persona_snapshot_with_agent_config_fallback(&persona, Some("gpt-4"), Some("openai"));
assert_eq!(
snap.runtime.as_deref(),
Some("goose"),
"persona runtime Some must be copied verbatim into snapshot"
);
let mut no_runtime = sample_persona();
no_runtime.runtime = None;
let snap =
persona_snapshot_with_agent_config_fallback(&no_runtime, Some("gpt-4"), Some("openai"));
assert_eq!(
snap.runtime, None,
"persona runtime None must produce None snapshot (clears stale materialized value)"
);
}
// ── persona_snapshot_with_agent_config_fallback ────────────────────────────
/// Helper: a persona with no model/provider configured.
desktop/src-tauri/src/managed_agents/restore.rs
+3 -34
View File
@@ -61,23 +61,8 @@ pub fn backfill_persona_snapshots(app: &tauri::AppHandle) -> Result<(), String>
// Layer precedence at read time: persona env < agent env. When the
// persona leaves model/provider blank, the record's own configured
// values are preserved — a blank persona must not clobber a
// user-configured agent.
let snapshot = super::persona_events::persona_snapshot_with_agent_config_fallback(
persona,
record.model.as_deref(), // fallback: record.model
record.provider.as_deref(), // fallback: record.provider
);
if let Some(prompt) = snapshot.system_prompt {
record.system_prompt = Some(prompt);
}
record.model = snapshot.model;
record.provider = snapshot.provider;
// env_vars stay overrides-only; see the create-path comment. Self-heal
// pre-refresh records that baked persona env in as pseudo-overrides.
record
.env_vars
.retain(|k, v| persona.env_vars.get(k) != Some(v));
record.persona_source_version = Some(snapshot.source_version);
// user-configured agent. See `apply_persona_snapshot`.
super::persona_events::apply_persona_snapshot(record, persona);
record.updated_at = util::now_iso();
changed = true;
}
@@ -195,23 +180,7 @@ pub async fn restore_managed_agents_on_launch(
let Some(persona) = personas_for_snapshot.iter().find(|p| p.id == persona_id) else {
continue;
};
let snapshot = super::persona_events::persona_snapshot_with_agent_config_fallback(
persona,
record.model.as_deref(), // fallback: record.model
record.provider.as_deref(), // fallback: record.provider
);
if let Some(prompt) = snapshot.system_prompt {
record.system_prompt = Some(prompt);
}
record.model = snapshot.model;
record.provider = snapshot.provider;
// env_vars stay overrides-only; see the create-path comment.
// Self-heal pre-refresh records that baked persona env in as
// pseudo-overrides.
record
.env_vars
.retain(|k, v| persona.env_vars.get(k) != Some(v));
record.persona_source_version = Some(snapshot.source_version);
super::persona_events::apply_persona_snapshot(record, persona);
record.updated_at = util::now_iso();
changed = true;
}
desktop/src-tauri/src/managed_agents/spawn_hash.rs
+9 -21
View File
@@ -28,7 +28,7 @@ use std::hash::{DefaultHasher, Hash, Hasher};
use super::{
known_acp_runtime, normalize_agent_args,
persona_events::persona_snapshot_with_agent_config_fallback,
persona_events::apply_persona_snapshot,
resolve_effective_agent_env,
types::{ManagedAgentRecord, PersonaRecord},
};
@@ -60,29 +60,17 @@ pub(crate) fn spawn_config_hash(
personas: &[PersonaRecord],
workspace_relay: &str,
) -> u64 {
// Prospective re-snapshot: mirror the mutation start/restore apply to the
// record right before spawning, so the hash covers what a restart would
// actually run. Idempotent, so the spawn-time stamp (post-snapshot record)
// and later recomputes (persisted record) agree when nothing changed.
// Prospective re-snapshot: apply the same `apply_persona_snapshot` the
// start/restore paths run right before spawning, so the hash covers what a
// restart would actually run. Idempotent, so the spawn-time stamp
// (post-snapshot record) and later recomputes (persisted record) agree
// when nothing changed. The persona env itself reaches the hash through
// `resolve_effective_agent_env` below; `persona_source_version` is set on
// the clone but is not a hash input.
let mut record = record.clone();
if let Some(persona_id) = record.persona_id.clone() {
if let Some(persona) = personas.iter().find(|p| p.id == persona_id) {
let snapshot = persona_snapshot_with_agent_config_fallback(
persona,
record.model.as_deref(),
record.provider.as_deref(),
);
if let Some(prompt) = snapshot.system_prompt {
record.system_prompt = Some(prompt);
}
record.model = snapshot.model;
record.provider = snapshot.provider;
// Mirror the start/restore self-heal: overrides equal to the live
// persona value are treated as inherited. The persona env itself
// reaches the hash through `resolve_effective_agent_env` below.
record
.env_vars
.retain(|k, v| persona.env_vars.get(k) != Some(v));
apply_persona_snapshot(&mut record, persona);
}
}
let record = &record;
desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs
+56
View File
@@ -376,6 +376,62 @@ fn team_pack_records_keep_empty_vs_absent_prompt_distinction() {
);
}
/// (a) A definition-runtime edit must change spawn_config_hash for a
/// materialized, override-free record — the prospective re-snapshot now
/// copies the persona's runtime onto the record before hashing.
#[test]
fn definition_runtime_edit_changes_hash_for_materialized_record() {
let mut rec = record();
rec.persona_id = Some("pers".into());
rec.runtime = Some("goose".into()); // materialized runtime on instance
let before = [persona("pers", Some("goose"), "prompt")];
let after = [persona("pers", Some("claude"), "prompt")];
assert_ne!(
spawn_config_hash(&rec, &before, "wss://ws.example"),
spawn_config_hash(&rec, &after, "wss://ws.example"),
"definition runtime edit must badge a materialized, override-free instance"
);
}
/// (c) An explicit agent_command_override (ladder step 1) must beat a
/// changed definition runtime — the badge must NOT fire for a pinned instance.
#[test]
fn agent_command_override_beats_definition_runtime_change() {
let mut rec = record();
rec.persona_id = Some("pers".into());
rec.runtime = Some("goose".into()); // materialized runtime
rec.agent_command_override = Some("goose".into()); // explicit per-instance pin
let before = [persona("pers", Some("goose"), "prompt")];
let after = [persona("pers", Some("claude"), "prompt")];
assert_eq!(
spawn_config_hash(&rec, &before, "wss://ws.example"),
spawn_config_hash(&rec, &after, "wss://ws.example"),
"explicit override must win regardless of definition runtime change"
);
}
/// (d) When the linked definition is absent the prospective re-snapshot is
/// skipped entirely: the materialized runtime must still affect the hash.
#[test]
fn missing_definition_leaves_materialized_runtime_in_hash() {
let mut rec = record();
rec.persona_id = Some("missing".into());
rec.runtime = Some("goose".into()); // materialized runtime
let no_personas: &[PersonaRecord] = &[];
let mut no_runtime = rec.clone();
no_runtime.runtime = None;
assert_ne!(
spawn_config_hash(&rec, no_personas, "wss://ws.example"),
spawn_config_hash(&no_runtime, no_personas, "wss://ws.example"),
"materialized runtime must still affect hash when definition is absent"
);
}
#[test]
fn effective_spawn_prompt_matches_hash_semantics() {
// The env write and the hash share effective_spawn_prompt — this row
desktop/src-tauri/src/managed_agents/types.rs
+25 -109
View File
@@ -236,9 +236,19 @@ pub struct ManagedAgentRecord {
#[serde(default)]
pub agent_command_override: Option<String>,
pub agent_args: Vec<String>,
/// Create-time snapshot of the catalog MCP command. Never read at spawn —
/// the effective MCP command is always re-derived from the runtime catalog
/// (`known_acp_runtime`) — and no longer written by updates. Kept for
/// serde compatibility with existing stores.
pub mcp_command: String,
/// Deprecated: `BUZZ_ACP_TURN_TIMEOUT` is ignored by the harness and the
/// desktop no longer emits or edits it. Kept for serde compatibility with
/// existing stores; use `idle_timeout_seconds` or
/// `max_turn_duration_seconds` for turn-length control.
pub turn_timeout_seconds: u64,
/// Idle timeout in seconds. If set, overrides turn_timeout_seconds.
/// Idle timeout in seconds (`BUZZ_ACP_IDLE_TIMEOUT`): how long the agent
/// may stay silent on its ACP channel mid-turn before the harness times
/// the turn out.
#[serde(default)]
pub idle_timeout_seconds: Option<u64>,
/// Absolute wall-clock cap per turn.
@@ -334,7 +344,16 @@ pub struct ManagedAgentRecord {
/// Absorbed from `PersonaRecord.runtime` — the preferred ACP runtime ID
/// (e.g. 'goose', 'claude'). Record-first command resolution reads this
/// before falling back to legacy persona lookup; populated by the store
/// migration and at create time.
/// migration and at create time, and re-mirrored from the linked
/// definition at every snapshot apply (`apply_persona_snapshot`).
///
/// `None` means "inherit from the linked definition" (the Inherit sentinel
/// clears it). Serialization then omits the key, so boot-time
/// `materialize_agent_runtimes` re-inserts a mirror of the definition's
/// current runtime on the next launch — behaviorally identical, because
/// every apply site re-mirrors the live definition anyway. A literal
/// `"runtime": null` in the store (key present, e.g. hand-edited) is
/// honored: materialization skips it and it deserializes to `None`.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub runtime: Option<String>,
/// Pool of short thematic names for clones of this agent. Absorbed from
@@ -439,7 +458,11 @@ pub struct ManagedAgentSummary {
/// concrete pin (`agent_command` above is the resolved/effective command).
pub agent_command_override: Option<String>,
pub agent_args: Vec<String>,
/// Catalog-derived from the effective harness (not the record's stored
/// field), so the UI always shows what a spawn would actually use.
pub mcp_command: String,
/// Deprecated passthrough of the stored record value; the harness ignores
/// it. Kept for wire compatibility.
pub turn_timeout_seconds: u64,
pub idle_timeout_seconds: Option<u64>,
pub max_turn_duration_seconds: Option<u64>,
@@ -488,56 +511,6 @@ pub struct ManagedAgentSummary {
pub respond_to_allowlist: Vec<String>,
}
#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct CreateManagedAgentRequest {
pub name: String,
#[serde(default)]
pub persona_id: Option<String>,
pub relay_url: Option<String>,
pub acp_command: Option<String>,
pub agent_command: Option<String>,
/// True when `agent_command` is a runtime command the user deliberately
/// picked for a linked persona. Distinguishes a real selection, including an
/// installed alias, from a missing-runtime fallback so a persona-backed
/// create only stores an `agent_command_override` for the former.
#[serde(default)]
pub harness_override: bool,
#[serde(default)]
pub agent_args: Vec<String>,
pub mcp_command: Option<String>,
pub turn_timeout_seconds: Option<u64>,
pub idle_timeout_seconds: Option<u64>,
pub max_turn_duration_seconds: Option<u64>,
pub parallelism: Option<u32>,
pub system_prompt: Option<String>,
pub avatar_url: Option<String>,
pub model: Option<String>,
pub provider: Option<String>,
pub mcp_toolsets: Option<String>,
/// Environment variables for this agent. Layered on top of persona env.
#[serde(default)]
pub env_vars: BTreeMap<String, String>,
#[serde(default)]
pub spawn_after_create: bool,
#[serde(default = "default_start_on_app_launch")]
pub start_on_app_launch: bool,
#[serde(default)]
pub backend: BackendKind,
/// `None` = caller expressed no preference: the definition's
/// `respond_to` default applies when linked, `RespondTo::default()`
/// otherwise. `Some` is an explicit instance-level choice and always
/// wins over the definition default.
#[serde(default)]
pub respond_to: Option<RespondTo>,
/// Raw allowlist as received from the frontend. Validated and normalized
/// before being written to the record.
#[serde(default)]
pub respond_to_allowlist: Vec<String>,
#[serde(default)]
pub relay_mesh: Option<RelayMeshConfig>,
}
#[derive(Debug, Serialize)]
pub struct CreateManagedAgentResponse {
pub agent: ManagedAgentSummary,
@@ -616,63 +589,6 @@ pub struct ManagedAgentPrereqsInfo {
pub mcp: CommandAvailabilityInfo,
}
/// Patch request for updating a managed agent's mutable fields.
///
/// Tri-state nullable semantics via `Option<Option<T>>`:
/// - Field absent in JSON → `None` (don't touch)
/// - `"field": null` → `Some(None)` (clear to default)
/// - `"field": "value"` → `Some(Some("value"))` (set)
#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct UpdateManagedAgentRequest {
pub pubkey: String,
/// Absent = don't touch. Present = rename the agent.
#[serde(default)]
pub name: Option<String>,
/// Absent = don't touch. null = clear to agent default. "id" = set.
#[serde(default)]
pub model: Option<Option<String>>,
#[serde(default)]
pub system_prompt: Option<Option<String>>,
#[serde(default)]
pub mcp_toolsets: Option<Option<String>>,
/// Absent = don't touch. Present = replace the env_vars map entirely.
#[serde(default)]
pub env_vars: Option<BTreeMap<String, String>>,
#[serde(default)]
pub parallelism: Option<u32>,
#[serde(default)]
pub turn_timeout_seconds: Option<u64>,
#[serde(default)]
pub relay_url: Option<String>,
#[serde(default)]
pub acp_command: Option<String>,
#[serde(default)]
pub agent_command: Option<String>,
/// True when the accompanying `agent_command` is a runtime/Custom command
/// the user deliberately picked for a linked persona (i.e. the dialog is
/// not inheriting). Distinguishes a real pin — including one that maps to
/// the persona's own runtime — from a persona-authoritative restatement,
/// so a same-runtime pick is preserved instead of being dropped back to
/// inherit. Ignored when `agent_command` is absent or the inherit sentinel.
#[serde(default)]
pub harness_override: bool,
#[serde(default)]
pub agent_args: Option<Vec<String>>,
#[serde(default)]
pub mcp_command: Option<String>,
/// Absent = don't touch. null = clear to runtime default. "id" = set.
#[serde(default, deserialize_with = "crate::util::double_option")]
pub provider: Option<Option<String>>,
/// Absent = don't touch. Present = set mode.
#[serde(default)]
pub respond_to: Option<RespondTo>,
/// Absent = don't touch. Present = replace the allowlist (validated &
/// normalized server-side).
#[serde(default)]
pub respond_to_allowlist: Option<Vec<String>>,
}
#[derive(Debug, Serialize)]
pub struct UpdateManagedAgentResponse {
pub agent: ManagedAgentSummary,
desktop/src-tauri/src/managed_agents/types/requests.rs
+134 -2
View File
@@ -1,10 +1,14 @@
//! Persona command request types, split from `types.rs` (file-size cap).
//! Persona and managed-agent command request types, split from `types.rs`
//! (file-size cap).
use std::collections::BTreeMap;
use serde::Deserialize;
use super::{validate_respond_to_allowlist, PersonaRecord, RespondTo};
use super::{
default_start_on_app_launch, validate_respond_to_allowlist, BackendKind, PersonaRecord,
RelayMeshConfig, RespondTo,
};
/// The NIP-AP behavioral quad as one grouped request field.
///
@@ -125,6 +129,134 @@ pub struct UpdatePersonaRequest {
pub behavior: Option<PersonaBehaviorRequest>,
}
#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct CreateManagedAgentRequest {
pub name: String,
#[serde(default)]
pub persona_id: Option<String>,
pub relay_url: Option<String>,
pub acp_command: Option<String>,
pub agent_command: Option<String>,
/// True when `agent_command` is a runtime command the user deliberately
/// picked for a linked persona. Distinguishes a real selection, including an
/// installed alias, from a missing-runtime fallback so a persona-backed
/// create only stores an `agent_command_override` for the former.
#[serde(default)]
pub harness_override: bool,
#[serde(default)]
pub agent_args: Vec<String>,
/// Accepted for wire compatibility; not applied to the record. The
/// effective MCP command is always derived from the runtime catalog at
/// spawn time — a per-record override is never read.
///
/// @deprecated — sending this field has no effect.
#[allow(dead_code)]
pub mcp_command: Option<String>,
/// Accepted for wire compatibility; not applied to the record.
/// `BUZZ_ACP_TURN_TIMEOUT` is deprecated and ignored by the harness.
///
/// @deprecated — sending this field has no effect.
#[allow(dead_code)]
pub turn_timeout_seconds: Option<u64>,
pub idle_timeout_seconds: Option<u64>,
pub max_turn_duration_seconds: Option<u64>,
pub parallelism: Option<u32>,
pub system_prompt: Option<String>,
pub avatar_url: Option<String>,
pub model: Option<String>,
pub provider: Option<String>,
pub mcp_toolsets: Option<String>,
/// Environment variables for this agent. Layered on top of persona env.
#[serde(default)]
pub env_vars: BTreeMap<String, String>,
#[serde(default)]
pub spawn_after_create: bool,
#[serde(default = "default_start_on_app_launch")]
pub start_on_app_launch: bool,
#[serde(default)]
pub backend: BackendKind,
/// `None` = caller expressed no preference: the definition's
/// `respond_to` default applies when linked, `RespondTo::default()`
/// otherwise. `Some` is an explicit instance-level choice and always
/// wins over the definition default.
#[serde(default)]
pub respond_to: Option<RespondTo>,
/// Raw allowlist as received from the frontend. Validated and normalized
/// before being written to the record.
#[serde(default)]
pub respond_to_allowlist: Vec<String>,
#[serde(default)]
pub relay_mesh: Option<RelayMeshConfig>,
}
/// Patch request for updating a managed agent's mutable fields.
///
/// Tri-state nullable semantics via `Option<Option<T>>`:
/// - Field absent in JSON → `None` (don't touch)
/// - `"field": null` → `Some(None)` (clear to default)
/// - `"field": "value"` → `Some(Some("value"))` (set)
#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct UpdateManagedAgentRequest {
pub pubkey: String,
/// Absent = don't touch. Present = rename the agent.
#[serde(default)]
pub name: Option<String>,
/// Absent = don't touch. null = clear to agent default. "id" = set.
#[serde(default)]
pub model: Option<Option<String>>,
#[serde(default)]
pub system_prompt: Option<Option<String>>,
#[serde(default)]
pub mcp_toolsets: Option<Option<String>>,
/// Absent = don't touch. Present = replace the env_vars map entirely.
#[serde(default)]
pub env_vars: Option<BTreeMap<String, String>>,
#[serde(default)]
pub parallelism: Option<u32>,
/// Accepted for wire compatibility; not applied to the stored record.
/// `BUZZ_ACP_TURN_TIMEOUT` is deprecated and ignored by the harness.
///
/// @deprecated — sending this field has no effect.
#[allow(dead_code)]
#[serde(default)]
pub turn_timeout_seconds: Option<u64>,
#[serde(default)]
pub relay_url: Option<String>,
#[serde(default)]
pub acp_command: Option<String>,
#[serde(default)]
pub agent_command: Option<String>,
/// True when the accompanying `agent_command` is a runtime/Custom command
/// the user deliberately picked for a linked persona (i.e. the dialog is
/// not inheriting). Distinguishes a real pin — including one that maps to
/// the persona's own runtime — from a persona-authoritative restatement,
/// so a same-runtime pick is preserved instead of being dropped back to
/// inherit. Ignored when `agent_command` is absent or the inherit sentinel.
#[serde(default)]
pub harness_override: bool,
#[serde(default)]
pub agent_args: Option<Vec<String>>,
/// Accepted for wire compatibility; not applied to the stored record.
/// The effective MCP command is always catalog-derived at spawn time.
///
/// @deprecated — sending this field has no effect.
#[allow(dead_code)]
#[serde(default)]
pub mcp_command: Option<String>,
/// Absent = don't touch. null = clear to runtime default. "id" = set.
#[serde(default, deserialize_with = "crate::util::double_option")]
pub provider: Option<Option<String>>,
/// Absent = don't touch. Present = set mode.
#[serde(default)]
pub respond_to: Option<RespondTo>,
/// Absent = don't touch. Present = replace the allowlist (validated &
/// normalized server-side).
#[serde(default)]
pub respond_to_allowlist: Option<Vec<String>>,
}
#[cfg(test)]
mod tests {
use super::*;
desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx
-20
View File
@@ -96,11 +96,7 @@ export function AgentInstanceEditDialog({
agent.personaId != null && agent.agentCommandOverride == null,
);
const [agentArgs, setAgentArgs] = React.useState(agent.agentArgs.join(","));
const [mcpCommand, setMcpCommand] = React.useState(agent.mcpCommand);
const [mcpToolsets, setMcpToolsets] = React.useState(agent.mcpToolsets ?? "");
const [turnTimeoutSeconds, setTurnTimeoutSeconds] = React.useState(
String(agent.turnTimeoutSeconds),
);
const [parallelism, setParallelism] = React.useState(
String(agent.parallelism),
);
@@ -155,9 +151,7 @@ export function AgentInstanceEditDialog({
agent.personaId != null && agent.agentCommandOverride == null,
);
setAgentArgs(agent.agentArgs.join(","));
setMcpCommand(agent.mcpCommand);
setMcpToolsets(agent.mcpToolsets ?? "");
setTurnTimeoutSeconds(String(agent.turnTimeoutSeconds));
setParallelism(String(agent.parallelism));
setSystemPrompt(agent.systemPrompt ?? "");
setModel(agent.model ?? "");
@@ -458,7 +452,6 @@ export function AgentInstanceEditDialog({
computeEditAgentFormValidity({
name,
parallelism,
turnTimeoutSeconds,
agentAcpCommand: agent.acpCommand,
acpCommand,
respondTo,
@@ -474,7 +467,6 @@ export function AgentInstanceEditDialog({
async function handleSubmit() {
try {
const parsedParallelism = Number.parseInt(parallelism, 10);
const parsedTimeout = Number.parseInt(turnTimeoutSeconds, 10);
const parsedArgs = agentArgs
.split(",")
.map((v) => v.trim())
@@ -532,18 +524,10 @@ export function AgentInstanceEditDialog({
parsedArgs.join(",") !== agent.agentArgs.join(",")
? parsedArgs
: undefined,
mcpCommand:
mcpCommand.trim() !== agent.mcpCommand
? mcpCommand.trim()
: undefined,
mcpToolsets:
(mcpToolsets.trim() || null) !== agent.mcpToolsets
? mcpToolsets.trim() || null
: undefined,
turnTimeoutSeconds:
parsedTimeout > 0 && parsedTimeout !== agent.turnTimeoutSeconds
? parsedTimeout
: undefined,
parallelism:
parsedParallelism > 0 && parsedParallelism !== agent.parallelism
? parsedParallelism
@@ -943,26 +927,22 @@ export function AgentInstanceEditDialog({
inheritedEnvVars={inheritedEnvVars}
inheritHarness={inheritHarness}
linkedPersona={linkedPersona}
mcpCommand={mcpCommand}
mcpToolsets={mcpToolsets}
parallelism={parallelism}
relayUrl={relayUrl}
requiredEnvKeys={requiredEnvKeys}
selectedRuntimeId={selectedRuntimeId}
systemPrompt={systemPrompt}
turnTimeoutSeconds={turnTimeoutSeconds}
onAcpCommandChange={setAcpCommand}
onAgentArgsChange={setAgentArgs}
onAgentCommandChange={setAgentCommand}
onAutoRestartChange={setAutoRestartOnConfigChange}
onEnvVarsChange={setEnvVars}
onInheritHarnessChange={setInheritHarness}
onMcpCommandChange={setMcpCommand}
onMcpToolsetsChange={setMcpToolsets}
onParallelismChange={setParallelism}
onRelayUrlChange={setRelayUrl}
onSystemPromptChange={setSystemPrompt}
onTurnTimeoutChange={setTurnTimeoutSeconds}
/>
</motion.div>
) : null}
desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx
+25 -94
View File
@@ -20,26 +20,22 @@ export function EditAgentAdvancedFields({
inheritedEnvVars,
inheritHarness,
linkedPersona,
mcpCommand,
mcpToolsets,
parallelism,
relayUrl,
requiredEnvKeys,
selectedRuntimeId,
systemPrompt,
turnTimeoutSeconds,
onAcpCommandChange,
onAgentArgsChange,
onAgentCommandChange,
onEnvVarsChange,
onInheritHarnessChange,
onMcpCommandChange,
onMcpToolsetsChange,
onParallelismChange,
onRelayUrlChange,
onAutoRestartChange,
onSystemPromptChange,
onTurnTimeoutChange,
}: {
acpCommand: string;
agentArgs: string;
@@ -51,26 +47,22 @@ export function EditAgentAdvancedFields({
inheritedEnvVars: Record<string, string>;
inheritHarness: boolean;
linkedPersona: AgentPersona | null;
mcpCommand: string;
mcpToolsets: string;
parallelism: string;
relayUrl: string;
requiredEnvKeys: readonly string[];
selectedRuntimeId: string;
systemPrompt: string;
turnTimeoutSeconds: string;
onAcpCommandChange: (value: string) => void;
onAgentArgsChange: (value: string) => void;
onAgentCommandChange: (value: string) => void;
onEnvVarsChange: (value: EnvVarsValue) => void;
onInheritHarnessChange: (value: boolean) => void;
onMcpCommandChange: (value: string) => void;
onMcpToolsetsChange: (value: string) => void;
onParallelismChange: (value: string) => void;
onRelayUrlChange: (value: string) => void;
onAutoRestartChange: (value: boolean) => void;
onSystemPromptChange: (value: string) => void;
onTurnTimeoutChange: (value: string) => void;
}) {
return (
<div className="space-y-5 pt-2">
@@ -181,36 +173,6 @@ export function EditAgentAdvancedFields({
</div>
</div>
{/* MCP command */}
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-mcp-command"
>
MCP command
<span className={PERSONA_LABEL_OPTIONAL_CLASS}>Optional</span>
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-mcp-command"
onChange={(event) => onMcpCommandChange(event.target.value)}
placeholder="Optional MCP server command"
value={mcpCommand}
/>
</div>
</div>
{/* MCP toolsets */}
<div className="space-y-1.5">
<label
@@ -244,64 +206,33 @@ export function EditAgentAdvancedFields({
</p>
</div>
{/* Turn timeout + Parallelism side by side */}
<div className="grid gap-4 md:grid-cols-2">
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-timeout"
>
Turn timeout
<span className={PERSONA_LABEL_OPTIONAL_CLASS}>seconds</span>
</label>
<div
{/* Parallelism */}
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-parallelism"
>
Parallelism
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-timeout"
onChange={(event) => onTurnTimeoutChange(event.target.value)}
placeholder="300"
value={turnTimeoutSeconds}
/>
</div>
</div>
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-parallelism"
>
Parallelism
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-parallelism"
inputMode="numeric"
onChange={(event) => onParallelismChange(event.target.value)}
placeholder="1"
value={parallelism}
/>
</div>
disabled={disabled}
id="edit-agent-parallelism"
inputMode="numeric"
onChange={(event) => onParallelismChange(event.target.value)}
placeholder="1"
value={parallelism}
/>
</div>
</div>
desktop/src/features/agents/ui/agentInstanceEditPinning.test.mjs
-2
View File
@@ -165,7 +165,6 @@ test("rehost_gate_blocksSave_whenSubmissionSnapshotLacksCredential", () => {
const canSubmit = computeEditAgentFormValidity({
name: pinnedAgent.name,
parallelism: "1",
turnTimeoutSeconds: "60",
agentAcpCommand: pinnedAgent.acpCommand,
acpCommand: pinnedAgent.acpCommand,
respondTo: "mentions",
@@ -257,7 +256,6 @@ test("editValidity_allowlistWithEmptyList_blocksSave", () => {
const base = {
name: pinnedAgent.name,
parallelism: "1",
turnTimeoutSeconds: "60",
agentAcpCommand: pinnedAgent.acpCommand,
acpCommand: pinnedAgent.acpCommand,
selectedRuntimeId: "claude",
desktop/src/features/agents/ui/editAgentProviderDiscovery.test.mjs
-2
View File
@@ -604,7 +604,6 @@ test("editAgent_missingRequiredEnvKey_blocksSaveViaValidity", () => {
const base = {
name: "My Agent",
parallelism: "",
turnTimeoutSeconds: "",
agentAcpCommand: "",
acpCommand: "",
respondTo: "all",
@@ -634,7 +633,6 @@ test("editAgent_customCommandPinned_blocksSaveWhenCommandEmpty", () => {
const base = {
name: "My Agent",
parallelism: "",
turnTimeoutSeconds: "",
agentAcpCommand: "",
acpCommand: "",
respondTo: "all",
desktop/src/features/agents/ui/personaRuntimeModel.ts
-5
View File
@@ -210,7 +210,6 @@ export function resolveInheritedRuntimeSubmission(input: {
export interface EditAgentFormValidityInput {
name: string;
parallelism: string;
turnTimeoutSeconds: string;
/** The command already persisted on the agent (empty when inheriting). */
agentAcpCommand: string;
acpCommand: string;
@@ -249,9 +248,6 @@ export function computeEditAgentFormValidity(
const parallelismValid =
input.parallelism.trim() === "" ||
!Number.isNaN(Number.parseInt(input.parallelism, 10));
const timeoutValid =
input.turnTimeoutSeconds.trim() === "" ||
!Number.isNaN(Number.parseInt(input.turnTimeoutSeconds, 10));
const acpCommandValid = !(
input.agentAcpCommand && input.acpCommand.trim() === ""
);
@@ -266,7 +262,6 @@ export function computeEditAgentFormValidity(
return (
input.name.trim().length > 0 &&
parallelismValid &&
timeoutValid &&
acpCommandValid &&
respondToValid &&
customCommandValid &&
desktop/src/shared/api/tauriPersonas.ts
+26 -20
View File
@@ -71,6 +71,8 @@ export type RawPersona = {
parallelism?: number | null;
created_at: string;
updated_at: string;
/** Non-null when the pack `.persona.md` write-back failed (non-fatal). */
writeback_warning?: string | null;
};
export function fromRawPersona(persona: RawPersona): AgentPersona {
@@ -123,26 +125,30 @@ export async function createPersona(
export async function updatePersona(
input: UpdatePersonaInput,
): Promise<AgentPersona> {
return fromRawPersona(
await invokeTauri<RawPersona>("update_persona", {
input: {
id: input.id,
displayName: input.displayName,
avatarUrl: input.avatarUrl,
systemPrompt: input.systemPrompt,
runtime: input.runtime,
model: input.model,
provider: input.provider,
namePool: input.namePool ?? [],
// Send envVars only when caller explicitly provided it; omitting
// tells the backend "don't touch the stored env vars" so editing
// unrelated fields can't silently wipe saved credentials.
envVars: input.envVars,
// Same absent-vs-present contract as envVars for the behavioral quad.
behavior: input.behavior,
},
}),
);
const raw = await invokeTauri<RawPersona>("update_persona", {
input: {
id: input.id,
displayName: input.displayName,
avatarUrl: input.avatarUrl,
systemPrompt: input.systemPrompt,
runtime: input.runtime,
model: input.model,
provider: input.provider,
namePool: input.namePool ?? [],
// Send envVars only when caller explicitly provided it; omitting
// tells the backend "don't touch the stored env vars" so editing
// unrelated fields can't silently wipe saved credentials.
envVars: input.envVars,
// Same absent-vs-present contract as envVars for the behavioral quad.
behavior: input.behavior,
},
});
if (raw.writeback_warning) {
console.warn(
`[updatePersona] pack write-back failed (edit saved locally): ${raw.writeback_warning}`,
);
}
return fromRawPersona(raw);
}
export async function deletePersona(id: string): Promise<void> {