diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index d1e07bdb6..91e8c6359 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -31,7 +31,7 @@ const rules = [ // Exceptions should stay rare and temporary. Prefer splitting files instead. const overrides = new Map([ ["src-tauri/src/managed_agents/nest.rs", 1420], // version-gated AGENTS.md + SKILL.md refresh + .agents/.claude symlink migration + ensure_skill_symlinks (all known providers) + managed section upsert + dynamic agent context + tests - ["src-tauri/src/managed_agents/personas.rs", 950], // built-in persona system prompts (Solo + Kit + Scout) + merge_personas inequality checks + persona pack import/uninstall/list + uninstall safety check + ["src-tauri/src/managed_agents/personas.rs", 980], // built-in persona system prompts (Solo + Kit + Scout) + merge_personas inequality checks + persona pack import/uninstall/list + uninstall safety check + retired persona migration (RETIRED_PERSONAS constant + migrate_retired_personas) ["src-tauri/src/managed_agents/teams.rs", 580], // built-in team registry (Kit & Scout) + merge_teams + validate_team_deletion + JSON export/import + tests ["src-tauri/src/managed_agents/persona_card.rs", 970], // PNG/ZIP/MD persona card codec + pack-zip detection + nested root finder + provider/model/namePool fields + 27 unit tests ["src/app/AppShell.tsx", 835], // message edit state + handlers + ChannelPane edit prop threading + scrollback pagination + workflows view + projects view + memory-leak safeguards + home-badge state lifted here so it consumes the same NIP-RS read-state as the sidebar (single ReadStateManager) + dock bounce wiring + mark-all-read context + channel notification callback + desktopEnabled guard @@ -46,6 +46,7 @@ const overrides = new Map([ ["src/features/settings/ui/SettingsView.tsx", 600], ["src/features/sidebar/ui/AppSidebar.tsx", 860], // channels + forums creation forms + Pulse nav ["src/shared/api/relayClientSession.ts", 1040], // durable websocket session manager with reconnect/replay/recovery state + sendTypingIndicator + fetchChannelHistoryBefore + subscribeToChannelLive (huddle TTS) + subscribeToHuddleEvents (huddle indicator) + disconnect() for workspace switch teardown + fetchEvents/subscribeLive/publishEvent for NIP-RS read state + publishUserStatus/subscribeToUserStatusUpdates (NIP-38) + ConnectionState plumbing & stall-watchdog wiring for half-open WS detection (Warp orange-icon case) + terminal session latch (auth rejection no longer racing back to reconnecting) — emitter + watchdog + reconnect policy logic extracted to relayConnectionStateEmitter.ts / relayStallWatchdog.ts / relayReconnectPolicy.ts + ["src-tauri/src/migration.rs", 630], // worktree shared-agent-data symlink sync (SHARED_AGENT_FILES symlink-to-canonical) + mcp_command provider reconciliation + tests ["src-tauri/src/commands/media.rs", 730], // ffmpeg video transcode + poster frame extraction + run_ffmpeg_with_timeout (find_ffmpeg via resolve_command, is_video_file, transcode_to_mp4, extract_poster_frame, transcode_and_extract_poster) + spawn_blocking wrappers + tests ["src-tauri/src/commands/agents.rs", 881], // remote agent lifecycle routing (local + provider branches) + scope enforcement + persona pack metadata wiring + mcp_toolsets field + NIP-OA auth_tag in deploy payload ["src-tauri/src/commands/messages.rs", 515], // feed multi-query + NIP-50 search + forum thread resolution + thread ref + reactions via REQ + edit_message media_tags param (Slack-style attachment-editable edits) @@ -77,7 +78,7 @@ const overrides = new Map([ ["src-tauri/src/huddle/tts.rs", 1380], // TTS pipeline + session warmup + cancel/shutdown handling + apply_fade_out (fade-out only — leading fade removed 2026-05-18 after onset-attenuation regression measured in examples/pocket_onset_probe.rs) + FIRST_APPEND_LEAD_IN_SAMPLES + build_sentence_append_plan (pure helper enforcing the lead-in fires exactly once per utterance, not per sentence — see lead_in_pad_fires_exactly_once_per_utterance regression test) + normalize_for_playback (per-sentence peak normalization to -3 dBFS ceiling with MAX_GAIN cap) + 30 unit tests (18 interrupt + 5 fade-out + 1 first-append-lead-in + 3 build-sentence-append-plan + 6 normalize) ["src-tauri/src/relay.rs", 510], // +4 lines for NIP-OA auth tag injection in profile sync (build_profile_event) + verification test ["src-tauri/src/commands/pairing.rs", 600], // NIP-AB pairing actor: 3 Tauri commands + background WS task + NIP-42 auth + NIP-43 probe + event parsing helpers - ["src-tauri/src/lib.rs", 730], // +4 lines for PairingHandle managed state + 3 pairing command registrations + parse_message_deep_link helper extracted with 6 unit tests covering empty-param filter regression + ["src-tauri/src/lib.rs", 733], // +4 lines for PairingHandle managed state + 3 pairing command registrations + parse_message_deep_link helper extracted with 6 unit tests covering empty-param filter regression + mod migration + sync_shared_agent_data/reconcile_provider_mcp_commands calls on launch ["src/shared/api/tauri.ts", 1212], // pairing command wrappers + applyWorkspace + NIP-44 encrypt/decrypt wrappers + observer_url field + relay member API functions (list/get/add/remove/change-role) + prevent sleep + AcpProviderCatalogEntry raw types + fromRawAcpProviderCatalogEntry converter + installAcpRuntime ]); diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 6f51609c0..a8029911f 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -4,6 +4,7 @@ mod events; mod huddle; mod managed_agents; mod media_proxy; +mod migration; mod models; pub mod nostr_convert; mod prevent_sleep; @@ -398,6 +399,12 @@ pub fn run() { let app_handle = app.handle().clone(); let shutdown_started = Arc::clone(&restore_shutdown_started); + // Sync shared agent data from the canonical dev data directory to + // this worktree's data directory. Must run before + // restore_managed_agents_on_launch (which reads managed-agents.json). + migration::sync_shared_agent_data(&app_handle); + migration::reconcile_provider_mcp_commands(&app_handle); + // Resolve persisted identity key (env var → file → generate+save). // This is fatal — the app should not start with an ephemeral identity // that will be lost on restart, as that silently breaks channel diff --git a/desktop/src-tauri/src/managed_agents/personas.rs b/desktop/src-tauri/src/managed_agents/personas.rs index 2bd74c3bc..187fee529 100644 --- a/desktop/src-tauri/src/managed_agents/personas.rs +++ b/desktop/src-tauri/src/managed_agents/personas.rs @@ -461,6 +461,33 @@ Your name is Scout. You are friendly and helpful. You are understated, but have }, ]; +const RETIRED_PERSONAS: &[(&str, &str)] = &[ + ( + "builtin:orchestrator", + "You are an orchestration agent. Coordinate multi-step work across specialized agents, keep the overall plan moving, and synthesize results into a clear final outcome. When another agent should take a task, @mention them explicitly with the assignment, expected deliverable, and any relevant constraints or deadlines.", + ), + ( + "builtin:researcher", + "You are a research agent. Gather relevant information, compare sources, call out uncertainty, and return concise findings with evidence.", + ), + ( + "builtin:planner", + "You are a planning agent. Turn ambiguous requests into structured plans with milestones, dependencies, risks, and clear next actions. Do not implement the work yourself unless asked.", + ), + ( + "builtin:implementer", + "You are a builder agent. Execute tasks directly, make code and configuration changes carefully, validate the result, and explain important decisions and follow-up items.", + ), + ( + "builtin:refactor", + "You are a refactoring agent. Improve structure, naming, duplication, and module boundaries without changing externally observable behavior. Keep changes incremental, preserve compatibility, and add or update validation when behavior could drift.", + ), + ( + "builtin:reviewer", + "You are a review agent. Inspect plans, code, and outputs for bugs, regressions, edge cases, security issues, and missing tests. Prioritize findings by severity, cite concrete evidence, and keep summaries secondary to the actual review.", + ), +]; + fn personas_store_path(app: &AppHandle) -> Result { Ok(managed_agents_base_dir(app)?.join("personas.json")) } @@ -562,10 +589,55 @@ fn merge_personas(mut stored: Vec, now: &str) -> (Vec bool { + let mut changed = false; + + for record in stored.iter_mut() { + if let Some((_, original_prompt)) = RETIRED_PERSONAS.iter().find(|(id, _)| *id == record.id) + { + let retired_suffix = " (retired)"; + let needs_suffix = !record.display_name.ends_with(retired_suffix); + if needs_suffix || record.is_active { + let was_unmodified = record.system_prompt == *original_prompt; + eprintln!( + "sprout-desktop: persona-migration: retiring {} persona '{}' → '{} (retired)'", + if was_unmodified { + "unmodified" + } else { + "customized" + }, + record.display_name, + record.display_name, + ); + if needs_suffix { + record.display_name = format!("{}{}", record.display_name, retired_suffix); + } + record.is_active = false; + record.updated_at = now.to_string(); + changed = true; + } + } + } + + changed +} + pub fn ensure_persona_is_active( personas: &[PersonaRecord], persona_id: &str, @@ -893,7 +965,7 @@ pub fn save_personas(app: &AppHandle, records: &[PersonaRecord]) -> Result<(), S let path = personas_store_path(app)?; let payload = serde_json::to_vec_pretty(&sorted) .map_err(|error| format!("failed to serialize persona store: {error}"))?; - fs::write(&path, payload).map_err(|error| format!("failed to write persona store: {error}")) + crate::managed_agents::storage::atomic_write_json(&path, &payload) } #[cfg(test)] diff --git a/desktop/src-tauri/src/managed_agents/personas/tests.rs b/desktop/src-tauri/src/managed_agents/personas/tests.rs index 71dbb637b..633e6d2a1 100644 --- a/desktop/src-tauri/src/managed_agents/personas/tests.rs +++ b/desktop/src-tauri/src/managed_agents/personas/tests.rs @@ -1,6 +1,7 @@ use super::{ - ensure_persona_ids_are_active, ensure_persona_is_active, merge_personas, validate_pack_id, - validate_persona_activation_change, validate_persona_deletion, BUILT_IN_PERSONAS, + ensure_persona_ids_are_active, ensure_persona_is_active, merge_personas, + migrate_retired_personas, validate_pack_id, validate_persona_activation_change, + validate_persona_deletion, BUILT_IN_PERSONAS, RETIRED_PERSONAS, }; use crate::managed_agents::PersonaRecord; @@ -161,6 +162,9 @@ fn merge_personas_backfills_new_builtins_for_existing_store() { #[test] fn merge_personas_demotes_retired_builtins() { + // custom_persona uses "Custom prompt", which doesn't match the original + // retired system prompt, so the migration pass soft-deprecates rather + // than removes the record. let mut retired = custom_persona("builtin:reviewer", "Reviewer"); retired.is_builtin = true; retired.is_active = true; @@ -172,9 +176,11 @@ fn merge_personas_demotes_retired_builtins() { let demoted = records .iter() .find(|record| record.id == "builtin:reviewer") - .expect("retired built-in should be retained as a custom persona"); + .expect("retired built-in should be retained as a soft-deprecated custom persona"); assert!(!demoted.is_builtin); - assert!(demoted.is_active); + // migrate_retired_personas deactivates customized retired personas. + assert!(!demoted.is_active); + assert_eq!(demoted.display_name, "Reviewer (retired)"); assert_eq!(demoted.created_at, original_created_at); assert_eq!(demoted.updated_at, "2026-04-01T00:00:00Z"); } @@ -344,3 +350,111 @@ fn pack_id_rejects_too_long() { let max_id = "a".repeat(128); assert!(validate_pack_id(&max_id).is_ok()); } + +// ── migrate_retired_personas ────────────────────────────────────────────────── + +#[test] +fn migrate_retires_unmodified_personas() { + let now = "2026-04-01T00:00:00Z"; + // Simulate a store from before the Solo/Kit/Scout transition: all 6 + // retired personas with original system prompts. + let mut stored: Vec = RETIRED_PERSONAS + .iter() + .map(|(id, prompt)| PersonaRecord { + id: id.to_string(), + system_prompt: prompt.to_string(), + is_builtin: false, // already demoted by merge_personas + ..custom_persona(id, "Test Persona") + }) + .collect(); + + let changed = migrate_retired_personas(&mut stored, now); + + assert!(changed); + assert_eq!( + stored.len(), + RETIRED_PERSONAS.len(), + "all retired personas should be soft-deprecated, not removed", + ); + assert!( + stored + .iter() + .all(|r| r.display_name.ends_with(" (retired)")), + "all retired personas should have ' (retired)' suffix", + ); + assert!( + stored.iter().all(|r| !r.is_active), + "all retired personas should be inactive", + ); + assert!( + stored.iter().all(|r| r.updated_at == now), + "all retired personas should have refreshed updated_at", + ); +} + +#[test] +fn migrate_preserves_customized_personas() { + let now = "2026-04-01T00:00:00Z"; + let mut stored = vec![PersonaRecord { + id: "builtin:researcher".to_string(), + display_name: "My Researcher".to_string(), + system_prompt: "My custom research workflow with special instructions".to_string(), + is_builtin: false, + is_active: true, + ..custom_persona("builtin:researcher", "My Researcher") + }]; + + let changed = migrate_retired_personas(&mut stored, now); + + assert!(changed); + assert_eq!(stored.len(), 1); + let record = &stored[0]; + assert_eq!(record.display_name, "My Researcher (retired)"); + assert!(!record.is_active); + assert_eq!( + record.system_prompt, + "My custom research workflow with special instructions" + ); + assert_eq!(record.updated_at, now); +} + +#[test] +fn migrate_is_idempotent() { + let now = "2026-04-01T00:00:00Z"; + + // 1. Non-retired persona — no-op. + let mut stored = vec![custom_persona("custom:test", "Custom")]; + assert!(!migrate_retired_personas(&mut stored, now)); + assert_eq!(stored.len(), 1); + + // 2. Already-retired persona (display_name ends with " (retired)") — no-op. + let mut stored_with_retired = vec![PersonaRecord { + id: "builtin:researcher".to_string(), + display_name: "Researcher (retired)".to_string(), + system_prompt: "My custom prompt".to_string(), + is_builtin: false, + is_active: false, + ..custom_persona("builtin:researcher", "Researcher (retired)") + }]; + assert!( + !migrate_retired_personas(&mut stored_with_retired, now), + "already-retired persona should not trigger another change" + ); + + // 3. Retired persona still marked is_builtin: true (pre-demotion). + // migrate_retired_personas should still soft-deprecate it. + let mut stored_pre_demotion = vec![PersonaRecord { + id: "builtin:reviewer".to_string(), + display_name: "Reviewer".to_string(), + system_prompt: "Custom review prompt".to_string(), + is_builtin: true, + is_active: true, + ..custom_persona("builtin:reviewer", "Reviewer") + }]; + assert!(migrate_retired_personas(&mut stored_pre_demotion, now)); + assert_eq!(stored_pre_demotion[0].display_name, "Reviewer (retired)"); + assert!(!stored_pre_demotion[0].is_active); + + // 4. Run again on result of (3) — should be no-op. + assert!(!migrate_retired_personas(&mut stored_pre_demotion, now)); +} diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index 265224d66..f373a1226 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -56,14 +56,18 @@ pub fn save_managed_agents(app: &AppHandle, records: &[ManagedAgentRecord]) -> R let payload = serde_json::to_vec_pretty(&sorted) .map_err(|error| format!("failed to serialize agent store: {error}"))?; - // Atomic write: write to a temp file then rename. This prevents partial - // writes from corrupting the store if the process crashes mid-write. - // rename() is atomic on the same filesystem on both macOS and Linux. - let tmp_path = path.with_extension("json.tmp"); - fs::write(&tmp_path, &payload) - .map_err(|error| format!("failed to write temp agent store: {error}"))?; - fs::rename(&tmp_path, &path) - .map_err(|error| format!("failed to rename temp agent store: {error}")) + atomic_write_json(&path, &payload) +} + +/// Atomic, symlink-preserving JSON write. +/// Resolves symlinks so the tmp+rename happens at the real target path, +/// preserving any symlink at `path`. +pub(crate) fn atomic_write_json(path: &Path, payload: &[u8]) -> Result<(), String> { + let resolved = std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()); + let tmp = resolved.with_extension("json.tmp"); + std::fs::write(&tmp, payload).map_err(|e| format!("failed to write {}: {e}", tmp.display()))?; + std::fs::rename(&tmp, &resolved) + .map_err(|e| format!("failed to rename {}: {e}", resolved.display())) } /// Maximum log file size before rotation (10 MB). diff --git a/desktop/src-tauri/src/managed_agents/teams.rs b/desktop/src-tauri/src/managed_agents/teams.rs index 71807bded..2860678f5 100644 --- a/desktop/src-tauri/src/managed_agents/teams.rs +++ b/desktop/src-tauri/src/managed_agents/teams.rs @@ -153,7 +153,7 @@ pub fn save_teams(app: &AppHandle, records: &[TeamRecord]) -> Result<(), String> let path = teams_store_path(app)?; let payload = serde_json::to_vec_pretty(&sorted) .map_err(|error| format!("failed to serialize teams store: {error}"))?; - fs::write(&path, payload).map_err(|error| format!("failed to write teams store: {error}")) + crate::managed_agents::storage::atomic_write_json(&path, &payload) } // --------------------------------------------------------------------------- diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs new file mode 100644 index 000000000..e7b636a25 --- /dev/null +++ b/desktop/src-tauri/src/migration.rs @@ -0,0 +1,623 @@ +//! Worktree data sync and on-launch reconciliation for the Sprout desktop app. +//! +//! **Worktree sync** (`sync_shared_agent_data`): Per-launch symlink creation +//! from the current worktree data directory to the canonical dev data +//! directory (`xyz.block.sprout.app.dev`). Only runs when +//! `SPROUT_SHARE_IDENTITY=1` and `SPROUT_PRIVATE_KEY` is set. All dev +//! instances share the same physical files — edits in any worktree are +//! immediately visible to all others. +//! +//! **Provider reconciliation** (`reconcile_provider_mcp_commands`): Per-launch +//! fix-up of `mcp_command` values in `managed-agents.json` against the +//! discovery table. Ensures known providers always have their canonical +//! `mcp_command`; unknown/custom agents are left untouched. + +use std::path::{Path, PathBuf}; +use tauri::Manager; + +const CANONICAL_DEV_IDENTIFIER: &str = "xyz.block.sprout.app.dev"; + +/// JSON files symlinked from worktree data directories to the canonical +/// dev data directory. Only data files — never `agent-pids/` or `logs/`. +/// `identity.key` is deliberately excluded because worktree instances +/// receive their identity via the `SPROUT_PRIVATE_KEY` env var. +/// +/// NOTE: `agents/packs/` is intentionally excluded — recursive directory +/// symlink is out of scope. Pack personas will appear in the worktree but +/// agents with `persona_pack_path` may fail if the ACP reads pack files +/// at runtime. Install packs in the worktree separately if needed. +const SHARED_AGENT_FILES: &[&str] = &[ + "agents/managed-agents.json", + "agents/personas.json", + "agents/teams.json", +]; + +fn canonical_dev_data_dir(current: &Path) -> Option { + current.parent().map(|p| p.join(CANONICAL_DEV_IDENTIFIER)) +} + +/// Read a JSON array of objects from `path`, apply `f` to each object, +/// and write back if any mutation returned `true`. +fn patch_json_records( + path: &Path, + mut f: impl FnMut(&mut serde_json::Map) -> bool, +) { + let Ok(content) = std::fs::read_to_string(path) else { + return; + }; + let Ok(mut records) = serde_json::from_str::>(&content) else { + eprintln!( + "sprout-desktop: patch-json-records: failed to parse {}", + path.display() + ); + return; + }; + let mut changed = false; + for record in &mut records { + if let Some(obj) = record.as_object_mut() { + changed |= f(obj); + } + } + if changed { + if let Ok(bytes) = serde_json::to_vec_pretty(&records) { + let _ = std::fs::write(path, bytes); + } + } +} + +/// Create symlinks for shared agent data files from the current (worktree) +/// data directory to the canonical dev data directory. +/// +/// Guards: +/// - `SPROUT_SHARE_IDENTITY` must be `"1"` +/// - `SPROUT_PRIVATE_KEY` must parse as valid `nostr::Keys` +/// - The canonical dir must differ from the current dir (skip if we ARE canonical) +/// - The canonical dir must exist +pub fn sync_shared_agent_data(app: &tauri::AppHandle) { + // Guard: only runs when sharing identity with a worktree. + let is_shared = std::env::var("SPROUT_SHARE_IDENTITY") + .map(|v| v == "1") + .unwrap_or(false); + if !is_shared { + return; + } + + // Guard: SPROUT_PRIVATE_KEY must be a valid nostr key. + let has_valid_key = std::env::var("SPROUT_PRIVATE_KEY") + .ok() + .and_then(|k| k.parse::().ok()) + .is_some(); + if !has_valid_key { + eprintln!( + "sprout-desktop: shared-agent-sync: SPROUT_PRIVATE_KEY missing or invalid, skipping" + ); + return; + } + + let current_dir = match app.path().app_data_dir() { + Ok(dir) => dir, + Err(e) => { + eprintln!("sprout-desktop: shared-agent-sync: cannot resolve app data dir: {e}"); + return; + } + }; + + let canonical_dir = match canonical_dev_data_dir(¤t_dir) { + Some(dir) => dir, + None => { + eprintln!( + "sprout-desktop: shared-agent-sync: cannot compute canonical dir (no parent)" + ); + return; + } + }; + + // Guard: skip if we ARE the canonical instance. + // Use canonicalize to handle case-insensitive FS and symlinks. + let current_canonical = + std::fs::canonicalize(¤t_dir).unwrap_or_else(|_| current_dir.clone()); + let source_canonical = + std::fs::canonicalize(&canonical_dir).unwrap_or_else(|_| canonical_dir.clone()); + if current_canonical == source_canonical { + return; + } + + // Guard: skip if canonical dir doesn't exist. + if !canonical_dir.exists() { + eprintln!( + "sprout-desktop: shared-agent-sync: canonical dir does not exist: {}", + canonical_dir.display() + ); + return; + } + + let mut synced = 0u32; + for rel in SHARED_AGENT_FILES { + let src = canonical_dir.join(rel); + let dst = current_dir.join(rel); + + if !src.exists() { + continue; + } + + if let Some(parent) = dst.parent() { + if let Err(e) = std::fs::create_dir_all(parent) { + eprintln!( + "sprout-desktop: shared-agent-sync: failed to create {}: {e}", + parent.display() + ); + continue; + } + } + + // Already a correct symlink — nothing to do. + if dst.is_symlink() { + if let Ok(target) = std::fs::read_link(&dst) { + if target == src { + continue; + } + } + } + + // Remove whatever's at dst (regular file, wrong symlink, broken symlink). + if dst.exists() || dst.is_symlink() { + let _ = std::fs::remove_file(&dst); + } + + match std::os::unix::fs::symlink(&src, &dst) { + Ok(_) => synced += 1, + Err(e) => { + eprintln!("sprout-desktop: shared-agent-sync: failed to symlink {rel}: {e}"); + } + } + } + + if synced > 0 { + eprintln!( + "sprout-desktop: shared-agent-sync: {synced} file(s) linked to {}", + canonical_dir.display() + ); + } +} + +fn reconcile_mcp_commands_in_file(path: &Path) { + patch_json_records(path, |obj| { + let agent_command = match obj.get("agent_command").and_then(|v| v.as_str()) { + Some(cmd) => cmd.to_string(), + None => return false, + }; + let Some(provider) = crate::managed_agents::known_acp_provider(&agent_command) else { + return false; + }; + let expected = provider.mcp_command.unwrap_or(""); + let current = obj + .get("mcp_command") + .and_then(|v| v.as_str()) + .unwrap_or(""); + if current != expected { + eprintln!( + "sprout-desktop: provider-reconcile: {:?} ({:?}): mcp_command {:?} → {:?}", + obj.get("name").and_then(|v| v.as_str()).unwrap_or("?"), + agent_command, + current, + expected, + ); + obj.insert( + "mcp_command".to_string(), + serde_json::Value::String(expected.to_string()), + ); + true + } else { + false + } + }); +} + +/// Reconcile `mcp_command` values in managed-agents.json against the +/// discovery table. Known providers get their canonical mcp_command; +/// unknown/custom agents are left untouched. +pub fn reconcile_provider_mcp_commands(app: &tauri::AppHandle) { + let Ok(dir) = app.path().app_data_dir() else { + return; + }; + let path = dir.join("agents/managed-agents.json"); + if !path.exists() { + return; + } + reconcile_mcp_commands_in_file(&path); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn canonical_dev_data_dir_replaces_last_component() { + let current = PathBuf::from( + "/Users/me/Library/Application Support/xyz.block.sprout.app.dev.my-branch", + ); + let canonical = canonical_dev_data_dir(¤t).unwrap(); + assert_eq!( + canonical, + PathBuf::from("/Users/me/Library/Application Support/xyz.block.sprout.app.dev") + ); + } + + #[test] + fn canonical_dev_data_dir_returns_none_for_root() { + // A root path has no parent — should return None. + assert!(canonical_dev_data_dir(Path::new("/")).is_none()); + } + + /// Helper: create a temp dir structure mimicking canonical + worktree layout. + /// Returns `(parent_dir_handle, canonical_dir, worktree_dir)`. + fn setup_sync_layout() -> (tempfile::TempDir, PathBuf, PathBuf) { + let parent = tempfile::tempdir().unwrap(); + let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); + let worktree = parent.path().join("xyz.block.sprout.app.dev.my-branch"); + + // Populate canonical with agent data. + std::fs::create_dir_all(canonical.join("agents")).unwrap(); + std::fs::write( + canonical.join("agents/managed-agents.json"), + r#"[{"id":"agent-1"}]"#, + ) + .unwrap(); + std::fs::write( + canonical.join("agents/personas.json"), + r#"[{"id":"builtin:solo"}]"#, + ) + .unwrap(); + std::fs::write(canonical.join("agents/teams.json"), r#"[{"id":"team-1"}]"#).unwrap(); + + (parent, canonical, worktree) + } + + /// Helper: sync files directly (without a Tauri AppHandle) for unit testing. + /// Mirrors the symlink loop of `sync_shared_agent_data` but takes explicit + /// paths. `sync_shared_agent_data` requires a live Tauri AppHandle and + /// cannot be unit-tested directly. + fn sync_files(canonical: &Path, worktree: &Path) -> u32 { + let mut synced = 0u32; + for rel in SHARED_AGENT_FILES { + let src = canonical.join(rel); + let dst = worktree.join(rel); + if !src.exists() { + continue; + } + if let Some(parent) = dst.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + if dst.is_symlink() { + if let Ok(target) = std::fs::read_link(&dst) { + if target == src { + continue; + } + } + } + if dst.exists() || dst.is_symlink() { + let _ = std::fs::remove_file(&dst); + } + std::os::unix::fs::symlink(&src, &dst).unwrap(); + synced += 1; + } + synced + } + + #[test] + fn sync_creates_symlinks_to_fresh_worktree() { + let (_parent, canonical, worktree) = setup_sync_layout(); + let synced = sync_files(&canonical, &worktree); + assert_eq!(synced, 3); + for rel in SHARED_AGENT_FILES { + let dst = worktree.join(rel); + assert!(dst.is_symlink(), "{rel} should be a symlink"); + assert_eq!(std::fs::read_link(&dst).unwrap(), canonical.join(rel)); + } + // Content is readable through symlinks. + assert_eq!( + std::fs::read_to_string(worktree.join("agents/managed-agents.json")).unwrap(), + r#"[{"id":"agent-1"}]"#, + ); + } + + #[test] + fn sync_replaces_existing_files_with_symlinks() { + let (_parent, canonical, worktree) = setup_sync_layout(); + std::fs::create_dir_all(worktree.join("agents")).unwrap(); + std::fs::write(worktree.join("agents/managed-agents.json"), "[]").unwrap(); + std::fs::write(worktree.join("agents/personas.json"), "[]").unwrap(); + std::fs::write(worktree.join("agents/teams.json"), "[]").unwrap(); + + let synced = sync_files(&canonical, &worktree); + + assert_eq!(synced, 3); + for rel in SHARED_AGENT_FILES { + let dst = worktree.join(rel); + assert!( + dst.is_symlink(), + "{rel} should be a symlink after replacing regular file" + ); + assert_eq!(std::fs::read_link(&dst).unwrap(), canonical.join(rel)); + } + assert_eq!( + std::fs::read_to_string(worktree.join("agents/managed-agents.json")).unwrap(), + r#"[{"id":"agent-1"}]"#, + ); + } + + #[test] + fn sync_preserves_correct_symlinks() { + let (_parent, canonical, worktree) = setup_sync_layout(); + // First sync creates symlinks. + assert_eq!(sync_files(&canonical, &worktree), 3); + // Second sync should be a no-op. + assert_eq!(sync_files(&canonical, &worktree), 0); + for rel in SHARED_AGENT_FILES { + let dst = worktree.join(rel); + assert!(dst.is_symlink()); + assert_eq!(std::fs::read_link(&dst).unwrap(), canonical.join(rel)); + } + } + + #[test] + fn sync_replaces_wrong_symlinks() { + let (_parent, canonical, worktree) = setup_sync_layout(); + let wrong_target = PathBuf::from("/nonexistent/wrong-target.json"); + std::fs::create_dir_all(worktree.join("agents")).unwrap(); + for rel in SHARED_AGENT_FILES { + std::os::unix::fs::symlink(&wrong_target, worktree.join(rel)).unwrap(); + } + let synced = sync_files(&canonical, &worktree); + assert_eq!(synced, 3); + for rel in SHARED_AGENT_FILES { + assert_eq!( + std::fs::read_link(worktree.join(rel)).unwrap(), + canonical.join(rel) + ); + } + } + + #[test] + fn sync_handles_broken_symlinks() { + let (_parent, canonical, worktree) = setup_sync_layout(); + std::fs::create_dir_all(worktree.join("agents")).unwrap(); + let broken_target = PathBuf::from("/this/does/not/exist.json"); + for rel in SHARED_AGENT_FILES { + std::os::unix::fs::symlink(&broken_target, worktree.join(rel)).unwrap(); + } + let synced = sync_files(&canonical, &worktree); + assert_eq!(synced, 3); + for rel in SHARED_AGENT_FILES { + let dst = worktree.join(rel); + assert!(dst.is_symlink()); + assert_eq!(std::fs::read_link(&dst).unwrap(), canonical.join(rel)); + // Content should be readable through the fixed symlink. + assert!(std::fs::read_to_string(&dst).is_ok()); + } + } + + #[test] + fn writes_through_symlink_reach_canonical() { + let (_parent, canonical, worktree) = setup_sync_layout(); + sync_files(&canonical, &worktree); + + let worktree_path = worktree.join("agents/personas.json"); + let canonical_path = canonical.join("agents/personas.json"); + + // Write through the symlink using the same pattern as atomic_write_json. + let new_content = r#"[{"id":"builtin:solo","updated":true}]"#; + let resolved = std::fs::canonicalize(&worktree_path).unwrap(); + let tmp = resolved.with_extension("json.tmp"); + std::fs::write(&tmp, new_content.as_bytes()).unwrap(); + std::fs::rename(&tmp, &resolved).unwrap(); + + // The canonical file should have the new content. + assert_eq!( + std::fs::read_to_string(&canonical_path).unwrap(), + new_content + ); + // The worktree path should still be a symlink. + assert!(worktree_path.is_symlink()); + // Reading through the symlink should return the new content. + assert_eq!( + std::fs::read_to_string(&worktree_path).unwrap(), + new_content + ); + } + + #[test] + fn canonical_dev_data_dir_returns_self_for_canonical_instance() { + // When the current app data dir IS the canonical dev identifier, + // canonical_dev_data_dir returns the exact same path — the caller + // (sync_shared_agent_data) uses this equality to skip the sync. + // The env-var guards (SPROUT_SHARE_IDENTITY, SPROUT_PRIVATE_KEY) + // require a live Tauri AppHandle and are covered by integration + // testing only. + let current = + PathBuf::from("/Users/me/Library/Application Support/xyz.block.sprout.app.dev"); + assert_eq!(canonical_dev_data_dir(¤t).unwrap(), current); + + // Also verify with a temp dir on the real filesystem. + let parent = tempfile::tempdir().unwrap(); + let canonical = parent.path().join(CANONICAL_DEV_IDENTIFIER); + assert_eq!(canonical_dev_data_dir(&canonical).unwrap(), canonical); + } + + fn write_agents_json(dir: &Path, records: &serde_json::Value) { + std::fs::create_dir_all(dir.join("agents")).unwrap(); + std::fs::write( + dir.join("agents/managed-agents.json"), + serde_json::to_vec_pretty(records).unwrap(), + ) + .unwrap(); + } + + fn read_agents_json(dir: &Path) -> Vec { + let content = std::fs::read_to_string(dir.join("agents/managed-agents.json")).unwrap(); + serde_json::from_str(&content).unwrap() + } + + #[test] + fn reconcile_clears_mcp_command_for_goose() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Scout", + "agent_command": "goose", + "mcp_command": "sprout-mcp-server" + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], ""); + } + + #[test] + fn reconcile_clears_mcp_command_for_claude() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Claude Agent", + "agent_command": "claude-agent-acp", + "mcp_command": "sprout-mcp-server" + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], ""); + } + + #[test] + fn reconcile_preserves_sprout_dev_mcp() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Solo", + "agent_command": "sprout-agent", + "mcp_command": "sprout-dev-mcp" + }]), + ); + let before = + std::fs::read_to_string(dir.path().join("agents/managed-agents.json")).unwrap(); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let after = std::fs::read_to_string(dir.path().join("agents/managed-agents.json")).unwrap(); + assert_eq!( + before, after, + "file should not be rewritten when already correct" + ); + } + + #[test] + fn reconcile_fixes_sprout_agent_if_stale() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Solo", + "agent_command": "sprout-agent", + "mcp_command": "sprout-mcp-server" + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], "sprout-dev-mcp"); + } + + #[test] + fn reconcile_leaves_unknown_agent_untouched() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Custom Bot", + "agent_command": "my-custom-agent", + "mcp_command": "my-custom-mcp" + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], "my-custom-mcp"); + } + + #[test] + fn reconcile_is_idempotent() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Scout", + "agent_command": "goose", + "mcp_command": "sprout-mcp-server" + }]), + ); + let path = dir.path().join("agents/managed-agents.json"); + reconcile_mcp_commands_in_file(&path); + let after_first = std::fs::read_to_string(&path).unwrap(); + reconcile_mcp_commands_in_file(&path); + let after_second = std::fs::read_to_string(&path).unwrap(); + assert_eq!(after_first, after_second); + } + + #[test] + fn reconcile_handles_mixed_records() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([ + {"name": "Scout", "agent_command": "goose", "mcp_command": "sprout-mcp-server"}, + {"name": "Claude", "agent_command": "claude-agent-acp", "mcp_command": "sprout-mcp-server"}, + {"name": "Solo", "agent_command": "sprout-agent", "mcp_command": "sprout-dev-mcp"}, + {"name": "Custom", "agent_command": "my-bot", "mcp_command": "my-mcp"}, + {"name": "Codex", "agent_command": "codex-acp", "mcp_command": "sprout-mcp-server"} + ]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], "", "goose should be cleared"); + assert_eq!(records[1]["mcp_command"], "", "claude should be cleared"); + assert_eq!( + records[2]["mcp_command"], "sprout-dev-mcp", + "sprout-agent preserved" + ); + assert_eq!( + records[3]["mcp_command"], "my-mcp", + "custom agent untouched" + ); + assert_eq!(records[4]["mcp_command"], "", "codex should be cleared"); + } + + #[test] + fn reconcile_adds_mcp_command_when_key_absent() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Solo", + "agent_command": "sprout-agent" + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], "sprout-dev-mcp"); + } + + #[test] + fn reconcile_treats_null_mcp_command_as_empty() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Solo", + "agent_command": "sprout-agent", + "mcp_command": null + }]), + ); + reconcile_mcp_commands_in_file(&dir.path().join("agents/managed-agents.json")); + let records = read_agents_json(dir.path()); + assert_eq!(records[0]["mcp_command"], "sprout-dev-mcp"); + } +} diff --git a/desktop/src/features/agents/lib/catalog.ts b/desktop/src/features/agents/lib/catalog.ts index 0fd64a820..acc23df96 100644 --- a/desktop/src/features/agents/lib/catalog.ts +++ b/desktop/src/features/agents/lib/catalog.ts @@ -13,7 +13,7 @@ export type PersonaLibraryState = { }; export function isPersonaActive(persona: AgentPersona) { - return !persona.isBuiltIn || persona.isActive; + return persona.isActive; } export function getActivePersonas(personas: readonly AgentPersona[]) { diff --git a/desktop/src/features/agents/ui/AgentsView.tsx b/desktop/src/features/agents/ui/AgentsView.tsx index cc9fd2eae..52508e30d 100644 --- a/desktop/src/features/agents/ui/AgentsView.tsx +++ b/desktop/src/features/agents/ui/AgentsView.tsx @@ -100,7 +100,7 @@ export function AgentsView() { selectedLogAgentPubkey={agents.logAgentPubkey} // Persona props canChooseCatalog={personas.catalogPersonas.length > 0} - personas={personas.personasQuery.data ?? []} + personas={personas.libraryPersonas} personasError={ personas.personasQuery.error instanceof Error ? personas.personasQuery.error