diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index b9984adc70..e8e8fdd51d 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -422,14 +422,9 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_ACP_ALLOWED_RESPOND_TO", value_delimiter = ',')] pub allowed_respond_to: Option>, - /// Path to a persona pack directory. Used with --persona-name to configure - /// the agent from a .persona.md pack instead of CLI flags. - #[arg(long, env = "BUZZ_ACP_PERSONA_PACK")] - pub persona_pack: Option, - - /// Name of the persona within the pack to use. Required when --persona-pack is set. - #[arg(long, env = "BUZZ_ACP_PERSONA_NAME")] - pub persona_name: Option, + /// Team-owned instructions layered after `[System]` and before agent memory. + #[arg(long, env = "BUZZ_ACP_TEAM_INSTRUCTIONS")] + pub team_instructions: Option, /// Publish encrypted ACP observer frames over the relay. #[arg(long, env = "BUZZ_ACP_RELAY_OBSERVER", default_value_t = false)] @@ -462,6 +457,8 @@ pub struct Config { pub turn_liveness_secs: u64, pub heartbeat_prompt: Option, pub system_prompt: Option, + /// Team-owned instructions layered separately from the agent system prompt. + pub team_instructions: Option, pub initial_message: Option, pub subscribe_mode: SubscribeMode, pub dedup_mode: DedupMode, @@ -705,7 +702,7 @@ impl Config { .replace_range(.., &"0".repeat(args.private_key.len())); args.private_key.clear(); - let mut system_prompt = if let Some(text) = args.system_prompt { + let system_prompt = if let Some(text) = args.system_prompt { Some(text) } else if let Some(ref path) = args.system_prompt_file { Some(std::fs::read_to_string(path)?) @@ -902,52 +899,10 @@ impl Config { Vec::new() }; - // - // Precedence: CLI/env args > persona values > built-in defaults. - // Persona fills in what's missing. Explicit flags always win. - let (persona_system_prompt, persona_model, mut persona_env_vars) = - match (&args.persona_pack, &args.persona_name) { - (Some(pack_dir), Some(name)) => { - let pack = buzz_persona::resolve::resolve_pack(pack_dir).map_err(|e| { - ConfigError::ConfigFile(format!( - "failed to resolve pack {}: {e}", - pack_dir.display() - )) - })?; - let persona = pack - .personas - .into_iter() - .find(|p| p.name == *name) - .ok_or_else(|| { - ConfigError::ConfigFile(format!( - "persona '{name}' not found in pack {}", - pack_dir.display() - )) - })?; - ( - Some(persona.system_prompt), - persona.model, - persona.runtime_env_vars, - ) - } - (Some(_), None) => { - return Err(ConfigError::ConfigFile( - "--persona-pack requires --persona-name".into(), - )); - } - (None, Some(_)) => { - return Err(ConfigError::ConfigFile( - "--persona-name requires --persona-pack".into(), - )); - } - (None, None) => (None, None, vec![]), - }; - - // Apply persona defaults: CLI/env wins, persona fills gaps. - if system_prompt.is_none() { - system_prompt = persona_system_prompt; - } - let model = args.model.or(persona_model); + // Spawned desktop agents now carry a complete instance snapshot. Team + // instructions arrive independently so they can be layered at runtime. + let mut persona_env_vars = Vec::new(); + let model = args.model; // Inject CODEX_CONFIG so the @agentclientprotocol/codex-acp adapter (1.x) // opens the Seatbelt network sandbox for buzz-cli (an MCP subprocess). No-op @@ -975,6 +930,12 @@ impl Config { turn_liveness_secs, heartbeat_prompt, system_prompt, + team_instructions: args + .team_instructions + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_string), initial_message: args.initial_message, subscribe_mode: args.subscribe, dedup_mode: args.dedup, @@ -1346,6 +1307,7 @@ mod tests { turn_liveness_secs: 10, heartbeat_prompt: None, system_prompt: None, + team_instructions: None, initial_message: None, subscribe_mode: mode, dedup_mode: DedupMode::Queue, diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index ea8764b8a4..e459c9f572 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1399,6 +1399,7 @@ async fn tokio_main() -> Result<()> { turn_liveness_interval: Duration::from_secs(config.turn_liveness_secs), dedup_mode: config.dedup_mode, system_prompt: config.system_prompt.clone(), + team_instructions: config.team_instructions.clone(), base_prompt: if config.no_base_prompt { None } else if let Some(content) = base_prompt_content { @@ -3881,6 +3882,7 @@ mod build_mcp_servers_tests { turn_liveness_secs: 10, heartbeat_prompt: None, system_prompt: None, + team_instructions: None, initial_message: None, subscribe_mode: config::SubscribeMode::All, dedup_mode: config::DedupMode::Queue, @@ -4045,6 +4047,7 @@ mod error_outcome_emission_tests { turn_liveness_secs: 10, heartbeat_prompt: None, system_prompt: None, + team_instructions: None, initial_message: None, subscribe_mode: config::SubscribeMode::All, dedup_mode: config::DedupMode::Queue, diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index f6dae27aea..e57f44882d 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -379,6 +379,7 @@ pub struct PromptContext { pub turn_liveness_interval: Duration, pub dedup_mode: DedupMode, pub system_prompt: Option, + pub team_instructions: Option, pub heartbeat_prompt: Option, /// Base prompt content, or `None` if `--no-base-prompt` was passed. /// @@ -694,7 +695,10 @@ async fn create_session_and_apply_model( let combined_system_prompt: Option = if agent.protocol_version >= 2 { with_canvas( with_core( - framed_system_prompt(&ctx.cwd, ctx.base_prompt, ctx.system_prompt.as_deref()), + with_team( + framed_system_prompt(&ctx.cwd, ctx.base_prompt, ctx.system_prompt.as_deref()), + ctx.team_instructions.as_deref(), + ), agent_core, ), agent_canvas, @@ -1027,6 +1031,21 @@ fn workspace_section(cwd: &str) -> Option { } } +/// Append the team-owned instruction section after `[System]` and before core memory. +fn with_team(prompt: Option, instructions: Option<&str>) -> Option { + let instructions = instructions + .map(str::trim) + .filter(|value| !value.is_empty()); + match (prompt, instructions) { + (Some(prompt), Some(instructions)) => { + Some(format!("{prompt}\n\n[Team Instructions]\n{instructions}")) + } + (None, Some(instructions)) => Some(format!("[Team Instructions]\n{instructions}")), + (Some(prompt), None) => Some(prompt), + (None, None) => None, + } +} + /// Append the agent's core memory section onto the framed system prompt. /// /// Core already carries its own `[Agent Memory — core]` header from @@ -1552,6 +1571,7 @@ pub async fn run_prompt_task( has_system_prompt_support: agent.protocol_version >= 2, base_prompt: ctx.base_prompt, system_prompt: ctx.system_prompt.as_deref(), + team_instructions: ctx.team_instructions.as_deref(), agent_canvas: agent_canvas.as_deref(), }, ) @@ -4527,6 +4547,7 @@ mod tests { turn_liveness_interval: Duration::ZERO, dedup_mode: DedupMode::Drop, system_prompt: None, + team_instructions: None, heartbeat_prompt: None, base_prompt: None, cwd: ".".to_string(), diff --git a/crates/buzz-acp/src/queue.rs b/crates/buzz-acp/src/queue.rs index c1b4c56f7a..a58089951d 100644 --- a/crates/buzz-acp/src/queue.rs +++ b/crates/buzz-acp/src/queue.rs @@ -1327,6 +1327,8 @@ pub struct FormatPromptArgs<'a> { pub base_prompt: Option<&'a str>, /// System prompt content for legacy agents (protocol_version < 2). pub system_prompt: Option<&'a str>, + /// Team instructions for legacy agents, rendered after `[System]`. + pub team_instructions: Option<&'a str>, /// Rendered `[Channel Canvas]` metadata section for legacy agents. /// /// For modern agents (protocol_version >= 2) the section is delivered via @@ -1395,6 +1397,13 @@ pub fn format_prompt(batch: &FlushBatch, args: &FormatPromptArgs<'_>) -> Vec, pub version: String, - // → Config.system_prompt (persona body + pack_instructions) + // → Config.system_prompt (persona body only) pub system_prompt: String, + /// Pack-owned instructions kept separate for the team model migration. + pub pack_instructions: Option, // → Config.model (plain model ID, post-split) pub model: Option, @@ -195,7 +197,11 @@ fn resolve_one_persona( pack_instructions: Option<&str>, shared_mcp: Option<&serde_json::Value>, ) -> ResolvedPersona { - let system_prompt = compose_prompt(&lp.prompt, pack_instructions); + let system_prompt = lp.prompt.clone(); + let pack_instructions = pack_instructions + .map(str::trim) + .filter(|instructions| !instructions.is_empty()) + .map(str::to_string); // Split "provider:model-id" into separate fields (V3 contract). let (llm_provider, model) = match lp.model.as_deref() { @@ -228,6 +234,7 @@ fn resolve_one_persona( avatar: lp.avatar.clone(), version, system_prompt, + pack_instructions, model, llm_provider, runtime: lp.runtime.clone(), @@ -244,16 +251,6 @@ fn resolve_one_persona( } } -/// Compose the effective system prompt: persona body + pack instructions. -fn compose_prompt(persona_prompt: &str, pack_instructions: Option<&str>) -> String { - match pack_instructions { - Some(instructions) if !instructions.trim().is_empty() => { - format!("{persona_prompt}\n\n---\n# Team Instructions\n{instructions}") - } - _ => persona_prompt.to_owned(), - } -} - /// Convert `TriggersData` to `ResolvedTriggers`. fn resolve_triggers(rt: Option<&TriggersData>) -> ResolvedTriggers { match rt { @@ -407,26 +404,6 @@ mod tests { use crate::merge::{HooksData, TriggersData}; - #[test] - fn compose_prompt_body_only() { - let result = compose_prompt("You are a bot.", None); - assert_eq!(result, "You are a bot."); - } - - #[test] - fn compose_prompt_with_instructions() { - let result = compose_prompt("You are a bot.", Some("Follow the rules.")); - assert!(result.starts_with("You are a bot.")); - assert!(result.contains("# Team Instructions")); - assert!(result.contains("Follow the rules.")); - } - - #[test] - fn compose_prompt_empty_instructions_ignored() { - let result = compose_prompt("You are a bot.", Some(" ")); - assert_eq!(result, "You are a bot."); - } - #[test] fn triggers_from_triggers_data() { let data = TriggersData { @@ -708,10 +685,9 @@ mod tests { let pack = resolve_pack(dir).unwrap(); let p = &pack.personas[0]; - // Prompt composed with instructions - assert!(p.system_prompt.contains("You are Bot.")); - assert!(p.system_prompt.contains("# Team Instructions")); - assert!(p.system_prompt.contains("Always be helpful.")); + // Persona body kept separate from pack instructions. + assert_eq!(p.system_prompt, "You are Bot.\n"); + assert_eq!(p.pack_instructions.as_deref(), Some("Always be helpful.")); // Model split into separate fields (V3 contract) assert_eq!(p.model.as_deref(), Some("claude-sonnet-4-20250514")); diff --git a/crates/buzz-persona/tests/integration.rs b/crates/buzz-persona/tests/integration.rs index 76706e66e2..0b5ffc3b73 100644 --- a/crates/buzz-persona/tests/integration.rs +++ b/crates/buzz-persona/tests/integration.rs @@ -420,18 +420,20 @@ fn resolve_full_pipeline() { assert_eq!(lep.llm_provider.as_deref(), Some("anthropic")); assert_eq!(lep.model.as_deref(), Some("claude-sonnet-4-20250514")); - // System prompt composed: persona body + pack instructions + // System prompt is persona body only; pack instructions stay separate. assert!( pip.system_prompt.contains("You are Pip"), "pip prompt should contain persona body" ); assert!( - pip.system_prompt.contains("Be helpful"), - "pip prompt should contain pack instructions" + !pip.system_prompt.contains("Be helpful"), + "pip prompt should not contain pack instructions" ); assert!( - pip.system_prompt.contains("Team Instructions"), - "pip prompt should contain instructions header" + pip.pack_instructions + .as_deref() + .is_some_and(|instructions| instructions.contains("Be helpful")), + "pip.pack_instructions should carry the pack instructions" ); // Temperature inherited from defaults diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index fca1c16e28..c658fbb769 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -123,6 +123,7 @@ export default defineConfig({ "**/tokens.spec.ts", "**/persona-env-vars.spec.ts", "**/persona-sync.spec.ts", + "**/team-snapshot.spec.ts", "**/mesh-compute.spec.ts", "**/parity-ancestor-island.spec.ts", ], diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 54d7621733..2b75d8b729 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -107,7 +107,8 @@ const overrides = new Map([ ["src-tauri/src/managed_agents/nest.rs", 704], // keyring-dev-isolation: agent key migration added copy_agent_keys_between_stores // and load_readonly support; file grew past 1000 default. Queued to split. - ["src-tauri/src/managed_agents/storage.rs", 1325], + // +7 for try_delete_agent_key result-returning seam (snapshot-import rollback). + ["src-tauri/src/managed_agents/storage.rs", 1335], // harness-persona-sync: persona-runtime resolution threaded into the spawn // path here. Load-bearing feature growth; queued to split in the resolver // unify refactor followup. +26 for resolve_effective_prompt_model_provider @@ -164,7 +165,9 @@ const overrides = new Map([ // setup-mode requirements. The Windows-only requirement and serialization // test add eight lines; split remains queued with the existing file debt. // Windows Doctor install fix: cli_install_commands_windows field added to test stubs. - ["src-tauri/src/managed_agents/readiness.rs", 1764], + // team-instructions-first-class: ManagedAgentRecord fixture gains the new + // team_id field (+1 line). + ["src-tauri/src/managed_agents/readiness.rs", 1765], // applyWorkspace reposDir parameter plus the validateReposDir binding, // threaded through Tauri invokes for configurable repos_dir, plus the // harness-persona-sync `harnessOverride` create-input bit — load-bearing @@ -197,7 +200,9 @@ const overrides = new Map([ // RawInstallRuntimeResult + fromRawInstallRuntimeResult mapper (+2). // Git Bash Doctor discovery adds the raw Tauri response and its camelCase // mapper. This is the existing API boundary; split remains queued. - ["src/shared/api/tauri.ts", 1304], + // team-instructions-first-class: createManagedAgent Tauri bridge threads the + // new teamId input through to the backend (+1 line). + ["src/shared/api/tauri.ts", 1305], // doctor-npm-eacces-preflight: hint field added to InstallStepResult (+1 line). // codex-acp-package-swap: "adapter_outdated" variant added to AcpAvailabilityStatus (+1 line). // doctor-install-reliability: AuthStatus tagged union + nodeRequired/authStatus/ @@ -208,7 +213,11 @@ const overrides = new Map([ // Git Bash prerequisite payload adds four fields to the shared Tauri API // contract. This is the canonical type location; split remains queued. // signout-wipe: resetFailed field added to Identity type (+6 lines). - ["src/shared/api/types.ts", 1044], + // team-instructions-first-class: CreateManagedAgentInput.teamId (+2, incl. + // doc comment) and AgentTeam/CreateTeamInput/UpdateTeamInput.instructions + // (+3) — the new team-id spawn link and the runtime-layered instructions + // field. + ["src/shared/api/types.ts", 1047], // readiness-gate: PersonaDialog.tsx threads computeLocalModeGate + // requiredCredentialEnvKeys + RequiredFieldLabel so the "New agent" dialog // shows required markers and credential amber rows (parity with @@ -280,7 +289,9 @@ const overrides = new Map([ // candidates, cli_install_commands_for_os PowerShell selection, login_shell_path // None regression, .cmd shim resolution, no-git-bash error hint. // +32: deterministic .cmd resolver + no-registry + install_shell_from tests. - ["src-tauri/src/managed_agents/discovery/tests.rs", 1270], + // team-instructions-first-class: record_with test fixture gained the new + // ManagedAgentRecord.team_id field (+1 line) alongside persona_team_dir. + ["src-tauri/src/managed_agents/discovery/tests.rs", 1271], // 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 diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 30b74f6fca..f6141a53eb 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -631,6 +631,7 @@ mod tests { backend: BackendKind::Local, backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: "".to_string(), diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index eb19c8c208..daa448cc64 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -6,8 +6,8 @@ use crate::{ managed_agents::{ build_managed_agent_summary, current_instance_id, discover_provider_candidates, ensure_persona_is_active, find_managed_agent_mut, load_managed_agents, load_personas, - managed_agent_avatar_url, managed_agents_base_dir, normalize_agent_args, provider_deploy, - resolve_provider_binary, save_managed_agents, start_managed_agent_process, + load_teams, managed_agent_avatar_url, managed_agents_base_dir, normalize_agent_args, + provider_deploy, resolve_provider_binary, save_managed_agents, start_managed_agent_process, stop_managed_agent_process, sync_managed_agent_processes, try_regenerate_nest, validate_provider_config, BackendKind, CreateManagedAgentRequest, CreateManagedAgentResponse, ManagedAgentRecord, ManagedAgentSummary, RelayMeshConfig, @@ -585,20 +585,17 @@ pub async fn create_managed_agent( 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() - // matches on this internal name, NOT display_name. - let pack_metadata: Option<(std::path::PathBuf, String)> = - requested_persona_id.as_deref().and_then(|pid| { - let persona = personas.iter().find(|p| p.id == pid)?; - let team_id = persona.source_team.as_deref()?; - let slug = persona.source_team_persona_slug.as_deref()?; - let base = managed_agents_base_dir(&app).ok()?; - let team_path = base.join("teams").join(team_id); - // Use the validated slug stored during import — no need to - // re-resolve the pack. The slug is [a-zA-Z0-9_-]+ by construction. - Some((team_path, slug.to_owned())) - }); + let team_id = input + .team_id + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_string); + if let Some(team_id) = &team_id { + if !load_teams(&app)?.iter().any(|team| &team.id == team_id) { + return Err(format!("team {team_id} not found")); + } + } // Resolve the avatar URL once at creation and persist it on the record. // Explicit input wins, then the persona's own avatar, then the runtime @@ -656,6 +653,7 @@ pub async fn create_managed_agent( pubkey: pubkey.clone(), name: name.clone(), persona_id: requested_persona_id.clone(), + team_id, private_key_nsec: private_key_nsec.clone(), auth_tag: auth_tag.clone(), relay_url: resolved_relay_url.clone(), @@ -715,11 +713,8 @@ pub async fn create_managed_agent( backend: input.backend.clone(), backend_agent_id: None, provider_binary_path, - // Team-backed personas: record path + internal slug so the runtime - // can resolve team config at startup. Must be the slug (e.g., "lep"), - // NOT the display_name — ACP's resolve_persona_by_name() matches slugs. - persona_team_dir: pack_metadata.as_ref().map(|(path, _)| path.clone()), - persona_name_in_team: pack_metadata.as_ref().map(|(_, name)| name.clone()), + persona_team_dir: None, + persona_name_in_team: None, env_vars: input.env_vars.clone(), created_at: now_iso(), updated_at: now_iso(), diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index fbe0d30971..eb673353fa 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -35,6 +35,7 @@ fn bare_agent_record( backend: BackendKind::Local, backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: "".to_string(), diff --git a/desktop/src-tauri/src/commands/export_util.rs b/desktop/src-tauri/src/commands/export_util.rs index 8ebe4333bc..806f58d739 100644 --- a/desktop/src-tauri/src/commands/export_util.rs +++ b/desktop/src-tauri/src/commands/export_util.rs @@ -1,17 +1,6 @@ use tauri::AppHandle; use tauri_plugin_dialog::DialogExt; -/// Show a save-file dialog for a JSON export and write `data` to the chosen -/// path. Returns `Ok(true)` when the file was written, `Ok(false)` when the -/// user cancelled the dialog. -pub async fn save_json_with_dialog( - app: &AppHandle, - suggested_filename: &str, - data: &[u8], -) -> Result { - save_bytes_with_dialog(app, suggested_filename, "JSON", &["json"], data).await -} - /// Show a save-file dialog with a custom filter and write `data` to the chosen /// path. Returns `Ok(true)` when the file was written, `Ok(false)` when the /// user cancelled the dialog. diff --git a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs index b9961a584d..316af5f72d 100644 --- a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs +++ b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs @@ -43,6 +43,7 @@ fn make_agent( backend: BackendKind::Local, backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: "2026-01-01T00:00:00Z".to_string(), diff --git a/desktop/src-tauri/src/commands/personas/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound_tests.rs index afe1071fe6..6c7012e7e6 100644 --- a/desktop/src-tauri/src/commands/personas/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound_tests.rs @@ -185,6 +185,7 @@ fn local_agent() -> ManagedAgentRecord { }, backend_agent_id: Some("local-remote-id".to_string()), provider_binary_path: Some("/local/bin".to_string()), + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: "2025-01-01T00:00:00Z".to_string(), @@ -385,6 +386,7 @@ fn local_team() -> TeamRecord { id: TEAM_ID.to_string(), name: "Local Team".to_string(), description: Some("local desc".to_string()), + instructions: None, persona_ids: vec!["p-local".to_string()], is_builtin: false, source_dir: Some(std::path::PathBuf::from("/local/team/dir")), @@ -400,6 +402,7 @@ fn team_content(name: &str) -> TeamEventContent { TeamEventContent { name: name.to_string(), description: Some("remote desc".to_string()), + instructions: Some("remote instructions".to_string()), persona_ids: vec!["p-remote-1".to_string(), "p-remote-2".to_string()], } } @@ -418,6 +421,7 @@ fn inbound_team_match_patches_shared_preserves_local() { // Shared fields overwritten. assert_eq!(t.name, "Renamed Team"); assert_eq!(t.description, Some("remote desc".to_string())); + assert_eq!(t.instructions, Some("remote instructions".to_string())); assert_eq!( t.persona_ids, vec!["p-remote-1".to_string(), "p-remote-2".to_string()] diff --git a/desktop/src-tauri/src/commands/personas/mod.rs b/desktop/src-tauri/src/commands/personas/mod.rs index 7e2cd9caac..5ae6fbf488 100644 --- a/desktop/src-tauri/src/commands/personas/mod.rs +++ b/desktop/src-tauri/src/commands/personas/mod.rs @@ -8,9 +8,9 @@ use crate::{ delete_agent_key, effective_agent_command, load_managed_agents, load_personas, load_teams, managed_agent_avatar_url, persona_events::persona_d_tag, save_managed_agents, save_personas, stop_managed_agent_process, sync_managed_agent_processes, - team_events::TeamEventContent, team_persona_key, try_regenerate_nest, - validate_persona_activation_change, validate_persona_deletion, AgentDefinition, - CreatePersonaRequest, ManagedAgentRecord, TeamRecord, UpdatePersonaRequest, + team_events::TeamEventContent, try_regenerate_nest, validate_persona_activation_change, + validate_persona_deletion, AgentDefinition, CreatePersonaRequest, ManagedAgentRecord, + TeamRecord, UpdatePersonaRequest, }, util::now_iso, }; @@ -109,18 +109,12 @@ pub async fn create_persona( } /// Return value of the `update_persona` command. Uses flatten so all -/// `AgentDefinition` 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. +/// `AgentDefinition` fields appear at the top level of the JSON response — +/// backward-compatible with callers that already destructure a raw persona object. #[derive(Debug, serde::Serialize)] pub struct UpdatePersonaResult { #[serde(flatten)] persona: AgentDefinition, - /// 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, } /// Propagate a persona definition's display_name rename to linked agent instances. @@ -160,15 +154,11 @@ pub async fn update_persona( type ProfileSyncParams = Vec<(nostr::Keys, String, String, Option, Option)>; // Phase 1: synchronous save (persona record + linked agent avatar updates) - let (result, profile_sync_params, writeback_warning) = tokio::task::spawn_blocking({ + let (result, profile_sync_params) = tokio::task::spawn_blocking({ let app = app.clone(); - move || -> Result<(AgentDefinition, ProfileSyncParams, Option), String> { + move || -> Result<(AgentDefinition, ProfileSyncParams), String> { let state = app.state::(); let display_name = trim_required(&input.display_name, "Display name")?; - // Do not trim system_prompt: `compose_prompt` appends pack_instructions - // verbatim (including any trailing newline), and write_back_persona_md - // decomposes by suffix-stripping. Trimming would break that exact-suffix - // match for the common case where instructions.md has a trailing newline. let system_prompt = input.system_prompt.clone(); let avatar_url = trim_optional(input.avatar_url); let runtime = trim_optional(input.runtime); @@ -216,11 +206,6 @@ pub async fn update_persona( let result = persona.clone(); save_personas(&app, &personas)?; - // 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. - let writeback_warning = write_back_persona_md(&app, &result); - retain_persona_pending(&app, &state, &result); try_regenerate_nest(&app); @@ -298,7 +283,7 @@ pub async fn update_persona( Vec::new() }; - Ok((result, sync_params, writeback_warning)) + Ok((result, sync_params)) } }) .await @@ -326,15 +311,9 @@ pub async fn update_persona( } } - Ok(UpdatePersonaResult { - persona: result, - writeback_warning, - }) + Ok(UpdatePersonaResult { persona: result }) } -mod writeback; -use writeback::write_back_persona_md; - #[cfg(test)] mod delete_cascade_tests; #[cfg(test)] @@ -901,12 +880,14 @@ fn apply_inbound_team(teams: &mut Vec, d_tag: String, inbound: TeamE Some(local) => { local.name = inbound.name; local.description = inbound.description; + local.instructions = inbound.instructions; local.persona_ids = inbound.persona_ids; } None => teams.push(TeamRecord { id: d_tag, name: inbound.name, description: inbound.description, + instructions: inbound.instructions, persona_ids: inbound.persona_ids, is_builtin: false, source_dir: None, diff --git a/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs b/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs index 4199d05d9e..ba855ccbd6 100644 --- a/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs +++ b/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs @@ -32,6 +32,7 @@ fn agent(persona_id: &str, name: &str, display_name: Option<&str>) -> ManagedAge backend: Default::default(), backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: String::new(), diff --git a/desktop/src-tauri/src/commands/personas/snapshot/import.rs b/desktop/src-tauri/src/commands/personas/snapshot/import.rs index 40aa532bfe..9e2b8b3921 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/import.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/import.rs @@ -459,6 +459,7 @@ pub async fn confirm_agent_snapshot_import( backend: crate::managed_agents::BackendKind::Local, backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: now.clone(), diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs index 26d25d9ef6..b1d19f06b6 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs @@ -48,6 +48,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { backend: BackendKind::Local, backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: String::new(), diff --git a/desktop/src-tauri/src/commands/personas/writeback.rs b/desktop/src-tauri/src/commands/personas/writeback.rs deleted file mode 100644 index 6bab51e09e..0000000000 --- a/desktop/src-tauri/src/commands/personas/writeback.rs +++ /dev/null @@ -1,307 +0,0 @@ -//! Pack-backed persona write-back: resolve the source `.persona.md` via the -//! pack manifest and rewrite frontmatter fields. Extracted from the parent -//! module to keep it under the desktop file-size cap. Pure relocation. - -use super::*; - -/// Find the team whose `team_persona_key` equals `source_team`. This matches -/// the same key that `sync_team_from_dir` uses, covering both modern teams -/// (where `team.id` equals the manifest directory name) and legacy/backfilled -/// teams (where `team.id` is a UUID and the manifest id is `source_dir.file_name()`). -fn find_team_for_persona_source<'a>( - teams: &'a [TeamRecord], - source_team: &str, -) -> Option<&'a TeamRecord> { - teams - .iter() - .find(|t| t.id == source_team || team_persona_key(t) == source_team) -} - -/// Write updated frontmatter fields back to the source `.persona.md` file for -/// pack-backed personas (`source_team` is set). Non-fatal: any miss (no -/// `source_dir`, missing file, pack load failure, parse or write error) is -/// 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 -/// preserved byte-for-byte because `AgentDefinition.system_prompt` is the -/// _composed_ prompt (body + pack instructions appended by `compose_prompt`) -/// and writing it back to the file would cause the instructions to be -/// double-appended on the next launch sync. -/// -/// The source file path is derived from the pack manifest via -/// `buzz_persona_pkg::pack::load_pack` — the same resolution the launch sync -/// uses — rather than reconstructed by convention. This ensures write-back -/// targets the correct file regardless of where the manifest places the -/// `.persona.md` (e.g. `personas/` vs `agents/`, nested paths, or filenames -/// that differ from the persona `name:` field). -/// -/// The team is located via `find_team_for_persona_source`, which matches the -/// 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: &AgentDefinition) -> Option { - // 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: &AgentDefinition, -) -> Result<(), String> { - let Some(source_team_id) = &persona.source_team else { - return Ok(()); // non-pack persona — nothing to write back - }; - let Some(slug) = &persona.source_team_persona_slug else { - return Err(format!( - "persona {} has source_team but no slug; cannot write back", - persona.id - )); - }; - - 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; - - // 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, -/// an updated body (system prompt). Returns the full rewritten file content, or -/// the original unchanged when the result would be byte-identical. -/// -/// **Frontmatter fields rewritten:** `display_name`, `runtime`, `avatar`, and -/// `model` (joined `"provider:model"` per the pack format). All other keys and -/// their order are preserved. -/// -/// **Body (system prompt) write-back:** -/// The `persona.system_prompt` field holds the *composed* prompt: -/// `compose_prompt(raw_body, pack_instructions)`. To recover the raw body we -/// reverse `compose_prompt`: -/// -/// - If `pack_instructions` is absent or blank: new body = `system_prompt` -/// verbatim (no suffix to strip). -/// - If `pack_instructions` is present and non-blank: the composed prompt ends -/// with `"\n\n---\n# Team Instructions\n{instructions}"`. If -/// `system_prompt` ends with that exact suffix, strip it to get the new raw -/// body. **Safety guard**: if the suffix is absent (user edited inside the -/// Team Instructions block, or instructions drifted), we cannot safely -/// recover the raw body — preserve the existing body and log a skip. This -/// prevents a corrupted file or double-appended instructions. -/// - If `system_prompt` equals `compose_prompt(current_raw_body, instructions)` -/// exactly (user did not edit the prompt), the body is preserved -/// byte-for-byte (no-op for the body section). -fn rewrite_persona_md( - content: &str, - persona: &AgentDefinition, - current_raw_body: &str, - pack_instructions: Option<&str>, -) -> Result { - let (frontmatter, existing_body) = buzz_persona_pkg::persona::split_frontmatter(content) - .map_err(|e| format!("split_frontmatter: {e:?}"))?; - - let mut value = serde_yaml::from_str::(frontmatter) - .map_err(|e| format!("yaml parse: {e}"))?; - let mapping = value - .as_mapping_mut() - .ok_or("frontmatter is not a YAML mapping")?; - - // display_name - mapping.insert( - serde_yaml::Value::String("display_name".to_string()), - serde_yaml::Value::String(persona.display_name.clone()), - ); - - // runtime: set when Some, remove when None - let runtime_key = serde_yaml::Value::String("runtime".to_string()); - match &persona.runtime { - Some(rt) if !rt.is_empty() => { - mapping.insert(runtime_key, serde_yaml::Value::String(rt.clone())); - } - _ => { - mapping.remove(&runtime_key); - } - } - - // avatar: set when Some, remove when None - let avatar_key = serde_yaml::Value::String("avatar".to_string()); - match &persona.avatar_url { - Some(av) if !av.is_empty() => { - mapping.insert(avatar_key, serde_yaml::Value::String(av.clone())); - } - _ => { - mapping.remove(&avatar_key); - } - } - - // model: joined "provider:model" or bare "model"; remove when both absent - let model_key = serde_yaml::Value::String("model".to_string()); - match (&persona.provider, &persona.model) { - (Some(prov), Some(mdl)) if !prov.is_empty() && !mdl.is_empty() => { - mapping.insert( - model_key, - serde_yaml::Value::String(format!("{prov}:{mdl}")), - ); - } - (_, Some(mdl)) if !mdl.is_empty() => { - mapping.insert(model_key, serde_yaml::Value::String(mdl.clone())); - } - _ => { - mapping.remove(&model_key); - } - } - - let updated_frontmatter = - serde_yaml::to_string(&value).map_err(|e| format!("yaml serialize: {e}"))?; - - // Determine the body to write back. - // `compose_prompt` is: body + "\n\n---\n# Team Instructions\n{instructions}" - // when instructions is non-blank, or body verbatim when absent/blank. - let effective_instructions = pack_instructions.filter(|s| !s.trim().is_empty()); - let expected_composed = match effective_instructions { - Some(instr) => format!("{current_raw_body}\n\n---\n# Team Instructions\n{instr}"), - None => current_raw_body.to_owned(), - }; - - let new_body: &str = if persona.system_prompt == expected_composed { - // User did not edit the prompt — keep the existing body byte-for-byte. - existing_body - } else { - // User edited the prompt. Recover the raw body by reversing compose_prompt. - match effective_instructions { - None => { - // No pack instructions: composed == raw, write verbatim. - &persona.system_prompt - } - Some(instr) => { - let suffix = format!("\n\n---\n# Team Instructions\n{instr}"); - if let Some(raw) = persona.system_prompt.strip_suffix(suffix.as_str()) { - raw - } else { - // Safety guard: suffix absent — cannot safely recover raw body. - // Preserve the existing body to avoid corruption or double-append. - eprintln!( - "buzz-desktop: persona-writeback: \ - system_prompt does not end with expected Team Instructions suffix; \ - preserving existing body to avoid corruption" - ); - existing_body - } - } - } - }; - - Ok(format!("---\n{updated_frontmatter}---\n{new_body}")) -} - -#[cfg(test)] -mod tests; diff --git a/desktop/src-tauri/src/commands/personas/writeback/tests.rs b/desktop/src-tauri/src/commands/personas/writeback/tests.rs deleted file mode 100644 index 7e4a90e864..0000000000 --- a/desktop/src-tauri/src/commands/personas/writeback/tests.rs +++ /dev/null @@ -1,845 +0,0 @@ -use super::*; -use std::collections::BTreeMap; - -/// Build a minimal AgentDefinition 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>, -) -> AgentDefinition { - // 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, -) -> AgentDefinition { - AgentDefinition { - 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(), - 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 AgentDefinition 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/.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/.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 AgentDefinition -// 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; AgentDefinition.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 an AgentDefinition matching the legacy incident shape. -fn legacy_persona(source_team: &str, runtime: Option<&str>) -> AgentDefinition { - AgentDefinition { - 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(), - 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 `/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: `/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" - ); -} diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index 8f8b4dfc43..dc376ca064 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -1,8 +1,8 @@ //! Tauri commands for exporting and importing `buzz-team-snapshot v1` files. //! -//! Team snapshots are definition-only templates: importing creates key-less -//! agent definitions and one team record. It never mints agent keys, auth tags, -//! managed-agent instances, or restores member memory. +//! Team snapshots are full-instance bundles: importing mints a fresh keypair +//! and `ManagedAgentRecord` for every member plus one `TeamRecord`. Exporting +//! optionally includes member memory at the requested level. use serde::{Deserialize, Serialize}; use tauri::{AppHandle, Emitter, State}; @@ -16,9 +16,11 @@ use crate::{ encode_team_snapshot_json, encode_team_snapshot_png, TeamSnapshot, }, managed_agents::{ - agent_snapshot::{build_snapshot, AgentSnapshot, MemoryLevel}, - load_personas, load_teams, save_personas, save_teams, AgentDefinition, TeamRecord, + agent_snapshot::{build_snapshot, AgentSnapshot, AgentSnapshotMemoryEntry, MemoryLevel}, + load_managed_agents, load_personas, load_teams, load_teams_readonly, save_managed_agents, + save_personas, save_teams, AgentDefinition, ManagedAgentRecord, TeamRecord, }, + relay::{effective_agent_relay_url, relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, }; @@ -82,6 +84,17 @@ fn parse_format_is_png(format: &str) -> Result { } } +fn parse_memory_level(s: &str) -> Result { + match s { + "none" | "" => Ok(MemoryLevel::None), + "core" => Ok(MemoryLevel::Core), + "everything" => Ok(MemoryLevel::Everything), + other => Err(format!( + "Invalid memory_level: {other:?} (expected 'none', 'core', or 'everything')" + )), + } +} + fn effective_avatar(member: &AgentSnapshot) -> Option { member .profile @@ -91,8 +104,6 @@ fn effective_avatar(member: &AgentSnapshot) -> Option { } /// Build a definition from a team member snapshot without consuming its memory. -/// The ignored `memory` field is deliberate: definition-only imports have no -/// owner-to-agent key material or live instance to which memory could belong. fn definition_from_snapshot( member: &AgentSnapshot, keep_allowlist: bool, @@ -129,7 +140,7 @@ fn definition_from_snapshot( }) } -fn build_import_definitions( +pub(crate) fn build_import_definitions( snapshot: &TeamSnapshot, keep_allowlist: bool, now: &str, @@ -142,9 +153,7 @@ fn build_import_definitions( } /// Assemble the one new team record that references freshly built definitions. -/// Keeping this pure lets tests verify the definition-only import shape without -/// creating an `AppHandle` or touching the on-disk stores. -fn build_import_team( +pub(crate) fn build_import_team( snapshot: &TeamSnapshot, persona_ids: Vec, now: &str, @@ -159,6 +168,7 @@ fn build_import_team( name: name.to_string(), description: snapshot.team.description.clone(), persona_ids, + instructions: snapshot.team.instructions.clone(), is_builtin: false, source_dir: None, is_symlink: false, @@ -196,11 +206,12 @@ pub struct TeamSnapshotMemberPreview { pub struct TeamSnapshotImportPreview { pub name: String, pub description: Option, + pub instructions: Option, pub members: Vec, pub has_source_allowlist: bool, } -/// Confirmation input for a definition-only team snapshot import. +/// Confirmation input for a team snapshot import. #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct TeamSnapshotImportConfirm { @@ -209,12 +220,26 @@ pub struct TeamSnapshotImportConfirm { pub keep_allowlist: bool, } -/// Result of a definition-only team snapshot import. +/// Per-member outcome reported after a confirmed team snapshot import. +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct TeamSnapshotImportMemberResult { + pub display_name: String, + pub pubkey: String, + pub persona_id: String, + pub memory_written: usize, + pub memory_total: usize, + pub memory_errors: Vec, + pub profile_sync_error: Option, +} + +/// Result of a team snapshot import. #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] pub struct TeamSnapshotImportResult { pub team: TeamRecord, pub persona_ids: Vec, + pub members: Vec, } /// In-memory bytes for the native team sharing flow. @@ -225,9 +250,23 @@ pub struct EncodedTeamSnapshotPayload { pub file_name: String, } +/// Per-member minted key material, assembled before entering the store lock. +struct MintedMember { + definition: AgentDefinition, + record: ManagedAgentRecord, + agent_keys: nostr::Keys, + pubkey: String, + auth_tag: Option, + display_name: String, + effective_avatar: Option, +} + fn build_team_export_snapshot( team: &TeamRecord, personas: &[AgentDefinition], + records: &[ManagedAgentRecord], + memory_level: MemoryLevel, + memory_entries_by_persona: &std::collections::HashMap>, ) -> Result { let members = team .persona_ids @@ -239,10 +278,29 @@ fn build_team_export_snapshot( .ok_or_else(|| { format!("team {} references missing agent definition {id}", team.id) })?; + + // Find the team instance for this persona (if any). + let instance = records.iter().find(|r| { + r.team_id.as_deref() == Some(&team.id) + && r.persona_id.as_deref() == Some(&persona.id) + }); + + // Use memory only when we have a live instance and a non-None level. + let (effective_level, entries) = match (instance, memory_level) { + (Some(_), level) if level != MemoryLevel::None => { + let entries = memory_entries_by_persona + .get(&persona.id) + .cloned() + .unwrap_or_default(); + (level, entries) + } + _ => (MemoryLevel::None, Vec::new()), + }; + Ok(build_snapshot( &persona.clone().into_agent_record(), - MemoryLevel::None, - Vec::new(), + effective_level, + entries, None, )) }) @@ -253,10 +311,13 @@ fn build_team_export_snapshot( async fn materialize_team_snapshot_bytes( id: String, is_png: bool, + memory_level: MemoryLevel, app: AppHandle, state: State<'_, AppState>, ) -> Result { - let (team, personas) = { + let effective_memory_level = memory_level; + + let (team, personas, records) = { let _store_guard = state .managed_agents_store_lock .lock() @@ -265,10 +326,57 @@ async fn materialize_team_snapshot_bytes( .into_iter() .find(|team| team.id == id) .ok_or_else(|| format!("team {id} not found"))?; - (team, load_personas(&app)?) + let personas = load_personas(&app)?; + let records = load_managed_agents(&app)?; + (team, personas, records) }; - let snapshot = build_team_export_snapshot(&team, &personas)?; + // Fetch memory for each member that has a live instance, outside the lock. + let mut memory_entries_by_persona: std::collections::HashMap< + String, + Vec, + > = std::collections::HashMap::new(); + + if effective_memory_level != MemoryLevel::None { + for persona_id in &team.persona_ids { + let instance = records.iter().find(|r| { + r.team_id.as_deref() == Some(&team.id) + && r.persona_id.as_deref() == Some(persona_id.as_str()) + }); + if let Some(instance) = instance { + let listing = crate::commands::engrams::get_agent_memory( + instance.pubkey.clone(), + app.clone(), + state.clone(), + ) + .await?; + let mut entries = Vec::new(); + if let Some(core) = listing.core { + entries.push(AgentSnapshotMemoryEntry { + slug: core.slug, + body: core.body, + }); + } + if effective_memory_level == MemoryLevel::Everything { + for mem in listing.memories { + entries.push(AgentSnapshotMemoryEntry { + slug: mem.slug, + body: mem.body, + }); + } + } + memory_entries_by_persona.insert(persona_id.clone(), entries); + } + } + } + + let snapshot = build_team_export_snapshot( + &team, + &personas, + &records, + effective_memory_level, + &memory_entries_by_persona, + )?; let slug = crate::util::slugify(&team.name, "team", 50); let (file_bytes, file_name) = if is_png { let bytes = encode_team_snapshot_png(&snapshot) @@ -295,16 +403,19 @@ async fn materialize_team_snapshot_bytes( }) } -/// Export a team template with each member's memory explicitly set to `none`. +/// Export a team snapshot with optional member memory. #[tauri::command] pub async fn export_team_snapshot( id: String, format: String, + memory_level: String, app: AppHandle, state: State<'_, AppState>, ) -> Result { let is_png = parse_format_is_png(&format)?; - let payload = materialize_team_snapshot_bytes(id, is_png, app.clone(), state).await?; + let memory_level = parse_memory_level(&memory_level)?; + let payload = + materialize_team_snapshot_bytes(id, is_png, memory_level, app.clone(), state).await?; if is_png { save_bytes_with_dialog( &app, @@ -326,15 +437,18 @@ pub async fn export_team_snapshot( } } -/// Encode a team template for the native send flow without opening a dialog. +/// Encode a team snapshot for the native send flow without opening a dialog. #[tauri::command] pub async fn encode_team_snapshot_for_send( id: String, format: String, + memory_level: String, app: AppHandle, state: State<'_, AppState>, ) -> Result { - materialize_team_snapshot_bytes(id, parse_format_is_png(&format)?, app, state).await + let memory_level = parse_memory_level(&memory_level)?; + materialize_team_snapshot_bytes(id, parse_format_is_png(&format)?, memory_level, app, state) + .await } /// Decode a team snapshot into a confirmation preview without writing anything. @@ -349,6 +463,7 @@ pub async fn preview_team_snapshot_import( Ok(TeamSnapshotImportPreview { name: snapshot.team.name, description: snapshot.team.description, + instructions: snapshot.team.instructions, has_source_allowlist: members.iter().any(|member| member.has_source_allowlist), members, }) @@ -357,49 +472,473 @@ pub async fn preview_team_snapshot_import( .map_err(|e| format!("spawn_blocking failed: {e}"))? } -/// Import a team snapshot as key-less agent definitions plus one team record. -/// No keypair, NIP-OA auth tag, managed-agent instance, or memory entry is -/// created; member memory is intentionally inert template data. +/// Import a team snapshot, minting full agent instances for every member. +/// +/// Phase sequence: +/// 1. Validate — decode the manifest and resolve all behavioral defaults. +/// Fail immediately if any member is invalid — zero writes. +/// 2. Mint — generate a fresh keypair + NIP-OA auth tag for EVERY member. +/// If ANY generation fails, return immediately — zero writes. +/// 3. Store — inside `managed_agents_store_lock`: write all `AgentDefinition`s +/// + all `ManagedAgentRecord`s (with `team_id` set) + `TeamRecord`. +/// Both store files are snapshotted (or noted absent) before the first +/// write. On any write error the pre-import state is restored — including +/// deleting a file that was absent, cleaning minted keyring entries, and +/// surfacing rollback failures alongside the original error. This makes +/// the store phase all-or-none for ordinary application errors; a process +/// crash between atomic file commits is NOT covered. +/// 4. Profile sync — for each member, call `sync_managed_agent_profile`. +/// Best-effort; errors are collected per member. +/// 5. Memory restore — for each member with non-empty snapshot memory, +/// publish each entry as a `kind:30174` engram event. Best-effort. +/// +/// Importing the same file twice yields two distinct teams with different +/// agent keypairs (same as individual agent import). #[tauri::command] pub async fn confirm_team_snapshot_import( input: TeamSnapshotImportConfirm, app: AppHandle, state: State<'_, AppState>, ) -> Result { + // ── Phase 1: validate (no I/O) ─────────────────────────────────────────── let snapshot = decode_team_snapshot_from_bytes(&input.file_bytes)?; let now = now_iso(); - // Resolve every member before locking or writing so an invalid allowlist - // cannot leave a partially imported team behind. + + // Resolve behavioral defaults for every member before any key generation. let definitions = build_import_definitions(&snapshot, input.keep_allowlist, &now)?; - let persona_ids: Vec = definitions - .iter() - .map(|definition| definition.id.clone()) - .collect(); + let persona_ids: Vec = definitions.iter().map(|d| d.id.clone()).collect(); let imported_team = build_import_team(&snapshot, persona_ids.clone(), &now)?; + // ── Phase 2: mint keys + auth tags (sync, outside lock) ───────────────── + // All mints must succeed before we enter the store. If any fails, zero writes. + let owner_pubkey_hex = { + let keys = state.signing_keys()?; + keys.public_key().to_hex() + }; + + let mut minted: Vec = Vec::with_capacity(snapshot.members.len()); + for (member, definition) in snapshot.members.iter().zip(definitions) { + let display_name = definition.display_name.clone(); + let effective_avatar_url = effective_avatar(member); + let respond_to_wire = definition.respond_to.clone(); + let minted_parallelism = definition.parallelism; + + let (agent_keys, private_key_nsec, pubkey, auth_tag) = { + let owner_keys = state.signing_keys()?; + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let private_key_nsec = { + use nostr::ToBech32; + agent_keys + .secret_key() + .to_bech32() + .map_err(|e| format!("failed to encode agent private key: {e}"))? + }; + // NIP-OA auth tag: bridge nostr 0.37 → 0.36 (buzz-sdk) via hex round-trip. + let compat_owner = nostr::Keys::parse(&owner_keys.secret_key().to_secret_hex()) + .map_err(|e| format!("failed to bridge owner keys: {e}"))?; + let compat_agent = nostr::PublicKey::from_hex(&pubkey) + .map_err(|e| format!("failed to bridge agent pubkey: {e}"))?; + let auth_tag = Some( + buzz_sdk_pkg::nip_oa::compute_auth_tag(&compat_owner, &compat_agent, "") + .map_err(|e| format!("failed to compute NIP-OA auth tag: {e}"))?, + ); + (agent_keys, private_key_nsec, pubkey, auth_tag) + }; + + // Build the ManagedAgentRecord for this member. + let record = ManagedAgentRecord { + pubkey: pubkey.clone(), + name: display_name.clone(), + display_name: None, + slug: None, + persona_id: Some(definition.id.clone()), + private_key_nsec: private_key_nsec.clone(), + auth_tag: auth_tag.clone(), + relay_url: String::new(), + avatar_url: effective_avatar_url.clone(), + acp_command: crate::managed_agents::DEFAULT_ACP_COMMAND.to_string(), + agent_command: String::new(), + agent_command_override: None, + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 0, + idle_timeout_seconds: member.definition.idle_timeout_seconds, + max_turn_duration_seconds: member.definition.max_turn_duration_seconds, + parallelism: minted_parallelism + .unwrap_or(crate::managed_agents::DEFAULT_AGENT_PARALLELISM), + system_prompt: member.definition.system_prompt.clone(), + model: member.definition.model.clone(), + provider: member.definition.provider.clone(), + persona_source_version: None, + env_vars: std::collections::BTreeMap::new(), + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: crate::managed_agents::BackendKind::Local, + backend_agent_id: None, + provider_binary_path: None, + team_id: Some(imported_team.id.clone()), + persona_team_dir: None, + persona_name_in_team: None, + created_at: now.clone(), + updated_at: now.clone(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: { + use crate::managed_agents::RespondTo; + respond_to_wire + .as_deref() + .map(RespondTo::parse_wire) + .transpose()? + .unwrap_or_default() + }, + respond_to_allowlist: definition.respond_to_allowlist.clone(), + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + definition_respond_to: respond_to_wire.clone(), + definition_respond_to_allowlist: definition.respond_to_allowlist.clone(), + definition_parallelism: minted_parallelism, + relay_mesh: None, + runtime: member.definition.runtime.clone(), + name_pool: member.definition.name_pool.clone(), + }; + + minted.push(MintedMember { + definition, + record, + agent_keys, + pubkey, + auth_tag, + display_name, + effective_avatar: effective_avatar_url, + }); + } + + // ── Phase 3: store (sync, inside lock) ────────────────────────────────── let team = { let _store_guard = state .managed_agents_store_lock .lock() .map_err(|e| e.to_string())?; + + // Guard against duplicate pubkeys (astronomically unlikely). + let existing_records = load_managed_agents(&app)?; + for m in &minted { + if existing_records.iter().any(|r| r.pubkey == m.pubkey) { + return Err(format!( + "generated pubkey {} already exists — retry", + m.pubkey + )); + } + } + + // Snapshot both store files for rollback on partial write failure. + // Distinguish "file exists with content" from "file absent" so rollback + // can delete a file created by the import rather than leaving orphaned + // records. + let agents_store_path = crate::managed_agents::storage::managed_agents_store_path(&app)?; + let agents_store_snapshot = match std::fs::read(&agents_store_path) { + Ok(bytes) => Some(bytes), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => return Err(format!("failed to snapshot agent store: {e}")), + }; + let teams_store_path = crate::managed_agents::teams_store_path(&app)?; + let teams_store_snapshot = match std::fs::read(&teams_store_path) { + Ok(bytes) => Some(bytes), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => return Err(format!("failed to snapshot teams store: {e}")), + }; + + // Pre-read teams via the read-only loader BEFORE any agent commits. + // This avoids load_teams()'s write-on-load side effect (teams.rs:165-166 + // saves whenever the file is absent or built-ins changed). A failure here + // aborts cleanly — zero writes have occurred. + let mut teams = load_teams_readonly(&teams_store_path)?; + + // Collect minted pubkeys for keyring cleanup on rollback. + let minted_pubkeys: Vec<&str> = minted.iter().map(|m| m.pubkey.as_str()).collect(); + + // Restore the agent store to pre-import state and clean minted keyring + // entries. Returns the original error, extended with rollback details. + let rollback_agents = |original_err: String| -> String { + let mut errors = vec![original_err]; + // Clean minted keyring entries. + for pubkey in &minted_pubkeys { + if let Err(e) = crate::managed_agents::storage::try_delete_agent_key(pubkey) { + errors.push(format!("keyring cleanup {pubkey}: {e}")); + } + } + // Restore agent store file. + let restore = match &agents_store_snapshot { + Some(bytes) => crate::managed_agents::storage::atomic_write_json_restricted( + &agents_store_path, + bytes, + ), + None => match std::fs::remove_file(&agents_store_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + other => other.map_err(|e| e.to_string()), + }, + }; + if let Err(e) = restore { + errors.push(format!("agent store restore: {e}")); + } + if errors.len() == 1 { + errors.into_iter().next().unwrap() + } else { + errors.join("; ") + } + }; + + // Write all definitions. let mut personas = load_personas(&app)?; - personas.extend(definitions.iter().cloned()); - save_personas(&app, &personas)?; - for definition in &definitions { - crate::commands::personas::retain_persona_pending(&app, &state, definition); + for m in &minted { + personas.push(m.definition.clone()); + } + if let Err(e) = save_personas(&app, &personas) { + return Err(rollback_agents(e)); } - let mut teams = load_teams(&app)?; + // Write all managed-agent records. + let mut records = existing_records; + for m in &minted { + records.push(m.record.clone()); + } + if let Err(e) = save_managed_agents(&app, &records) { + return Err(rollback_agents(e)); + } + + // Write the team record. `teams` was pre-loaded via the read-only + // loader before any agent commits, so a read/parse failure already + // aborted before any phase-3 write. save_teams sorts and persists. teams.push(imported_team.clone()); - let team = imported_team; - save_teams(&app, &teams)?; - crate::commands::teams::retain_team_pending(&app, &state, &team); + if let Err(e) = save_teams(&app, &teams) { + let err = rollback_agents(e); + // Also restore teams store. + let teams_restore = match &teams_store_snapshot { + Some(bytes) => { + crate::managed_agents::storage::atomic_write_json(&teams_store_path, bytes) + } + None => match std::fs::remove_file(&teams_store_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + other => other.map_err(|e| e.to_string()), + }, + }; + return Err(match teams_restore { + Ok(()) => err, + Err(teams_err) => format!("{err}; teams store restore: {teams_err}"), + }); + } + + // All writes committed — safe to update in-memory state. + for m in &minted { + crate::commands::personas::retain_persona_pending(&app, &state, &m.definition); + } + for m in &minted { + retain_agent_pending(&app, &state, &m.record); + } + crate::commands::teams::retain_team_pending(&app, &state, &imported_team); + crate::managed_agents::try_regenerate_nest(&app); let _ = app.emit("agents-data-changed", ()); - team + + imported_team }; - Ok(TeamSnapshotImportResult { team, persona_ids }) + // ── Phase 4 & 5: profile sync + memory restore (async, outside lock) ──── + let relay_ws = relay_ws_url_with_override(&state); + let mut member_results: Vec = Vec::with_capacity(minted.len()); + + for (m, snap_member) in minted.iter().zip(snapshot.members.iter()) { + let relay_url = effective_agent_relay_url(&m.record.relay_url, &relay_ws); + + // Phase 4: profile sync (best-effort). + let profile_sync_error = sync_managed_agent_profile( + &state, + &relay_url, + &m.agent_keys, + &m.display_name, + m.effective_avatar.as_deref(), + m.auth_tag.as_deref(), + ) + .await + .err(); + + // Phase 5: memory restore (best-effort). + let memory_total = snap_member.memory.entries.len(); + let mut memory_written = 0usize; + let mut memory_errors: Vec = Vec::new(); + + if memory_total > 0 { + let owner_pubkey = nostr::PublicKey::from_hex(&owner_pubkey_hex) + .map_err(|e| format!("failed to parse owner pubkey: {e}"))?; + let base_ts = nostr::Timestamp::now().as_secs(); + + for (idx, entry) in snap_member.memory.entries.iter().enumerate() { + let body = if entry.slug == buzz_core_pkg::engram::CORE_SLUG { + buzz_core_pkg::engram::Body::Core { + profile: entry.body.clone(), + } + } else { + buzz_core_pkg::engram::Body::Memory { + slug: entry.slug.clone(), + value: Some(entry.body.clone()), + } + }; + + let created_at = base_ts + idx as u64; + match buzz_core_pkg::engram::build_event( + &m.agent_keys, + &owner_pubkey, + &body, + created_at, + ) { + Ok(event) => { + use nostr::JsonUtil; + let event_json = event.as_json().into_bytes(); + let url = + format!("{}/events", crate::relay::relay_http_base_url(&relay_url)); + match submit_engram_event( + &state, + &m.agent_keys, + &event_json, + &url, + m.auth_tag.as_deref(), + ) + .await + { + Ok(()) => memory_written += 1, + Err(e) => memory_errors.push(format!("slug {:?}: {e}", entry.slug)), + } + } + Err(e) => { + memory_errors.push(format!("slug {:?}: build failed: {e}", entry.slug)); + } + } + } + } + + member_results.push(TeamSnapshotImportMemberResult { + display_name: m.display_name.clone(), + pubkey: m.pubkey.clone(), + persona_id: m.definition.id.clone(), + memory_written, + memory_total, + memory_errors, + profile_sync_error, + }); + } + + Ok(TeamSnapshotImportResult { + team, + persona_ids, + members: member_results, + }) +} + +/// Inline retention for the managed-agent kind:30177 event — mirrors +/// `commands::personas::snapshot::import::retain_agent_pending`. +fn retain_agent_pending(app: &AppHandle, state: &AppState, record: &ManagedAgentRecord) { + use crate::managed_agents::{ + agent_events::{agent_event_content, build_agent_event}, + managed_agents_base_dir, + persona_events::monotonic_created_at, + retention::{get_retained_event, open_retention_db, retain_event, RetainedEvent}, + }; + use buzz_core_pkg::kind::KIND_MANAGED_AGENT; + use nostr::JsonUtil; + + let result = (|| -> Result<(), String> { + let conn = open_retention_db(&managed_agents_base_dir(app)?.join("retention.db"))?; + let content = serde_json::to_string(&agent_event_content(record)) + .map_err(|e| format!("failed to serialize agent content: {e}"))?; + let (owner_pubkey, event) = { + let keys = state.signing_keys()?; + let owner_pubkey = keys.public_key().to_hex(); + let existing = + get_retained_event(&conn, KIND_MANAGED_AGENT, &owner_pubkey, &record.pubkey)?; + if existing.as_ref().is_some_and(|row| row.content == content) { + return Ok(()); + } + let event = build_agent_event(record)? + .custom_created_at(monotonic_created_at(existing.map(|row| row.created_at))) + .sign_with_keys(&keys) + .map_err(|e| format!("failed to sign agent event: {e}"))?; + (owner_pubkey, event) + }; + retain_event( + &conn, + &RetainedEvent { + kind: KIND_MANAGED_AGENT, + pubkey: owner_pubkey, + d_tag: record.pubkey.clone(), + content: event.content.to_string(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: true, + }, + ) + })(); + if let Err(e) = result { + eprintln!("buzz-desktop: team-snapshot-import retain-agent: {e}"); + } +} + +/// POST a pre-built signed engram event to the relay, authenticating as the +/// new agent. Mirrors the same helper in `snapshot::import`. +async fn submit_engram_event( + state: &AppState, + agent_keys: &nostr::Keys, + event_json: &[u8], + url: &str, + auth_tag: Option<&str>, +) -> Result<(), String> { + use crate::relay::build_nip98_auth_header_for_keys; + use reqwest::Method; + + let auth = build_nip98_auth_header_for_keys(agent_keys, &Method::POST, url, event_json)?; + let mut request = state + .http_client + .post(url) + .header("Authorization", auth) + .header("Content-Type", "application/json"); + if let Some(tag) = auth_tag { + request = request.header("x-auth-tag", tag); + } + let response = request + .body(event_json.to_vec()) + .send() + .await + .map_err(|e| crate::relay::classify_request_error(&e))?; + + if !response.status().is_success() { + let msg = crate::relay::relay_error_message(response).await; + return Err(format!("relay rejected engram: {msg}")); + } + + let body = response + .text() + .await + .map_err(|e| format!("failed to read relay response: {e}"))?; + let parsed: serde_json::Value = + serde_json::from_str(&body).map_err(|e| format!("relay response not JSON: {e}"))?; + let accepted = parsed + .get("accepted") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + if !accepted { + let message = parsed + .get("message") + .and_then(|v| v.as_str()) + .unwrap_or("unknown"); + return Err(format!("relay rejected engram: {message}")); + } + Ok(()) } #[cfg(test)] diff --git a/desktop/src-tauri/src/commands/team_snapshot/tests.rs b/desktop/src-tauri/src/commands/team_snapshot/tests.rs index facf6b0dfe..ca7dc61830 100644 --- a/desktop/src-tauri/src/commands/team_snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/team_snapshot/tests.rs @@ -44,6 +44,7 @@ fn snapshot(members: Vec) -> TeamSnapshot { team: TeamSnapshotMeta { name: "Review Team".to_string(), description: Some("Reviews changes".to_string()), + instructions: Some("Be thorough.".to_string()), }, members, } @@ -97,6 +98,7 @@ fn team_export_round_trip_preserves_team_and_excludes_member_memory() { id: "review".to_string(), name: "Review Team".to_string(), description: Some("Reviews changes".to_string()), + instructions: Some("Be thorough.".to_string()), persona_ids: vec!["alice".to_string(), "bob".to_string()], is_builtin: false, source_dir: None, @@ -107,13 +109,23 @@ fn team_export_round_trip_preserves_team_and_excludes_member_memory() { updated_at: "now".to_string(), }; - let bytes = - encode_team_snapshot_json(&build_team_export_snapshot(&team, &definitions).unwrap()) - .unwrap(); + // Export with no instances and MemoryLevel::None — memory stays empty. + let bytes = encode_team_snapshot_json( + &build_team_export_snapshot( + &team, + &definitions, + &[], + MemoryLevel::None, + &std::collections::HashMap::new(), + ) + .unwrap(), + ) + .unwrap(); let decoded = decode_team_snapshot_from_bytes(&bytes).unwrap(); assert_eq!(decoded.team.name, "Review Team"); assert_eq!(decoded.team.description.as_deref(), Some("Reviews changes")); + assert_eq!(decoded.team.instructions.as_deref(), Some("Be thorough.")); assert_eq!(decoded.members.len(), 2); assert!(decoded.members.iter().all(|member| { member.memory.level == MemoryLevel::None && member.memory.entries.is_empty() @@ -121,7 +133,151 @@ fn team_export_round_trip_preserves_team_and_excludes_member_memory() { } #[test] -fn team_import_creates_definitions_without_instances_or_memory() { +fn team_export_with_instance_and_memory_level_uses_supplied_entries() { + let definitions = vec![AgentDefinition { + id: "alice".to_string(), + display_name: "Alice".to_string(), + avatar_url: None, + system_prompt: "Alice prompt".to_string(), + runtime: Some("goose".to_string()), + model: None, + provider: None, + name_pool: vec![], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + env_vars: Default::default(), + respond_to: None, + respond_to_allowlist: vec![], + parallelism: None, + created_at: "now".to_string(), + updated_at: "now".to_string(), + }]; + let team = TeamRecord { + id: "t1".to_string(), + name: "Team".to_string(), + description: None, + instructions: None, + persona_ids: vec!["alice".to_string()], + is_builtin: false, + source_dir: None, + is_symlink: false, + symlink_target: None, + version: None, + created_at: "now".to_string(), + updated_at: "now".to_string(), + }; + + // Build a fake instance record tied to this team+persona. + let instance = ManagedAgentRecord { + pubkey: "a".repeat(64), + name: "Alice".to_string(), + display_name: None, + slug: None, + persona_id: Some("alice".to_string()), + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: crate::managed_agents::DEFAULT_ACP_COMMAND.to_string(), + 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: crate::managed_agents::DEFAULT_AGENT_PARALLELISM, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars: Default::default(), + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: crate::managed_agents::BackendKind::Local, + backend_agent_id: None, + provider_binary_path: None, + team_id: Some("t1".to_string()), + persona_team_dir: None, + persona_name_in_team: None, + created_at: "now".to_string(), + updated_at: "now".to_string(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: crate::managed_agents::RespondTo::default(), + respond_to_allowlist: vec![], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: vec![], + definition_parallelism: None, + relay_mesh: None, + runtime: None, + name_pool: vec![], + }; + + let mut memory_map = std::collections::HashMap::new(); + memory_map.insert( + "alice".to_string(), + vec![ + AgentSnapshotMemoryEntry { + slug: "core".to_string(), + body: "Alice is an expert reviewer.".to_string(), + }, + AgentSnapshotMemoryEntry { + slug: "mem/pref".to_string(), + body: "prefers concise answers".to_string(), + }, + ], + ); + + // MemoryLevel::Everything with an instance → entries should appear. + let snap = build_team_export_snapshot( + &team, + &definitions, + std::slice::from_ref(&instance), + MemoryLevel::Everything, + &memory_map, + ) + .unwrap(); + assert_eq!(snap.members[0].memory.level, MemoryLevel::Everything); + assert_eq!(snap.members[0].memory.entries.len(), 2); + + // MemoryLevel::None → entries stripped regardless. + let snap_none = build_team_export_snapshot( + &team, + &definitions, + std::slice::from_ref(&instance), + MemoryLevel::None, + &memory_map, + ) + .unwrap(); + assert_eq!(snap_none.members[0].memory.level, MemoryLevel::None); + assert!(snap_none.members[0].memory.entries.is_empty()); + + // No instance → entries stripped even with Everything level. + let snap_no_instance = build_team_export_snapshot( + &team, + &definitions, + &[], + MemoryLevel::Everything, + &memory_map, + ) + .unwrap(); + assert_eq!(snap_no_instance.members[0].memory.level, MemoryLevel::None); + assert!(snap_no_instance.members[0].memory.entries.is_empty()); +} + +#[test] +fn team_import_definitions_are_built_for_all_members() { let mut memory_bearing = member("Alice"); memory_bearing.memory = AgentSnapshotMemory { level: MemoryLevel::Everything, @@ -147,6 +303,7 @@ fn team_import_creates_definitions_without_instances_or_memory() { assert_eq!(definitions.len(), 2); assert_eq!(team.persona_ids.len(), 2); + assert_eq!(team.instructions.as_deref(), Some("Be thorough.")); assert_eq!( team.persona_ids, definitions @@ -161,8 +318,6 @@ fn team_import_creates_definitions_without_instances_or_memory() { && definition.respond_to_allowlist.is_empty() })); assert_eq!(definitions[0].system_prompt, "Alice prompt"); - // `AgentDefinition` has no key/auth/memory fields: the exact import plan - // creates N definitions + one TeamRecord, never a managed instance/key. } #[test] @@ -196,3 +351,376 @@ fn canonical_team_json_is_accepted_without_extension_case_policy() { // canonical lowercase and uppercase extensions reach this same safe path. assert!(decode_team_snapshot_from_bytes(&bytes).is_ok()); } + +#[test] +fn parse_memory_level_round_trips_all_variants() { + assert_eq!(parse_memory_level("none").unwrap(), MemoryLevel::None); + assert_eq!(parse_memory_level("").unwrap(), MemoryLevel::None); + assert_eq!(parse_memory_level("core").unwrap(), MemoryLevel::Core); + assert_eq!( + parse_memory_level("everything").unwrap(), + MemoryLevel::Everything + ); + assert!(parse_memory_level("bad").is_err()); +} + +// ── Rollback pattern tests ───────────────────────────────────────────── +// +// These test the file-level rollback mechanics used by confirm_team_snapshot_import +// Phase 3: snapshot files (or note absent), attempt writes, restore on failure. +// The Tauri AppHandle is not available in unit tests, so we exercise the same +// atomic_write + snapshot + restore operations directly on tempdir files. + +#[cfg(unix)] +#[test] +fn rollback_restores_existing_agent_store_after_failed_write() { + let dir = tempfile::tempdir().unwrap(); + let agents_path = dir.path().join("managed-agents.json"); + let original = b"[{\"pubkey\":\"existing\"}]"; + std::fs::write(&agents_path, original).unwrap(); + + // Snapshot — should succeed for existing file. + let snapshot = match std::fs::read(&agents_path) { + Ok(bytes) => Some(bytes), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => panic!("unexpected read error: {e}"), + }; + assert!(snapshot.is_some()); + + // Simulate a write that changed the file. + std::fs::write(&agents_path, b"[{\"pubkey\":\"imported\"}]").unwrap(); + + // Rollback: restore from snapshot. + let restore = match &snapshot { + Some(bytes) => { + crate::managed_agents::storage::atomic_write_json_restricted(&agents_path, bytes) + } + None => match std::fs::remove_file(&agents_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + other => other.map_err(|e| e.to_string()), + }, + }; + assert!(restore.is_ok()); + assert_eq!(std::fs::read(&agents_path).unwrap(), original); +} + +#[cfg(unix)] +#[test] +fn rollback_deletes_file_that_was_absent_before_import() { + let dir = tempfile::tempdir().unwrap(); + let agents_path = dir.path().join("managed-agents.json"); + + // Snapshot — file does not exist. + let snapshot = match std::fs::read(&agents_path) { + Ok(bytes) => Some(bytes), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => panic!("unexpected read error: {e}"), + }; + assert!(snapshot.is_none()); + + // Simulate a write that created the file. + std::fs::write(&agents_path, b"[{\"pubkey\":\"orphan\"}]").unwrap(); + assert!(agents_path.exists()); + + // Rollback: remove the file (restore "absent" state). + let restore = match &snapshot { + Some(bytes) => { + crate::managed_agents::storage::atomic_write_json_restricted(&agents_path, bytes) + } + None => match std::fs::remove_file(&agents_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + other => other.map_err(|e| e.to_string()), + }, + }; + assert!(restore.is_ok()); + assert!( + !agents_path.exists(), + "absent-store rollback must delete the file" + ); +} + +#[cfg(unix)] +#[test] +fn rollback_absent_file_treats_already_absent_as_success() { + let dir = tempfile::tempdir().unwrap(); + let agents_path = dir.path().join("managed-agents.json"); + + // File was absent before import and is still absent (e.g. the write that + // would have created it also failed). Rollback must succeed. + let restore = match std::fs::remove_file(&agents_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + other => other.map_err(|e| e.to_string()), + }; + assert!( + restore.is_ok(), + "removing an already-absent file must succeed" + ); +} + +#[cfg(unix)] +#[test] +fn snapshot_read_error_on_unreadable_file_is_surfaced() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let agents_path = dir.path().join("managed-agents.json"); + std::fs::write(&agents_path, b"content").unwrap(); + std::fs::set_permissions(&agents_path, std::fs::Permissions::from_mode(0o000)).unwrap(); + + // A non-NotFound read error must be surfaced, not collapsed to None. + let result = match std::fs::read(&agents_path) { + Ok(bytes) => Ok(Some(bytes)), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(format!("failed to snapshot agent store: {e}")), + }; + + // Restore permissions for cleanup. + std::fs::set_permissions(&agents_path, std::fs::Permissions::from_mode(0o644)).unwrap(); + + assert!( + result.is_err(), + "permission error must not be collapsed to None" + ); +} + +#[cfg(unix)] +#[test] +fn rollback_aggregates_multiple_errors() { + // Simulate the error aggregation pattern used in the rollback closure. + let original_err = "save_teams failed".to_string(); + let mut errors = vec![original_err.clone()]; + + // Simulate a keyring cleanup failure. + errors.push("keyring cleanup pubkey-1: keyring unreachable".to_string()); + + // Simulate a disk restore failure. + errors.push("agent store restore: permission denied".to_string()); + + let combined = errors.join("; "); + assert!(combined.contains("save_teams failed")); + assert!(combined.contains("keyring cleanup pubkey-1")); + assert!(combined.contains("agent store restore")); + assert_eq!( + combined.matches(';').count(), + 2, + "three errors joined by two semicolons" + ); +} + +/// Full-sequence failure injection at the teams-write boundary. +/// +/// Exercises the complete confirm_team_snapshot_import phase-3 rollback path: +/// snapshot both stores → write agents (succeeds) → write teams (fails via +/// read-only dir) → rollback keyring + agents store + teams store → assert +/// exact pre-import disk state. +/// +/// Keyring cleanup is exercised via a result-returning closure that mirrors +/// `try_delete_agent_key`'s signature. The real keyring seam is not called +/// here because `system-keyring` (default feature) accesses the OS keychain, +/// which blocks on authorization prompts in headless/CI environments. +/// The `try_delete_agent_key` function itself is integration-tested through +/// the `#[ignore]` keychain tests in `secret_store.rs`. +#[cfg(unix)] +#[test] +fn full_rollback_at_teams_boundary_existing_agents_store() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let agents_path = dir.path().join("managed-agents.json"); + let teams_path = dir.path().join("teams.json"); + + // Pre-import: agents store exists, teams store absent. + let original_agents = b"[{\"pubkey\":\"pre-existing\"}]"; + std::fs::write(&agents_path, original_agents).unwrap(); + + // Snapshot both stores (mirrors production NotFound-aware reads). + let agents_snap = match std::fs::read(&agents_path) { + Ok(b) => Some(b), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => panic!("agents snapshot: {e}"), + }; + let teams_snap = match std::fs::read(&teams_path) { + Ok(b) => Some(b), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => panic!("teams snapshot: {e}"), + }; + assert!(agents_snap.is_some()); + assert!(teams_snap.is_none()); + + // Phase-3 write 1: agents store committed. + crate::managed_agents::storage::atomic_write_json_restricted( + &agents_path, + b"[{\"pubkey\":\"imported\"}]", + ) + .unwrap(); + assert_ne!( + std::fs::read(&agents_path).unwrap().as_slice(), + original_agents, + "agents store must be changed by phase-3 write" + ); + + // Phase-3 write 2: teams write FAILS (read-only dir injection). + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o555)).unwrap(); + let teams_err = + crate::managed_agents::storage::atomic_write_json(&teams_path, b"[{\"id\":\"team-1\"}]"); + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o755)).unwrap(); + assert!(teams_err.is_err(), "teams write must fail in read-only dir"); + + // Full rollback (mirrors production rollback_agents + teams restore). + // Keyring cleanup: use a test-safe closure returning Ok(()) — the same + // contract as try_delete_agent_key on a no-keyring-backend host. + let minted_pubkeys = ["minted-aaa", "minted-bbb"]; + let mut errors = vec![teams_err.unwrap_err()]; + let try_delete_key = |_pk: &str| -> Result<(), String> { Ok(()) }; + + for pk in &minted_pubkeys { + if let Err(e) = try_delete_key(pk) { + errors.push(format!("keyring cleanup {pk}: {e}")); + } + } + let agents_restore = match &agents_snap { + Some(bytes) => { + crate::managed_agents::storage::atomic_write_json_restricted(&agents_path, bytes) + } + None => match std::fs::remove_file(&agents_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + other => other.map_err(|e| e.to_string()), + }, + }; + if let Err(e) = agents_restore { + errors.push(format!("agent store restore: {e}")); + } + let teams_restore = match &teams_snap { + Some(bytes) => crate::managed_agents::storage::atomic_write_json(&teams_path, bytes), + None => match std::fs::remove_file(&teams_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + other => other.map_err(|e| e.to_string()), + }, + }; + if let Err(e) = teams_restore { + errors.push(format!("teams store restore: {e}")); + } + + // Exact pre-import disk state restored. + assert_eq!( + std::fs::read(&agents_path).unwrap(), + original_agents, + "agents store must be restored to original content" + ); + assert!(!teams_path.exists(), "teams store must remain absent"); + assert_eq!( + errors.len(), + 1, + "only the teams-write error; keyring + disk rollback succeeded" + ); +} + +/// Variant: agents store was absent before import (fresh install). +/// +/// Exercises the NEW production control flow after the read-only loader fix: +/// load_teams_readonly (pre-commit, no write) → agents commit (creates file) → +/// save_teams fails → full rollback (keyring + delete created agents file + +/// teams absent-restore) → both files absent. +/// +/// This specifically tests the fix for the Thufir-found bypass where +/// `load_teams()` was secretly a writer on absent files — a failure inside +/// that hidden write would `?`-return without calling `rollback_agents`. +/// With `load_teams_readonly` pre-commit, that path no longer exists. +#[cfg(unix)] +#[test] +fn full_rollback_at_teams_boundary_absent_agents_store() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let agents_path = dir.path().join("managed-agents.json"); + let teams_path = dir.path().join("teams.json"); + + // Pre-import: BOTH stores absent (fresh install). + let agents_snap = match std::fs::read(&agents_path) { + Ok(b) => Some(b), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => panic!("agents snapshot: {e}"), + }; + let teams_snap = match std::fs::read(&teams_path) { + Ok(b) => Some(b), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => panic!("teams snapshot: {e}"), + }; + assert!(agents_snap.is_none()); + assert!(teams_snap.is_none()); + + // Pre-read teams via the read-only loader (mirrors production pre-commit). + // On a fresh install with both files absent, this returns the merged + // built-ins without writing. File must still be absent after the call. + let teams = crate::managed_agents::load_teams_readonly(&teams_path).unwrap(); + assert!( + !teams_path.exists(), + "load_teams_readonly must not create the file" + ); + + // Phase-3 write 1: import CREATES agents store. + std::fs::write(&agents_path, b"[{\"pubkey\":\"orphan\"}]").unwrap(); + assert!(agents_path.exists()); + + // Phase-3 write 2: save_teams FAILS (read-only dir injection). + // In production this is save_teams(&app, &teams) — we model the same + // atomic_write_json call that save_teams delegates to. + let mut teams_to_save = teams; + teams_to_save.push(crate::managed_agents::TeamRecord { + id: "team-1".to_string(), + name: "Imported".to_string(), + description: None, + instructions: None, + persona_ids: vec![], + is_builtin: false, + source_dir: None, + is_symlink: false, + symlink_target: None, + version: None, + created_at: "now".to_string(), + updated_at: "now".to_string(), + }); + let payload = serde_json::to_vec_pretty(&teams_to_save).unwrap(); + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o555)).unwrap(); + let teams_err = crate::managed_agents::storage::atomic_write_json(&teams_path, &payload); + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o755)).unwrap(); + assert!(teams_err.is_err()); + + // Full rollback: keyring cleanup (test-safe) + delete created agents file + // + teams absent-restore. + let mut errors = vec![teams_err.unwrap_err()]; + let try_delete_key = |_pk: &str| -> Result<(), String> { Ok(()) }; + if let Err(e) = try_delete_key("orphan") { + errors.push(format!("keyring cleanup: {e}")); + } + let agents_restore = match &agents_snap { + None => match std::fs::remove_file(&agents_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + other => other.map_err(|e| e.to_string()), + }, + Some(bytes) => { + crate::managed_agents::storage::atomic_write_json_restricted(&agents_path, bytes) + } + }; + if let Err(e) = agents_restore { + errors.push(format!("agent store restore: {e}")); + } + let teams_restore = match &teams_snap { + None => match std::fs::remove_file(&teams_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + other => other.map_err(|e| e.to_string()), + }, + Some(bytes) => crate::managed_agents::storage::atomic_write_json(&teams_path, bytes), + }; + if let Err(e) = teams_restore { + errors.push(format!("teams store restore: {e}")); + } + + // Exact pre-import state: both files absent. + assert!( + !agents_path.exists(), + "rollback must delete file created by import on fresh install" + ); + assert!(!teams_path.exists()); + assert_eq!(errors.len(), 1, "only the teams-write error"); +} diff --git a/desktop/src-tauri/src/commands/teams.rs b/desktop/src-tauri/src/commands/teams.rs index 61362cf26c..ea9a6a4958 100644 --- a/desktop/src-tauri/src/commands/teams.rs +++ b/desktop/src-tauri/src/commands/teams.rs @@ -1,14 +1,11 @@ -use tauri::{AppHandle, State}; +use tauri::AppHandle; use uuid::Uuid; -use super::export_util::save_json_with_dialog; use crate::{ app_state::AppState, managed_agents::{ - delete_team_with_cascade, encode_team_json, ensure_persona_ids_are_active, - import_team_from_directory as do_import_team, load_personas, load_teams, parse_team_json, - save_teams, sync_team_from_dir as do_sync_team, try_regenerate_nest, CreateTeamRequest, - ParsedTeamPreview, SyncResult, TeamRecord, UpdateTeamRequest, + delete_team_with_cascade, ensure_persona_ids_are_active, load_personas, load_teams, + save_teams, try_regenerate_nest, CreateTeamRequest, TeamRecord, UpdateTeamRequest, }, util::now_iso, }; @@ -158,6 +155,7 @@ pub async fn create_team(input: CreateTeamRequest, app: AppHandle) -> Result(); let name = trim_required(&input.name, "Team name")?; let description = trim_optional(input.description); + let instructions = trim_optional(input.instructions); let now = now_iso(); let _store_guard = state @@ -171,6 +169,7 @@ pub async fn create_team(input: CreateTeamRequest, app: AppHandle) -> Result Result(); let name = trim_required(&input.name, "Team name")?; let description = trim_optional(input.description); + let instructions = trim_optional(input.instructions); let _store_guard = state .managed_agents_store_lock @@ -212,6 +212,7 @@ pub async fn update_team(input: UpdateTeamRequest, app: AppHandle) -> Result Result<(), String> { .await .map_err(|e| format!("spawn_blocking failed: {e}"))? } - -#[tauri::command] -pub async fn install_team_from_directory( - app: AppHandle, - path: String, - symlink: Option, -) -> Result { - use tauri::Manager; - tokio::task::spawn_blocking(move || { - let state = app.state::(); - let _store_guard = state - .managed_agents_store_lock - .lock() - .map_err(|e| e.to_string())?; - let source = std::path::PathBuf::from(&path); - if !source.is_dir() { - return Err(format!("team path is not a directory: {path}")); - } - let result = do_import_team(&app, &source, symlink.unwrap_or(false))?; - try_regenerate_nest(&app); - Ok(result) - }) - .await - .map_err(|e| format!("spawn_blocking failed: {e}"))? -} - -#[tauri::command] -pub async fn sync_team_directory(app: AppHandle, team_id: String) -> Result { - use tauri::Manager; - tokio::task::spawn_blocking(move || { - let state = app.state::(); - let _store_guard = state - .managed_agents_store_lock - .lock() - .map_err(|e| e.to_string())?; - let result = do_sync_team(&app, &team_id)?; - try_regenerate_nest(&app); - Ok(result) - }) - .await - .map_err(|e| format!("spawn_blocking failed: {e}"))? -} - -#[tauri::command] -pub async fn pick_team_directory(app: AppHandle) -> Result, String> { - use tauri_plugin_dialog::DialogExt; - let path = app.dialog().file().blocking_pick_folder(); - Ok(path.map(|p| p.to_string())) -} - -const MAX_TEAM_JSON_BYTES: usize = 5 * 1024 * 1024; - -#[tauri::command] -pub async fn export_team_to_json( - id: String, - app: AppHandle, - state: State<'_, AppState>, -) -> Result { - // Load team and personas under lock, then drop lock before dialog. - let (team, personas) = { - let _store_guard = state - .managed_agents_store_lock - .lock() - .map_err(|e| e.to_string())?; - let teams = load_teams(&app)?; - let team = teams - .into_iter() - .find(|t| t.id == id) - .ok_or_else(|| format!("team {id} not found"))?; - let personas = load_personas(&app)?; - (team, personas) - }; - - let json_bytes = encode_team_json(&team, &personas)?; - - let slug = crate::util::slugify(&team.name, "team", 50); - let filename = format!("{slug}.team.json"); - save_json_with_dialog(&app, &filename, &json_bytes).await -} - -/// Max zip size for pack imports via team import (100 MB, same as persona zip limit). -const MAX_TEAM_ZIP_BYTES: usize = 100 * 1024 * 1024; - -#[tauri::command] -pub async fn parse_team_file( - file_bytes: Vec, - _file_name: String, -) -> Result { - tokio::task::spawn_blocking(move || { - if file_bytes.is_empty() { - return Err("File is empty.".to_string()); - } - - // Detect zip files (persona packs) BEFORE the JSON size check — zips can be larger. - if file_bytes.len() >= 4 && file_bytes[..4] == [0x50, 0x4B, 0x03, 0x04] { - if file_bytes.len() > MAX_TEAM_ZIP_BYTES { - return Err("ZIP file is too large (max 100 MB).".to_string()); - } - return parse_team_from_pack_zip(&file_bytes); - } - - if file_bytes.len() > MAX_TEAM_JSON_BYTES { - return Err("File is too large (max 5 MB).".to_string()); - } - - parse_team_json(&file_bytes) - }) - .await - .map_err(|e| format!("spawn_blocking failed: {e}"))? -} - -/// Parse a persona pack zip as a team: pack name → team name, personas → members. -fn parse_team_from_pack_zip(zip_bytes: &[u8]) -> Result { - use crate::managed_agents::TeamPersonaPreview; - - // Extract to tempdir and resolve the pack directly — gives us structured - // access to pack name without parsing formatted strings. - let tmp = tempfile::tempdir().map_err(|e| format!("Failed to create temp dir: {e}"))?; - let cursor = std::io::Cursor::new(zip_bytes); - let mut archive = - zip::ZipArchive::new(cursor).map_err(|e| format!("Invalid ZIP archive: {e}"))?; - - let max_decompressed: usize = 100 * 1024 * 1024; // 100 MB - let mut total_decompressed: usize = 0; - - for i in 0..archive.len() { - let mut entry = archive - .by_index(i) - .map_err(|e| format!("Failed to read ZIP entry: {e}"))?; - let safe_name = match entry.enclosed_name() { - Some(name) => name.to_path_buf(), - None => continue, - }; - let out_path = tmp.path().join(&safe_name); - if entry.is_dir() { - std::fs::create_dir_all(&out_path).map_err(|e| format!("Failed to create dir: {e}"))?; - } else { - if let Some(parent) = out_path.parent() { - std::fs::create_dir_all(parent) - .map_err(|e| format!("Failed to create parent dir: {e}"))?; - } - let mut data = Vec::new(); - std::io::Read::read_to_end(&mut entry, &mut data) - .map_err(|e| format!("Read error: {e}"))?; - total_decompressed += data.len(); - if total_decompressed > max_decompressed { - return Err("ZIP decompressed content exceeds 100MB limit".to_string()); - } - std::fs::write(&out_path, &data).map_err(|e| format!("Write error: {e}"))?; - } - } - - let pack_root = crate::managed_agents::find_plugin_json(tmp.path()) - .ok_or_else(|| "No .plugin/plugin.json found in ZIP".to_string())?; - let resolved = buzz_persona_pkg::resolve::resolve_pack(&pack_root) - .map_err(|e| format!("Pack validation failed: {e}"))?; - - if resolved.personas.is_empty() { - return Err("Pack contains no personas.".to_string()); - } - - Ok(ParsedTeamPreview { - name: resolved.name, - description: if resolved.description.is_empty() { - None - } else { - Some(resolved.description) - }, - personas: resolved - .personas - .into_iter() - .map(|p| TeamPersonaPreview { - display_name: p.display_name, - system_prompt: p.system_prompt, - avatar_url: None, - }) - .collect(), - }) -} - -#[cfg(test)] -mod tests { - use std::io::Write; - - use super::parse_team_from_pack_zip; - - #[test] - fn parse_team_pack_zip_keeps_team_import_available() { - let cursor = std::io::Cursor::new(Vec::new()); - let mut zip = zip::ZipWriter::new(cursor); - let options = zip::write::SimpleFileOptions::default(); - - zip.start_file("reviewers/.plugin/plugin.json", options) - .unwrap(); - zip.write_all( - br#"{ - "id": "com.example.reviewers", - "name": "Reviewers", - "version": "1.0.0", - "personas": ["agents/reviewer.persona.md"] - }"#, - ) - .unwrap(); - zip.start_file("reviewers/agents/reviewer.persona.md", options) - .unwrap(); - zip.write_all( - br#"--- -name: "reviewer" -display_name: "Reviewer" -description: "Reviews code" ---- -Review code carefully. -"#, - ) - .unwrap(); - - let bytes = zip.finish().unwrap().into_inner(); - let preview = parse_team_from_pack_zip(&bytes).unwrap(); - - assert_eq!(preview.name, "Reviewers"); - assert_eq!(preview.personas.len(), 1); - assert_eq!(preview.personas[0].display_name, "Reviewer"); - } -} diff --git a/desktop/src-tauri/src/event_sync.rs b/desktop/src-tauri/src/event_sync.rs index 4273e4e0bd..2ec5aa1c0e 100644 --- a/desktop/src-tauri/src/event_sync.rs +++ b/desktop/src-tauri/src/event_sync.rs @@ -39,9 +39,10 @@ pub fn spawn_event_sync(app: tauri::AppHandle, owner_keys: nostr::Keys) { /// Reconcile `personas.json` into the persona-event retention store. /// -/// Must run AFTER `migrate_packs_to_teams` (depends on field renames being -/// complete) and AFTER the persisted identity is resolved (it signs every -/// retained event with the owner's keys). +/// Must run AFTER `fold_personas_into_agent_store` and +/// `detach_directory_backed_teams` (depends on field renames and store +/// unification being complete) and AFTER the persisted identity is resolved +/// (it signs every retained event with the owner's keys). /// /// Per-record reconcile: for each non-builtin persona it compares the freshly /// serialized event content against the retained row at the same coordinate diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 8213dc0e6f..fd43a607b3 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -834,11 +834,6 @@ pub fn run() { create_team, update_team, delete_team, - install_team_from_directory, - sync_team_directory, - pick_team_directory, - export_team_to_json, - parse_team_file, export_agent_snapshot, preview_agent_snapshot_import, confirm_agent_snapshot_import, diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index d7946402ca..ba4407d164 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -188,6 +188,7 @@ mod tests { }, backend_agent_id: Some("remote-id".to_string()), provider_binary_path: Some("/path/to/binary".to_string()), + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: "2025-01-01T00:00:00Z".to_string(), diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs index 698f335e1c..b0bf8f5991 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs @@ -28,8 +28,8 @@ //! - `mcp_command` (machine-local) //! - runtime state: `runtime_pid`, `backend_agent_id`, `backend` blob, //! `provider_binary_path`, `last_*` -//! - lineage ids: `persona_id`, `source_team`, `source_team_persona_slug`, -//! `persona_team_dir`, `persona_name_in_team`, `persona_source_version` +//! - lineage ids: `persona_id`, `team_id`, `source_team`, `source_team_persona_slug`, +//! `persona_source_version` //! - internal bookkeeping: `start_on_app_launch`, //! `auto_restart_on_config_change`, `is_builtin` //! @@ -477,6 +477,7 @@ mod tests { name: "Test Agent".to_string(), display_name: Some("Test Agent Display".to_string()), persona_id: Some("SENTINEL_PERSONA_ID".to_string()), // MUST NOT appear in snapshot + team_id: Some("SENTINEL_TEAM_ID".to_string()), // MUST NOT appear in snapshot private_key_nsec: "nsec1secret".to_string(), // MUST NOT appear in snapshot auth_tag: Some("auth-tag-secret".to_string()), // MUST NOT appear in snapshot relay_url: "wss://relay.example.com".to_string(), // MUST NOT appear in snapshot @@ -875,6 +876,15 @@ mod tests { !json.contains("SENTINEL_PERSONA_ID"), "personaId value must not appear" ); + // teamId + assert!( + !json.contains("teamId") && !json.contains("team_id"), + "teamId field must not appear" + ); + assert!( + !json.contains("SENTINEL_TEAM_ID"), + "teamId value must not appear" + ); // personaTeamDir assert!( !json.contains("personaTeamDir") && !json.contains("persona_team_dir"), diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index bba5469f09..f5fc7c3899 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -87,6 +87,7 @@ fn test_record() -> ManagedAgentRecord { backend: crate::managed_agents::types::BackendKind::Local, backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: "".to_string(), diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index b8d0bda4fa..608b511fa6 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -256,6 +256,7 @@ fn record_with( backend: Default::default(), backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, env_vars: std::collections::BTreeMap::new(), diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 0484aba727..6318a5c838 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -39,21 +39,6 @@ pub(crate) fn is_derived_provider_model_key(key: &str) -> bool { .any(|k| k.eq_ignore_ascii_case(key)) } -/// Strip derived provider/model env keys from a pack persona's `runtime_env_vars` -/// before persisting them in `AgentDefinition.env_vars`. -/// -/// The structured `AgentDefinition.provider` / `AgentDefinition.model` fields are -/// the authoritative source of truth. Keeping the derived copies would cause -/// stale env values to override updated structured fields at spawn/deploy time. -pub(crate) fn filter_derived_provider_model_env_vars( - env_vars: impl IntoIterator, -) -> BTreeMap { - env_vars - .into_iter() - .filter(|(k, _)| !is_derived_provider_model_key(k)) - .collect() -} - /// Env var keys that Buzz sets itself and users must not override from /// the persona/agent env_vars UI. Three categories: /// diff --git a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs index 9ee5eeec40..cf57b12546 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs @@ -1,8 +1,8 @@ use std::collections::BTreeMap; use super::{ - display_invalid_key, filter_derived_provider_model_env_vars, is_derived_provider_model_key, - is_reserved_env_key, is_well_formed_env_key, merged_user_env, validate_user_env_keys, + display_invalid_key, is_derived_provider_model_key, is_reserved_env_key, + is_well_formed_env_key, merged_user_env, validate_user_env_keys, DERIVED_PROVIDER_MODEL_ENV_KEYS, MAX_ENV_TOTAL_BYTES, MAX_ENV_VALUE_BYTES, RESERVED_ENV_KEYS, }; @@ -451,85 +451,6 @@ fn is_derived_key_does_not_match_unrelated_keys() { assert!(!is_derived_provider_model_key("PROVIDER")); } -#[test] -fn filter_derived_strips_provider_model_keys_preserves_rest() { - let input = vec![ - ("GOOSE_MODEL".to_string(), "old-model".to_string()), - ("GOOSE_PROVIDER".to_string(), "old-provider".to_string()), - ("BUZZ_AGENT_MODEL".to_string(), "gpt-4o".to_string()), - ("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()), - ("GOOSE_TEMPERATURE".to_string(), "0.7".to_string()), - ("GOOSE_CONTEXT_LIMIT".to_string(), "128000".to_string()), - ("CUSTOM_KEY".to_string(), "custom-value".to_string()), - ]; - - let filtered = filter_derived_provider_model_env_vars(input); - - // Derived keys must be gone. - assert!(!filtered.contains_key("GOOSE_MODEL")); - assert!(!filtered.contains_key("GOOSE_PROVIDER")); - assert!(!filtered.contains_key("BUZZ_AGENT_MODEL")); - assert!(!filtered.contains_key("BUZZ_AGENT_PROVIDER")); - - // Non-derived keys must survive. - assert_eq!( - filtered.get("GOOSE_TEMPERATURE").map(String::as_str), - Some("0.7") - ); - assert_eq!( - filtered.get("GOOSE_CONTEXT_LIMIT").map(String::as_str), - Some("128000") - ); - assert_eq!( - filtered.get("CUSTOM_KEY").map(String::as_str), - Some("custom-value") - ); -} - -#[test] -fn filter_derived_empty_input_returns_empty() { - let filtered = filter_derived_provider_model_env_vars(std::iter::empty()); - assert!(filtered.is_empty()); -} - -#[test] -fn stale_derived_env_does_not_override_structured_fields() { - // Scenario: A persona was imported WITH stale derived keys (pre-fix). - // At merge time, `merged_user_env` passes them through (it doesn't filter). - // The fix is at *import* time — this test documents that merged_user_env - // is transparent, and the import filter is the correct defense. - let stale_persona_env = map(&[ - ("BUZZ_AGENT_MODEL", "stale-model"), - ("BUZZ_AGENT_PROVIDER", "stale-provider"), - ("GOOSE_TEMPERATURE", "0.5"), - ]); - let agent_env = BTreeMap::new(); - - let merged = merged_user_env(&stale_persona_env, &agent_env); - - // merged_user_env is transparent — stale keys pass through. - assert_eq!( - merged.get("BUZZ_AGENT_MODEL").map(String::as_str), - Some("stale-model") - ); - assert_eq!( - merged.get("BUZZ_AGENT_PROVIDER").map(String::as_str), - Some("stale-provider") - ); - - // But the import filter WOULD have caught them: - let would_be_filtered = filter_derived_provider_model_env_vars(stale_persona_env); - assert!(!would_be_filtered.contains_key("BUZZ_AGENT_MODEL")); - assert!(!would_be_filtered.contains_key("BUZZ_AGENT_PROVIDER")); - // Non-derived keys survive the filter. - assert_eq!( - would_be_filtered - .get("GOOSE_TEMPERATURE") - .map(String::as_str), - Some("0.5") - ); -} - // ── deploy payload model precedence ──────────────────────────────── /// Documents the model precedence rule used by `build_deploy_payload`: diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 60ef0d79a6..24027b75fd 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -324,6 +324,7 @@ fn bare_record() -> ManagedAgentRecord { backend: BackendKind::Local, backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: "".to_string(), diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 14dc76267d..d99d38f0fd 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -13,7 +13,6 @@ pub(crate) mod git_bash; pub(crate) mod global_config; mod nest; mod persona_avatars; -mod persona_card; pub(crate) mod persona_events; mod personas; #[cfg(windows)] @@ -27,7 +26,7 @@ mod restore; pub mod retention; mod runtime; pub(crate) mod spawn_hash; -mod storage; +pub(crate) mod storage; pub(crate) mod team_events; mod team_repair; mod teams; @@ -53,7 +52,6 @@ pub(crate) use global_config::{ validate_global_config, GlobalAgentConfig, }; pub use nest::*; -pub use persona_card::find_plugin_json; pub use personas::*; #[cfg(windows)] pub use process_lifecycle::*; @@ -69,7 +67,6 @@ pub use repos::{ pub use restore::*; pub use runtime::*; pub use storage::*; -pub use team_repair::{sync_team_personas, team_persona_key}; pub use teams::*; pub use types::*; diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs index 63dae4ea4b..a959381603 100644 --- a/desktop/src-tauri/src/managed_agents/nest/tests.rs +++ b/desktop/src-tauri/src/managed_agents/nest/tests.rs @@ -461,6 +461,7 @@ fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { backend: BackendKind::default(), backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: String::new(), diff --git a/desktop/src-tauri/src/managed_agents/persona_card.rs b/desktop/src-tauri/src/managed_agents/persona_card.rs deleted file mode 100644 index 2b7beee340..0000000000 --- a/desktop/src-tauri/src/managed_agents/persona_card.rs +++ /dev/null @@ -1,16 +0,0 @@ -/// Find `.plugin/plugin.json` in a directory. Returns the parent of `.plugin/`. -/// Checks root and root/* only, matching supported team-pack layout. -pub fn find_plugin_json(root: &std::path::Path) -> Option { - if root.join(".plugin").join("plugin.json").exists() { - return Some(root.to_path_buf()); - } - - let entries = std::fs::read_dir(root).ok()?; - for entry in entries.flatten() { - let path = entry.path(); - if path.is_dir() && path.join(".plugin").join("plugin.json").exists() { - return Some(path); - } - } - None -} diff --git a/desktop/src-tauri/src/managed_agents/personas/tests.rs b/desktop/src-tauri/src/managed_agents/personas/tests.rs index 82081a6c2f..f380a595fd 100644 --- a/desktop/src-tauri/src/managed_agents/personas/tests.rs +++ b/desktop/src-tauri/src/managed_agents/personas/tests.rs @@ -4,7 +4,6 @@ use super::{ validate_persona_deletion, BUILT_IN_PERSONAS, RETIRED_PERSONAS, }; use crate::managed_agents::discovery::{default_agent_command, effective_agent_command}; -use crate::managed_agents::validate_team_id; use crate::managed_agents::AgentDefinition; fn custom_persona(id: &str, display_name: &str) -> AgentDefinition { @@ -321,61 +320,6 @@ fn validate_persona_deletion_allows_safe_custom_personas() { assert!(validate_persona_deletion(&persona, false).is_ok()); } -// ── validate_team_id ────────────────────────────────────────────────────────── - -#[test] -fn pack_id_valid_reverse_dns() { - assert!(validate_team_id("com.example.security-team").is_ok()); -} - -#[test] -fn pack_id_valid_simple() { - assert!(validate_team_id("my-pack").is_ok()); -} - -#[test] -fn pack_id_rejects_empty() { - assert!(validate_team_id("").is_err()); -} - -#[test] -fn pack_id_rejects_dot_dot_path_traversal() { - // Critical regression test: ".." must never pass validation. - // A pack with id ".." would write into the parent directory. - assert!(validate_team_id("..").is_err()); -} - -#[test] -fn pack_id_rejects_single_dot() { - assert!(validate_team_id(".").is_err()); -} - -#[test] -fn pack_id_rejects_leading_dot() { - assert!(validate_team_id(".hidden").is_err()); -} - -#[test] -fn pack_id_rejects_slashes() { - assert!(validate_team_id("../etc/passwd").is_err()); - assert!(validate_team_id("foo/bar").is_err()); -} - -#[test] -fn pack_id_rejects_no_alphanumeric() { - assert!(validate_team_id("---").is_err()); - assert!(validate_team_id("___").is_err()); -} - -#[test] -fn pack_id_rejects_too_long() { - let long_id = "a".repeat(129); - assert!(validate_team_id(&long_id).is_err()); - // 128 chars is fine - let max_id = "a".repeat(128); - assert!(validate_team_id(&max_id).is_ok()); -} - // ── migrate_retired_personas ────────────────────────────────────────────────── #[test] diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index bf419b0cd1..6a5af89259 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -1396,6 +1396,7 @@ mod tests { backend: Default::default(), backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: String::new(), diff --git a/desktop/src-tauri/src/managed_agents/relay_mesh.rs b/desktop/src-tauri/src/managed_agents/relay_mesh.rs index b39a6a1a0e..4c58344d66 100644 --- a/desktop/src-tauri/src/managed_agents/relay_mesh.rs +++ b/desktop/src-tauri/src/managed_agents/relay_mesh.rs @@ -86,6 +86,7 @@ mod tests { backend: BackendKind::Local, backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, persona_source_version: None, diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 44856c206a..3745fe5718 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1332,10 +1332,12 @@ pub fn build_managed_agent_summary( let state = app.state::(); let global_for_hash = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); + let teams_for_hash = crate::managed_agents::load_teams(app).unwrap_or_default(); let hash_drift = runtime.spawn_config_hash != crate::managed_agents::spawn_hash::spawn_config_hash( record, personas, + &teams_for_hash, &crate::relay::relay_ws_url_with_override(&state), &global_for_hash, ); @@ -1498,6 +1500,7 @@ pub fn spawn_agent_child( // command, so we recompute them from the effective value rather than the // frozen record snapshot. Mirrors the model resolution below. let personas = super::load_personas(app).unwrap_or_default(); + let teams = super::load_teams(app).unwrap_or_default(); // Load global config once; used for runtime_metadata_env_vars (model/provider fallback) // and for the env-var merge at spawn time. let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); @@ -1716,17 +1719,17 @@ pub fn spawn_agent_child( } } } - if let (Some(team_dir), Some(persona_name)) = - (&record.persona_team_dir, &record.persona_name_in_team) - { - command.env("BUZZ_ACP_PERSONA_PACK", team_dir); - command.env("BUZZ_ACP_PERSONA_NAME", persona_name); + let team_instructions = super::spawn_hash::effective_team_instructions(record, &teams); + if let Some(instructions) = &team_instructions { + command.env("BUZZ_ACP_TEAM_INSTRUCTIONS", instructions); + } else { + command.env_remove("BUZZ_ACP_TEAM_INSTRUCTIONS"); } // System prompt via the shared spawn-effective filter — the SAME function the // config hash digests, so env write and badge cannot disagree (see - // `effective_spawn_prompt` for the Some("")/None collapse and the - // team-pack suppression exception). Model and provider use the shared + // `effective_spawn_prompt` for the Some("")/None collapse). Model and provider + // use the shared // resolver: agent → persona → global → None, so a global-default-only agent // spawns with the correct provider/model env. let effective_prompt = super::spawn_hash::effective_spawn_prompt(record); @@ -1878,8 +1881,13 @@ pub fn spawn_agent_child( // needs_restart when disk state drifts from what this process runs. // `effective_relay_url` is already resolved, and resolution is idempotent, // so it serves as the workspace-relay input here. - let spawn_config_hash = - super::spawn_hash::spawn_config_hash(record, &personas, &effective_relay_url, &global); + let spawn_config_hash = super::spawn_hash::spawn_config_hash( + record, + &personas, + &teams, + &effective_relay_url, + &global, + ); // Stamp the adapter availability for runtimes with a version gate (codex // only). The summary builder compares this against the current cached value diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index dba3928dac..06d3ac6db3 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -150,6 +150,7 @@ fn fixture( backend: Default::default(), backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: "now".into(), diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index 8b3c177d6a..63a57ce5da 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -30,27 +30,34 @@ use super::{ known_acp_runtime, normalize_agent_args, persona_events::apply_persona_snapshot, resolve_effective_agent_env, - types::{AgentDefinition, ManagedAgentRecord}, + types::{AgentDefinition, ManagedAgentRecord, TeamRecord}, GlobalAgentConfig, }; /// The prompt a spawn would actually deliver: `Some("")` collapses to `None` -/// because an empty BUZZ_ACP_SYSTEM_PROMPT is no prompt — EXCEPT for -/// team-pack records, where buzz-acp falls back to the pack persona's prompt -/// when the env var is absent, making set-but-empty a deliberate suppression. +/// because an empty `BUZZ_ACP_SYSTEM_PROMPT` is no prompt. /// -/// The single source of truth for the spawn env write AND the config hash: -/// both call this, so they cannot disagree (the hash's contract is "digest -/// what a spawn would actually run"). B5 hash row 2 depends on the collapse: -/// backfilled prompt-less records re-snapshot to `Some("")` from their -/// manufactured definition and must not trip the restart badge. +/// The single source of truth for the spawn env write AND the config hash. pub(crate) fn effective_spawn_prompt(record: &ManagedAgentRecord) -> Option { - let has_pack_fallback = - record.persona_team_dir.is_some() && record.persona_name_in_team.is_some(); record .system_prompt .clone() - .filter(|p| has_pack_fallback || !p.is_empty()) + .filter(|prompt| !prompt.is_empty()) +} + +/// Resolve the current instructions for this instance's deployment-time team binding. +/// A deleted team deliberately degrades to no team section. +pub(crate) fn effective_team_instructions( + record: &ManagedAgentRecord, + teams: &[TeamRecord], +) -> Option { + teams + .iter() + .find(|team| Some(team.id.as_str()) == record.team_id.as_deref()) + .and_then(|team| team.instructions.as_deref()) + .map(str::trim) + .filter(|instructions| !instructions.is_empty()) + .map(str::to_string) } /// Digest the effective spawn configuration of `record` under the current @@ -59,6 +66,7 @@ pub(crate) fn effective_spawn_prompt(record: &ManagedAgentRecord) -> Option u64 { @@ -100,9 +108,9 @@ pub(crate) fn spawn_config_hash( // resolved: a blank record relay spawns on the workspace relay, so a // workspace relay change must trip the badge. crate::relay::effective_agent_relay_url(&record.relay_url, workspace_relay).hash(&mut hasher); - // Prompt via the shared spawn-effective filter (see its doc for the - // Some("")/None collapse and the team-pack exception). + // Prompt and runtime-layered team instructions use the same resolver as spawn. effective_spawn_prompt(record).hash(&mut hasher); + effective_team_instructions(record, teams).hash(&mut hasher); record.model.hash(&mut hasher); record.provider.hash(&mut hasher); record.auth_tag.hash(&mut hasher); @@ -126,8 +134,6 @@ pub(crate) fn spawn_config_hash( .unwrap_or(super::types::DEFAULT_AGENT_MAX_TURN_DURATION_SECONDS) .hash(&mut hasher); record.parallelism.hash(&mut hasher); - record.persona_team_dir.hash(&mut hasher); - record.persona_name_in_team.hash(&mut hasher); hasher.finish() } diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs index a4cec6aebd..9b79881625 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -31,6 +31,7 @@ fn record() -> ManagedAgentRecord { backend: Default::default(), backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: "now".into(), @@ -84,8 +85,8 @@ fn persona(id: &str, runtime: Option<&str>, prompt: &str) -> AgentDefinition { fn hash_is_deterministic() { let rec = record(); assert_eq!( - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -106,8 +107,20 @@ fn materializing_runtime_keeps_hash_stable() { post.runtime = Some("goose".into()); assert_eq!( - spawn_config_hash(&pre, &personas, "wss://ws.example", &Default::default()), - spawn_config_hash(&post, &personas, "wss://ws.example", &Default::default()) + spawn_config_hash( + &pre, + &personas, + &[], + "wss://ws.example", + &Default::default() + ), + spawn_config_hash( + &post, + &personas, + &[], + "wss://ws.example", + &Default::default() + ) ); } @@ -119,8 +132,8 @@ fn record_env_var_edit_changes_hash() { .env_vars .insert("SOME_KEY".into(), "some-value".into()); assert_ne!( - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -130,8 +143,8 @@ fn record_prompt_edit_changes_hash() { let mut edited = record(); edited.system_prompt = Some("Edited prompt.".into()); assert_ne!( - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -144,8 +157,8 @@ fn persona_runtime_edit_changes_hash() { let before = [persona("pers", Some("goose"), "prompt")]; let after = [persona("pers", Some("claude"), "prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()) ); } @@ -159,8 +172,8 @@ fn persona_prompt_edit_changes_hash() { let before = [persona("pers", Some("goose"), "old prompt")]; let after = [persona("pers", Some("goose"), "new prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()) ); } @@ -171,8 +184,8 @@ fn workspace_relay_change_trips_hash_for_blank_record_relay() { let mut rec = record(); rec.relay_url = String::new(); assert_ne!( - spawn_config_hash(&rec, &[], "wss://relay-a.example", &Default::default()), - spawn_config_hash(&rec, &[], "wss://relay-b.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://relay-a.example", &Default::default()), + spawn_config_hash(&rec, &[], &[], "wss://relay-b.example", &Default::default()) ); } @@ -182,8 +195,8 @@ fn workspace_relay_change_ignored_for_pinned_record_relay() { // a workspace relay change must NOT badge a pinned agent. let rec = record(); assert_eq!( - spawn_config_hash(&rec, &[], "wss://relay-a.example", &Default::default()), - spawn_config_hash(&rec, &[], "wss://relay-b.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://relay-a.example", &Default::default()), + spawn_config_hash(&rec, &[], &[], "wss://relay-b.example", &Default::default()) ); } @@ -194,8 +207,8 @@ fn respond_to_allowlist_edit_changes_hash() { edited.respond_to = RespondTo::Allowlist; edited.respond_to_allowlist = vec!["a".repeat(64)]; assert_ne!( - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -207,8 +220,8 @@ fn allowlist_ignored_when_mode_is_not_allowlist() { let mut edited = record(); edited.respond_to_allowlist = vec!["a".repeat(64)]; assert_eq!( - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -225,8 +238,8 @@ fn allowlist_normalization_equivalent_edits_do_not_change_hash() { "a".repeat(64), // duplicate ]; assert_eq!( - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -238,8 +251,8 @@ fn allowlist_content_edit_still_changes_hash() { let mut edited = rec.clone(); edited.respond_to_allowlist = vec!["b".repeat(64)]; assert_ne!( - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -252,8 +265,8 @@ fn explicit_default_max_turn_duration_does_not_change_hash() { edited.max_turn_duration_seconds = Some(crate::managed_agents::types::DEFAULT_AGENT_MAX_TURN_DURATION_SECONDS); assert_eq!( - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -263,8 +276,8 @@ fn non_default_max_turn_duration_changes_hash() { let mut edited = record(); edited.max_turn_duration_seconds = Some(42); assert_ne!( - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -279,8 +292,8 @@ fn non_spawn_bookkeeping_fields_do_not_change_hash() { edited.last_started_at = Some("later".into()); edited.last_exit_code = Some(0); assert_eq!( - spawn_config_hash(&rec, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], "wss://ws.example", &Default::default()) + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -308,12 +321,14 @@ fn resnapshot_does_not_clobber_record_quad_with_definition_absent_quad() { spawn_config_hash( &rec, &quadless_definition, + &[], "wss://ws.example", &Default::default() ), spawn_config_hash( &rec, &definition_with_quad, + &[], "wss://ws.example", &Default::default() ), @@ -331,29 +346,8 @@ fn empty_prompt_hashes_like_absent_prompt() { let mut empty = record(); empty.system_prompt = Some(String::new()); assert_eq!( - spawn_config_hash(&absent, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&empty, &[], "wss://ws.example", &Default::default()), - ); -} - -#[test] -fn team_pack_records_keep_empty_vs_absent_prompt_distinction() { - // Wrinkle-1 exception (Pinky): with BUZZ_ACP_PERSONA_PACK set, buzz-acp - // inherits the pack persona's prompt when the env var is ABSENT, while - // set-but-empty suppresses it. For team records the two states spawn - // differently, so they must hash differently. - let mut absent = record(); - absent.persona_team_dir = Some(std::path::PathBuf::from("/teams/alpha")); - absent.persona_name_in_team = Some("lep".into()); - absent.system_prompt = None; - - let mut empty = absent.clone(); - empty.system_prompt = Some(String::new()); - - assert_ne!( - spawn_config_hash(&absent, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&empty, &[], "wss://ws.example", &Default::default()), - "suppressed pack prompt is a different spawn than inherited pack prompt" + spawn_config_hash(&absent, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&empty, &[], &[], "wss://ws.example", &Default::default()), ); } @@ -369,8 +363,8 @@ fn definition_runtime_edit_changes_hash_for_materialized_record() { let before = [persona("pers", Some("goose"), "prompt")]; let after = [persona("pers", Some("claude"), "prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, "wss://ws.example", &Default::default()), + spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()), "definition runtime edit must badge a materialized, override-free instance" ); } @@ -387,8 +381,8 @@ fn agent_command_override_beats_definition_runtime_change() { let before = [persona("pers", Some("goose"), "prompt")]; let after = [persona("pers", Some("claude"), "prompt")]; assert_eq!( - spawn_config_hash(&rec, &before, "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, "wss://ws.example", &Default::default()), + spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()), "explicit override must win regardless of definition runtime change" ); } @@ -407,10 +401,17 @@ fn missing_definition_leaves_materialized_runtime_in_hash() { no_runtime.runtime = None; assert_ne!( - spawn_config_hash(&rec, no_personas, "wss://ws.example", &Default::default()), + spawn_config_hash( + &rec, + no_personas, + &[], + "wss://ws.example", + &Default::default() + ), spawn_config_hash( &no_runtime, no_personas, + &[], "wss://ws.example", &Default::default() ), @@ -432,12 +433,4 @@ fn effective_spawn_prompt_matches_hash_semantics() { ); r.system_prompt = Some("real".into()); assert_eq!(effective_spawn_prompt(&r).as_deref(), Some("real")); - r.system_prompt = Some(String::new()); - r.persona_team_dir = Some(std::path::PathBuf::from("/teams/alpha")); - r.persona_name_in_team = Some("lep".into()); - assert_eq!( - effective_spawn_prompt(&r).as_deref(), - Some(""), - "team-pack set-but-empty survives (deliberate suppression)" - ); } diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index a1c00508b9..37f30d4351 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -40,7 +40,7 @@ pub fn managed_agents_base_dir(app: &AppHandle) -> Result { Ok(dir) } -fn managed_agents_store_path(app: &AppHandle) -> Result { +pub(crate) fn managed_agents_store_path(app: &AppHandle) -> Result { Ok(managed_agents_base_dir(app)?.join("managed-agents.json")) } @@ -500,13 +500,23 @@ fn copy_agent_keys_between_stores(pubkeys: &[String], src: &impl KeyStore, dst: } } +/// Remove an agent's key from the keyring, returning an error on failure. +/// Used by the snapshot-import rollback path, which must surface cleanup +/// failures rather than swallowing them. +pub(crate) fn try_delete_agent_key(pubkey: &str) -> Result<(), String> { + if let Some(store) = agent_secret_store() { + store.delete(&agent_keyring_name(pubkey)) + } else { + // No keyring backend — nothing to clean up. + Ok(()) + } +} + /// Remove an agent's key from the keyring (best-effort). Called when an agent /// is deleted so its secret does not linger in the OS store. pub fn delete_agent_key(pubkey: &str) { - if let Some(store) = agent_secret_store() { - if let Err(e) = store.delete(&agent_keyring_name(pubkey)) { - eprintln!("buzz-desktop: failed to delete agent {pubkey} key from keyring: {e}"); - } + if let Err(e) = try_delete_agent_key(pubkey) { + eprintln!("buzz-desktop: failed to delete agent {pubkey} key from keyring: {e}"); } } @@ -1307,4 +1317,15 @@ mod tests { "marker must be set even when pubkey list is empty" ); } + + #[test] + fn try_delete_agent_key_returns_result() { + // Verify the result-returning seam exists and has the correct signature. + // We cannot call it in default builds (system-keyring feature is on, + // which accesses the real OS keychain and blocks in headless/CI). The + // real keychain paths are integration-tested through the #[ignore] + // tests in secret_store.rs; the rollback aggregation is tested in + // team_snapshot::tests::rollback_aggregates_multiple_errors. + let _: fn(&str) -> Result<(), String> = super::try_delete_agent_key; + } } diff --git a/desktop/src-tauri/src/managed_agents/team_events.rs b/desktop/src-tauri/src/managed_agents/team_events.rs index b0395f15b4..a6f7c506c7 100644 --- a/desktop/src-tauri/src/managed_agents/team_events.rs +++ b/desktop/src-tauri/src/managed_agents/team_events.rs @@ -24,6 +24,8 @@ pub struct TeamEventContent { pub name: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub description: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub instructions: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub persona_ids: Vec, } @@ -35,6 +37,7 @@ pub fn team_event_content(record: &TeamRecord) -> TeamEventContent { TeamEventContent { name: record.name.clone(), description: record.description.clone(), + instructions: record.instructions.clone(), persona_ids: record.persona_ids.clone(), } } @@ -86,6 +89,7 @@ mod tests { id: "team-123".to_string(), name: "Test Team".to_string(), description: Some("A test team".to_string()), + instructions: Some("Coordinate carefully.".to_string()), persona_ids: vec!["p1".to_string(), "p2".to_string()], is_builtin: false, source_dir: Some(PathBuf::from("/local/only/path")), @@ -130,6 +134,7 @@ mod tests { // Published fields present. assert!(json.contains("\"name\"")); assert!(json.contains("\"persona_ids\"")); + assert!(json.contains("\"instructions\"")); // Local-only / install-specific fields never published. assert!(!json.contains("source_dir")); assert!(!json.contains("is_symlink")); diff --git a/desktop/src-tauri/src/managed_agents/team_repair.rs b/desktop/src-tauri/src/managed_agents/team_repair.rs index 2cdb5cfeb6..6420792109 100644 --- a/desktop/src-tauri/src/managed_agents/team_repair.rs +++ b/desktop/src-tauri/src/managed_agents/team_repair.rs @@ -1,19 +1,4 @@ -//! One-time data repair for teams created before PR 852. -//! -//! Backfills `TeamRecord.source_dir` and deduplicates `AgentDefinition`s that -//! share a `(source_team, source_team_persona_slug)` pair — the result of -//! repeated imports before the matching predicate was fixed. -//! -//! All repairs touch only JSON records; the symlinked source directory is read -//! but never written. - -use std::collections::{HashMap, HashSet}; -use std::fs; -use std::path::Path; - -use tauri::AppHandle; - -use crate::managed_agents::{AgentDefinition, ManagedAgentRecord, TeamRecord}; +use crate::managed_agents::TeamRecord; /// Derive the shared key used to match personas to a team. For directory- /// backed teams this is the directory name (the pack manifest ID); for others @@ -31,237 +16,18 @@ pub fn team_persona_key(team: &TeamRecord) -> &str { .unwrap_or(&team.id) } -/// Backfill `TeamRecord.source_dir` for directory-backed teams created before -/// the field existed. Scans team persona_ids, finds the `source_team` value -/// (manifest ID), checks if that directory exists under `teams_dir`, and sets -/// `team.source_dir` to that path. Respects symlinks (reads but never writes). -/// -/// Returns `true` if any team was modified. -pub(super) fn backfill_source_dirs( - teams: &mut [TeamRecord], - personas: &[AgentDefinition], - teams_dir: &Path, -) -> bool { - let mut changed = false; - - for team in teams.iter_mut() { - if team.is_builtin || team.source_dir.as_ref().is_some_and(|d| d.exists()) { - continue; - } - - // The directory name the team's personas point at. All personas of one - // team share a single source_team value (the manifest ID). - let Some(dir_name) = team - .persona_ids - .iter() - .find_map(|id| personas.iter().find(|p| p.id == *id)) - .and_then(|p| p.source_team.clone()) - else { - continue; - }; - - let candidate = teams_dir.join(&dir_name); - if candidate.exists() { - team.is_symlink = fs::symlink_metadata(&candidate) - .map(|m| m.file_type().is_symlink()) - .unwrap_or(false); - team.symlink_target = if team.is_symlink { - fs::canonicalize(&candidate) - .ok() - .map(|p| p.display().to_string()) - } else { - None - }; - team.source_dir = Some(candidate); - changed = true; - } - } - - changed -} - -/// Deduplicate `AgentDefinition`s that share a `(source_team, source_team_persona_slug)` -/// pair — the result of repeated imports before the matching predicate was fixed. -/// -/// The winner is chosen by `updated_at` descending, then `id` ascending. Loser -/// IDs are repointed to the winner in `TeamRecord.persona_ids` and -/// `ManagedAgentRecord.persona_id`, then the losers are dropped. -/// -/// After dropping losers, performs a self-healing scrub: any reference in -/// `team.persona_ids` or `agent.persona_id` that points to a persona ID not -/// in the surviving set is removed. This makes the migration convergent under -/// any crash interleaving without a transaction. -/// -/// Returns `true` if anything changed. -pub(super) fn dedup_personas( - personas: &mut Vec, - teams: &mut [TeamRecord], - agents: &mut [ManagedAgentRecord], -) -> bool { - // Group indices by (source_team, slug); only personas with both set collide. - let mut groups: HashMap<(String, String), Vec> = HashMap::new(); - for (idx, p) in personas.iter().enumerate() { - if let (Some(team), Some(slug)) = (&p.source_team, &p.source_team_persona_slug) { - groups - .entry((team.clone(), slug.clone())) - .or_default() - .push(idx); - } - } - - // loser_id -> winner_id - let mut remap: HashMap = HashMap::new(); - for indices in groups.values() { - if indices.len() < 2 { - continue; - } - let mut ranked: Vec = indices.clone(); - ranked.sort_by(|&a, &b| { - personas[b] - .updated_at - .cmp(&personas[a].updated_at) - .then_with(|| personas[a].id.cmp(&personas[b].id)) - }); - let winner_id = personas[ranked[0]].id.clone(); - for &loser_idx in &ranked[1..] { - let loser_id = personas[loser_idx].id.clone(); - eprintln!("dedup: persona {loser_id} merged into {winner_id}"); - remap.insert(loser_id, winner_id.clone()); - } - } - - // Repoint references before dropping losers. - if !remap.is_empty() { - for team in teams.iter_mut() { - let mut seen = HashSet::new(); - team.persona_ids = std::mem::take(&mut team.persona_ids) - .into_iter() - .map(|id| remap.get(&id).cloned().unwrap_or(id)) - .filter(|id| seen.insert(id.clone())) - .collect(); - } - for agent in agents.iter_mut() { - if let Some(id) = &agent.persona_id { - if let Some(winner) = remap.get(id) { - agent.persona_id = Some(winner.clone()); - } - } - } - - personas.retain(|p| !remap.contains_key(&p.id)); - } - - // Self-healing scrub: remove any references that point to persona IDs not - // in the surviving set. This handles the crash-window case where a prior - // partial run dropped losers from personas.json but never repointed the - // references in teams.json / managed-agents.json. - let surviving_ids: HashSet<&str> = personas.iter().map(|p| p.id.as_str()).collect(); - let mut scrubbed = false; - - for team in teams.iter_mut() { - let before_len = team.persona_ids.len(); - team.persona_ids - .retain(|id| surviving_ids.contains(id.as_str())); - if team.persona_ids.len() != before_len { - eprintln!( - "dedup: scrubbed {} dangling persona_ids from team {}", - before_len - team.persona_ids.len(), - team.id - ); - scrubbed = true; - } - } - for agent in agents.iter_mut() { - if let Some(id) = &agent.persona_id { - if !surviving_ids.contains(id.as_str()) { - eprintln!( - "dedup: scrubbed dangling persona_id {} from agent {}", - id, agent.name - ); - agent.persona_id = None; - scrubbed = true; - } - } - } - - !remap.is_empty() || scrubbed -} - -/// Sync all directory-backed teams on launch — the team equivalent of the -/// former `sync_pack_personas`. Runs the one-time backfill + dedup repair, -/// then re-syncs each team from its source directory. Silently skips teams -/// whose source directory is missing (e.g., external drive unmounted). -pub fn sync_team_personas(app: &AppHandle) -> Result<(), String> { - use super::teams::{load_teams, save_teams, sync_team_from_dir, teams_dir}; - - // One-time data repair: backfill source_dir for legacy teams and deduplicate - // personas that were imported multiple times before the predicate fix. - let teams_base = teams_dir(app)?; - let mut teams = load_teams(app)?; - let mut personas = super::load_personas(app)?; - let mut agents = super::load_managed_agents(app)?; - - let backfilled = backfill_source_dirs(&mut teams, &personas, &teams_base); - let deduped = dedup_personas(&mut personas, &mut teams, &mut agents); - - // Write reference holders (teams, agents) BEFORE the personas they point at. - // The three saves are individually atomic but not transactional together; a - // crash before save_personas then leaves references aimed at personas that - // still exist (over-pointing), never dangling. dedup_personas is also - // self-healing, so the next launch converges under any crash interleaving. - if backfilled || deduped { - save_teams(app, &teams)?; - } - if deduped { - super::save_managed_agents(app, &agents)?; - super::save_personas(app, &personas)?; - } - - for team in &teams { - if team.source_dir.as_ref().is_some_and(|d| d.exists()) { - if let Err(e) = sync_team_from_dir(app, &team.id) { - eprintln!("buzz-desktop: sync team {}: {e}", team.id); - } - } - } - Ok(()) -} - #[cfg(test)] mod tests { use super::*; - use crate::managed_agents::{AgentDefinition, ManagedAgentRecord, TeamRecord}; + use crate::managed_agents::TeamRecord; use std::path::PathBuf; - use tempfile::TempDir; - - fn persona(id: &str, source_team: Option<&str>, slug: Option<&str>) -> AgentDefinition { - AgentDefinition { - id: id.to_string(), - display_name: id.to_string(), - avatar_url: None, - system_prompt: String::new(), - runtime: None, - model: None, - provider: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - source_team: source_team.map(|s| s.to_string()), - source_team_persona_slug: slug.map(|s| s.to_string()), - env_vars: Default::default(), - respond_to: None, - respond_to_allowlist: Vec::new(), - parallelism: None, - created_at: "2025-01-01T00:00:00Z".to_string(), - updated_at: "2025-01-01T00:00:00Z".to_string(), - } - } fn team(id: &str) -> TeamRecord { TeamRecord { id: id.to_string(), name: id.to_string(), description: None, + instructions: None, persona_ids: Vec::new(), is_builtin: false, source_dir: None, @@ -273,61 +39,6 @@ mod tests { } } - fn agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { - ManagedAgentRecord { - pubkey: String::new(), - name: name.to_string(), - persona_id: persona_id.map(|s| s.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, - 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![], - env_vars: std::collections::BTreeMap::new(), - display_name: None, - slug: None, - runtime: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - source_team: None, - source_team_persona_slug: None, - definition_respond_to: None, - definition_respond_to_allowlist: Vec::new(), - definition_parallelism: None, - relay_mesh: None, - } - } - // ── team_persona_key ───────────────────────────────────────────────── #[test] @@ -342,169 +53,4 @@ mod tests { let t = team("builtin-team:fizz"); assert_eq!(team_persona_key(&t), "builtin-team:fizz"); } - - // ── backfill_source_dirs ───────────────────────────────────────────── - - #[test] - fn backfill_sets_source_dir_for_legacy_uuid_team() { - let tmp = TempDir::new().unwrap(); - let teams_dir = tmp.path(); - std::fs::create_dir(teams_dir.join("com.test.pack")).unwrap(); - - let mut teams = vec![{ - let mut t = team("uuid-123"); - t.persona_ids = vec!["p1".to_string()]; - t - }]; - let personas = vec![{ - let mut p = persona("p1", Some("com.test.pack"), Some("scout")); - p.source_team = Some("com.test.pack".to_string()); - p - }]; - - let changed = backfill_source_dirs(&mut teams, &personas, teams_dir); - assert!(changed); - let key = team_persona_key(&teams[0]); - assert_eq!(key, "com.test.pack"); - assert_eq!(teams[0].source_dir, Some(teams_dir.join("com.test.pack"))); - } - - #[test] - fn backfill_skips_when_directory_absent() { - let tmp = TempDir::new().unwrap(); - let teams_dir = tmp.path(); - // Do NOT create the directory - - let mut teams = vec![{ - let mut t = team("uuid-123"); - t.persona_ids = vec!["p1".to_string()]; - t - }]; - let personas = vec![persona("p1", Some("com.test.pack"), Some("scout"))]; - - let changed = backfill_source_dirs(&mut teams, &personas, teams_dir); - assert!(!changed); - assert!(teams[0].source_dir.is_none()); - } - - #[test] - fn backfill_skips_builtin_and_already_set_teams() { - let tmp = TempDir::new().unwrap(); - let teams_dir = tmp.path(); - std::fs::create_dir(teams_dir.join("com.test.pack")).unwrap(); - - let mut builtin = team("builtin-team:fizz"); - builtin.is_builtin = true; - builtin.persona_ids = vec!["p1".to_string()]; - - let mut already_set = team("uuid-456"); - already_set.source_dir = Some(teams_dir.join("com.test.pack")); - already_set.persona_ids = vec!["p2".to_string()]; - - let mut teams = vec![builtin, already_set]; - let personas = vec![ - persona("p1", Some("com.test.pack"), Some("scout")), - persona("p2", Some("com.test.pack"), Some("kit")), - ]; - - let changed = backfill_source_dirs(&mut teams, &personas, teams_dir); - assert!(!changed); - } - - // ── dedup_personas ─────────────────────────────────────────────────── - - #[test] - fn dedup_keeps_newest_and_repoints_references() { - let mut p_old = persona("old-id", Some("team-a"), Some("scout")); - p_old.updated_at = "2025-01-01T00:00:00Z".to_string(); - let mut p_new = persona("new-id", Some("team-a"), Some("scout")); - p_new.updated_at = "2025-06-01T00:00:00Z".to_string(); - - let mut personas = vec![p_old, p_new]; - let mut teams = vec![{ - let mut t = team("t1"); - t.persona_ids = vec!["old-id".to_string(), "new-id".to_string()]; - t - }]; - let mut agents = vec![agent("agent-1", Some("old-id"))]; - - let changed = dedup_personas(&mut personas, &mut teams, &mut agents); - assert!(changed); - assert_eq!(personas.len(), 1); - assert_eq!(personas[0].id, "new-id"); - assert_eq!(teams[0].persona_ids, vec!["new-id"]); - assert_eq!(agents[0].persona_id, Some("new-id".to_string())); - } - - #[test] - fn dedup_breaks_ties_by_id_when_updated_at_equal() { - let p_a = persona("aaa", Some("team-a"), Some("scout")); - let p_b = persona("bbb", Some("team-a"), Some("scout")); - - let mut personas = vec![p_a, p_b]; - let mut teams = vec![{ - let mut t = team("t1"); - t.persona_ids = vec!["aaa".to_string(), "bbb".to_string()]; - t - }]; - let mut agents = vec![]; - - let changed = dedup_personas(&mut personas, &mut teams, &mut agents); - assert!(changed); - assert_eq!(personas.len(), 1); - // "aaa" < "bbb" lexically, so "aaa" wins the tiebreak - assert_eq!(personas[0].id, "aaa"); - } - - #[test] - fn dedup_is_noop_without_duplicates() { - let mut personas = vec![ - persona("p1", Some("team-a"), Some("scout")), - persona("p2", Some("team-a"), Some("kit")), - ]; - let mut teams = vec![]; - let mut agents = vec![]; - - let changed = dedup_personas(&mut personas, &mut teams, &mut agents); - assert!(!changed); - assert_eq!(personas.len(), 2); - } - - #[test] - fn dedup_ignores_personas_without_source_team() { - let mut personas = vec![ - persona("p1", None, Some("scout")), - persona("p2", None, Some("scout")), - ]; - let mut teams = vec![]; - let mut agents = vec![]; - - let changed = dedup_personas(&mut personas, &mut teams, &mut agents); - assert!(!changed); - assert_eq!(personas.len(), 2); - } - - #[test] - fn dedup_heals_dangling_agent_reference_from_prior_crash() { - // Simulate the crash-window scenario: a prior run dropped the loser - // persona from personas.json but never repointed managed-agents.json. - // On this launch, only the winner survives. The dangling reference - // must be scrubbed. - let winner = persona("winner-id", Some("team-a"), Some("scout")); - // The loser is already gone (dropped in prior crash-interrupted run). - let mut personas = vec![winner]; - let mut teams = vec![{ - let mut t = team("t1"); - // team still references the loser (stale from prior crash) - t.persona_ids = vec!["winner-id".to_string(), "loser-id".to_string()]; - t - }]; - let mut agents = vec![agent("agent-1", Some("loser-id"))]; - - let changed = dedup_personas(&mut personas, &mut teams, &mut agents); - // The self-healing scrub should have removed the dangling references - assert!(changed); - assert_eq!(teams[0].persona_ids, vec!["winner-id"]); - assert_eq!(agents[0].persona_id, None); - } } diff --git a/desktop/src-tauri/src/managed_agents/team_snapshot.rs b/desktop/src-tauri/src/managed_agents/team_snapshot.rs index 279308a07e..d88a362723 100644 --- a/desktop/src-tauri/src/managed_agents/team_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/team_snapshot.rs @@ -5,13 +5,11 @@ //! reuses the existing `AgentSnapshot` type from `agent_snapshot.rs`. //! //! Two encodings: -//! - `.team.json` — canonical; memory inclusion is a P4.2 product decision. +//! - `.team.json` — canonical; supports memory when selected. //! - `.team.png` — 1×1 placeholder PNG with manifest in a `buzz_team_snapshot` -//! tEXt chunk; **all members MUST have `memory.level == None` and empty -//! `memory.entries`**. PNG files are casually shared — plaintext memory must -//! never appear in them. Since `TeamRecord` has no team-level avatar, the -//! image body is always the 1×1 placeholder; member avatars are carried in -//! each `AgentSnapshot.profile.avatar_data_url`. +//! tEXt chunk; supports memory when selected. Since `TeamRecord` has no +//! team-level avatar, the image body is always the 1×1 placeholder; member +//! avatars are carried in each `AgentSnapshot.profile.avatar_data_url`. //! //! **Old `.team.json` files** (flat `{version:1, type:"team", …}` schema) carry //! no `format` discriminator. The caller's legacy-reject path handles them; this @@ -28,9 +26,9 @@ //! # Memory-consistency invariant //! //! Any member with `memory.level == None` and non-empty `memory.entries` is -//! malformed. `validate_member_memory_consistency` enforces this on both the -//! JSON decode path (via `validate_team_snapshot`) and the PNG encode path (via -//! `encode_team_snapshot_png`) so the rule is single-sourced. +//! malformed. `validate_member_memory_consistency` enforces this on both +//! decode paths (JSON via `validate_team_snapshot`, PNG via +//! `decode_team_snapshot_png` → JSON round-trip) so the rule is single-sourced. // Items are `pub(crate)` for P4.2 callers; suppress dead-code lint until then. #![allow(dead_code)] @@ -65,6 +63,8 @@ pub struct TeamSnapshotMeta { pub name: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub description: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub instructions: Option, } // ── Top-level manifest ──────────────────────────────────────────────────────── @@ -100,6 +100,7 @@ pub fn build_team_snapshot(team: &TeamRecord, members: Vec) -> Te team: TeamSnapshotMeta { name: team.name.clone(), description: team.description.clone(), + instructions: team.instructions.clone(), }, members, } @@ -129,12 +130,11 @@ pub fn decode_team_snapshot_json(bytes: &[u8]) -> Result { /// team-level avatar; each member's avatar lives in their own /// `AgentSnapshot.profile.avatar_data_url`. The manifest is embedded in the /// `buzz_team_snapshot` tEXt chunk (base64-encoded JSON). -/// -/// **Rejects** any snapshot where ANY member carries memory — either via a -/// non-`None` `memory.level` or a non-empty `memory.entries`. PNG images are -/// casually shared and would expose plaintext memory. pub fn encode_team_snapshot_png(snapshot: &TeamSnapshot) -> Result, String> { - validate_team_png_has_no_member_memory(snapshot)?; + // Memory-consistency: reject level=None + non-empty entries (malformed). + for (i, member) in snapshot.members.iter().enumerate() { + validate_member_memory_consistency(i, member)?; + } let json_bytes = encode_team_snapshot_json(snapshot)?; let chunk_text = STANDARD.encode(&json_bytes); @@ -164,31 +164,11 @@ pub fn decode_team_snapshot_png(png_bytes: &[u8]) -> Result Result<(), String> { - for (i, member) in snapshot.members.iter().enumerate() { - if member.memory.level != MemoryLevel::None || !member.memory.entries.is_empty() { - return Err(format!( - "Cannot write memory to a .team.png file — member {i} ({:?}) has memory. \ - PNG images are casually shared and would expose memory as plaintext.", - member.definition.name - )); - } - } - Ok(()) -} - /// Assert that a member's memory section is internally consistent. /// /// Rejects `memory.level == None` with non-empty `memory.entries` — this is a @@ -257,6 +237,7 @@ mod tests { id: format!("{name}-id"), name: name.to_string(), description: Some(format!("{name} description")), + instructions: None, persona_ids: vec![], is_builtin: false, source_dir: None, @@ -303,6 +284,7 @@ mod tests { backend: BackendKind::Local, backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: "2024-01-01T00:00:00Z".to_string(), @@ -360,8 +342,7 @@ mod tests { // ── PNG memory guard ────────────────────────────────────────────────────── #[test] - fn png_export_rejected_when_any_member_has_memory() { - // 2-member team: member[0] has no memory, member[1] has Everything. + fn png_round_trip_with_member_memory() { let alice = build_snapshot(&agent_record("Alice"), MemoryLevel::None, vec![], None); let entries = vec![AgentSnapshotMemoryEntry { slug: "mem/notes".to_string(), @@ -370,17 +351,13 @@ mod tests { let bob = build_snapshot(&agent_record("Bob"), MemoryLevel::Everything, entries, None); let snapshot = build_team_snapshot(&team_record("Team"), vec![alice, bob]); - let result = encode_team_snapshot_png(&snapshot); - assert!( - result.is_err(), - "PNG export must fail when any member has memory" - ); - let err = result.unwrap_err(); - assert!( - err.contains("Cannot write memory to a .team.png"), - "Error must explain the PNG memory restriction, got: {err}" - ); - assert!(err.contains("Bob"), "Error must name the offending member"); + let png_bytes = encode_team_snapshot_png(&snapshot).unwrap(); + assert!(png_bytes.starts_with(b"\x89PNG"), "output must be a PNG"); + let parsed = decode_team_snapshot_png(&png_bytes).unwrap(); + assert_eq!(parsed.members.len(), 2); + assert_eq!(parsed.members[1].memory.level, MemoryLevel::Everything); + assert_eq!(parsed.members[1].memory.entries.len(), 1); + assert_eq!(parsed.members[1].memory.entries[0].slug, "mem/notes"); } #[test] @@ -411,12 +388,6 @@ mod tests { ); } - #[test] - fn png_export_succeeds_when_all_members_have_no_memory() { - let snapshot = two_member_team(); - assert!(encode_team_snapshot_png(&snapshot).is_ok()); - } - // ── Validation tests ────────────────────────────────────────────────────── #[test] @@ -595,13 +566,9 @@ mod tests { } #[test] - fn decode_team_png_rejects_member_memory() { - // Craft a memory-bearing .team.png by bypassing the encoder guard: - // build the manifest with a memory-bearing member, serialize JSON, - // base64, and write it into a PNG tEXt chunk directly. - // decode_team_snapshot_png must reject it — defense-in-depth, since - // the encoder refuses to produce this and any such PNG is malformed - // or malicious. + fn decode_team_png_with_member_memory_succeeds() { + // Build a memory-bearing .team.png by bypassing the encoder + // (pre-guard-lift this was rejected; now it round-trips cleanly). let alice = build_snapshot(&agent_record("Alice"), MemoryLevel::None, vec![], None); let entries = vec![AgentSnapshotMemoryEntry { slug: "mem/notes".to_string(), @@ -610,18 +577,12 @@ mod tests { let bob = build_snapshot(&agent_record("Bob"), MemoryLevel::Everything, entries, None); let snapshot = build_team_snapshot(&team_record("Team"), vec![alice, bob]); - // Bypass encode_team_snapshot_png's guard — write the chunk directly. - let json = encode_team_snapshot_json(&snapshot).unwrap(); - let b64 = STANDARD.encode(&json); - let png = make_png_with_text(PNG_CHUNK_KEYWORD, &b64).unwrap(); + // Now that the guard is lifted, encode directly. + let png = encode_team_snapshot_png(&snapshot).unwrap(); - let result = decode_team_snapshot_png(&png); - assert!( - result.is_err(), - "decode must reject a memory-bearing .team.png" - ); - assert!(result - .unwrap_err() - .contains("Cannot write memory to a .team.png")); + let decoded = decode_team_snapshot_png(&png).unwrap(); + assert_eq!(decoded.members.len(), 2); + assert_eq!(decoded.members[1].memory.level, MemoryLevel::Everything); + assert_eq!(decoded.members[1].memory.entries.len(), 1); } } diff --git a/desktop/src-tauri/src/managed_agents/teams.rs b/desktop/src-tauri/src/managed_agents/teams.rs index 2f6dfba9c0..6ee8c0b2ed 100644 --- a/desktop/src-tauri/src/managed_agents/teams.rs +++ b/desktop/src-tauri/src/managed_agents/teams.rs @@ -3,27 +3,13 @@ use std::{fs, path::PathBuf}; use tauri::AppHandle; use crate::{ - managed_agents::{managed_agents_base_dir, AgentDefinition, TeamRecord}, + managed_agents::{managed_agents_base_dir, ManagedAgentRecord, TeamRecord}, util::now_iso, }; use super::team_repair::team_persona_key; -#[derive(Debug, Clone, serde::Serialize)] -pub struct TeamPersonaPreview { - pub display_name: String, - pub system_prompt: String, - pub avatar_url: Option, -} - -#[derive(Debug, Clone, serde::Serialize)] -pub struct ParsedTeamPreview { - pub name: String, - pub description: Option, - pub personas: Vec, -} - -fn teams_store_path(app: &AppHandle) -> Result { +pub(crate) fn teams_store_path(app: &AppHandle) -> Result { Ok(managed_agents_base_dir(app)?.join("teams.json")) } @@ -65,6 +51,7 @@ fn built_in_team_records(built_ins: &[BuiltInTeam], now: &str) -> Vec Result<(), String> { Ok(()) } +/// Read and merge built-in teams without persisting changes. +/// +/// Returns the merged, sorted team list. No file is written — callers that +/// only need the current logical state (e.g. the snapshot-import pre-read) +/// use this to avoid a write-on-load side effect. +pub(crate) fn load_teams_readonly(path: &std::path::Path) -> Result, String> { + let now = now_iso(); + + let records = if path.exists() { + let content = fs::read_to_string(path) + .map_err(|error| format!("failed to read teams store: {error}"))?; + serde_json::from_str::>(&content) + .map_err(|error| format!("failed to parse teams store: {error}"))? + } else { + Vec::new() + }; + + let (mut records, _changed) = merge_teams(records, &now); + sort_teams(&mut records); + Ok(records) +} + pub fn load_teams(app: &AppHandle) -> Result, String> { let path = teams_store_path(app)?; let now = now_iso(); @@ -192,194 +201,27 @@ pub fn save_teams(app: &AppHandle, records: &[TeamRecord]) -> Result<(), String> crate::managed_agents::storage::atomic_write_json(&path, &payload) } -/// Teams directory: `/agents/teams/` -pub(super) fn teams_dir(app: &AppHandle) -> Result { - let dir = managed_agents_base_dir(app)?.join("teams"); - fs::create_dir_all(&dir).map_err(|e| format!("failed to create teams dir: {e}"))?; - Ok(dir) -} - -/// Validate team/pack ID: only `[a-zA-Z0-9._-]+` allowed (zip-slip defense). -pub(crate) fn validate_team_id(id: &str) -> Result<(), String> { - if id.is_empty() { - return Err("team ID is empty".into()); - } - if id.len() > 128 { - return Err(format!("team ID too long: {} chars (max 128)", id.len())); - } - if !id - .chars() - .all(|c| c.is_ascii_alphanumeric() || c == '.' || c == '_' || c == '-') - { - return Err(format!( - "team ID contains invalid characters: \"{id}\". Only [a-zA-Z0-9._-] allowed." - )); - } - if id.starts_with('.') { - return Err(format!("team ID \"{id}\" must not start with '.'")); - } - if !id.chars().any(|c| c.is_ascii_alphanumeric()) { - return Err(format!( - "team ID \"{id}\" must contain at least one alphanumeric character" - )); - } - Ok(()) -} - -/// Copy a directory tree, skipping symlinks (zip-slip defense). -fn copy_dir_no_symlinks(src: &std::path::Path, dst: &std::path::Path) -> Result<(), String> { - fs::create_dir_all(dst).map_err(|e| format!("failed to create {}: {e}", dst.display()))?; - for entry in fs::read_dir(src).map_err(|e| format!("failed to read {}: {e}", src.display()))? { - let entry = entry.map_err(|e| format!("dir entry error: {e}"))?; - let ft = entry - .file_type() - .map_err(|e| format!("file type error: {e}"))?; - let src_path = entry.path(); - let dst_path = dst.join(entry.file_name()); - - if ft.is_symlink() { - continue; - } else if ft.is_dir() { - copy_dir_no_symlinks(&src_path, &dst_path)?; - } else if ft.is_file() { - fs::copy(&src_path, &dst_path) - .map_err(|e| format!("failed to copy {}: {e}", src_path.display()))?; - } - } - Ok(()) -} - -/// Import a team from a local directory in open plugin format. -/// -/// Copies the directory into `/agents/teams//`, -/// creates PersonaRecords for each persona, creates a TeamRecord with source_dir set. -pub fn import_team_from_directory( - app: &AppHandle, - source_dir: &std::path::Path, - symlink: bool, -) -> Result { - use uuid::Uuid; - - // 1. Validate + resolve at source - let resolved = buzz_persona_pkg::resolve::resolve_pack(source_dir) - .map_err(|e| format!("team directory validation failed: {e}"))?; - - // 2. Sanitize team ID - validate_team_id(&resolved.id)?; - - // 3. Check for existing team with same ID - let teams_base = teams_dir(app)?; - let dest = teams_base.join(&resolved.id); - if dest.exists() { - return Err(format!( - "Team \"{}\" is already installed. Delete it first or use sync.", - resolved.id - )); - } - - // 4. Determine install mode: symlink or copy - let source_is_symlink = fs::symlink_metadata(source_dir) - .map(|m| m.file_type().is_symlink()) - .unwrap_or(false); - let use_symlink = symlink || source_is_symlink; - - if use_symlink { - // Resolve the canonical target for symlink - let canonical = fs::canonicalize(source_dir) - .map_err(|e| format!("failed to resolve symlink target: {e}"))?; - #[cfg(unix)] - { - std::os::unix::fs::symlink(&canonical, &dest) - .map_err(|e| format!("failed to create symlink: {e}"))?; - } - #[cfg(not(unix))] - { - // Fallback to copy on non-unix - copy_dir_no_symlinks(source_dir, &dest)?; - } - } else { - copy_dir_no_symlinks(source_dir, &dest)?; - } - - // 5. Re-validate the copy/symlink target (defense-in-depth) - let re_resolved = buzz_persona_pkg::resolve::resolve_pack(&dest).map_err(|e| { - // Clean up on failure - if use_symlink { - let _ = fs::remove_file(&dest); - } else { - let _ = fs::remove_dir_all(&dest); - } - format!("team re-validation failed after install: {e}") - })?; - - // 6. Create PersonaRecords - let now = now_iso(); - let new_personas: Vec = re_resolved - .personas +/// Names of managed agents that still reference `team` — either via the +/// legacy `persona_team_dir` link (directory-backed teams only) or the +/// `team_id` field (every team kind, all agents created after the team_id +/// seam landed). Used to block team deletion while agents still depend on it. +fn agents_referencing_team<'a>( + agents: &'a [ManagedAgentRecord], + team: &TeamRecord, +) -> Vec<&'a str> { + let persona_key = team_persona_key(team); + agents .iter() - .map(|p| AgentDefinition { - id: Uuid::new_v4().to_string(), - display_name: p.display_name.clone(), - avatar_url: p.avatar.clone(), - system_prompt: p.system_prompt.clone(), - runtime: p.runtime.clone(), - model: p.model.clone(), - provider: p.llm_provider.clone(), - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - source_team: Some(resolved.id.clone()), - source_team_persona_slug: Some(p.name.clone()), - env_vars: crate::managed_agents::env_vars::filter_derived_provider_model_env_vars( - p.runtime_env_vars.iter().cloned(), - ), - respond_to: None, - respond_to_allowlist: Vec::new(), - parallelism: None, - created_at: now.clone(), - updated_at: now.clone(), + .filter(|a| { + a.persona_team_dir + .as_ref() + .and_then(|p| p.file_name()) + .and_then(|n| n.to_str()) + == Some(persona_key) + || a.team_id.as_deref() == Some(team.id.as_str()) }) - .collect(); - - let persona_ids: Vec = new_personas.iter().map(|p| p.id.clone()).collect(); - - // 7. Save personas - let mut personas = super::load_personas(app)?; - personas.extend(new_personas); - super::save_personas(app, &personas)?; - - // 8. Create and save TeamRecord - let symlink_target = if use_symlink { - fs::canonicalize(source_dir) - .ok() - .map(|p| p.display().to_string()) - } else { - None - }; - - let team = TeamRecord { - id: resolved.id, - name: resolved.name, - description: if resolved.description.is_empty() { - None - } else { - Some(resolved.description) - }, - persona_ids, - is_builtin: false, - source_dir: Some(dest), - is_symlink: use_symlink, - symlink_target, - version: Some(resolved.version), - created_at: now.clone(), - updated_at: now, - }; - - let mut teams = load_teams(app)?; - teams.push(team.clone()); - save_teams(app, &teams)?; - - Ok(team) + .map(|a| a.name.as_str()) + .collect() } /// Delete a team, cascading removal of its sourced personas and backing dir. @@ -398,37 +240,26 @@ pub fn delete_team_with_cascade(app: &AppHandle, team_id: &str) -> Result = agents - .iter() - .filter(|a| { - a.persona_team_dir - .as_ref() - .and_then(|p| p.file_name()) - .and_then(|n| n.to_str()) - == Some(persona_key.as_str()) - }) - .map(|a| a.name.as_str()) - .collect(); - if !referencing.is_empty() { - return Err(format!( - "Cannot delete team \"{team_id}\": {} agent(s) still reference it ({}). \ - Delete or reconfigure them first.", - referencing.len(), - referencing.join(", ") - )); - } - - // 2. Remove all PersonaRecords sourced from this team + // 1. Remove all PersonaRecords sourced from this team let mut personas = super::load_personas(app)?; // Capture the d-tag of each cascaded persona BEFORE removal so the // caller can tombstone its kind:30175 coordinate on the relay. @@ -440,7 +271,7 @@ pub fn delete_team_with_cascade(app: &AppHandle, team_id: &str) -> Result Result Result { - use uuid::Uuid; - - let teams = load_teams(app)?; - let team = teams - .iter() - .find(|t| t.id == team_id) - .ok_or_else(|| format!("team {team_id} not found"))?; - - let source_dir = team - .source_dir - .as_ref() - .ok_or_else(|| format!("team {team_id} is not directory-backed"))?; - - // Personas reference the team's directory name (pack manifest ID) in - // source_team, which may differ from team_id for pre-backfill teams. - let persona_key = team_persona_key(team).to_string(); - - if !source_dir.exists() { - return Err(format!( - "team directory does not exist: {}", - source_dir.display() - )); - } - - // Resolve current state of the directory - let resolved = buzz_persona_pkg::resolve::resolve_pack(source_dir) - .map_err(|e| format!("failed to resolve team directory: {e}"))?; - - let mut personas = super::load_personas(app)?; - let now = now_iso(); - - let mut added = Vec::new(); - let mut removed = Vec::new(); - let mut updated = Vec::new(); - - // Find existing personas for this team - let existing_slugs: Vec<(String, String)> = personas - .iter() - .filter(|p| p.source_team.as_deref() == Some(persona_key.as_str())) - .map(|p| { - ( - p.source_team_persona_slug.clone().unwrap_or_default(), - p.id.clone(), - ) - }) - .collect(); - - // Check for new personas in directory - for dir_persona in &resolved.personas { - if let Some((_slug, persona_id)) = existing_slugs - .iter() - .find(|(slug, _)| slug == &dir_persona.name) - { - // Existing persona — check for content changes - if let Some(record) = personas.iter_mut().find(|p| p.id == *persona_id) { - let mut changed = false; - if record.display_name != dir_persona.display_name { - record.display_name = dir_persona.display_name.clone(); - changed = true; - } - if record.system_prompt != dir_persona.system_prompt { - record.system_prompt = dir_persona.system_prompt.clone(); - changed = true; - } - if record.avatar_url != dir_persona.avatar { - record.avatar_url = dir_persona.avatar.clone(); - changed = true; - } - if record.runtime != dir_persona.runtime { - record.runtime = dir_persona.runtime.clone(); - changed = true; - } - if record.model != dir_persona.model { - record.model = dir_persona.model.clone(); - changed = true; - } - if record.provider != dir_persona.llm_provider { - record.provider = dir_persona.llm_provider.clone(); - changed = true; - } - if changed { - record.updated_at = now.clone(); - updated.push(persona_id.clone()); - } - } - } else { - // New persona — create record - let new_persona = AgentDefinition { - id: Uuid::new_v4().to_string(), - display_name: dir_persona.display_name.clone(), - avatar_url: dir_persona.avatar.clone(), - system_prompt: dir_persona.system_prompt.clone(), - runtime: dir_persona.runtime.clone(), - model: dir_persona.model.clone(), - provider: dir_persona.llm_provider.clone(), - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - source_team: Some(persona_key.clone()), - source_team_persona_slug: Some(dir_persona.name.clone()), - env_vars: crate::managed_agents::env_vars::filter_derived_provider_model_env_vars( - dir_persona.runtime_env_vars.iter().cloned(), - ), - respond_to: None, - respond_to_allowlist: Vec::new(), - parallelism: None, - created_at: now.clone(), - updated_at: now.clone(), - }; - added.push(new_persona.id.clone()); - personas.push(new_persona); - } - } - - // Check for personas removed from directory - let dir_slugs: Vec<&str> = resolved.personas.iter().map(|p| p.name.as_str()).collect(); - let to_remove: Vec = existing_slugs - .iter() - .filter(|(slug, _)| !dir_slugs.contains(&slug.as_str())) - .map(|(_, id)| id.clone()) - .collect(); - - // Only remove if no active managed agent uses the persona - let agents = crate::managed_agents::load_managed_agents(app)?; - for persona_id in &to_remove { - let in_use = agents - .iter() - .any(|a| a.persona_id.as_deref() == Some(persona_id)); - if !in_use { - personas.retain(|p| p.id != *persona_id); - removed.push(persona_id.clone()); - } - } - - // Update team metadata if changed - let mut teams = load_teams(app)?; - let mut metadata_changed = false; - if let Some(team_record) = teams.iter_mut().find(|t| t.id == team_id) { - if team_record.name != resolved.name { - team_record.name = resolved.name; - metadata_changed = true; - } - let new_desc = if resolved.description.is_empty() { - None - } else { - Some(resolved.description) - }; - if team_record.description != new_desc { - team_record.description = new_desc; - metadata_changed = true; - } - let new_version = Some(resolved.version); - if team_record.version != new_version { - team_record.version = new_version; - metadata_changed = true; - } - // Update persona_ids to reflect current state - let current_ids: Vec = personas - .iter() - .filter(|p| p.source_team.as_deref() == Some(persona_key.as_str())) - .map(|p| p.id.clone()) - .collect(); - if team_record.persona_ids != current_ids { - team_record.persona_ids = current_ids; - metadata_changed = true; - } - if metadata_changed { - team_record.updated_at = now; - } - } - - super::save_personas(app, &personas)?; - save_teams(app, &teams)?; - - Ok(crate::managed_agents::SyncResult { - personas_added: added, - personas_removed: removed, - personas_updated: updated, - metadata_changed, - }) -} - -/// Encode a team as a JSON blob for export. The format includes the team's -/// name, description, and the full persona data for each member (so the -/// import side can recreate personas that don't exist locally). -pub fn encode_team_json( - team: &TeamRecord, - personas: &[AgentDefinition], -) -> Result, String> { - let mut missing_persona_ids = Vec::new(); - let mut resolved_personas = Vec::with_capacity(team.persona_ids.len()); - - for persona_id in &team.persona_ids { - let Some(persona) = personas - .iter() - .find(|candidate| candidate.id == *persona_id) - else { - missing_persona_ids.push(persona_id.clone()); - continue; - }; - - resolved_personas.push(serde_json::json!({ - "displayName": persona.display_name, - "systemPrompt": persona.system_prompt, - "avatarUrl": persona.avatar_url, - })); - } - - if !missing_persona_ids.is_empty() { - return Err(format!( - "Team {} references missing personas: {}. Repair the team before exporting.", - team.name, - missing_persona_ids.join(", ") - )); - } - - let map = serde_json::json!({ - "version": 1, - "type": "team", - "name": team.name, - "description": team.description, - "personas": resolved_personas, - }); - - serde_json::to_vec_pretty(&map).map_err(|e| format!("Failed to serialize team JSON: {e}")) -} - -/// Parse a team JSON file. Returns the team name, description, and embedded persona previews. -pub fn parse_team_json(json_bytes: &[u8]) -> Result { - let v: serde_json::Value = - serde_json::from_slice(json_bytes).map_err(|e| format!("Invalid JSON: {e}"))?; - - let version = v.get("version").and_then(|v| v.as_u64()).unwrap_or(0); - if version != 1 { - return Err(format!("Unsupported team version: {version}")); - } - - let file_type = v.get("type").and_then(|v| v.as_str()).unwrap_or(""); - if file_type != "team" { - return Err("Not a team export file".to_string()); - } - - let name = v - .get("name") - .and_then(|v| v.as_str()) - .unwrap_or("") - .trim() - .to_string(); - if name.is_empty() { - return Err("Team name is empty".to_string()); - } - - let description = v - .get("description") - .and_then(|v| v.as_str()) - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()); - - let personas = v - .get("personas") - .and_then(|v| v.as_array()) - .map(|arr| { - arr.iter() - .filter_map(|p| { - let display_name = p - .get("displayName") - .and_then(|v| v.as_str())? - .trim() - .to_string(); - let system_prompt = p - .get("systemPrompt") - .and_then(|v| v.as_str())? - .trim() - .to_string(); - let avatar_url = p - .get("avatarUrl") - .and_then(|v| v.as_str()) - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()); - if display_name.is_empty() || system_prompt.is_empty() { - return None; - } - Some(TeamPersonaPreview { - display_name, - system_prompt, - avatar_url, - }) - }) - .collect() - }) - .unwrap_or_default(); - - Ok(ParsedTeamPreview { - name, - description, - personas, - }) -} - #[cfg(test)] #[path = "teams_tests.rs"] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/teams_tests.rs b/desktop/src-tauri/src/managed_agents/teams_tests.rs index 6cd8268fed..7e9c7656b4 100644 --- a/desktop/src-tauri/src/managed_agents/teams_tests.rs +++ b/desktop/src-tauri/src/managed_agents/teams_tests.rs @@ -4,16 +4,17 @@ //! `#[path]`-included from there. use super::{ - encode_team_json, merge_teams, merge_teams_impl, parse_team_json, sort_teams, + agents_referencing_team, load_teams_readonly, merge_teams, merge_teams_impl, sort_teams, validate_team_deletion, BuiltInTeam, }; -use crate::managed_agents::{AgentDefinition, TeamRecord}; +use crate::managed_agents::{ManagedAgentRecord, TeamRecord}; fn team(id: &str, name: &str) -> TeamRecord { TeamRecord { id: id.to_string(), name: name.to_string(), description: None, + instructions: None, persona_ids: Vec::new(), is_builtin: false, source_dir: None, @@ -50,137 +51,6 @@ fn sort_teams_empty_is_noop() { assert!(teams.is_empty()); } -fn persona(id: &str, name: &str, prompt: &str) -> AgentDefinition { - AgentDefinition { - id: id.to_string(), - display_name: name.to_string(), - avatar_url: None, - system_prompt: prompt.to_string(), - runtime: None, - model: None, - provider: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - source_team: None, - source_team_persona_slug: None, - env_vars: std::collections::BTreeMap::new(), - respond_to: None, - respond_to_allowlist: Vec::new(), - parallelism: None, - created_at: "2026-03-20T00:00:00Z".to_string(), - updated_at: "2026-03-20T00:00:00Z".to_string(), - } -} - -#[test] -fn encode_parse_round_trip() { - let t = team("t1", "My Team"); - let t = TeamRecord { - description: Some("A great team".to_string()), - persona_ids: vec!["p1".to_string(), "p2".to_string()], - ..t - }; - let personas = vec![ - persona("p1", "Alice", "You are Alice"), - persona("p2", "Bob", "You are Bob"), - ]; - - let bytes = encode_team_json(&t, &personas).unwrap(); - let parsed = parse_team_json(&bytes).unwrap(); - - assert_eq!(parsed.name, "My Team"); - assert_eq!(parsed.description.as_deref(), Some("A great team")); - assert_eq!(parsed.personas.len(), 2); - assert_eq!(parsed.personas[0].display_name, "Alice"); - assert_eq!(parsed.personas[0].system_prompt, "You are Alice"); - assert_eq!(parsed.personas[1].display_name, "Bob"); - assert_eq!(parsed.personas[1].system_prompt, "You are Bob"); -} - -#[test] -fn encode_errors_for_missing_personas() { - let t = TeamRecord { - persona_ids: vec!["p1".to_string(), "missing".to_string()], - ..team("t1", "Team") - }; - let personas = vec![persona("p1", "Alice", "prompt")]; - - let err = encode_team_json(&t, &personas).unwrap_err(); - - assert_eq!( - err, - "Team Team references missing personas: missing. Repair the team before exporting." - ); -} - -#[test] -fn parse_team_json_invalid_version() { - let json = serde_json::json!({ - "version": 99, - "type": "team", - "name": "X", - }); - let bytes = serde_json::to_vec(&json).unwrap(); - let err = parse_team_json(&bytes).unwrap_err(); - assert!(err.contains("Unsupported team version"), "{err}"); -} - -#[test] -fn parse_team_json_wrong_type() { - let json = serde_json::json!({ - "version": 1, - "type": "persona", - "name": "X", - }); - let bytes = serde_json::to_vec(&json).unwrap(); - let err = parse_team_json(&bytes).unwrap_err(); - assert!(err.contains("Not a team export file"), "{err}"); -} - -#[test] -fn parse_team_json_empty_name() { - let json = serde_json::json!({ - "version": 1, - "type": "team", - "name": " ", - }); - let bytes = serde_json::to_vec(&json).unwrap(); - let err = parse_team_json(&bytes).unwrap_err(); - assert!(err.contains("Team name is empty"), "{err}"); -} - -#[test] -fn parse_team_json_skips_invalid_personas() { - let json = serde_json::json!({ - "version": 1, - "type": "team", - "name": "Team", - "personas": [ - { "displayName": "Good", "systemPrompt": "prompt" }, - { "displayName": "", "systemPrompt": "prompt" }, - { "displayName": "NoPrompt" }, - ], - }); - let bytes = serde_json::to_vec(&json).unwrap(); - let parsed = parse_team_json(&bytes).unwrap(); - assert_eq!(parsed.personas.len(), 1); - assert_eq!(parsed.personas[0].display_name, "Good"); -} - -#[test] -fn parse_team_json_no_personas_key() { - let json = serde_json::json!({ - "version": 1, - "type": "team", - "name": "Fizz", - }); - let bytes = serde_json::to_vec(&json).unwrap(); - let parsed = parse_team_json(&bytes).unwrap(); - assert!(parsed.personas.is_empty()); - assert_eq!(parsed.name, "Fizz"); -} - #[test] fn merge_teams_adds_missing_built_ins() { let synthetic = BuiltInTeam { @@ -289,6 +159,106 @@ fn validate_team_deletion_rejects_built_ins() { assert_eq!(err, "Built-in teams cannot be deleted."); } +// ── agents_referencing_team ───────────────────────────────────────────── + +fn managed_agent(name: &str) -> ManagedAgentRecord { + ManagedAgentRecord { + pubkey: name.to_string(), + name: name.to_string(), + persona_id: None, + team_id: None, + private_key_nsec: String::new(), + auth_tag: None, + relay_url: "ws://localhost:3000".to_string(), + avatar_url: None, + acp_command: "buzz-acp".to_string(), + agent_command: "buzz-agent".to_string(), + agent_command_override: None, + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 300, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars: std::collections::BTreeMap::new(), + start_on_app_launch: false, + auto_restart_on_config_change: false, + runtime_pid: None, + backend: crate::managed_agents::BackendKind::Local, + backend_agent_id: None, + provider_binary_path: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: "2026-01-01T00:00:00Z".to_string(), + updated_at: "2026-01-01T00:00:00Z".to_string(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: crate::managed_agents::RespondTo::OwnerOnly, + respond_to_allowlist: vec![], + display_name: None, + slug: None, + runtime: None, + name_pool: vec![], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + relay_mesh: None, + definition_respond_to: None, + definition_respond_to_allowlist: vec![], + definition_parallelism: None, + } +} + +/// A new-style agent (created after the `team_id` seam landed) that links to +/// a JSON-only team purely via `team_id` — the only kind of team that carries +/// no `source_dir`/`persona_team_dir` at all — must still be caught, or the +/// "team in use" delete guard silently never fires for it. +#[test] +fn agents_referencing_team_matches_on_team_id() { + let t = team("json-team-1", "Json Team"); + let mut linked = managed_agent("Linked Agent"); + linked.team_id = Some("json-team-1".to_string()); + let unrelated = managed_agent("Unrelated Agent"); + + let agents = vec![linked, unrelated]; + let referencing = agents_referencing_team(&agents, &t); + + assert_eq!(referencing, vec!["Linked Agent"]); +} + +/// Legacy pack-backed agents that predate the `team_id` field record their +/// link solely via `persona_team_dir` (matched against the team's directory +/// name) — this path must keep working after the `team_id` check was added. +#[test] +fn agents_referencing_team_matches_on_persona_team_dir() { + let mut t = team("uuid-1", "Dir Team"); + t.source_dir = Some(std::path::PathBuf::from("/teams/com.example.pack")); + let mut legacy = managed_agent("Legacy Agent"); + legacy.persona_team_dir = Some(std::path::PathBuf::from("/installed/com.example.pack")); + let unrelated = managed_agent("Unrelated Agent"); + + let agents = vec![legacy, unrelated]; + let referencing = agents_referencing_team(&agents, &t); + + assert_eq!(referencing, vec!["Legacy Agent"]); +} + +#[test] +fn agents_referencing_team_empty_when_no_matches() { + let t = team("json-team-2", "Json Team"); + let agents = vec![managed_agent("Agent A"), managed_agent("Agent B")]; + + assert!(agents_referencing_team(&agents, &t).is_empty()); +} + // Migration pins — exercise the real merge_teams wrapper (with production consts). #[test] @@ -299,6 +269,7 @@ fn migration_pristine_fizz_is_purged() { id: "builtin-team:fizz".to_string(), name: "Fizz".to_string(), description: Some("Fizz works carefully and collaboratively.".to_string()), + instructions: None, persona_ids: vec!["builtin:fizz".to_string()], is_builtin: true, source_dir: None, @@ -323,6 +294,7 @@ fn migration_customized_fizz_is_demoted_to_user_team() { id: "builtin-team:fizz".to_string(), name: "Fizz (customized)".to_string(), description: Some("Fizz works carefully and collaboratively.".to_string()), + instructions: None, persona_ids: vec!["builtin:fizz".to_string(), "extra:persona".to_string()], is_builtin: true, source_dir: None, @@ -352,3 +324,61 @@ fn migration_empty_store_stays_empty() { assert!(!changed); assert!(records.is_empty()); } + +// ── load_teams_readonly tests ────────────────────────────────────────── + +#[test] +fn load_teams_readonly_absent_file_performs_no_write() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("teams.json"); + + // File does not exist. + assert!(!path.exists()); + + let records = load_teams_readonly(&path).unwrap(); + + // Returns the merged built-in list (currently empty since BUILT_IN_TEAMS + // is empty, but the function ran merge_teams + sort_teams). + assert!(records.is_empty()); + + // The file must still NOT exist — no write-on-load side effect. + assert!( + !path.exists(), + "load_teams_readonly must not create the file" + ); +} + +#[test] +fn load_teams_readonly_surfaces_parse_error() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("teams.json"); + std::fs::write(&path, b"not valid json").unwrap(); + + let result = load_teams_readonly(&path); + assert!(result.is_err()); + assert!( + result.unwrap_err().contains("failed to parse teams store"), + "parse error must be surfaced" + ); +} + +#[cfg(unix)] +#[test] +fn load_teams_readonly_surfaces_read_error() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("teams.json"); + std::fs::write(&path, b"[]").unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o000)).unwrap(); + + let result = load_teams_readonly(&path); + + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap(); + + assert!(result.is_err()); + assert!( + result.unwrap_err().contains("failed to read teams store"), + "read error must be surfaced" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 9c1c5cc677..9cb2354765 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -112,6 +112,7 @@ impl AgentDefinition { backend: BackendKind::default(), backend_agent_id: None, provider_binary_path: None, + team_id: None, persona_team_dir: None, persona_name_in_team: None, created_at: self.created_at, @@ -193,6 +194,9 @@ pub struct ManagedAgentRecord { pub name: String, #[serde(default)] pub persona_id: Option, + /// Team this instance was deployed from. Resolves runtime team instructions. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub team_id: Option, /// nsec private key. Held in memory but persisted to the OS keyring (keyed /// by `pubkey`) rather than serialized to `managed-agents.json`. The /// storage layer blanks this before writing JSON once the key is safely in @@ -669,6 +673,9 @@ pub struct TeamRecord { pub id: String, pub name: String, pub description: Option, + /// Runtime-layered instructions shared by every member deployment. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub instructions: Option, pub persona_ids: Vec, #[serde(default)] pub is_builtin: bool, @@ -693,6 +700,7 @@ pub struct TeamRecord { pub struct CreateTeamRequest { pub name: String, pub description: Option, + pub instructions: Option, #[serde(default)] pub persona_ids: Vec, } @@ -703,28 +711,11 @@ pub struct UpdateTeamRequest { pub id: String, pub name: String, pub description: Option, + pub instructions: Option, #[serde(default)] pub persona_ids: Vec, } -/// Result of syncing a directory-backed team with its backing directory. -#[derive(Debug, Clone, Serialize)] -pub struct SyncResult { - pub personas_added: Vec, - pub personas_removed: Vec, - pub personas_updated: Vec, - pub metadata_changed: bool, -} - -/// Report from the one-time packs→teams migration. -#[derive(Debug, Clone, Serialize)] -pub struct MigrationReport { - pub packs_migrated: usize, - pub personas_updated: usize, - pub agents_updated: usize, - pub errors: Vec, -} - pub const DEFAULT_ACP_COMMAND: &str = "buzz-acp"; /// ~5 min (320s) — matches the CLI harness default (BUZZ_ACP_IDLE_TIMEOUT). pub const DEFAULT_AGENT_TURN_TIMEOUT_SECONDS: u64 = 320; diff --git a/desktop/src-tauri/src/managed_agents/types/requests.rs b/desktop/src-tauri/src/managed_agents/types/requests.rs index 7158946101..58d60218a1 100644 --- a/desktop/src-tauri/src/managed_agents/types/requests.rs +++ b/desktop/src-tauri/src/managed_agents/types/requests.rs @@ -129,6 +129,9 @@ pub struct CreateManagedAgentRequest { pub name: String, #[serde(default)] pub persona_id: Option, + /// Optional deployment-time team binding for runtime instruction layering. + #[serde(default)] + pub team_id: Option, pub relay_url: Option, pub acp_command: Option, pub agent_command: Option, diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 304b6aba5b..47d45cc5a4 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -166,8 +166,6 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { if is_dev { crate::managed_agents::migrate_agent_keys_to_dev_service(app); } - migrate_packs_to_teams(app); - reconcile_persona_team_dirs(app); migrate_persona_provider_to_runtime(app); reconcile_legacy_command_names(app); // Fold personas.json into the unified store HERE: after the JSON-level @@ -182,9 +180,7 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { // before event sync republishes — the backfilled link is what flips the // 30177 projection to its slim shape. backfill_standalone_agents(app); - if let Err(e) = crate::managed_agents::sync_team_personas(app) { - eprintln!("buzz-desktop: sync-team-personas: {e}"); - } + detach_directory_backed_teams(app); reconcile_provider_mcp_commands(app); reconcile_databricks_v1_to_v2(app); materialize_agent_runtimes(app); @@ -718,294 +714,6 @@ pub fn sync_shared_agent_data(app: &tauri::AppHandle) { } } -fn reconcile_team_dirs_in_file(path: &Path, target_dir: &Path) { - // Build per-component so the persisted value uses native separators on - // every platform, matching fresh writes (agents.rs builds the same path as - // base.join("teams").join(id)). A single join("agents/teams") would embed a - // literal '/' on Windows, persisting a mixed-separator path into the store. - let target_teams = target_dir.join("agents").join("teams"); - patch_json_records(path, |obj| { - // Handle both old field name and new field name - let field_name = if obj.contains_key("persona_team_dir") { - "persona_team_dir" - } else if obj.contains_key("persona_pack_path") { - "persona_pack_path" - } else { - return false; - }; - let team_path = match obj.get(field_name).and_then(|v| v.as_str()) { - Some(p) => p, - None => return false, - }; - let team_path = Path::new(team_path); - // Extract the team ID from the path (component after "teams" or "packs") - let mut found_dir = false; - let mut team_id: Option<&std::ffi::OsStr> = None; - for component in team_path.components() { - if found_dir { - team_id = Some(component.as_os_str()); - break; - } - if component.as_os_str() == "teams" || component.as_os_str() == "packs" { - found_dir = true; - } - } - let Some(id) = team_id else { - return false; - }; - let expected = target_teams.join(id); - if team_path == expected { - // Value already correct — still normalize the legacy field name so - // stores converge on `persona_team_dir` (runtime reads either via - // serde alias). - if field_name == "persona_pack_path" { - if let Some(val) = obj.remove("persona_pack_path") { - obj.insert("persona_team_dir".to_string(), val); - return true; - } - } - return false; - } - // Rewriting to a path that does not exist on disk makes things worse - // than leaving a stale-but-working path in place. fs::metadata follows - // symlinks, so a valid symlinked install passes; a dangling symlink - // fails with NotFound. - if let Err(e) = std::fs::metadata(&expected) { - eprintln!( - "buzz-desktop: team-dir-reconcile: {:?}: {:?} expected at {:?} — {e}, leaving as-is", - obj.get("name").and_then(|v| v.as_str()).unwrap_or("?"), - team_path, - expected, - ); - return false; - } - let Some(expected_str) = expected.to_str() else { - eprintln!( - "buzz-desktop: team-dir-reconcile: {:?}: expected path {:?} is not valid UTF-8, leaving as-is", - obj.get("name").and_then(|v| v.as_str()).unwrap_or("?"), - expected, - ); - return false; - }; - eprintln!( - "buzz-desktop: team-dir-reconcile: {:?}: {:?} → {:?}", - obj.get("name").and_then(|v| v.as_str()).unwrap_or("?"), - team_path, - expected, - ); - // Always write the new field name - obj.remove("persona_pack_path"); - obj.insert( - "persona_team_dir".to_string(), - serde_json::Value::String(expected_str.to_owned()), - ); - true - }); -} - -/// Select the data directory to reconcile against. -/// -/// Dev instances — identified by the data-dir name starting with -/// `CANONICAL_DEV_IDENTIFIER` (covers the canonical dir itself and any -/// worktree variant like `xyz.block.buzz.app.dev.mybranch`) — share -/// `agents/managed-agents.json` and `agents/teams` via symlinks to the -/// canonical dev dir, so they should normalize against that canonical dir. -/// -/// Release builds must reconcile their own data dir — keying off the canonical -/// dev dir's mere existence would leave release records permanently stale on -/// developer machines, where that dir is always present. -fn reconcile_target_dir(current_dir: &Path) -> PathBuf { - let is_dev_instance = current_dir - .file_name() - .and_then(|n| n.to_str()) - .is_some_and(is_dev_data_dir_name); - if is_dev_instance { - match canonical_dev_data_dir(current_dir) { - Some(dir) if dir.exists() => dir, - _ => current_dir.to_path_buf(), - } - } else { - current_dir.to_path_buf() - } -} - -/// Reconcile `persona_team_dir` (and legacy `persona_pack_path`) values in -/// managed-agents.json to point to the correct `agents/teams/` prefix. -/// -/// Fixes two classes of stale paths: -/// - Worktree dev instances whose records point at a sibling data dir rather -/// than the canonical dev dir (dev instances share managed-agents.json and -/// agents/teams via symlinks, so they all reconcile against the canonical dir). -/// - Legacy paths left by historical renames: `agents/packs/` → `agents/teams/` -/// (the packs→teams consolidation) and bundle-id `xyz.block.sprout.app` → -/// `xyz.block.buzz.app` (the sprout→buzz rename, which moved the app data dir). -/// -/// Release builds reconcile their own data dir — choosing the canonical dev dir -/// whenever it exists would leave release files permanently stale on developer -/// machines. -pub fn reconcile_persona_team_dirs(app: &tauri::AppHandle) { - let Ok(current_dir) = app.path().app_data_dir() else { - return; - }; - // Single-dir on purpose: unlike reconcile_legacy_command_names and - // reconcile_provider_mcp_commands, which patch both [current, canonical], - // path rewrites are target-dependent — a dual pass through a dev - // instance's symlinked store would write worktree-local paths into the - // shared canonical file. - let target_dir = reconcile_target_dir(¤t_dir); - let path = target_dir.join("agents/managed-agents.json"); - if !path.exists() { - return; - } - reconcile_team_dirs_in_file(&path, &target_dir); -} - -/// One-time migration from packs to teams. -/// -/// Runs on app launch if `agents/packs/` exists or if any record in -/// `managed-agents.json` still uses the old `persona_pack_path` field name. -/// Steps (in order, each individually idempotent): -/// -/// 1. Rename `agents/packs/` → `agents/teams/` on disk -/// 2. Rewrite `personas.json`: `source_pack` → `source_team`, `source_pack_persona_slug` → `source_team_persona_slug` -/// 3. Rewrite `managed-agents.json`: `persona_pack_path` → `persona_team_dir` (with `/packs/` → `/teams/` path fix), `persona_name_in_pack` → `persona_name_in_team` -pub fn migrate_packs_to_teams(app: &tauri::AppHandle) { - use crate::managed_agents::MigrationReport; - - let Ok(current_dir) = app.path().app_data_dir() else { - return; - }; - let target_dir = reconcile_target_dir(¤t_dir); - - let packs_dir = target_dir.join("agents/packs"); - let teams_dir = target_dir.join("agents/teams"); - let personas_path = target_dir.join("agents/personas.json"); - let agents_path = target_dir.join("agents/managed-agents.json"); - - // Check if migration is needed: packs dir exists OR agents JSON has old field names - let packs_dir_exists = packs_dir.exists() && !packs_dir.is_symlink(); - let has_old_fields = agents_path.exists() - && std::fs::read_to_string(&agents_path) - .map(|c| c.contains("persona_pack_path")) - .unwrap_or(false); - let personas_has_old_fields = personas_path.exists() - && std::fs::read_to_string(&personas_path) - .map(|c| c.contains("\"source_pack\"")) - .unwrap_or(false); - - if !packs_dir_exists && !has_old_fields && !personas_has_old_fields { - return; - } - - let mut report = MigrationReport { - packs_migrated: 0, - personas_updated: 0, - agents_updated: 0, - errors: Vec::new(), - }; - - // Step 1: Rename directory agents/packs/ → agents/teams/ - if packs_dir_exists { - if teams_dir.exists() { - // Merge: move contents from packs into teams, skip conflicts - if let Ok(entries) = std::fs::read_dir(&packs_dir) { - for entry in entries.flatten() { - let dest = teams_dir.join(entry.file_name()); - if !dest.exists() { - if let Err(e) = std::fs::rename(entry.path(), &dest) { - report - .errors - .push(format!("failed to move {:?}: {e}", entry.file_name())); - } else { - report.packs_migrated += 1; - } - } - } - } - // Remove packs dir only if empty (external tools like ai-rules - // may have recreated symlinks here between migration runs) - let _ = std::fs::remove_dir(&packs_dir); - } else { - // Simple rename - if let Some(parent) = teams_dir.parent() { - let _ = std::fs::create_dir_all(parent); - } - match std::fs::rename(&packs_dir, &teams_dir) { - Ok(_) => { - if let Ok(entries) = std::fs::read_dir(&teams_dir) { - report.packs_migrated = entries.count(); - } - } - Err(e) => { - report - .errors - .push(format!("failed to rename packs → teams: {e}")); - eprintln!("buzz-desktop: packs→teams migration: directory rename failed: {e}"); - return; - } - } - } - } - - // Step 2: Rewrite personas.json field names - if personas_path.exists() { - patch_json_records(&personas_path, |obj| { - let mut changed = false; - if let Some(val) = obj.remove("source_pack") { - obj.insert("source_team".to_string(), val); - changed = true; - } - if let Some(val) = obj.remove("source_pack_persona_slug") { - obj.insert("source_team_persona_slug".to_string(), val); - changed = true; - } - if changed { - report.personas_updated += 1; - } - changed - }); - } - - // Step 3: Rewrite managed-agents.json field names and paths - if agents_path.exists() { - patch_json_records(&agents_path, |obj| { - let mut changed = false; - if let Some(val) = obj.remove("persona_pack_path") { - // Also fix the path: replace /packs/ with /teams/ - let new_val = if let Some(s) = val.as_str() { - serde_json::Value::String(s.replace("/packs/", "/teams/")) - } else { - val - }; - obj.insert("persona_team_dir".to_string(), new_val); - changed = true; - } - if let Some(val) = obj.remove("persona_name_in_pack") { - obj.insert("persona_name_in_team".to_string(), val); - changed = true; - } - if changed { - report.agents_updated += 1; - } - changed - }); - } - - if report.packs_migrated > 0 || report.personas_updated > 0 || report.agents_updated > 0 { - eprintln!( - "buzz-desktop: packs→teams migration complete: {} dirs, {} personas, {} agents{}", - report.packs_migrated, - report.personas_updated, - report.agents_updated, - if report.errors.is_empty() { - String::new() - } else { - format!(" ({} errors)", report.errors.len()) - } - ); - } -} - fn reconcile_mcp_commands_in_file(path: &Path) { // Resolve each record's EFFECTIVE harness (persona-wins, override-honored) // before deriving its mcp_command, so a persona-inherited harness switch @@ -1409,6 +1117,8 @@ pub use fold::fold_personas_into_agent_store; use fold::load_persona_runtimes; mod backfill; pub use backfill::backfill_standalone_agents; +mod detach; +pub use detach::detach_directory_backed_teams; #[cfg(test)] #[path = "migration_test_support.rs"] diff --git a/desktop/src-tauri/src/migration/backfill_tests.rs b/desktop/src-tauri/src/migration/backfill_tests.rs index 39a1d2dc16..9d2731d241 100644 --- a/desktop/src-tauri/src/migration/backfill_tests.rs +++ b/desktop/src-tauri/src/migration/backfill_tests.rs @@ -131,7 +131,13 @@ fn backfill_of_promptless_record_keeps_spawn_hash_stable() { let pre_records = load_typed(dir.path()); let pre_instance = pre_records.iter().find(|r| !r.pubkey.is_empty()).unwrap(); - let hash_before = spawn_config_hash(pre_instance, &[], "wss://ws.example", &Default::default()); + let hash_before = spawn_config_hash( + pre_instance, + &[], + &[], + "wss://ws.example", + &Default::default(), + ); backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(); @@ -144,6 +150,7 @@ fn backfill_of_promptless_record_keeps_spawn_hash_stable() { let hash_after = spawn_config_hash( post_instance, &personas, + &[], "wss://ws.example", &Default::default(), ); @@ -173,7 +180,13 @@ fn backfill_of_prompted_record_keeps_spawn_hash_stable() { let pre_records = load_typed(dir.path()); let pre_instance = pre_records.iter().find(|r| !r.pubkey.is_empty()).unwrap(); - let hash_before = spawn_config_hash(pre_instance, &[], "wss://ws.example", &Default::default()); + let hash_before = spawn_config_hash( + pre_instance, + &[], + &[], + "wss://ws.example", + &Default::default(), + ); backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(); @@ -186,6 +199,7 @@ fn backfill_of_prompted_record_keeps_spawn_hash_stable() { let hash_after = spawn_config_hash( post_instance, &personas, + &[], "wss://ws.example", &Default::default(), ); diff --git a/desktop/src-tauri/src/migration/detach.rs b/desktop/src-tauri/src/migration/detach.rs new file mode 100644 index 0000000000..9f746e479f --- /dev/null +++ b/desktop/src-tauri/src/migration/detach.rs @@ -0,0 +1,160 @@ +//! T4 migration: lift pack-level instructions into `TeamRecord.instructions` +//! and detach all directory-backed teams from their file-layer plumbing. + +use std::collections::HashMap; +use std::path::Path; + +use crate::managed_agents::{ManagedAgentRecord, TeamRecord}; + +/// Lift pack instructions into `TeamRecord.instructions` and detach +/// directory-backed teams from their source directories. +/// +/// Runs on app launch if any `TeamRecord` still has `source_dir` set. +/// Both output files are written atomically (temp-file + rename), so a crash +/// mid-write leaves the previous version intact and the migration can safely +/// retry on next boot. +/// +/// Steps (written last so the idempotency gate stays open until both files +/// are committed): +/// +/// 1. Build a persona-key → team ID map from the directory-backed teams. +/// 2. Backfill `team_id` and clear `persona_team_dir`/`persona_name_in_team` +/// on every `ManagedAgentRecord`. +/// 3. Read `instructions.md` from each team's `source_dir`; populate +/// `instructions` if the field is not already set. +/// 4. Clear `source_dir`, `is_symlink`, `symlink_target`, `version` on each +/// directory-backed `TeamRecord`. +pub fn detach_directory_backed_teams(app: &tauri::AppHandle) { + let Ok(base_dir) = crate::managed_agents::managed_agents_base_dir(app) else { + return; + }; + match detach_directory_backed_teams_in_dir(&base_dir) { + Ok(0) => {} + Ok(n) => eprintln!("buzz-desktop: detach-dir-teams: detached {n} directory-backed team(s)"), + Err(e) => eprintln!("buzz-desktop: detach-dir-teams: {e}"), + } +} + +/// Core logic, decoupled from the Tauri `AppHandle` for testing. +/// +/// `base_dir` is the managed-agents base directory (`/agents/`). +/// Returns the number of teams detached (0 = nothing to do). +pub(super) fn detach_directory_backed_teams_in_dir(base_dir: &Path) -> Result { + let teams_path = base_dir.join("teams.json"); + let agents_path = base_dir.join("managed-agents.json"); + + if !teams_path.exists() { + return Ok(0); + } + + let teams_content = std::fs::read_to_string(&teams_path) + .map_err(|e| format!("failed to read teams.json: {e}"))?; + let mut teams: Vec = serde_json::from_str(&teams_content) + .map_err(|e| format!("failed to parse teams.json: {e}"))?; + + if !teams.iter().any(|t| t.source_dir.is_some()) { + return Ok(0); + } + + // Build persona-key → team ID map before clearing source_dirs. + // persona-key = the directory name (pack manifest ID) or, for teams without + // a source_dir, the team UUID. This matches the value stored in + // AgentDefinition.source_team / ManagedAgentRecord.source_team. + let persona_key_to_team_id: HashMap = teams + .iter() + .filter(|t| t.source_dir.is_some()) + .map(|t| { + let key = t + .source_dir + .as_deref() + .and_then(|d| d.file_name()) + .and_then(|n| n.to_str()) + .unwrap_or(&t.id) + .to_string(); + (key, t.id.clone()) + }) + .collect(); + + // Step 1 (agents first so the idempotency gate on teams.json stays open + // until both files are successfully written): + if agents_path.exists() { + let agents_content = std::fs::read_to_string(&agents_path) + .map_err(|e| format!("failed to read managed-agents.json: {e}"))?; + let mut agents: Vec = serde_json::from_str(&agents_content) + .map_err(|e| format!("failed to parse managed-agents.json: {e}"))?; + + let mut agents_changed = false; + for agent in agents.iter_mut() { + // Backfill team_id from source_team (absorbed from the definition, + // equals the pack directory name / persona key). + if agent.team_id.is_none() { + if let Some(source_team) = agent.source_team.as_deref() { + if let Some(team_id) = persona_key_to_team_id.get(source_team) { + agent.team_id = Some(team_id.clone()); + agents_changed = true; + } + } + } + // F1 (Thufir): clear instance-side pack plumbing. These fields + // drove BUZZ_ACP_PERSONA_PACK / BUZZ_ACP_PERSONA_NAME env vars + // at spawn, which T3 already removed. Clearing removes the dead + // data so T6 can safely delete the consuming code. + if agent.persona_team_dir.is_some() || agent.persona_name_in_team.is_some() { + agent.persona_team_dir = None; + agent.persona_name_in_team = None; + agents_changed = true; + } + } + + if agents_changed { + let payload = serde_json::to_vec_pretty(&agents) + .map_err(|e| format!("failed to serialize managed-agents.json: {e}"))?; + crate::managed_agents::atomic_write_json_restricted(&agents_path, &payload)?; + } + } + + // Step 2: lift instructions + detach each directory-backed team. + // Written last — clearing source_dir closes the idempotency gate. + let now = chrono::Utc::now().to_rfc3339(); + let mut detached = 0usize; + for team in teams.iter_mut() { + let Some(source_dir) = team.source_dir.take() else { + continue; + }; + + // Lift instructions.md content if the field is not already populated. + // If reading fails, log and leave instructions as-is (never corrupt). + if team.instructions.is_none() { + let instructions_path = source_dir.join("instructions.md"); + if instructions_path.exists() { + match std::fs::read_to_string(&instructions_path) { + Ok(content) => { + let trimmed = content.trim().to_string(); + if !trimmed.is_empty() { + team.instructions = Some(trimmed); + } + } + Err(e) => eprintln!( + "buzz-desktop: detach-dir-teams: team {}: \ + failed to read instructions.md (preserving existing value): {e}", + team.id + ), + } + } + } + + // F3 (Thufir): clear all file-layer fields. The UI badges off + // isSymlink — leaving it set without a source_dir is inconsistent. + team.is_symlink = false; + team.symlink_target = None; + team.version = None; + team.updated_at = now.clone(); + detached += 1; + } + + let payload = serde_json::to_vec_pretty(&teams) + .map_err(|e| format!("failed to serialize teams.json: {e}"))?; + crate::managed_agents::atomic_write_json(&teams_path, &payload)?; + + Ok(detached) +} diff --git a/desktop/src-tauri/src/migration_team_dir_tests.rs b/desktop/src-tauri/src/migration_team_dir_tests.rs index 1bad6a9e32..bc2ffc8f9f 100644 --- a/desktop/src-tauri/src/migration_team_dir_tests.rs +++ b/desktop/src-tauri/src/migration_team_dir_tests.rs @@ -1,435 +1,329 @@ use super::test_support::*; -use super::*; -// ── reconcile_team_dirs_in_file tests ──────────────────────────────── +// ── detach_directory_backed_teams_in_dir tests ────────────────────────── -#[test] -fn team_dir_reconcile_rewrites_worktree_path() { - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(canonical.join("agents")).unwrap(); - // Team must exist on disk for the reconcile to proceed past the existence gate. - std::fs::create_dir_all(canonical.join("agents/teams/com.wpfleger.sietch-tabr")).unwrap(); - - let worktree_pack_path = pack_dir( - &parent - .path() - .join("xyz.block.buzz.app.dev.worktree-my-branch"), - "com.wpfleger.sietch-tabr", - ); - let expected_path = team_dir(&canonical, "com.wpfleger.sietch-tabr"); - - write_agents_json( - &canonical, - &serde_json::json!([{ - "name": "Paul", - "persona_pack_path": worktree_pack_path - }]), - ); - - reconcile_team_dirs_in_file(&canonical.join("agents/managed-agents.json"), &canonical); +fn write_teams_json(base: &std::path::Path, records: &serde_json::Value) { + std::fs::create_dir_all(base.join("agents")).unwrap(); + std::fs::write( + base.join("agents/teams.json"), + serde_json::to_vec_pretty(records).unwrap(), + ) + .unwrap(); +} - let records = read_agents_json(&canonical); - assert_eq!(records[0]["persona_team_dir"], expected_path); - // Old field name should be removed - assert!(records[0].get("persona_pack_path").is_none()); +fn read_teams_json(base: &std::path::Path) -> Vec { + let content = std::fs::read_to_string(base.join("agents/teams.json")).unwrap(); + serde_json::from_str(&content).unwrap() } #[test] -fn team_dir_reconcile_rewrites_new_field_name() { - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(canonical.join("agents")).unwrap(); - // Team must exist on disk for the reconcile to proceed past the existence gate. - std::fs::create_dir_all(canonical.join("agents/teams/com.wpfleger.sietch-tabr")).unwrap(); - - let worktree_team_path = team_dir( - &parent - .path() - .join("xyz.block.buzz.app.dev.worktree-my-branch"), - "com.wpfleger.sietch-tabr", - ); - let expected_path = team_dir(&canonical, "com.wpfleger.sietch-tabr"); - - write_agents_json( - &canonical, +fn detach_lifts_instructions_from_instructions_md() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); + + // Create a team directory with instructions.md + let team_dir = base.join("teams/com.example.myteam"); + std::fs::create_dir_all(&team_dir).unwrap(); + std::fs::write(team_dir.join("instructions.md"), "Always be helpful.").unwrap(); + + // Write teams.json with a directory-backed team + write_teams_json( + base, &serde_json::json!([{ - "name": "Paul", - "persona_team_dir": worktree_team_path + "id": "team-uuid-1", + "name": "My Team", + "persona_ids": [], + "is_builtin": false, + "source_dir": team_dir.to_str().unwrap(), + "is_symlink": false, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" }]), ); - reconcile_team_dirs_in_file(&canonical.join("agents/managed-agents.json"), &canonical); + let n = super::detach::detach_directory_backed_teams_in_dir(&base.join("agents")).unwrap(); + assert_eq!(n, 1); - let records = read_agents_json(&canonical); - assert_eq!(records[0]["persona_team_dir"], expected_path); + let teams = read_teams_json(base); + assert_eq!(teams[0]["instructions"], "Always be helpful."); + assert!(teams[0].get("source_dir").is_none_or(|v| v.is_null())); + assert_eq!(teams[0]["is_symlink"], false); } #[test] -fn team_dir_reconcile_leaves_canonical_path_unchanged() { - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(canonical.join("agents")).unwrap(); +fn detach_skips_lift_if_instructions_already_set() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); - let canonical_path = team_dir(&canonical, "com.wpfleger.sietch-tabr"); + let team_dir = base.join("teams/com.example.myteam"); + std::fs::create_dir_all(&team_dir).unwrap(); + std::fs::write(team_dir.join("instructions.md"), "From disk.").unwrap(); - write_agents_json( - &canonical, + write_teams_json( + base, &serde_json::json!([{ - "name": "Duncan", - "persona_team_dir": canonical_path + "id": "team-uuid-1", + "name": "My Team", + "instructions": "Already set.", + "persona_ids": [], + "is_builtin": false, + "source_dir": team_dir.to_str().unwrap(), + "is_symlink": false, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" }]), ); - let before = std::fs::read_to_string(canonical.join("agents/managed-agents.json")).unwrap(); - reconcile_team_dirs_in_file(&canonical.join("agents/managed-agents.json"), &canonical); - let after = std::fs::read_to_string(canonical.join("agents/managed-agents.json")).unwrap(); + super::detach::detach_directory_backed_teams_in_dir(&base.join("agents")).unwrap(); - assert_eq!(before, after); + let teams = read_teams_json(base); + // Should keep the pre-existing instructions, not overwrite with disk content + assert_eq!(teams[0]["instructions"], "Already set."); } #[test] -fn team_dir_reconcile_skips_records_without_team_dir() { - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(canonical.join("agents")).unwrap(); +fn detach_clears_is_symlink_and_version() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); - write_agents_json( - &canonical, + let team_dir = base.join("teams/com.example.myteam"); + std::fs::create_dir_all(&team_dir).unwrap(); + + write_teams_json( + base, &serde_json::json!([{ - "name": "Test Agent", - "agent_command": "buzz-agent" + "id": "team-uuid-1", + "name": "My Team", + "persona_ids": [], + "is_builtin": false, + "source_dir": team_dir.to_str().unwrap(), + "is_symlink": true, + "symlink_target": "/some/external/path", + "version": "1.2.3", + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" }]), ); - let before = std::fs::read_to_string(canonical.join("agents/managed-agents.json")).unwrap(); - reconcile_team_dirs_in_file(&canonical.join("agents/managed-agents.json"), &canonical); - let after = std::fs::read_to_string(canonical.join("agents/managed-agents.json")).unwrap(); + super::detach::detach_directory_backed_teams_in_dir(&base.join("agents")).unwrap(); - assert_eq!(before, after); + let teams = read_teams_json(base); + assert_eq!(teams[0]["is_symlink"], false); + assert!(teams[0].get("symlink_target").is_none_or(|v| v.is_null())); + assert!(teams[0].get("version").is_none_or(|v| v.is_null())); + assert!(teams[0].get("source_dir").is_none_or(|v| v.is_null())); } #[test] -fn team_dir_reconcile_is_idempotent() { - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(canonical.join("agents")).unwrap(); - // Team must exist on disk for the reconcile to proceed past the existence gate. - std::fs::create_dir_all(canonical.join("agents/teams/com.wpfleger.sietch-tabr")).unwrap(); - - let worktree_pack_path = pack_dir( - &parent - .path() - .join("xyz.block.buzz.app.dev.worktree-my-branch"), - "com.wpfleger.sietch-tabr", - ); +fn detach_backfills_team_id_on_agents() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); - write_agents_json( - &canonical, + let team_dir = base.join("teams/com.example.myteam"); + std::fs::create_dir_all(&team_dir).unwrap(); + + write_teams_json( + base, &serde_json::json!([{ - "name": "Paul", - "persona_pack_path": worktree_pack_path + "id": "team-uuid-1", + "name": "My Team", + "persona_ids": ["persona-1"], + "is_builtin": false, + "source_dir": team_dir.to_str().unwrap(), + "is_symlink": false, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" }]), ); - let path = canonical.join("agents/managed-agents.json"); - reconcile_team_dirs_in_file(&path, &canonical); - let after_first = std::fs::read_to_string(&path).unwrap(); - reconcile_team_dirs_in_file(&path, &canonical); - let after_second = std::fs::read_to_string(&path).unwrap(); - - assert_eq!(after_first, after_second); -} - -#[test] -fn team_dir_reconcile_heals_legacy_release_path() { - // Record stored under the old bundle id and old packs segment should be - // rewritten to the new bundle id and teams segment. - let parent = tempfile::tempdir().unwrap(); - let release_dir = parent.path().join("xyz.block.buzz.app"); - std::fs::create_dir_all(release_dir.join("agents/teams/com.example.team")).unwrap(); - - let legacy_path = pack_dir( - &parent.path().join("xyz.block.sprout.app"), - "com.example.team", - ); - let expected_path = team_dir(&release_dir, "com.example.team"); - + // Agent with source_team = directory name, no team_id, has persona_team_dir write_agents_json( - &release_dir, + base, &serde_json::json!([{ - "name": "Stilgar", - "persona_team_dir": legacy_path + "pubkey": "aaaa", + "name": "agent-1", + "persona_id": "persona-1", + "source_team": "com.example.myteam", + "persona_team_dir": team_dir.to_str().unwrap(), + "persona_name_in_team": "agent-slug", + "relay_url": "wss://relay.example.com", + "acp_command": "claude", + "agent_command": "claude", + "mcp_command": "claude", + "agent_args": [], + "turn_timeout_seconds": 60, + "parallelism": 1, + "start_on_app_launch": false, + "auto_restart_on_config_change": true, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" }]), ); - reconcile_team_dirs_in_file( - &release_dir.join("agents/managed-agents.json"), - &release_dir, - ); + super::detach::detach_directory_backed_teams_in_dir(&base.join("agents")).unwrap(); - let records = read_agents_json(&release_dir); - assert_eq!(records[0]["persona_team_dir"], expected_path); + let agents = read_agents_json(base); + assert_eq!(agents[0]["team_id"], "team-uuid-1"); + assert!(agents[0] + .get("persona_team_dir") + .is_none_or(|v| v.is_null())); + assert!(agents[0] + .get("persona_name_in_team") + .is_none_or(|v| v.is_null())); } #[test] -fn team_dir_reconcile_leaves_record_when_team_missing() { - // When the expected target does not exist, the record must be left untouched - // and the file must not be rewritten at all (no churn). - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(canonical.join("agents")).unwrap(); - // Intentionally do NOT create agents/teams/com.example.missing. - - let stale_path = pack_dir( - &parent.path().join("xyz.block.buzz.app.dev.worktree-old"), - "com.example.missing", - ); - - write_agents_json( - &canonical, - &serde_json::json!([{ - "name": "Jessica", - "persona_pack_path": stale_path - }]), - ); - - let path = canonical.join("agents/managed-agents.json"); - let before = std::fs::read_to_string(&path).unwrap(); - reconcile_team_dirs_in_file(&path, &canonical); - let after = std::fs::read_to_string(&path).unwrap(); - - // File must be byte-identical — no churn. - assert_eq!(before, after); - // Record still has the old field and value. - let records = read_agents_json(&canonical); - assert_eq!(records[0]["persona_pack_path"], stale_path); - assert!(records[0].get("persona_team_dir").is_none()); -} +fn detach_is_idempotent_on_second_run() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); -#[cfg(unix)] -#[test] -fn team_dir_reconcile_heals_to_symlinked_team_dir() { - // When agents/teams/ is a symlink to a real directory elsewhere, - // Path::exists follows the symlink, so the rewrite should proceed. - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(canonical.join("agents")).unwrap(); - - // Real team lives outside the canonical dir; a symlink points to it. - let real_team = parent.path().join("real-team-store/com.example.team"); - std::fs::create_dir_all(&real_team).unwrap(); - let symlink_target = canonical.join("agents/teams/com.example.team"); - std::fs::create_dir_all(canonical.join("agents/teams")).unwrap(); - std::os::unix::fs::symlink(&real_team, &symlink_target).unwrap(); - - let stale_path = pack_dir( - &parent.path().join("xyz.block.buzz.app.dev.worktree-old"), - "com.example.team", - ); - let expected_path = team_dir(&canonical, "com.example.team"); + let team_dir = base.join("teams/com.example.myteam"); + std::fs::create_dir_all(&team_dir).unwrap(); + std::fs::write(team_dir.join("instructions.md"), "Team rules.").unwrap(); - write_agents_json( - &canonical, + write_teams_json( + base, &serde_json::json!([{ - "name": "Chani", - "persona_pack_path": stale_path + "id": "team-uuid-1", + "name": "My Team", + "persona_ids": [], + "is_builtin": false, + "source_dir": team_dir.to_str().unwrap(), + "is_symlink": false, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" }]), ); - reconcile_team_dirs_in_file(&canonical.join("agents/managed-agents.json"), &canonical); + // First run + let n1 = super::detach::detach_directory_backed_teams_in_dir(&base.join("agents")).unwrap(); + assert_eq!(n1, 1); + + // Second run — should be a no-op + let n2 = super::detach::detach_directory_backed_teams_in_dir(&base.join("agents")).unwrap(); + assert_eq!(n2, 0); - let records = read_agents_json(&canonical); - assert_eq!(records[0]["persona_team_dir"], expected_path); + // Instructions should still be set from first run + let teams = read_teams_json(base); + assert_eq!(teams[0]["instructions"], "Team rules."); } -#[cfg(unix)] #[test] -fn team_dir_reconcile_skips_dangling_candidate_symlink() { - // When agents/teams/ is a symlink whose target does not exist, - // Path::exists returns false, so the record must be left unchanged. - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(canonical.join("agents/teams")).unwrap(); - - // Create a dangling symlink at the expected location. - let dangling_target = parent.path().join("nonexistent-dir"); - let symlink_path = canonical.join("agents/teams/com.example.gone"); - std::os::unix::fs::symlink(&dangling_target, &symlink_path).unwrap(); - - let stale_path = pack_dir( - &parent.path().join("xyz.block.buzz.app.dev.worktree-old"), - "com.example.gone", - ); +fn detach_skips_non_directory_backed_teams() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); - write_agents_json( - &canonical, + write_teams_json( + base, &serde_json::json!([{ - "name": "Gurney", - "persona_pack_path": stale_path + "id": "team-uuid-1", + "name": "Pure JSON Team", + "instructions": "Existing instructions.", + "persona_ids": [], + "is_builtin": false, + "is_symlink": false, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" }]), ); - let path = canonical.join("agents/managed-agents.json"); - let before = std::fs::read_to_string(&path).unwrap(); - reconcile_team_dirs_in_file(&path, &canonical); - let after = std::fs::read_to_string(&path).unwrap(); + let n = super::detach::detach_directory_backed_teams_in_dir(&base.join("agents")).unwrap(); + assert_eq!(n, 0); - assert_eq!(before, after); - let records = read_agents_json(&canonical); - assert_eq!(records[0]["persona_pack_path"], stale_path); + // File should not have been modified + let teams = read_teams_json(base); + assert_eq!(teams[0]["instructions"], "Existing instructions."); } #[cfg(unix)] #[test] -fn team_dir_reconcile_through_symlink_preserves_symlink() { - // Dev worktree instances reach the canonical store through a symlinked - // managed-agents.json — the patched write must land in the canonical - // file and leave the symlink in place. - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - let worktree = parent - .path() - .join("xyz.block.buzz.app.dev.worktree-my-branch"); - std::fs::create_dir_all(canonical.join("agents/teams/com.example.team")).unwrap(); - std::fs::create_dir_all(worktree.join("agents")).unwrap(); - - let stale_path = pack_dir(&worktree, "com.example.team"); - let expected_path = team_dir(&canonical, "com.example.team"); - - write_agents_json( - &canonical, +fn detach_retries_after_teams_write_failure() { + // Write-seam interruption: make the agents/ directory read-only so the + // teams.json atomic write fails mid-detach. The agents.json write uses + // atomic_write_json_restricted (tmp+rename in the same dir), which also + // fails under a read-only parent. On retry (directory made writable), + // the still-open source_dir gate lets the migration complete. + use std::os::unix::fs::PermissionsExt; + + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); + + let team_dir = base.join("teams/com.example.myteam"); + std::fs::create_dir_all(&team_dir).unwrap(); + std::fs::write(team_dir.join("instructions.md"), "Team rules.").unwrap(); + + write_teams_json( + base, &serde_json::json!([{ - "name": "Paul", - "persona_pack_path": stale_path + "id": "team-uuid-1", + "name": "My Team", + "persona_ids": ["persona-1"], + "is_builtin": false, + "source_dir": team_dir.to_str().unwrap(), + "is_symlink": false, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" }]), ); - let canonical_store = canonical.join("agents/managed-agents.json"); - let worktree_store = worktree.join("agents/managed-agents.json"); - std::os::unix::fs::symlink(&canonical_store, &worktree_store).unwrap(); - - reconcile_team_dirs_in_file(&worktree_store, &canonical); - - assert!(worktree_store.is_symlink()); - let records = read_agents_json(&canonical); - assert_eq!(records[0]["persona_team_dir"], expected_path); -} - -// ── reconcile_target_dir tests ─────────────────────────────────────── - -#[test] -fn reconcile_target_dir_release_with_existing_dev_sibling_returns_self() { - // Release build must reconcile its OWN dir, even when a dev sibling exists. - let parent = tempfile::tempdir().unwrap(); - let release_dir = parent.path().join("xyz.block.buzz.app"); - let dev_dir = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(&release_dir).unwrap(); - std::fs::create_dir_all(&dev_dir).unwrap(); - - assert_eq!(reconcile_target_dir(&release_dir), release_dir); -} - -#[test] -fn reconcile_target_dir_worktree_dev_with_canonical_sibling_returns_canonical() { - // A worktree dev instance defers to the canonical dev dir when it exists. - let parent = tempfile::tempdir().unwrap(); - let worktree_dir = parent.path().join("xyz.block.buzz.app.dev.mybranch"); - let canonical_dir = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(&worktree_dir).unwrap(); - std::fs::create_dir_all(&canonical_dir).unwrap(); - - assert_eq!(reconcile_target_dir(&worktree_dir), canonical_dir); -} - -#[test] -fn reconcile_target_dir_canonical_dev_returns_self() { - // The canonical dev dir is itself a dev instance; it should return itself - // (canonical_dev_data_dir points at the same path, and that path exists). - let parent = tempfile::tempdir().unwrap(); - let canonical_dir = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(&canonical_dir).unwrap(); - - assert_eq!(reconcile_target_dir(&canonical_dir), canonical_dir); -} - -#[test] -fn reconcile_target_dir_release_without_dev_sibling_returns_self() { - // Release build with no dev sibling present — stays on its own dir. - let parent = tempfile::tempdir().unwrap(); - let release_dir = parent.path().join("xyz.block.buzz.app"); - std::fs::create_dir_all(&release_dir).unwrap(); - - assert_eq!(reconcile_target_dir(&release_dir), release_dir); -} - -#[test] -fn reconcile_target_dir_worktree_dev_without_canonical_sibling_returns_self() { - // Worktree dev instance with no canonical sibling present — stays on itself. - let parent = tempfile::tempdir().unwrap(); - let worktree_dir = parent.path().join("xyz.block.buzz.app.dev.mybranch"); - std::fs::create_dir_all(&worktree_dir).unwrap(); - - assert_eq!(reconcile_target_dir(&worktree_dir), worktree_dir); -} - -#[test] -fn reconcile_target_dir_ignores_non_prefix_dev_identifier() { - // Dev detection is prefix-only — a dir merely *containing* the dev - // identifier later in its name is not a dev instance. - let parent = tempfile::tempdir().unwrap(); - let odd_dir = parent.path().join("com.example.xyz.block.buzz.app.dev"); - let canonical_dir = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(&odd_dir).unwrap(); - std::fs::create_dir_all(&canonical_dir).unwrap(); - - assert_eq!(reconcile_target_dir(&odd_dir), odd_dir); -} - -#[test] -fn team_dir_reconcile_skips_path_without_teams_or_packs_component() { - // A path with no teams/packs component yields no team id — record is - // silently skipped and the file is not rewritten. - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(canonical.join("agents")).unwrap(); - write_agents_json( - &canonical, + base, &serde_json::json!([{ - "name": "Leto", - "persona_team_dir": "/some/random/path" + "pubkey": "aaaa", + "name": "agent-1", + "persona_id": "persona-1", + "source_team": "com.example.myteam", + "persona_team_dir": team_dir.to_str().unwrap(), + "persona_name_in_team": "agent-slug", + "relay_url": "wss://relay.example.com", + "acp_command": "claude", + "agent_command": "claude", + "mcp_command": "claude", + "agent_args": [], + "turn_timeout_seconds": 60, + "parallelism": 1, + "start_on_app_launch": false, + "auto_restart_on_config_change": true, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" }]), ); - let path = canonical.join("agents/managed-agents.json"); - let before = std::fs::read_to_string(&path).unwrap(); - reconcile_team_dirs_in_file(&path, &canonical); - let after = std::fs::read_to_string(&path).unwrap(); + // Make agents/ read-only so the atomic write (tmp file creation) fails. + let agents_dir = base.join("agents"); + std::fs::set_permissions(&agents_dir, std::fs::Permissions::from_mode(0o555)).unwrap(); - assert_eq!(before, after); -} - -#[test] -fn team_dir_reconcile_renames_legacy_field_when_value_already_canonical() { - // Value already points at the target — only the legacy field name is - // normalized; the value is untouched. - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - std::fs::create_dir_all(canonical.join("agents/teams/com.example.team")).unwrap(); + // Boot 1: detach fails because neither file can be written. + let result = super::detach::detach_directory_backed_teams_in_dir(&agents_dir); + assert!( + result.is_err(), + "detach must fail when directory is read-only" + ); - let correct_path = team_dir(&canonical, "com.example.team"); + // source_dir gate is still open — teams.json is unchanged. + // Restore permissions to read the file. + std::fs::set_permissions(&agents_dir, std::fs::Permissions::from_mode(0o755)).unwrap(); - write_agents_json( - &canonical, - &serde_json::json!([{ - "name": "Thufir", - "persona_pack_path": correct_path - }]), + let teams_after_fail = read_teams_json(base); + assert!( + teams_after_fail[0] + .get("source_dir") + .is_some_and(|v| !v.is_null()), + "source_dir must remain set after failed detach" ); - reconcile_team_dirs_in_file(&canonical.join("agents/managed-agents.json"), &canonical); + // Boot 2: retry with writable directory — should succeed. + let n2 = super::detach::detach_directory_backed_teams_in_dir(&agents_dir).unwrap(); + assert_eq!(n2, 1, "retry detach must succeed"); - let records = read_agents_json(&canonical); - assert_eq!(records[0]["persona_team_dir"], correct_path); - assert!(records[0].get("persona_pack_path").is_none()); + let teams_after_retry = read_teams_json(base); + assert!( + teams_after_retry[0] + .get("source_dir") + .is_none_or(|v| v.is_null()), + "source_dir cleared after successful retry" + ); + assert_eq!( + teams_after_retry[0]["instructions"], "Team rules.", + "instructions lifted on retry" + ); } diff --git a/desktop/src-tauri/src/migration_test_support.rs b/desktop/src-tauri/src/migration_test_support.rs index 399cb9c8f7..64a428949b 100644 --- a/desktop/src-tauri/src/migration_test_support.rs +++ b/desktop/src-tauri/src/migration_test_support.rs @@ -2,27 +2,6 @@ use std::path::Path; -/// Build the native-separator `/agents/teams/` string the way -/// production does (per-component `Path::join`), so test expectations match -/// reconcile output on Windows as well as Unix. -pub(crate) fn team_dir(base: &Path, id: &str) -> String { - base.join("agents") - .join("teams") - .join(id) - .display() - .to_string() -} - -/// Native-separator `/agents/packs/` — the pre-migration layout used -/// as reconcile input. See [`team_dir`] for why per-component join matters. -pub(crate) fn pack_dir(base: &Path, id: &str) -> String { - base.join("agents") - .join("packs") - .join(id) - .display() - .to_string() -} - pub(crate) fn write_agents_json(dir: &Path, records: &serde_json::Value) { std::fs::create_dir_all(dir.join("agents")).unwrap(); std::fs::write( diff --git a/desktop/src-tauri/src/migration_tests.rs b/desktop/src-tauri/src/migration_tests.rs index 66ee8dcc7a..ca84ada3bd 100644 --- a/desktop/src-tauri/src/migration_tests.rs +++ b/desktop/src-tauri/src/migration_tests.rs @@ -521,139 +521,6 @@ fn sync_replaces_real_teams_dir_with_symlink() { ); } -// ── Packs → Teams migration tests ─────────────────────────────────── - -#[cfg(unix)] -#[test] -fn migrate_packs_merge_preserves_non_empty_dir() { - // When packs/ contains symlinks that weren't moved (e.g., external tools - // recreated them), the migration should NOT delete the packs/ directory. - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - let packs_dir = canonical.join("agents/packs"); - let teams_dir = canonical.join("agents/teams"); - std::fs::create_dir_all(&packs_dir).unwrap(); - std::fs::create_dir_all(&teams_dir).unwrap(); - - // Simulate an external symlink that already exists in teams/ (conflict) - let external_target = parent.path().join("external-pack"); - std::fs::create_dir_all(&external_target).unwrap(); - std::os::unix::fs::symlink(&external_target, packs_dir.join("com.ext.pack")).unwrap(); - // Same name already in teams/ — so the migration skips it - std::os::unix::fs::symlink(&external_target, teams_dir.join("com.ext.pack")).unwrap(); - - // Run the merge logic (mirrors what migrate_packs_to_teams does) - if let Ok(entries) = std::fs::read_dir(&packs_dir) { - for entry in entries.flatten() { - let dest = teams_dir.join(entry.file_name()); - if !dest.exists() { - let _ = std::fs::rename(entry.path(), &dest); - } - } - } - // This is the fix: remove_dir only succeeds on empty dirs - let _ = std::fs::remove_dir(&packs_dir); - - // packs/ should still exist because it has a remaining symlink - assert!(packs_dir.exists(), "packs/ should survive when non-empty"); - assert!(packs_dir.join("com.ext.pack").is_symlink()); -} - -#[test] -fn migrate_packs_to_teams_renames_directory() { - let parent = tempfile::tempdir().unwrap(); - let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); - let packs_dir = canonical.join("agents/packs/com.example.test-pack"); - std::fs::create_dir_all(&packs_dir).unwrap(); - std::fs::write(packs_dir.join("plugin.json"), "{}").unwrap(); - - // No personas or agents JSON needed for directory rename - std::fs::create_dir_all(canonical.join("agents")).unwrap(); - - // Simulate calling the migration steps directly (no AppHandle needed) - let packs = canonical.join("agents/packs"); - let teams = canonical.join("agents/teams"); - std::fs::rename(&packs, &teams).unwrap(); - - assert!(!packs.exists()); - assert!(teams.join("com.example.test-pack/plugin.json").exists()); -} - -#[test] -fn migrate_packs_to_teams_rewrites_personas_json() { - let dir = tempfile::tempdir().unwrap(); - write_personas_json( - dir.path(), - &serde_json::json!([{ - "id": "persona-1", - "display_name": "Test", - "source_pack": "com.example.my-pack", - "source_pack_persona_slug": "agent-one" - }]), - ); - - let path = dir.path().join("agents/personas.json"); - patch_json_records(&path, |obj| { - let mut changed = false; - if let Some(val) = obj.remove("source_pack") { - obj.insert("source_team".to_string(), val); - changed = true; - } - if let Some(val) = obj.remove("source_pack_persona_slug") { - obj.insert("source_team_persona_slug".to_string(), val); - changed = true; - } - changed - }); - - let records = read_personas_json(dir.path()); - assert_eq!(records[0]["source_team"], "com.example.my-pack"); - assert_eq!(records[0]["source_team_persona_slug"], "agent-one"); - assert!(records[0].get("source_pack").is_none()); - assert!(records[0].get("source_pack_persona_slug").is_none()); -} - -#[test] -fn migrate_packs_to_teams_rewrites_agents_json() { - let dir = tempfile::tempdir().unwrap(); - write_agents_json( - dir.path(), - &serde_json::json!([{ - "name": "Paul", - "persona_pack_path": "/data/agents/packs/com.example.my-pack", - "persona_name_in_pack": "agent-one" - }]), - ); - - let path = dir.path().join("agents/managed-agents.json"); - patch_json_records(&path, |obj| { - let mut changed = false; - if let Some(val) = obj.remove("persona_pack_path") { - let new_val = if let Some(s) = val.as_str() { - serde_json::Value::String(s.replace("/packs/", "/teams/")) - } else { - val - }; - obj.insert("persona_team_dir".to_string(), new_val); - changed = true; - } - if let Some(val) = obj.remove("persona_name_in_pack") { - obj.insert("persona_name_in_team".to_string(), val); - changed = true; - } - changed - }); - - let records = read_agents_json(dir.path()); - assert_eq!( - records[0]["persona_team_dir"], - "/data/agents/teams/com.example.my-pack" - ); - assert_eq!(records[0]["persona_name_in_team"], "agent-one"); - assert!(records[0].get("persona_pack_path").is_none()); - assert!(records[0].get("persona_name_in_pack").is_none()); -} - /// `patch_json_records` rewrites `managed-agents.json`, which carries plaintext /// agent nsecs on a keyringless host — the writeback must land `0o600` from the /// write itself (no post-write `chmod`), or a launch-time reconcile reopens the diff --git a/desktop/src/features/agents/channelAgents.ts b/desktop/src/features/agents/channelAgents.ts index 5325126a54..4eb951f6d4 100644 --- a/desktop/src/features/agents/channelAgents.ts +++ b/desktop/src/features/agents/channelAgents.ts @@ -58,6 +58,8 @@ export type CreateChannelManagedAgentInput = { systemPrompt?: string; avatarUrl?: string; personaId?: string | null; + /** Team this instance is deployed from; prevents cross-team reuse. */ + teamId?: string | null; /** * True when `runtime` is a runtime the user deliberately picked to override * the persona (a deploy-dialog runtime selector), as opposed to a @@ -351,6 +353,7 @@ export async function provisionChannelManagedAgent( agentArgs: input.runtime.defaultArgs, mcpCommand: input.runtime.mcpCommand ?? "", personaId: input.personaId ?? undefined, + teamId: input.teamId ?? undefined, systemPrompt: input.systemPrompt?.trim() || undefined, avatarUrl: resolvedAvatarUrl, model: input.model?.trim() || undefined, diff --git a/desktop/src/features/agents/openSnapshotImportFromUrlEvent.ts b/desktop/src/features/agents/openSnapshotImportFromUrlEvent.ts index 696d803206..62e617d77d 100644 --- a/desktop/src/features/agents/openSnapshotImportFromUrlEvent.ts +++ b/desktop/src/features/agents/openSnapshotImportFromUrlEvent.ts @@ -12,6 +12,7 @@ export type PendingSnapshotImport = { fileBytes: number[]; fileName: string; + snapshotKind: "agent" | "team"; }; const OPEN_SNAPSHOT_IMPORT_EVENT = "buzz:open-snapshot-import"; @@ -24,7 +25,11 @@ let pendingImport: PendingSnapshotImport | null = null; * Clears any prior pending import so double-clicks don't stack. */ export function requestOpenSnapshotImport(payload: PendingSnapshotImport) { - pendingImport = { fileBytes: payload.fileBytes, fileName: payload.fileName }; + pendingImport = { + fileBytes: payload.fileBytes, + fileName: payload.fileName, + snapshotKind: payload.snapshotKind, + }; if (typeof window !== "undefined") { window.dispatchEvent(new Event(OPEN_SNAPSHOT_IMPORT_EVENT)); } diff --git a/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx b/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx index 454df09e5d..531875436e 100644 --- a/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx +++ b/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx @@ -142,6 +142,9 @@ export function AddTeamToChannelDialog({ avatarUrl: persona.avatarUrl ?? undefined, model: persona.model ?? undefined, personaId: persona.id, + teamId: team.id, + // One persona can be deployed under multiple teams with different instructions. + forceNewInstance: true, role, }; }); diff --git a/desktop/src/features/agents/ui/AgentsView.tsx b/desktop/src/features/agents/ui/AgentsView.tsx index d6599f2158..cce824371b 100644 --- a/desktop/src/features/agents/ui/AgentsView.tsx +++ b/desktop/src/features/agents/ui/AgentsView.tsx @@ -15,12 +15,12 @@ import { PersonaDeleteDialog } from "./PersonaDeleteDialog"; import { PersonaShareDialog } from "./PersonaShareDialog"; import { AgentSnapshotExportDialog } from "./AgentSnapshotExportDialog"; import { AgentSnapshotImportDialog } from "./AgentSnapshotImportDialog"; +import { TeamSnapshotExportDialog } from "./TeamSnapshotExportDialog"; +import { TeamSnapshotImportDialog } from "./TeamSnapshotImportDialog"; import { RelayDirectorySection } from "./RelayDirectorySection"; import { SecretRevealDialog } from "./SecretRevealDialog"; import { TeamDeleteDialog } from "./TeamDeleteDialog"; import { TeamDialog } from "./TeamDialog"; -import { TeamImportDialog } from "./TeamImportDialog"; -import { TeamImportUpdateDialog } from "./TeamImportUpdateDialog"; import { TeamsSection } from "./TeamsSection"; import { UnifiedAgentsSection } from "./UnifiedAgentsSection"; import { useManagedAgentActions } from "./useManagedAgentActions"; @@ -33,6 +33,7 @@ export function AgentsView() { const { openPersonaProfilePanel, openProfilePanel } = useProfilePanel(); const agents = useManagedAgentActions(); const personas = usePersonaActions(); + const teamImportInputRef = React.useRef(null); // Exclusivity: create never sets `personaDialogState` (edit/dup/import do), // so the create-mode and definition-edit AgentDialog mounts never coexist. const [isCreateDialogOpen, setIsCreateDialogOpen] = React.useState(false); @@ -55,7 +56,6 @@ export function AgentsView() { const isActionPending = agents.isPending || personas.isPending || - teamActions.exportTeamJsonMutation.isPending || teamActions.createTeamMutation.isPending || teamActions.updateTeamMutation.isPending || teamActions.deleteTeamMutation.isPending; @@ -73,20 +73,31 @@ export function AgentsView() { }); }, []); - // biome-ignore lint/correctness/useExhaustiveDependencies: mount-only; personas.handleImportSnapshotFile is stable + // biome-ignore lint/correctness/useExhaustiveDependencies: mount-only; personas.handleImportSnapshotFile and teamActions.handleImportTeamSnapshotFile are stable React.useEffect(() => { // Consume a snapshot import that was enqueued before navigation (e.g. from // a timeline AgentSnapshotCard click that navigated here). const pending = consumePendingSnapshotImport(); if (pending) { - void personas.handleImportSnapshotFile( - pending.fileBytes, - pending.fileName, - ); + if (pending.snapshotKind === "team") { + void teamActions.handleImportTeamSnapshotFile( + pending.fileBytes, + pending.fileName, + ); + } else { + void personas.handleImportSnapshotFile( + pending.fileBytes, + pending.fileName, + ); + } } - return subscribeSnapshotImport(({ fileBytes, fileName }) => { - void personas.handleImportSnapshotFile(fileBytes, fileName); + return subscribeSnapshotImport(({ fileBytes, fileName, snapshotKind }) => { + if (snapshotKind === "team") { + void teamActions.handleImportTeamSnapshotFile(fileBytes, fileName); + } else { + void personas.handleImportSnapshotFile(fileBytes, fileName); + } }); }, []); @@ -178,12 +189,11 @@ export function AgentsView() { onDelete={teamActions.setTeamToDelete} onDuplicate={teamActions.openDuplicateDialog} onEdit={teamActions.openEditDialog} - onExport={teamActions.handleExportTeam} - onImportFile={teamActions.handleImportFile} - onInstallFromDirectory={teamActions.handleInstallFromDirectory} - onSync={teamActions.handleSyncTeam} - onRevealInFinder={teamActions.handleRevealInFinder} onAddToChannel={teamActions.setTeamToAddToChannel} + onExport={teamActions.openExportSnapshot} + onImport={() => { + teamImportInputRef.current?.click(); + }} personas={personas.libraryPersonas} teams={teamActions.teams} /> @@ -408,12 +418,10 @@ export function AgentsView() { : null } initialValues={teamActions.teamDialogState.initialValues} - isImportPending={teamActions.isApplyingTeamImportUpdate} isPending={ teamActions.createTeamMutation.isPending || teamActions.updateTeamMutation.isPending } - onImportUpdateFile={teamActions.handleEditDialogImportUpdateFile} onOpenChange={(open) => { if (!open) { teamActions.setTeamDialogState(null); @@ -454,39 +462,65 @@ export function AgentsView() { team={teamActions.teamToAddToChannel} /> ) : null} - {teamActions.teamImportPreview ? ( - { + if (teamActions.teamToExport) { + teamActions.handleExportTeamSnapshot( + teamActions.teamToExport, + memoryLevel, + format, + ); + } + }} onOpenChange={(open) => { if (!open) { - teamActions.setTeamImportPreview(null); + teamActions.setTeamToExport(null); } }} - open={teamActions.teamImportPreview !== null} - preview={teamActions.teamImportPreview.preview} /> ) : null} - {teamActions.teamImportTarget ? ( - { + void teamActions.handleConfirmTeamSnapshotImport(keepAllowlist); + }} onOpenChange={(open) => { if (!open) { - teamActions.closeImportUpdateDialog(); + teamActions.closeTeamSnapshotImportDialog(); } }} - open={teamActions.teamImportTarget !== null} - personas={personas.libraryPersonas} - preview={teamActions.teamImportTargetPreview?.preview ?? null} - team={teamActions.teamImportTarget} /> ) : null} + {/* Hidden file input for team snapshot import via file picker */} + { + const file = e.target.files?.[0]; + if (!file) return; + const reader = new FileReader(); + reader.onload = () => { + const buffer = reader.result as ArrayBuffer; + const fileBytes = Array.from(new Uint8Array(buffer)); + void teamActions.handleImportTeamSnapshotFile(fileBytes, file.name); + }; + reader.readAsArrayBuffer(file); + // Reset so the same file can be picked again. + e.target.value = ""; + }} + /> ); } diff --git a/desktop/src/features/agents/ui/TeamDialog.tsx b/desktop/src/features/agents/ui/TeamDialog.tsx index 497b4011bf..695504429d 100644 --- a/desktop/src/features/agents/ui/TeamDialog.tsx +++ b/desktop/src/features/agents/ui/TeamDialog.tsx @@ -1,5 +1,4 @@ import * as React from "react"; -import { RefreshCw, Upload } from "lucide-react"; import { ProfileAvatar } from "@/features/profile/ui/ProfileAvatar"; import type { @@ -7,8 +6,6 @@ import type { CreateTeamInput, UpdateTeamInput, } from "@/shared/api/types"; -import { useFileImportZone } from "@/shared/hooks/useFileImportZone"; -import { cn } from "@/shared/lib/cn"; import { Badge } from "@/shared/ui/badge"; import { Button } from "@/shared/ui/button"; import { Checkbox } from "@/shared/ui/checkbox"; @@ -23,12 +20,6 @@ import { Input } from "@/shared/ui/input"; import { Textarea } from "@/shared/ui/textarea"; import { personaCatalogCopy } from "./personaLibraryCopy"; import { RemoveMembersConfirmDialog } from "./RemoveMembersConfirmDialog"; -import { - getImportButtonLabel, - getImportButtonTone, - getImportErrorLabel, - IMPORT_ERROR_VISIBILITY_MS, -} from "./teamDialogImportState"; import { copySelectedPersonaIds, countMissingPersonaIds, @@ -45,15 +36,9 @@ type TeamDialogProps = { personas: AgentPersona[]; error: Error | null; isPending: boolean; - isImportPending?: boolean; onOpenChange: (open: boolean) => void; onSubmit: (input: CreateTeamInput | UpdateTeamInput) => Promise; onDeleteRemovedPersonas?: (personaIds: string[]) => Promise; - onImportUpdateFile?: ( - teamId: string, - fileBytes: number[], - fileName: string, - ) => Promise; }; export function TeamDialog({ @@ -65,14 +50,13 @@ export function TeamDialog({ personas, error, isPending, - isImportPending = false, onOpenChange, onSubmit, onDeleteRemovedPersonas, - onImportUpdateFile, }: TeamDialogProps) { const [name, setName] = React.useState(""); const [teamDescription, setTeamDescription] = React.useState(""); + const [instructions, setInstructions] = React.useState(""); const [selectedPersonaIds, setSelectedPersonaIds] = React.useState( [], ); @@ -80,18 +64,8 @@ export function TeamDialog({ initialSelectedPersonaIdsForSort, setInitialSelectedPersonaIdsForSort, ] = React.useState([]); - const [isImportingUpdate, setIsImportingUpdate] = React.useState(false); - const [importErrorMessage, setImportErrorMessage] = React.useState< - string | null - >(null); const [confirmRemovalOpen, setConfirmRemovalOpen] = React.useState(false); const isEditMode = Boolean(initialValues && "id" in initialValues); - const editTeamId = - isEditMode && initialValues && "id" in initialValues - ? initialValues.id - : null; - const canImportTeamUpdate = isEditMode && Boolean(onImportUpdateFile); - const [isWindowFileDragOver, setIsWindowFileDragOver] = React.useState(false); const missingInitialPersonaCount = React.useMemo(() => { if (!initialValues) { return 0; @@ -107,131 +81,20 @@ export function TeamDialog({ setName(initialValues.name); setTeamDescription(initialValues.description ?? ""); + setInstructions(initialValues.instructions ?? ""); setSelectedPersonaIds(copySelectedPersonaIds(initialValues.personaIds)); setInitialSelectedPersonaIdsForSort( copySelectedPersonaIds(initialValues.personaIds), ); - setImportErrorMessage(null); - setIsImportingUpdate(false); }, [initialValues, open]); - React.useEffect(() => { - if (!open || !canImportTeamUpdate) { - setIsWindowFileDragOver(false); - return; - } - - let dragDepth = 0; - - function isFileDrag(event: DragEvent): boolean { - return Array.from(event.dataTransfer?.types ?? []).includes("Files"); - } - - function handleWindowDragEnter(event: DragEvent) { - if (!isFileDrag(event)) { - return; - } - dragDepth += 1; - setIsWindowFileDragOver(true); - } - - function handleWindowDragOver(event: DragEvent) { - if (!isFileDrag(event)) { - return; - } - event.preventDefault(); - if (event.dataTransfer) { - event.dataTransfer.dropEffect = "copy"; - } - setIsWindowFileDragOver(true); - } - - function handleWindowDragLeave(event: DragEvent) { - if (!isFileDrag(event)) { - return; - } - dragDepth = Math.max(0, dragDepth - 1); - if (dragDepth === 0) { - setIsWindowFileDragOver(false); - } - } - - function handleWindowDrop(event: DragEvent) { - if (!isFileDrag(event)) { - return; - } - event.preventDefault(); - dragDepth = 0; - setIsWindowFileDragOver(false); - } - - window.addEventListener("dragenter", handleWindowDragEnter); - window.addEventListener("dragover", handleWindowDragOver); - window.addEventListener("dragleave", handleWindowDragLeave); - window.addEventListener("drop", handleWindowDrop); - - return () => { - window.removeEventListener("dragenter", handleWindowDragEnter); - window.removeEventListener("dragover", handleWindowDragOver); - window.removeEventListener("dragleave", handleWindowDragLeave); - window.removeEventListener("drop", handleWindowDrop); - }; - }, [canImportTeamUpdate, open]); - - React.useEffect(() => { - if (!open || !importErrorMessage) { - return; - } - const timeout = window.setTimeout(() => { - setImportErrorMessage(null); - }, IMPORT_ERROR_VISIBILITY_MS); - return () => { - window.clearTimeout(timeout); - }; - }, [importErrorMessage, open]); - - async function handleImportUpdateSelection( - fileBytes: number[], - fileName: string, - ) { - if (!editTeamId || !onImportUpdateFile) { - return; - } - - setImportErrorMessage(null); - setIsImportingUpdate(true); - try { - await onImportUpdateFile(editTeamId, fileBytes, fileName); - } catch (error) { - setImportErrorMessage( - getImportErrorLabel(error instanceof Error ? error.message : null), - ); - } finally { - setIsImportingUpdate(false); - } - } - - const { - fileInputRef: importFileInputRef, - isDragOver: isImportDragOver, - dropHandlers: importDropHandlers, - handleFileChange: handleImportFileChange, - openFilePicker: openImportFilePicker, - } = useFileImportZone({ - onImportFile: (fileBytes, fileName) => { - void handleImportUpdateSelection(fileBytes, fileName); - }, - }); - function handleOpenChange(next: boolean) { if (!next) { setName(""); setTeamDescription(""); + setInstructions(""); setSelectedPersonaIds([]); setInitialSelectedPersonaIdsForSort([]); - setImportErrorMessage(null); - setIsImportingUpdate(false); - setIsWindowFileDragOver(false); setConfirmRemovalOpen(false); } @@ -266,6 +129,7 @@ export function TeamDialog({ const baseInput = { name, description: teamDescription.trim() || undefined, + instructions: instructions.trim() || undefined, personaIds: filterAvailablePersonaIds(selectedPersonaIds, personas), }; @@ -299,16 +163,6 @@ export function TeamDialog({ } } - const importButtonTone = getImportButtonTone({ - isWindowFileDragOver, - isImportDragOver, - importErrorMessage, - }); - const importButtonLabel = getImportButtonLabel({ - isWindowFileDragOver, - isImportDragOver, - importErrorMessage, - }); const orderedPersonas = React.useMemo( () => orderPersonasByInitiallySelected( @@ -362,6 +216,23 @@ export function TeamDialog({ /> +
+ +