diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 09e46abf5..32f341dad 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -30,7 +30,7 @@ const rules = [ // Do not add to this list; split the file instead. Remove each entry as its // file is broken up. Tracked as a follow-up. const overrides = new Map([ - ["src-tauri/src/commands/agents.rs", 1208], + ["src-tauri/src/commands/agents.rs", 1287], ["src-tauri/src/managed_agents/nest.rs", 1420], ["src-tauri/src/managed_agents/runtime.rs", 1465], ["src-tauri/src/managed_agents/persona_card.rs", 1050], diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 2c6933ede..57039b407 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -695,17 +695,25 @@ pub async fn create_managed_agent( } /// Data needed for background profile reconciliation after agent start. -struct ProfileReconcileData { - private_key_nsec: String, - name: String, - relay_url: String, - /// Expected avatar URL for the published profile. Resolved at start from the - /// record's persisted `avatar_url` (the exact URL published at creation), - /// falling back to persona/command derivation only for pre-existing records - /// that have no stored value — so old records still self-heal without - /// regressing a user-overridden avatar. - avatar_url: Option, - auth_tag: Option, +pub(crate) struct ProfileReconcileData { + pub(crate) private_key_nsec: String, + pub(crate) name: String, + pub(crate) relay_url: String, + /// Expected avatar URL for the published profile. `None` for legacy records + /// that predate the `avatar_url` field — these will be backfilled from the + /// relay's existing kind:0 profile on first reconciliation. + pub(crate) avatar_url: Option, + pub(crate) auth_tag: Option, + /// The agent's pubkey (hex). Needed to update the persisted record during + /// avatar backfill migration. + pub(crate) pubkey: String, + /// The agent's command (e.g. "goose"). Used as fallback when no profile + /// exists on the relay during avatar backfill. + pub(crate) agent_command: String, + /// Persona ID if this agent was created from a persona. Used during avatar + /// backfill to recover the correct avatar from the persona record when the + /// relay profile has been corrupted. + pub(crate) persona_id: Option, } #[tauri::command] @@ -745,14 +753,15 @@ pub async fn start_managed_agent( let record = find_managed_agent_mut(&mut records, &pubkey)?; - let expected_avatar = reconcile_avatar(record.avatar_url.as_deref(), &record.agent_command); - let reconcile = ProfileReconcileData { private_key_nsec: record.private_key_nsec.clone(), name: record.name.clone(), relay_url: record.relay_url.clone(), - avatar_url: expected_avatar, + avatar_url: record.avatar_url.clone(), auth_tag: record.auth_tag.clone(), + pubkey: record.pubkey.clone(), + agent_command: record.agent_command.clone(), + persona_id: record.persona_id.clone(), }; let target = if record.backend == BackendKind::Local { @@ -812,7 +821,8 @@ pub async fn start_managed_agent( // ── Profile reconciliation (fire-and-forget) ──────────────────────────── // On successful start, spawn a background task to ensure the agent's kind:0 // profile is published on the relay. This self-heals cases where the initial - // profile sync at creation time failed silently. + // profile sync at creation time failed silently. For legacy records (pre-PR-921) + // with no persisted avatar, this also backfills the avatar from the relay. if result.is_ok() { let reconcile_pubkey = pubkey.clone(); let reconcile_app = app.clone(); @@ -820,7 +830,8 @@ pub async fn start_managed_agent( use tauri::Manager; let state = reconcile_app.state::(); if let Err(e) = - reconcile_agent_profile(&state, &reconcile_pubkey, &reconcile_data).await + reconcile_agent_profile(&state, &reconcile_app, &reconcile_pubkey, &reconcile_data) + .await { eprintln!( "sprout-desktop: profile reconciliation failed for agent {reconcile_pubkey}: {e}" @@ -832,29 +843,95 @@ pub async fn start_managed_agent( result } +/// Resolve the avatar to backfill for a legacy agent record (pre-PR-921, no +/// stored `avatar_url`). +/// +/// Priority: the persona's avatar wins, because the old reconciliation code +/// could have overwritten the relay's kind:0 `picture` with the command default +/// — making the relay an unreliable source for persona-backed agents. Only fall +/// back to the relay's `picture`, then the command icon, for agents with no +/// persona avatar to recover from. +fn resolve_legacy_avatar( + persona_avatar: Option, + relay_picture: Option, + agent_command: &str, +) -> String { + persona_avatar + .or(relay_picture) + .or_else(|| managed_agent_avatar_url(agent_command)) + .unwrap_or_default() +} + /// Reconcile an agent's kind:0 profile on the relay. /// /// Queries the relay for the agent's existing profile and re-publishes if missing /// or stale (display_name or picture mismatch). This is fire-and-forget — errors /// are returned to the caller for logging but never block agent startup. /// +/// For legacy records (pre-PR-921) where `avatar_url` is `None`, this function +/// backfills via `resolve_legacy_avatar` — preferring the persona record's avatar +/// over the relay's `picture`, since the old code may have corrupted the relay +/// profile — and persists the updated record. After backfill, normal +/// reconciliation proceeds. +/// /// Query and publish both target the agent's stored `relay_url` so that, under /// an active workspace relay override, reconciliation reads and writes the same /// relay the agent's profile actually lives on. -async fn reconcile_agent_profile( +pub(crate) async fn reconcile_agent_profile( state: &AppState, + app: &AppHandle, agent_pubkey: &str, data: &ProfileReconcileData, ) -> Result<(), String> { use crate::relay::{query_agent_profile, sync_managed_agent_profile}; - // Compare against the avatar persisted at creation time — never re-derive it. - let expected_avatar = data.avatar_url.as_deref(); - - // Query the same relay the profile is published to (the stored relay_url). + // Query the relay for the agent's existing kind:0 profile. let existing = query_agent_profile(state, &data.relay_url, agent_pubkey).await?; - if !profile_needs_sync(existing.as_ref(), &data.name, expected_avatar) { + // Resolve the expected avatar — backfilling for legacy records that have no + // stored avatar_url yet. + let expected_avatar = match data.avatar_url.as_deref() { + Some(url) => url.to_string(), + None => { + // Legacy record: the relay profile may have been corrupted by the + // old reconciliation code (it overwrote the persona avatar with the + // command default), so the persona record is the authoritative source. + let persona_avatar = data.persona_id.as_ref().and_then(|pid| { + load_personas(app) + .ok()? + .into_iter() + .find(|p| p.id == *pid)? + .avatar_url + }); + + let backfilled = resolve_legacy_avatar( + persona_avatar, + existing.as_ref().and_then(|info| info.picture.clone()), + &data.agent_command, + ); + + // Persist the backfilled avatar so this migration only runs once. + if !backfilled.is_empty() { + let _store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|e| e.to_string())?; + let mut records = load_managed_agents(app)?; + if let Some(record) = records.iter_mut().find(|r| r.pubkey == data.pubkey) { + record.avatar_url = Some(backfilled.clone()); + save_managed_agents(app, &records)?; + } + } + + backfilled + } + }; + + if expected_avatar.is_empty() { + return Ok(()); + } + + if !profile_needs_sync(existing.as_ref(), &data.name, Some(&expected_avatar)) { return Ok(()); } @@ -866,7 +943,7 @@ async fn reconcile_agent_profile( &data.relay_url, &agent_keys, &data.name, - expected_avatar, + Some(&expected_avatar), data.auth_tag.as_deref(), ) .await @@ -890,18 +967,6 @@ fn profile_needs_sync( } } -/// Resolve the avatar a managed agent's profile should reconcile against. -/// Stored value (persisted at creation) wins; legacy records that predate the -/// field (`stored == None`) fall back to the command-based derivation — the -/// same source the create path used. Persona config is never consulted: doing -/// so diverges from what was published and overwrites user intent on restart. -fn reconcile_avatar(stored: Option<&str>, agent_command: &str) -> Option { - match stored { - Some(url) => Some(url.to_string()), - None => managed_agent_avatar_url(agent_command), - } -} - #[tauri::command] pub fn stop_managed_agent( pubkey: String, @@ -1182,26 +1247,40 @@ mod tests { assert!(profile_needs_sync(Some(&existing), "Duncan", None)); } - /// Legacy records (`avatar_url: None`) must reconcile against - /// `managed_agent_avatar_url(agent_command)` — never persona config — - /// matching what the original create path published. #[test] - fn reconcile_avatar_legacy_record_uses_command_not_persona() { - let resolved = reconcile_avatar(None, "goose"); - - assert_eq!(resolved, managed_agent_avatar_url("goose")); - assert!( - resolved.is_some(), - "goose command should have a known avatar" + fn legacy_avatar_prefers_persona_over_corrupted_relay_picture() { + // The regression: the relay picture was overwritten with the command + // default. The persona avatar must win so the correct avatar is restored. + let resolved = resolve_legacy_avatar( + Some("https://x/persona.png".to_string()), + Some("https://x/default-icon.png".to_string()), + "goose", ); + + assert_eq!(resolved, "https://x/persona.png"); + } + + #[test] + fn legacy_avatar_falls_back_to_relay_picture_without_persona() { + let resolved = + resolve_legacy_avatar(None, Some("https://x/relay.png".to_string()), "goose"); + + assert_eq!(resolved, "https://x/relay.png"); + } + + #[test] + fn legacy_avatar_falls_back_to_command_icon_when_no_persona_or_relay() { + use crate::managed_agents::managed_agent_avatar_url; + + let resolved = resolve_legacy_avatar(None, None, "goose"); + + assert_eq!(resolved, managed_agent_avatar_url("goose").unwrap()); } - /// New records persist their avatar at creation; the stored value is used - /// verbatim, never falling back to command derivation. #[test] - fn reconcile_avatar_stored_value_wins() { - let resolved = reconcile_avatar(Some("https://custom/avatar.png"), "goose"); + fn legacy_avatar_empty_when_nothing_resolves() { + let resolved = resolve_legacy_avatar(None, None, "totally-unknown-command"); - assert_eq!(resolved.as_deref(), Some("https://custom/avatar.png")); + assert!(resolved.is_empty()); } } diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index d3bd52300..15a9109b0 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -164,6 +164,8 @@ pub async fn restore_managed_agents_on_launch( .lock() .map_err(|error| error.to_string())?; + let mut successfully_spawned: Vec = Vec::new(); + for (pubkey, result) in spawn_results { let record = match find_managed_agent_mut(&mut records, &pubkey) { Ok(r) => r, @@ -178,7 +180,8 @@ pub async fn restore_managed_agents_on_launch( record.last_stopped_at = None; record.last_exit_code = None; record.last_error = None; - runtimes.insert(pubkey, ManagedAgentProcess { child, log_path }); + runtimes.insert(pubkey.clone(), ManagedAgentProcess { child, log_path }); + successfully_spawned.push(pubkey); } Err(error) => { record.updated_at = util::now_iso(); @@ -187,8 +190,49 @@ pub async fn restore_managed_agents_on_launch( } } + // Collect profile reconciliation data for successfully spawned agents before + // releasing the lock. This mirrors the fire-and-forget pattern in + // start_managed_agent — ensuring boot-restored agents get the same profile + // self-healing as UI-started agents. + let reconcile_items: Vec<(String, crate::commands::ProfileReconcileData)> = + successfully_spawned + .iter() + .filter_map(|pubkey| { + let record = records.iter().find(|r| r.pubkey == *pubkey)?; + Some(( + pubkey.clone(), + crate::commands::ProfileReconcileData { + private_key_nsec: record.private_key_nsec.clone(), + name: record.name.clone(), + relay_url: record.relay_url.clone(), + avatar_url: record.avatar_url.clone(), + auth_tag: record.auth_tag.clone(), + pubkey: record.pubkey.clone(), + agent_command: record.agent_command.clone(), + persona_id: record.persona_id.clone(), + }, + )) + }) + .collect(); + save_managed_agents(app, &records)?; + // ── Profile reconciliation (fire-and-forget) ──────────────────────────── + // Spawn background tasks to ensure each restored agent's kind:0 profile is + // published on the relay. Same pattern as the UI start path. + for (pubkey, data) in reconcile_items { + let reconcile_app = app.clone(); + tauri::async_runtime::spawn(async move { + let state = reconcile_app.state::(); + if let Err(e) = + crate::commands::reconcile_agent_profile(&state, &reconcile_app, &pubkey, &data) + .await + { + eprintln!("sprout-desktop: profile reconciliation failed for agent {pubkey}: {e}"); + } + }); + } + Ok(()) }