From 67eb982b1f006a54a8245e9882886e47ee93a323 Mon Sep 17 00:00:00 2001 From: klopez4212 Date: Tue, 23 Jun 2026 16:48:14 +0100 Subject: [PATCH 1/5] Add agent avatar plumbing --- .../src-tauri/src/commands/agent_models.rs | 72 ++- desktop/src-tauri/src/commands/agents.rs | 255 +++++--- .../src-tauri/src/commands/agents_tests.rs | 112 +++- desktop/src-tauri/src/commands/personas.rs | 46 +- desktop/src-tauri/src/managed_agents/nest.rs | 1 + .../src/managed_agents/persona_card.rs | 551 +++--------------- .../src/managed_agents/persona_card_tests.rs | 530 +++++++++++++++++ .../src/managed_agents/relay_mesh.rs | 1 + .../src-tauri/src/managed_agents/restore.rs | 1 + .../src-tauri/src/managed_agents/runtime.rs | 17 +- .../src/managed_agents/runtime/tests.rs | 1 + .../src/managed_agents/team_repair.rs | 1 + desktop/src-tauri/src/managed_agents/types.rs | 13 + .../features/agents/ui/personaDialogState.ts | 3 +- desktop/src/features/profile/hooks.ts | 5 +- desktop/src/shared/api/tauri.ts | 8 +- desktop/src/shared/api/tauriPersonas.ts | 5 + desktop/src/shared/api/types.ts | 2 + .../avatars/gooseAppAvatarRefs.test.mjs | 77 +++ .../src/shared/avatars/gooseAppAvatarRefs.ts | 91 +++ desktop/src/shared/lib/runtimeAvatar.ts | 13 + 21 files changed, 1190 insertions(+), 615 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/persona_card_tests.rs create mode 100644 desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs create mode 100644 desktop/src/shared/avatars/gooseAppAvatarRefs.ts create mode 100644 desktop/src/shared/lib/runtimeAvatar.ts diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 8a4e8401b..261f501a6 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -10,13 +10,40 @@ use crate::{ known_acp_runtime, load_managed_agents, load_personas, managed_agent_avatar_url, missing_command_message, normalize_agent_args, resolve_command, resolve_effective_prompt_model_provider, save_managed_agents, sync_managed_agent_processes, - try_regenerate_nest, AgentModelInfo, AgentModelsResponse, UpdateManagedAgentRequest, - UpdateManagedAgentResponse, + try_regenerate_nest, AgentModelInfo, AgentModelsResponse, ManagedAgentRecord, + UpdateManagedAgentRequest, UpdateManagedAgentResponse, }, relay::{relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, }; +fn trim_optional(value: Option) -> Option { + value + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) +} + +fn is_persona_runtime_avatar(record: &ManagedAgentRecord, avatar_url: &str) -> bool { + record.persona_id.is_some() + && managed_agent_avatar_url(&record.agent_command) + .as_deref() + .is_some_and(|runtime_avatar_url| runtime_avatar_url == avatar_url.trim()) +} + +fn profile_sync_avatar_url(record: &ManagedAgentRecord) -> Option { + record + .avatar_url + .clone() + .filter(|avatar_url| !is_persona_runtime_avatar(record, avatar_url)) + .or_else(|| { + if record.persona_id.is_none() { + managed_agent_avatar_url(&record.agent_command) + } else { + None + } + }) +} + /// Query available models from an agent via `buzz-acp models --json`. /// /// Spawns a short-lived subprocess (no relay connection needed). The subprocess @@ -139,7 +166,8 @@ pub async fn get_agent_models( /// /// Does NOT auto-restart the agent. Runtime config changes (system prompt, /// parallelism, commands, toolsets) take effect on the next agent spawn. -/// Name changes are synced to the relay immediately via a kind:0 re-publish. +/// Name and avatar changes are synced to the relay immediately via a kind:0 +/// re-publish. #[tauri::command] pub async fn update_managed_agent( input: UpdateManagedAgentRequest, @@ -162,6 +190,7 @@ pub async fn update_managed_agent( let record = find_managed_agent_mut(&mut records, &input.pubkey)?; let mut name_changed = false; + let mut avatar_changed = false; if let Some(name_update) = input.name { let trimmed = name_update.trim().to_string(); if !trimmed.is_empty() && trimmed != record.name { @@ -169,6 +198,15 @@ pub async fn update_managed_agent( name_changed = true; } } + if let Some(avatar_update) = input.avatar_url { + let normalized = trim_optional(avatar_update); + let avatar_url_cleared = normalized.is_none(); + if normalized != record.avatar_url || avatar_url_cleared != record.avatar_url_cleared { + record.avatar_url = normalized; + record.avatar_url_cleared = avatar_url_cleared; + avatar_changed = true; + } + } if let Some(model_update) = input.model { record.model = model_update; } @@ -188,11 +226,13 @@ pub async fn update_managed_agent( if let Some(turn_timeout_seconds) = input.turn_timeout_seconds { record.turn_timeout_seconds = turn_timeout_seconds; } - // Store the relay override exactly as supplied (trimmed). An explicit - // value pins the agent; empty falls back to the workspace relay at - // read-time. A name-only edit (relay_url == None) leaves the pin intact. if let Some(relay_url) = input.relay_url { - record.relay_url = relay_url.trim().to_string(); + let trimmed = relay_url.trim(); + record.relay_url = if trimmed.is_empty() { + relay_ws_url_with_override(&state) + } else { + trimmed.to_string() + }; } if let Some(acp_command) = input.acp_command { record.acp_command = acp_command; @@ -243,20 +283,16 @@ pub async fn update_managed_agent( .find(|r| r.pubkey == input.pubkey) .ok_or_else(|| format!("agent {} not found", input.pubkey))?; - let sync_params = if name_changed { + let sync_params = if name_changed || avatar_changed { let agent_keys = Keys::parse(&record.private_key_nsec) .map_err(|e| format!("failed to parse agent keys: {e}"))?; - // Re-publish the renamed profile to the agent's effective relay: - // an explicit per-agent relay wins; empty falls back to workspace. - let relay_url = crate::relay::effective_agent_relay_url( - &record.relay_url, - &relay_ws_url_with_override(&state), - ); + let relay_url = record.relay_url.clone(); let display_name = record.name.clone(); - let avatar_url = record - .avatar_url - .clone() - .or_else(|| managed_agent_avatar_url(&record.agent_command)); + let avatar_url = if avatar_changed || record.avatar_url_cleared { + record.avatar_url.clone() + } else { + profile_sync_avatar_url(record) + }; let auth_tag = record.auth_tag.clone(); Some((agent_keys, relay_url, display_name, avatar_url, auth_tag)) } else { diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 5112c3ead..8d65b3ea8 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -60,6 +60,7 @@ fn resolve_created_avatar_url( requested_avatar_url: Option<&str>, persona_avatar_url: Option, agent_command: &str, + use_command_fallback: bool, ) -> Option { requested_avatar_url .and_then(trim_to_optional_string) @@ -68,7 +69,35 @@ fn resolve_created_avatar_url( .as_deref() .and_then(trim_to_optional_string) }) - .or_else(|| managed_agent_avatar_url(agent_command)) + .or_else(|| { + if use_command_fallback { + managed_agent_avatar_url(agent_command) + } else { + None + } + }) +} + +fn is_retired_fizz_data_url(persona_id: Option<&str>, avatar_url: &str) -> bool { + persona_id == Some("builtin:fizz") && avatar_url.trim_start().starts_with("data:image/") +} + +fn is_command_avatar_for_persona( + persona_id: Option<&str>, + agent_command: &str, + avatar_url: &str, +) -> bool { + persona_id.is_some() + && managed_agent_avatar_url(agent_command) + .as_deref() + .is_some_and(|command_avatar_url| command_avatar_url == avatar_url.trim()) +} + +fn filter_retired_fizz_avatar( + persona_id: Option<&str>, + avatar_url: Option, +) -> Option { + avatar_url.filter(|url| !is_retired_fizz_data_url(persona_id, url)) } #[cfg(feature = "mesh-llm")] @@ -145,7 +174,6 @@ async fn start_local_agent_with_preflight( /// empty map would surface as an opaque 401 from the provider. fn build_deploy_payload( app: &AppHandle, - state: &AppState, record: &ManagedAgentRecord, ) -> Result { // Merge persona env_vars + agent env_vars for provider deploy. Same @@ -175,15 +203,7 @@ fn build_deploy_payload( Ok(serde_json::json!({ "name": &record.name, - // Resolve the per-agent pin against the active workspace relay here: - // this payload crosses the host boundary to a remote provider harness - // that has no notion of the desktop's workspace, so the blank→workspace - // fallback (otherwise applied at read-time in `effective_agent_relay_url`) - // must be materialized into a concrete URL before serializing. - "relay_url": crate::relay::effective_agent_relay_url( - &record.relay_url, - &relay_ws_url_with_override(state), - ), + "relay_url": &record.relay_url, "private_key_nsec": &record.private_key_nsec, "auth_tag": &record.auth_tag, "agent_command": &record.agent_command, @@ -393,15 +413,13 @@ pub async fn create_managed_agent( .to_bech32() .map_err(|error| format!("failed to encode private key: {error}"))?; - // Store the relay override exactly as supplied (trimmed). An explicit - // value pins the agent; empty stays empty and resolves to the active - // workspace relay at read-time. Uniform for Local and Provider. let resolved_relay_url = input .relay_url .as_deref() .map(str::trim) - .unwrap_or("") - .to_string(); + .filter(|value| !value.is_empty()) + .map(str::to_string) + .unwrap_or_else(|| relay_ws_url_with_override(&state)); (keys, private_key_nsec, pubkey, resolved_relay_url, input) }; @@ -522,6 +540,7 @@ pub async fn create_managed_agent( input.avatar_url.as_deref(), persona_avatar_url, &agent_command, + requested_persona_id.is_none(), ); let record = crate::managed_agents::ManagedAgentRecord { @@ -532,6 +551,7 @@ pub async fn create_managed_agent( auth_tag: auth_tag.clone(), relay_url: resolved_relay_url.clone(), avatar_url: resolved_avatar_url.clone(), + avatar_url_cleared: false, acp_command: input .acp_command .as_deref() @@ -676,7 +696,7 @@ pub async fn create_managed_agent( .iter() .find(|r| r.pubkey == pubkey) .ok_or_else(|| "agent disappeared".to_string())?; - build_deploy_payload(&app, &state, rec) + build_deploy_payload(&app, rec) }; // The agent was already persisted in Phase 3 — converting a // persona-resolution failure into `spawn_error` (rather than @@ -743,10 +763,11 @@ 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. + /// Expected avatar URL for the published profile. `None` can mean either a + /// legacy missing value or an explicit clear; `avatar_url_cleared` + /// disambiguates those cases. pub(crate) avatar_url: Option, + pub(crate) avatar_url_cleared: bool, pub(crate) auth_tag: Option, /// The agent's pubkey (hex). Needed to update the persisted record during /// avatar backfill migration. @@ -802,6 +823,7 @@ pub async fn start_managed_agent( name: record.name.clone(), relay_url: record.relay_url.clone(), avatar_url: record.avatar_url.clone(), + avatar_url_cleared: record.avatar_url_cleared, auth_tag: record.auth_tag.clone(), pubkey: record.pubkey.clone(), agent_command: record.agent_command.clone(), @@ -814,7 +836,7 @@ pub async fn start_managed_agent( StartTarget::Provider { backend: record.backend.clone(), cached_binary_path: record.provider_binary_path.clone(), - agent_json: build_deploy_payload(&app, &state, record)?, + agent_json: build_deploy_payload(&app, record)?, } }; @@ -900,30 +922,44 @@ fn resolve_legacy_avatar( persona_avatar: Option, relay_picture: Option, agent_command: &str, + use_command_fallback: bool, ) -> String { persona_avatar .or(relay_picture) - .or_else(|| managed_agent_avatar_url(agent_command)) + .or_else(|| { + if use_command_fallback { + managed_agent_avatar_url(agent_command) + } else { + None + } + }) .unwrap_or_default() } +fn should_skip_legacy_command_avatar( + stored_avatar_was_retired_fizz: bool, + relay_picture_was_retired_fizz: bool, + persona_avatar: Option<&str>, + relay_picture: Option<&str>, +) -> bool { + (stored_avatar_was_retired_fizz || relay_picture_was_retired_fizz) + && persona_avatar.is_none() + && relay_picture.is_none() +} + /// 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. +/// or stale. 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 target the relay returned by `effective_agent_relay_url` -/// for every agent regardless of backend: an explicit per-agent `relay_url` -/// wins, and a blank one falls back to the active workspace relay. This keeps -/// reconciliation following the session's relay for never-pinned agents while -/// honoring a deliberate pin wherever it points. +/// 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. pub(crate) async fn reconcile_agent_profile( state: &AppState, app: &AppHandle, @@ -932,60 +968,119 @@ pub(crate) async fn reconcile_agent_profile( ) -> Result<(), String> { use crate::relay::{query_agent_profile, sync_managed_agent_profile}; - // An explicit per-agent relay wins; an empty one falls back to the active - // workspace relay. Resolved once and used for both the read and write-back. - let relay_url = crate::relay::effective_agent_relay_url( - &data.relay_url, - &relay_ws_url_with_override(state), - ); - // Query the relay for the agent's existing kind:0 profile. - let existing = query_agent_profile(state, &relay_url, agent_pubkey).await?; - - // 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, - ); + let existing = query_agent_profile(state, &data.relay_url, agent_pubkey).await?; + + // Resolve the expected avatar. A user-initiated clear is intentionally + // `None` and must not be backfilled from persona/relay/runtime defaults. + // For legacy records that have no stored avatar_url yet, `None` still means + // backfill from the best available historical source. + let stored_avatar = + filter_retired_fizz_avatar(data.persona_id.as_deref(), data.avatar_url.clone()); + let stored_avatar_was_retired_fizz = data + .avatar_url + .as_deref() + .is_some_and(|url| is_retired_fizz_data_url(data.persona_id.as_deref(), url)); + let stored_avatar_was_command_fallback = stored_avatar.as_deref().is_some_and(|url| { + is_command_avatar_for_persona(data.persona_id.as_deref(), &data.agent_command, url) + }); + let stored_avatar = stored_avatar.filter(|url| { + !is_command_avatar_for_persona(data.persona_id.as_deref(), &data.agent_command, url) + }); + let expected_avatar = if data.avatar_url_cleared && stored_avatar.is_none() { + None + } else { + match stored_avatar { + Some(url) => Some(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 = filter_retired_fizz_avatar( + data.persona_id.as_deref(), + data.persona_id.as_ref().and_then(|pid| { + load_personas(app) + .ok()? + .into_iter() + .find(|p| p.id == *pid)? + .avatar_url + }), + ); + let relay_picture_raw = existing.as_ref().and_then(|info| info.picture.clone()); + let relay_picture_was_retired_fizz = relay_picture_raw + .as_deref() + .is_some_and(|url| is_retired_fizz_data_url(data.persona_id.as_deref(), url)); + let relay_picture = + filter_retired_fizz_avatar(data.persona_id.as_deref(), relay_picture_raw); + let relay_picture_was_command_fallback = + relay_picture.as_deref().is_some_and(|url| { + is_command_avatar_for_persona( + data.persona_id.as_deref(), + &data.agent_command, + url, + ) + }); + let relay_picture = relay_picture.filter(|url| { + !is_command_avatar_for_persona( + data.persona_id.as_deref(), + &data.agent_command, + url, + ) + }); + + let skip_command_fallback = should_skip_legacy_command_avatar( + stored_avatar_was_retired_fizz, + relay_picture_was_retired_fizz, + persona_avatar.as_deref(), + relay_picture.as_deref(), + ); + let backfilled = if skip_command_fallback { + String::new() + } else { + resolve_legacy_avatar( + persona_avatar, + relay_picture, + &data.agent_command, + data.persona_id.is_none(), + ) + }; + + // Persist the backfilled avatar so this migration only runs once, + // or clear the retired built-in Fizz data URL if there is no + // current profile image to backfill. + let should_persist_avatar = stored_avatar_was_retired_fizz + || relay_picture_was_retired_fizz + || stored_avatar_was_command_fallback + || relay_picture_was_command_fallback + || (!backfilled.is_empty() + && data.avatar_url.as_deref() != Some(backfilled.as_str())); + if should_persist_avatar { + 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 = if backfilled.is_empty() { + None + } else { + Some(backfilled.clone()) + }; + record.avatar_url_cleared = backfilled.is_empty(); + save_managed_agents(app, &records)?; + } + } - // 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)?; + if backfilled.is_empty() { + None + } else { + Some(backfilled) } } - - backfilled } }; - if expected_avatar.is_empty() { - return Ok(()); - } - - if !profile_needs_sync(existing.as_ref(), &data.name, Some(&expected_avatar)) { + if !profile_needs_sync(existing.as_ref(), &data.name, expected_avatar.as_deref()) { return Ok(()); } @@ -994,10 +1089,10 @@ pub(crate) async fn reconcile_agent_profile( sync_managed_agent_profile( state, - &relay_url, + &data.relay_url, &agent_keys, &data.name, - Some(&expected_avatar), + expected_avatar.as_deref(), data.auth_tag.as_deref(), ) .await diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index f141c6ae5..a13be10ff 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -48,6 +48,7 @@ fn created_avatar_prefers_explicit_input() { Some(" https://x/input.png "), Some("https://x/persona.png".to_string()), "goose", + true, ); assert_eq!(resolved.as_deref(), Some("https://x/input.png")); @@ -55,8 +56,12 @@ fn created_avatar_prefers_explicit_input() { #[test] fn created_avatar_uses_persona_before_command_fallback() { - let resolved = - resolve_created_avatar_url(None, Some(" https://x/persona.png ".to_string()), "goose"); + let resolved = resolve_created_avatar_url( + None, + Some(" https://x/persona.png ".to_string()), + "goose", + true, + ); assert_eq!(resolved.as_deref(), Some("https://x/persona.png")); } @@ -65,11 +70,45 @@ fn created_avatar_uses_persona_before_command_fallback() { fn created_avatar_uses_command_fallback_without_input_or_persona() { use crate::managed_agents::managed_agent_avatar_url; - let resolved = resolve_created_avatar_url(None, None, "goose"); + let resolved = resolve_created_avatar_url(None, None, "goose", true); assert_eq!(resolved, managed_agent_avatar_url("goose")); } +#[test] +fn created_persona_avatar_does_not_use_command_fallback() { + let resolved = resolve_created_avatar_url(None, None, "goose", false); + + assert_eq!(resolved, None); +} + +#[test] +fn retired_fizz_data_url_is_treated_as_absent() { + assert_eq!( + filter_retired_fizz_avatar( + Some("builtin:fizz"), + Some("data:image/png;base64,old-demo".to_string()), + ), + None, + ); + assert_eq!( + filter_retired_fizz_avatar( + Some("custom:fizz"), + Some("data:image/png;base64,user-avatar".to_string()), + ) + .as_deref(), + Some("data:image/png;base64,user-avatar"), + ); + assert_eq!( + filter_retired_fizz_avatar( + Some("builtin:fizz"), + Some("https://relay.example/avatar.png".to_string()), + ) + .as_deref(), + Some("https://relay.example/avatar.png"), + ); +} + fn profile(name: Option<&str>, picture: Option<&str>) -> crate::relay::AgentProfileInfo { crate::relay::AgentProfileInfo { display_name: name.map(str::to_string), @@ -142,6 +181,7 @@ fn legacy_avatar_prefers_persona_over_corrupted_relay_picture() { Some("https://x/persona.png".to_string()), Some("https://x/default-icon.png".to_string()), "goose", + false, ); assert_eq!(resolved, "https://x/persona.png"); @@ -149,7 +189,12 @@ fn legacy_avatar_prefers_persona_over_corrupted_relay_picture() { #[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"); + let resolved = resolve_legacy_avatar( + None, + Some("https://x/relay.png".to_string()), + "goose", + false, + ); assert_eq!(resolved, "https://x/relay.png"); } @@ -158,14 +203,69 @@ fn legacy_avatar_falls_back_to_relay_picture_without_persona() { 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"); + let resolved = resolve_legacy_avatar(None, None, "goose", true); assert_eq!(resolved, managed_agent_avatar_url("goose").unwrap()); } #[test] fn legacy_avatar_empty_when_nothing_resolves() { - let resolved = resolve_legacy_avatar(None, None, "totally-unknown-command"); + let resolved = resolve_legacy_avatar(None, None, "totally-unknown-command", true); assert!(resolved.is_empty()); } + +#[test] +fn legacy_persona_avatar_does_not_use_command_fallback() { + let resolved = resolve_legacy_avatar(None, None, "goose", false); + + assert!(resolved.is_empty()); +} + +#[test] +fn detects_command_avatar_for_persona_agents() { + let command_avatar = crate::managed_agents::managed_agent_avatar_url("goose") + .expect("goose avatar should resolve"); + + assert!(is_command_avatar_for_persona( + Some("builtin:fizz"), + "goose", + &command_avatar, + )); + assert!(!is_command_avatar_for_persona( + None, + "goose", + &command_avatar, + )); + assert!(!is_command_avatar_for_persona( + Some("builtin:fizz"), + "goose", + "https://x/fizz.png", + )); +} + +#[test] +fn legacy_avatar_skips_command_icon_for_retired_stored_fizz_avatar() { + assert!(should_skip_legacy_command_avatar(true, false, None, None)); +} + +#[test] +fn legacy_avatar_skips_command_icon_for_retired_relay_fizz_avatar() { + assert!(should_skip_legacy_command_avatar(false, true, None, None)); +} + +#[test] +fn legacy_avatar_keeps_command_icon_when_retired_fizz_has_current_avatar_source() { + assert!(!should_skip_legacy_command_avatar( + false, + true, + Some("https://x/persona.png"), + None, + )); + assert!(!should_skip_legacy_command_avatar( + false, + true, + None, + Some("https://x/relay.png"), + )); +} diff --git a/desktop/src-tauri/src/commands/personas.rs b/desktop/src-tauri/src/commands/personas.rs index 56f6742f4..db8df524e 100644 --- a/desktop/src-tauri/src/commands/personas.rs +++ b/desktop/src-tauri/src/commands/personas.rs @@ -298,9 +298,11 @@ pub fn parse_persona_files( }); } - // .persona.md: YAML frontmatter starts with "---" + // Persona markdown: YAML frontmatter starts with "---". + // Goose Internal exports plain .md agent files, while Buzz historically + // used .persona.md; parse both through the same validated importer. let lower_name = file_name.to_ascii_lowercase(); - if lower_name.ends_with(".persona.md") { + if lower_name.ends_with(".md") { if file_bytes.len() > MAX_JSON_BYTES { return Err("Markdown file is too large (max 5 MB).".to_string()); } @@ -312,17 +314,7 @@ pub fn parse_persona_files( }); } - // If it's a .md file but not .persona.md, give a specific hint. - if lower_name.ends_with(".md") { - return Err( - "Only .persona.md files are supported. Rename to .persona.md".to_string(), - ); - } - - Err( - "Unsupported file format. Expected .persona.md, .persona.png, .persona.json, or .zip" - .to_string(), - ) + Err("Unsupported file format. Expected .md, .persona.png, .persona.json, or .zip".to_string()) } #[tauri::command] @@ -373,3 +365,31 @@ pub async fn export_persona_to_json( let filename = format!("{slug}.persona.json"); save_json_with_dialog(&app, &filename, &json_bytes).await } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_persona_files_accepts_plain_md_with_avatar_ref() { + let md = br#"--- +name: fizz +display_name: Fizz +avatar: app-avatar:pollies-12 +runtime: goose +--- +You are Fizz. +"#; + + let result = parse_persona_files(md.to_vec(), "fizz.md".to_string()).unwrap(); + + assert_eq!(result.personas.len(), 1); + assert!(result.skipped.is_empty()); + assert_eq!(result.personas[0].display_name, "Fizz"); + assert_eq!( + result.personas[0].avatar_ref.as_deref(), + Some("app-avatar:pollies-12") + ); + assert_eq!(result.personas[0].source_file, "fizz.md"); + } +} diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index 8b1f3c62d..b21d965c8 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -964,6 +964,7 @@ mod tests { auth_tag: None, relay_url: String::new(), avatar_url: None, + avatar_url_cleared: false, acp_command: String::new(), agent_command: String::new(), agent_args: vec![], diff --git a/desktop/src-tauri/src/managed_agents/persona_card.rs b/desktop/src-tauri/src/managed_agents/persona_card.rs index 68d9b30a4..abb5803b6 100644 --- a/desktop/src-tauri/src/managed_agents/persona_card.rs +++ b/desktop/src-tauri/src/managed_agents/persona_card.rs @@ -1,6 +1,6 @@ use base64::{engine::general_purpose::STANDARD, Engine as _}; use png::Decoder; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use serde_json::Value; use std::io::{Cursor, Read}; @@ -13,6 +13,7 @@ pub struct ParsedPersonaPreview { pub display_name: String, pub system_prompt: String, pub avatar_data_url: Option, + pub avatar_ref: Option, pub runtime: Option, pub model: Option, pub provider: Option, @@ -81,6 +82,7 @@ pub fn parse_png_persona(png_bytes: &[u8]) -> Result Result Result { let content = - std::str::from_utf8(md_bytes).map_err(|e| format!("Invalid UTF-8 in .persona.md: {e}"))?; - let config = buzz_persona_pkg::persona::parse_persona_md(content) - .map_err(|e| format!("Failed to parse .persona.md: {e}"))?; - - // Split "provider:model" into separate fields for the preview. - let model = match config.model.as_deref() { - Some(s) if !s.is_empty() => { - let (_prov, id) = buzz_persona_pkg::persona::split_model(s); - Some(id.to_owned()) - } - _ => None, - }; + std::str::from_utf8(md_bytes).map_err(|e| format!("Invalid UTF-8 in Markdown: {e}"))?; + match buzz_persona_pkg::persona::parse_persona_md(content) { + Ok(config) => Ok(parsed_preview_from_md_config(config)), + Err(strict_err) => parse_lenient_md_persona(content) + .map_err(|_| format!("Failed to parse persona Markdown: {strict_err}")), + } +} - Ok(ParsedPersonaPreview { +fn parsed_preview_from_md_config( + config: buzz_persona_pkg::persona::PersonaConfig, +) -> ParsedPersonaPreview { + let (provider, model) = split_preview_model(config.model.as_deref()); + + ParsedPersonaPreview { display_name: config.display_name, system_prompt: config.prompt, - avatar_data_url: None, // .persona.md avatars are paths, not data URIs + avatar_data_url: None, // Markdown avatars are paths, not data URIs + avatar_ref: config.avatar, runtime: config.runtime, model, - provider: None, // .persona.md format does not carry llmProvider + provider, + name_pool: Vec::new(), + source_file: String::new(), + } +} + +fn split_preview_model(model: Option<&str>) -> (Option, Option) { + match model.map(str::trim).filter(|s| !s.is_empty()) { + Some(raw_model) => { + let (provider, id) = buzz_persona_pkg::persona::split_model(raw_model); + (provider.map(str::to_owned), Some(id.to_owned())) + } + None => (None, None), + } +} + +#[derive(Debug, Deserialize)] +struct LenientMdFrontmatter { + name: Option, + display_name: Option, + avatar: Option, + runtime: Option, + model: Option, +} + +fn parse_lenient_md_persona(content: &str) -> Result { + let (frontmatter, body) = buzz_persona_pkg::persona::split_frontmatter(content) + .map_err(|e| format!("Missing frontmatter: {e}"))?; + let fields: LenientMdFrontmatter = + serde_yaml::from_str(frontmatter).map_err(|e| format!("Invalid YAML frontmatter: {e}"))?; + let display_name = fields + .display_name + .or(fields.name) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .ok_or_else(|| "Missing display name".to_string())?; + let (provider, model) = split_preview_model(fields.model.as_deref()); + + Ok(ParsedPersonaPreview { + display_name, + system_prompt: body.to_string(), + avatar_data_url: None, // Markdown avatars are paths, not data URIs + avatar_ref: fields.avatar, + runtime: fields.runtime, + model, + provider, name_pool: Vec::new(), source_file: String::new(), }) @@ -405,6 +454,7 @@ pub fn parse_zip_pack(zip_bytes: &[u8]) -> Result Result Result Result Vec { - let mut buf = Vec::new(); - { - let mut enc = Encoder::new(Cursor::new(&mut buf), 1, 1); - enc.set_color(ColorType::Rgba); - enc.set_depth(BitDepth::Eight); - enc.add_text_chunk(keyword.to_string(), text.to_string()) - .unwrap(); - let mut w = enc.write_header().unwrap(); - w.write_image_data(&[0, 0, 0, 255]).unwrap(); - } - buf - } - - /// Helper: build a PNG with a buzz_persona_pkg tEXt chunk for the given name/prompt. - fn make_test_persona_png(name: &str, prompt: &str) -> Vec { - let payload = serde_json::json!({ - "version": 1, - "displayName": name, - "systemPrompt": prompt, - }); - let b64 = STANDARD.encode(payload.to_string().as_bytes()); - make_png_with_text("buzz_persona_pkg", &b64) - } - - /// Helper: build a plain PNG with no metadata. - fn make_plain_png() -> Vec { - let mut buf = Vec::new(); - { - let mut enc = Encoder::new(Cursor::new(&mut buf), 1, 1); - enc.set_color(ColorType::Rgba); - enc.set_depth(BitDepth::Eight); - let mut w = enc.write_header().unwrap(); - w.write_image_data(&[0, 0, 0, 255]).unwrap(); - } - buf - } - - /// Helper: create a ZIP from name→data pairs. - fn make_test_zip(entries: &[(&str, &[u8])]) -> Vec { - let mut buf = Cursor::new(Vec::new()); - let mut zip = ZipWriter::new(&mut buf); - let options = SimpleFileOptions::default(); - for (name, data) in entries { - zip.start_file(*name, options).unwrap(); - zip.write_all(data).unwrap(); - } - zip.finish().unwrap(); - buf.into_inner() - } - - #[test] - fn parse_png_round_trip() { - let png = make_test_persona_png("George Costanza", "You are George."); - let result = parse_png_persona(&png).unwrap(); - assert_eq!(result.display_name, "George Costanza"); - assert_eq!(result.system_prompt, "You are George."); - assert!(result - .avatar_data_url - .unwrap() - .starts_with("data:image/png;base64,")); - } - - #[test] - fn parse_png_no_metadata() { - let png = make_plain_png(); - let err = parse_png_persona(&png).unwrap_err(); - assert!(err.contains("doesn't contain persona data")); - } - - #[test] - fn parse_png_unknown_version() { - let payload = serde_json::json!({"version": 99, "displayName": "X", "systemPrompt": "Y"}); - let b64 = STANDARD.encode(payload.to_string().as_bytes()); - let png = make_png_with_text("buzz_persona_pkg", &b64); - let err = parse_png_persona(&png).unwrap_err(); - assert!(err.contains("Unsupported persona version")); - } - - #[test] - fn parse_png_malformed_base64() { - let png = make_png_with_text("buzz_persona_pkg", "!!!not-base64!!!"); - let err = parse_png_persona(&png).unwrap_err(); - assert!(err.contains("Invalid base64")); - } - - #[test] - fn parse_png_malformed_json() { - let b64 = STANDARD.encode(b"not json at all"); - let png = make_png_with_text("buzz_persona_pkg", &b64); - let err = parse_png_persona(&png).unwrap_err(); - assert!(err.contains("Invalid JSON")); - } - - #[test] - fn parse_png_empty_fields() { - let payload = serde_json::json!({"version": 1, "displayName": "", "systemPrompt": "Y"}); - let b64 = STANDARD.encode(payload.to_string().as_bytes()); - let png = make_png_with_text("buzz_persona_pkg", &b64); - let err = parse_png_persona(&png).unwrap_err(); - assert!(err.contains("displayName is empty")); - } - - #[test] - fn parse_png_chara_fallback() { - let chara = serde_json::json!({ - "spec": "chara_card_v2", - "spec_version": "2.0", - "data": { - "name": "Kramer", - "system_prompt": "You are Kramer.", - "description": "" - } - }); - let b64 = STANDARD.encode(chara.to_string().as_bytes()); - let png = make_png_with_text("chara", &b64); - let result = parse_png_persona(&png).unwrap(); - assert_eq!(result.display_name, "Kramer"); - assert_eq!(result.system_prompt, "You are Kramer."); - } - - #[test] - fn parse_png_chara_ignored_when_buzz_present() { - // Build a PNG with both buzz_persona_pkg and chara chunks. - let buzz = serde_json::json!({"version": 1, "displayName": "Buzz Name", "systemPrompt": "Buzz prompt"}); - let chara = serde_json::json!({ - "spec": "chara_card_v2", "spec_version": "2.0", - "data": {"name": "Chara Name", "system_prompt": "Chara prompt", "description": ""} - }); - let buzz_b64 = STANDARD.encode(buzz.to_string().as_bytes()); - let chara_b64 = STANDARD.encode(chara.to_string().as_bytes()); - - let mut buf = Vec::new(); - { - let mut enc = Encoder::new(Cursor::new(&mut buf), 1, 1); - enc.set_color(ColorType::Rgba); - enc.set_depth(BitDepth::Eight); - enc.add_text_chunk("buzz_persona_pkg".to_string(), buzz_b64) - .unwrap(); - enc.add_text_chunk("chara".to_string(), chara_b64).unwrap(); - let mut w = enc.write_header().unwrap(); - w.write_image_data(&[0, 0, 0, 255]).unwrap(); - } - - let result = parse_png_persona(&buf).unwrap(); - assert_eq!(result.display_name, "Buzz Name"); - assert_eq!(result.system_prompt, "Buzz prompt"); - } - - #[test] - fn parse_zip_valid_pack() { - let p1 = make_test_persona_png("Alice", "Prompt A"); - let p2 = make_test_persona_png("Bob", "Prompt B"); - let p3 = make_test_persona_png("Carol", "Prompt C"); - let zip = make_test_zip(&[("alice.png", &p1), ("bob.png", &p2), ("carol.png", &p3)]); - let result = parse_zip_personas(&zip).unwrap(); - assert_eq!(result.personas.len(), 3); - assert!(result.skipped.is_empty()); - assert_eq!(result.personas[0].source_file, "alice.png"); - } - - #[test] - fn parse_zip_mixed() { - let valid1 = make_test_persona_png("Alice", "Prompt A"); - let valid2 = make_test_persona_png("Bob", "Prompt B"); - let bad_png = make_plain_png(); // no metadata - let zip = make_test_zip(&[ - ("alice.png", &valid1), - ("bob.png", &valid2), - ("bad.png", &bad_png), - ("readme.txt", b"hello"), - ]); - let result = parse_zip_personas(&zip).unwrap(); - assert_eq!(result.personas.len(), 2); - assert_eq!(result.skipped.len(), 2); - } - - #[test] - fn parse_zip_no_pngs() { - let zip = make_test_zip(&[("readme.txt", b"hello"), ("data.csv", b"a,b")]); - let err = parse_zip_personas(&zip).unwrap_err(); - assert!(err.contains("No persona files found")); - } - - #[test] - fn parse_zip_exceeds_entry_limit() { - let png = make_test_persona_png("X", "Y"); - let entries: Vec<(String, &[u8])> = (0..51) - .map(|i| (format!("{i}.png"), png.as_slice())) - .collect(); - let refs: Vec<(&str, &[u8])> = entries.iter().map(|(n, d)| (n.as_str(), *d)).collect(); - let zip = make_test_zip(&refs); - let err = parse_zip_personas(&zip).unwrap_err(); - assert!(err.contains("too many entries")); - } - - #[test] - fn parse_zip_path_traversal() { - let valid = make_test_persona_png("Safe", "Prompt"); - let evil = make_test_persona_png("Evil", "Prompt"); - let zip = make_test_zip(&[("safe.png", &valid), ("../evil.png", &evil)]); - let result = parse_zip_personas(&zip).unwrap(); - assert_eq!(result.personas.len(), 1); - assert_eq!(result.skipped.len(), 1); - assert!(result.skipped[0].reason.contains("Path traversal")); - } - - #[test] - fn parse_png_duplicate_chunks() { - // Two buzz_persona_pkg chunks — should use the first and ignore the second. - let payload1 = - serde_json::json!({"version": 1, "displayName": "First", "systemPrompt": "Prompt 1"}); - let payload2 = - serde_json::json!({"version": 1, "displayName": "Second", "systemPrompt": "Prompt 2"}); - let b64_1 = STANDARD.encode(payload1.to_string().as_bytes()); - let b64_2 = STANDARD.encode(payload2.to_string().as_bytes()); - - let mut buf = Vec::new(); - { - let mut enc = Encoder::new(Cursor::new(&mut buf), 1, 1); - enc.set_color(ColorType::Rgba); - enc.set_depth(BitDepth::Eight); - enc.add_text_chunk("buzz_persona_pkg".to_string(), b64_1) - .unwrap(); - enc.add_text_chunk("buzz_persona_pkg".to_string(), b64_2) - .unwrap(); - let mut w = enc.write_header().unwrap(); - w.write_image_data(&[0, 0, 0, 255]).unwrap(); - } - - let result = parse_png_persona(&buf).unwrap(); - assert_eq!(result.display_name, "First"); - assert_eq!(result.system_prompt, "Prompt 1"); - } - - #[test] - fn parse_zip_exceeds_size_limit() { - // Create a ZIP with entries whose cumulative decompressed size exceeds 100MB. - let mut zip_buf = Cursor::new(Vec::new()); - { - let mut zip = ZipWriter::new(&mut zip_buf); - let options = SimpleFileOptions::default(); - zip.start_file("big.png", options).unwrap(); - let chunk = vec![0u8; 1024 * 1024]; // 1 MB - for _ in 0..101 { - zip.write_all(&chunk).unwrap(); - } - zip.finish().unwrap(); - } - let zip_bytes = zip_buf.into_inner(); - let err = parse_zip_personas(&zip_bytes).unwrap_err(); - assert!(err.contains("exceeds 100MB")); - } - - #[test] - fn parse_json_round_trip() { - let bytes = encode_persona_json( - "Ada Lovelace", - "You are Ada.", - Some("https://example.com/ada.png"), - None, - None, - None, - &[], - ) - .unwrap(); - let result = parse_json_persona(&bytes).unwrap(); - assert_eq!(result.display_name, "Ada Lovelace"); - assert_eq!(result.system_prompt, "You are Ada."); - assert_eq!( - result.avatar_data_url.as_deref(), - Some("https://example.com/ada.png") - ); - assert!(result.source_file.is_empty()); - } - - #[test] - fn parse_json_round_trip_no_avatar() { - let bytes = - encode_persona_json("Bob", "You are Bob.", None, None, None, None, &[]).unwrap(); - let result = parse_json_persona(&bytes).unwrap(); - assert_eq!(result.display_name, "Bob"); - assert_eq!(result.system_prompt, "You are Bob."); - assert!(result.avatar_data_url.is_none()); - } - - #[test] - fn parse_json_round_trip_data_uri_avatar() { - let data_uri = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUg=="; - let bytes = encode_persona_json( - "Carol", - "You are Carol.", - Some(data_uri), - None, - None, - None, - &[], - ) - .unwrap(); - let result = parse_json_persona(&bytes).unwrap(); - assert_eq!(result.display_name, "Carol"); - assert_eq!(result.avatar_data_url.as_deref(), Some(data_uri)); - } - - #[test] - fn parse_json_round_trip_with_runtime_and_model() { - let bytes = encode_persona_json( - "Agent Smith", - "You are an agent.", - None, - Some("goose"), - Some("claude-sonnet-4"), - None, - &[], - ) - .unwrap(); - let result = parse_json_persona(&bytes).unwrap(); - assert_eq!(result.display_name, "Agent Smith"); - assert_eq!(result.system_prompt, "You are an agent."); - assert!(result.avatar_data_url.is_none()); - assert_eq!(result.runtime.as_deref(), Some("goose")); - assert_eq!(result.model.as_deref(), Some("claude-sonnet-4")); - } - - #[test] - fn parse_json_round_trip_without_runtime_and_model() { - let bytes = - encode_persona_json("Bob", "You are Bob.", None, None, None, None, &[]).unwrap(); - let result = parse_json_persona(&bytes).unwrap(); - assert_eq!(result.display_name, "Bob"); - assert!(result.runtime.is_none()); - assert!(result.model.is_none()); - } - - #[test] - fn parse_json_backward_compat_no_runtime_model_fields() { - // Simulate a legacy persona JSON without runtime/model fields - let json = serde_json::json!({ - "version": 1, - "displayName": "Legacy Persona", - "systemPrompt": "Old school prompt" - }); - let bytes = serde_json::to_vec(&json).unwrap(); - let result = parse_json_persona(&bytes).unwrap(); - assert_eq!(result.display_name, "Legacy Persona"); - assert_eq!(result.system_prompt, "Old school prompt"); - assert!(result.runtime.is_none()); - assert!(result.model.is_none()); - } - - #[test] - fn parse_json_backward_compat_legacy_provider_key() { - // A JSON card written with the old "provider" key should still parse. - let json = serde_json::json!({ - "version": 1, - "displayName": "Legacy Agent", - "systemPrompt": "Old prompt", - "provider": "goose" - }); - let bytes = serde_json::to_vec(&json).unwrap(); - let result = parse_json_persona(&bytes).unwrap(); - assert_eq!(result.runtime.as_deref(), Some("goose")); - } - - #[test] - fn parse_json_invalid_version() { - let json = serde_json::json!({ - "version": 99, - "displayName": "X", - "systemPrompt": "Y" - }); - let bytes = serde_json::to_vec(&json).unwrap(); - let err = parse_json_persona(&bytes).unwrap_err(); - assert!(err.contains("Unsupported persona version")); - } - - #[test] - fn parse_json_empty_fields() { - let json_empty_name = serde_json::json!({ - "version": 1, - "displayName": "", - "systemPrompt": "Y" - }); - let err = parse_json_persona(&serde_json::to_vec(&json_empty_name).unwrap()).unwrap_err(); - assert!(err.contains("displayName is empty")); - - let json_empty_prompt = serde_json::json!({ - "version": 1, - "displayName": "X", - "systemPrompt": "" - }); - let err = parse_json_persona(&serde_json::to_vec(&json_empty_prompt).unwrap()).unwrap_err(); - assert!(err.contains("systemPrompt is empty")); - } - - #[test] - fn parse_json_malformed() { - let err = parse_json_persona(b"not json at all").unwrap_err(); - assert!(err.contains("Invalid JSON")); - } - - #[test] - fn parse_zip_with_json() { - let j1 = encode_persona_json("Alice", "Prompt A", None, None, None, None, &[]).unwrap(); - let j2 = encode_persona_json("Bob", "Prompt B", None, None, None, None, &[]).unwrap(); - let zip = make_test_zip(&[("alice.persona.json", &j1), ("bob.persona.json", &j2)]); - let result = parse_zip_personas(&zip).unwrap(); - assert_eq!(result.personas.len(), 2); - assert!(result.skipped.is_empty()); - assert_eq!(result.personas[0].display_name, "Alice"); - assert_eq!(result.personas[1].display_name, "Bob"); - } - - #[test] - fn parse_zip_mixed_png_and_json() { - let png = make_test_persona_png("PngPersona", "PNG prompt"); - let json = - encode_persona_json("JsonPersona", "JSON prompt", None, None, None, None, &[]).unwrap(); - let zip = make_test_zip(&[ - ("persona.png", &png), - ("persona.json", &json), - ("readme.txt", b"hello"), - ]); - let result = parse_zip_personas(&zip).unwrap(); - assert_eq!(result.personas.len(), 2); - // readme.txt should be skipped - assert_eq!(result.skipped.len(), 1); - assert!(result.skipped[0] - .reason - .contains("Not a .png, .json, or .persona.md file")); - } - - #[test] - fn parse_zip_ignores_macos_resource_forks() { - let j1 = - encode_persona_json("Frank", "You are Frank.", None, None, None, None, &[]).unwrap(); - let j2 = - encode_persona_json("Jackie", "You are Jackie.", None, None, None, None, &[]).unwrap(); - let zip = make_test_zip(&[ - ("frank-costanza.persona.json", &j1), - ("jackie-chiles.persona.json", &j2), - ("__MACOSX/._frank-costanza.persona.json", b"\x00\x05\x16"), - ("__MACOSX/._jackie-chiles.persona.json", b"\x00\x05\x16"), - ]); - let result = parse_zip_personas(&zip).unwrap(); - assert_eq!(result.personas.len(), 2); - // macOS resource forks should be silently ignored, not skipped with errors - assert!(result.skipped.is_empty()); - } -} +#[path = "persona_card_tests.rs"] +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/persona_card_tests.rs b/desktop/src-tauri/src/managed_agents/persona_card_tests.rs new file mode 100644 index 000000000..6bcf92481 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/persona_card_tests.rs @@ -0,0 +1,530 @@ +use super::*; +use png::{BitDepth, ColorType, Encoder}; +use std::io::Write; +use zip::write::{SimpleFileOptions, ZipWriter}; + +/// Helper: build a minimal valid PNG with a custom tEXt chunk. +fn make_png_with_text(keyword: &str, text: &str) -> Vec { + let mut buf = Vec::new(); + { + let mut enc = Encoder::new(Cursor::new(&mut buf), 1, 1); + enc.set_color(ColorType::Rgba); + enc.set_depth(BitDepth::Eight); + enc.add_text_chunk(keyword.to_string(), text.to_string()) + .unwrap(); + let mut w = enc.write_header().unwrap(); + w.write_image_data(&[0, 0, 0, 255]).unwrap(); + } + buf +} + +/// Helper: build a PNG with a buzz_persona_pkg tEXt chunk for the given name/prompt. +fn make_test_persona_png(name: &str, prompt: &str) -> Vec { + let payload = serde_json::json!({ + "version": 1, + "displayName": name, + "systemPrompt": prompt, + }); + let b64 = STANDARD.encode(payload.to_string().as_bytes()); + make_png_with_text("buzz_persona_pkg", &b64) +} + +/// Helper: build a plain PNG with no metadata. +fn make_plain_png() -> Vec { + let mut buf = Vec::new(); + { + let mut enc = Encoder::new(Cursor::new(&mut buf), 1, 1); + enc.set_color(ColorType::Rgba); + enc.set_depth(BitDepth::Eight); + let mut w = enc.write_header().unwrap(); + w.write_image_data(&[0, 0, 0, 255]).unwrap(); + } + buf +} + +/// Helper: create a ZIP from name→data pairs. +fn make_test_zip(entries: &[(&str, &[u8])]) -> Vec { + let mut buf = Cursor::new(Vec::new()); + let mut zip = ZipWriter::new(&mut buf); + let options = SimpleFileOptions::default(); + for (name, data) in entries { + zip.start_file(*name, options).unwrap(); + zip.write_all(data).unwrap(); + } + zip.finish().unwrap(); + buf.into_inner() +} + +#[test] +fn parse_png_round_trip() { + let png = make_test_persona_png("George Costanza", "You are George."); + let result = parse_png_persona(&png).unwrap(); + assert_eq!(result.display_name, "George Costanza"); + assert_eq!(result.system_prompt, "You are George."); + assert!(result + .avatar_data_url + .unwrap() + .starts_with("data:image/png;base64,")); +} + +#[test] +fn parse_png_no_metadata() { + let png = make_plain_png(); + let err = parse_png_persona(&png).unwrap_err(); + assert!(err.contains("doesn't contain persona data")); +} + +#[test] +fn parse_png_unknown_version() { + let payload = serde_json::json!({"version": 99, "displayName": "X", "systemPrompt": "Y"}); + let b64 = STANDARD.encode(payload.to_string().as_bytes()); + let png = make_png_with_text("buzz_persona_pkg", &b64); + let err = parse_png_persona(&png).unwrap_err(); + assert!(err.contains("Unsupported persona version")); +} + +#[test] +fn parse_png_malformed_base64() { + let png = make_png_with_text("buzz_persona_pkg", "!!!not-base64!!!"); + let err = parse_png_persona(&png).unwrap_err(); + assert!(err.contains("Invalid base64")); +} + +#[test] +fn parse_png_malformed_json() { + let b64 = STANDARD.encode(b"not json at all"); + let png = make_png_with_text("buzz_persona_pkg", &b64); + let err = parse_png_persona(&png).unwrap_err(); + assert!(err.contains("Invalid JSON")); +} + +#[test] +fn parse_png_empty_fields() { + let payload = serde_json::json!({"version": 1, "displayName": "", "systemPrompt": "Y"}); + let b64 = STANDARD.encode(payload.to_string().as_bytes()); + let png = make_png_with_text("buzz_persona_pkg", &b64); + let err = parse_png_persona(&png).unwrap_err(); + assert!(err.contains("displayName is empty")); +} + +#[test] +fn parse_png_chara_fallback() { + let chara = serde_json::json!({ + "spec": "chara_card_v2", + "spec_version": "2.0", + "data": { + "name": "Kramer", + "system_prompt": "You are Kramer.", + "description": "" + } + }); + let b64 = STANDARD.encode(chara.to_string().as_bytes()); + let png = make_png_with_text("chara", &b64); + let result = parse_png_persona(&png).unwrap(); + assert_eq!(result.display_name, "Kramer"); + assert_eq!(result.system_prompt, "You are Kramer."); +} + +#[test] +fn parse_png_chara_ignored_when_buzz_present() { + // Build a PNG with both buzz_persona_pkg and chara chunks. + let buzz = serde_json::json!({"version": 1, "displayName": "Buzz Name", "systemPrompt": "Buzz prompt"}); + let chara = serde_json::json!({ + "spec": "chara_card_v2", "spec_version": "2.0", + "data": {"name": "Chara Name", "system_prompt": "Chara prompt", "description": ""} + }); + let buzz_b64 = STANDARD.encode(buzz.to_string().as_bytes()); + let chara_b64 = STANDARD.encode(chara.to_string().as_bytes()); + + let mut buf = Vec::new(); + { + let mut enc = Encoder::new(Cursor::new(&mut buf), 1, 1); + enc.set_color(ColorType::Rgba); + enc.set_depth(BitDepth::Eight); + enc.add_text_chunk("buzz_persona_pkg".to_string(), buzz_b64) + .unwrap(); + enc.add_text_chunk("chara".to_string(), chara_b64).unwrap(); + let mut w = enc.write_header().unwrap(); + w.write_image_data(&[0, 0, 0, 255]).unwrap(); + } + + let result = parse_png_persona(&buf).unwrap(); + assert_eq!(result.display_name, "Buzz Name"); + assert_eq!(result.system_prompt, "Buzz prompt"); +} + +#[test] +fn parse_zip_valid_pack() { + let p1 = make_test_persona_png("Alice", "Prompt A"); + let p2 = make_test_persona_png("Bob", "Prompt B"); + let p3 = make_test_persona_png("Carol", "Prompt C"); + let zip = make_test_zip(&[("alice.png", &p1), ("bob.png", &p2), ("carol.png", &p3)]); + let result = parse_zip_personas(&zip).unwrap(); + assert_eq!(result.personas.len(), 3); + assert!(result.skipped.is_empty()); + assert_eq!(result.personas[0].source_file, "alice.png"); +} + +#[test] +fn parse_zip_mixed() { + let valid1 = make_test_persona_png("Alice", "Prompt A"); + let valid2 = make_test_persona_png("Bob", "Prompt B"); + let bad_png = make_plain_png(); // no metadata + let zip = make_test_zip(&[ + ("alice.png", &valid1), + ("bob.png", &valid2), + ("bad.png", &bad_png), + ("readme.txt", b"hello"), + ]); + let result = parse_zip_personas(&zip).unwrap(); + assert_eq!(result.personas.len(), 2); + assert_eq!(result.skipped.len(), 2); +} + +#[test] +fn parse_zip_no_pngs() { + let zip = make_test_zip(&[("readme.txt", b"hello"), ("data.csv", b"a,b")]); + let err = parse_zip_personas(&zip).unwrap_err(); + assert!(err.contains("No persona files found")); +} + +#[test] +fn parse_zip_exceeds_entry_limit() { + let png = make_test_persona_png("X", "Y"); + let entries: Vec<(String, &[u8])> = (0..51) + .map(|i| (format!("{i}.png"), png.as_slice())) + .collect(); + let refs: Vec<(&str, &[u8])> = entries.iter().map(|(n, d)| (n.as_str(), *d)).collect(); + let zip = make_test_zip(&refs); + let err = parse_zip_personas(&zip).unwrap_err(); + assert!(err.contains("too many entries")); +} + +#[test] +fn parse_zip_path_traversal() { + let valid = make_test_persona_png("Safe", "Prompt"); + let evil = make_test_persona_png("Evil", "Prompt"); + let zip = make_test_zip(&[("safe.png", &valid), ("../evil.png", &evil)]); + let result = parse_zip_personas(&zip).unwrap(); + assert_eq!(result.personas.len(), 1); + assert_eq!(result.skipped.len(), 1); + assert!(result.skipped[0].reason.contains("Path traversal")); +} + +#[test] +fn parse_png_duplicate_chunks() { + // Two buzz_persona_pkg chunks — should use the first and ignore the second. + let payload1 = + serde_json::json!({"version": 1, "displayName": "First", "systemPrompt": "Prompt 1"}); + let payload2 = + serde_json::json!({"version": 1, "displayName": "Second", "systemPrompt": "Prompt 2"}); + let b64_1 = STANDARD.encode(payload1.to_string().as_bytes()); + let b64_2 = STANDARD.encode(payload2.to_string().as_bytes()); + + let mut buf = Vec::new(); + { + let mut enc = Encoder::new(Cursor::new(&mut buf), 1, 1); + enc.set_color(ColorType::Rgba); + enc.set_depth(BitDepth::Eight); + enc.add_text_chunk("buzz_persona_pkg".to_string(), b64_1) + .unwrap(); + enc.add_text_chunk("buzz_persona_pkg".to_string(), b64_2) + .unwrap(); + let mut w = enc.write_header().unwrap(); + w.write_image_data(&[0, 0, 0, 255]).unwrap(); + } + + let result = parse_png_persona(&buf).unwrap(); + assert_eq!(result.display_name, "First"); + assert_eq!(result.system_prompt, "Prompt 1"); +} + +#[test] +fn parse_zip_exceeds_size_limit() { + // Create a ZIP with entries whose cumulative decompressed size exceeds 100MB. + let mut zip_buf = Cursor::new(Vec::new()); + { + let mut zip = ZipWriter::new(&mut zip_buf); + let options = SimpleFileOptions::default(); + zip.start_file("big.png", options).unwrap(); + let chunk = vec![0u8; 1024 * 1024]; // 1 MB + for _ in 0..101 { + zip.write_all(&chunk).unwrap(); + } + zip.finish().unwrap(); + } + let zip_bytes = zip_buf.into_inner(); + let err = parse_zip_personas(&zip_bytes).unwrap_err(); + assert!(err.contains("exceeds 100MB")); +} + +#[test] +fn parse_json_round_trip() { + let bytes = encode_persona_json( + "Ada Lovelace", + "You are Ada.", + Some("https://example.com/ada.png"), + None, + None, + None, + &[], + ) + .unwrap(); + let result = parse_json_persona(&bytes).unwrap(); + assert_eq!(result.display_name, "Ada Lovelace"); + assert_eq!(result.system_prompt, "You are Ada."); + assert_eq!( + result.avatar_data_url.as_deref(), + Some("https://example.com/ada.png") + ); + assert!(result.source_file.is_empty()); +} + +#[test] +fn parse_json_round_trip_no_avatar() { + let bytes = encode_persona_json("Bob", "You are Bob.", None, None, None, None, &[]).unwrap(); + let result = parse_json_persona(&bytes).unwrap(); + assert_eq!(result.display_name, "Bob"); + assert_eq!(result.system_prompt, "You are Bob."); + assert!(result.avatar_data_url.is_none()); +} + +#[test] +fn parse_json_round_trip_data_uri_avatar() { + let data_uri = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUg=="; + let bytes = encode_persona_json( + "Carol", + "You are Carol.", + Some(data_uri), + None, + None, + None, + &[], + ) + .unwrap(); + let result = parse_json_persona(&bytes).unwrap(); + assert_eq!(result.display_name, "Carol"); + assert_eq!(result.avatar_data_url.as_deref(), Some(data_uri)); +} + +#[test] +fn parse_json_round_trip_with_runtime_and_model() { + let bytes = encode_persona_json( + "Agent Smith", + "You are an agent.", + None, + Some("goose"), + Some("claude-sonnet-4"), + None, + &[], + ) + .unwrap(); + let result = parse_json_persona(&bytes).unwrap(); + assert_eq!(result.display_name, "Agent Smith"); + assert_eq!(result.system_prompt, "You are an agent."); + assert!(result.avatar_data_url.is_none()); + assert_eq!(result.runtime.as_deref(), Some("goose")); + assert_eq!(result.model.as_deref(), Some("claude-sonnet-4")); +} + +#[test] +fn parse_json_round_trip_without_runtime_and_model() { + let bytes = encode_persona_json("Bob", "You are Bob.", None, None, None, None, &[]).unwrap(); + let result = parse_json_persona(&bytes).unwrap(); + assert_eq!(result.display_name, "Bob"); + assert!(result.runtime.is_none()); + assert!(result.model.is_none()); +} + +#[test] +fn parse_json_backward_compat_no_runtime_model_fields() { + // Simulate a legacy persona JSON without runtime/model fields + let json = serde_json::json!({ + "version": 1, + "displayName": "Legacy Persona", + "systemPrompt": "Old school prompt" + }); + let bytes = serde_json::to_vec(&json).unwrap(); + let result = parse_json_persona(&bytes).unwrap(); + assert_eq!(result.display_name, "Legacy Persona"); + assert_eq!(result.system_prompt, "Old school prompt"); + assert!(result.runtime.is_none()); + assert!(result.model.is_none()); +} + +#[test] +fn parse_json_backward_compat_legacy_provider_key() { + // A JSON card written with the old "provider" key should still parse. + let json = serde_json::json!({ + "version": 1, + "displayName": "Legacy Agent", + "systemPrompt": "Old prompt", + "provider": "goose" + }); + let bytes = serde_json::to_vec(&json).unwrap(); + let result = parse_json_persona(&bytes).unwrap(); + assert_eq!(result.runtime.as_deref(), Some("goose")); +} + +#[test] +fn parse_json_invalid_version() { + let json = serde_json::json!({ + "version": 99, + "displayName": "X", + "systemPrompt": "Y" + }); + let bytes = serde_json::to_vec(&json).unwrap(); + let err = parse_json_persona(&bytes).unwrap_err(); + assert!(err.contains("Unsupported persona version")); +} + +#[test] +fn parse_json_empty_fields() { + let json_empty_name = serde_json::json!({ + "version": 1, + "displayName": "", + "systemPrompt": "Y" + }); + let err = parse_json_persona(&serde_json::to_vec(&json_empty_name).unwrap()).unwrap_err(); + assert!(err.contains("displayName is empty")); + + let json_empty_prompt = serde_json::json!({ + "version": 1, + "displayName": "X", + "systemPrompt": "" + }); + let err = parse_json_persona(&serde_json::to_vec(&json_empty_prompt).unwrap()).unwrap_err(); + assert!(err.contains("systemPrompt is empty")); +} + +#[test] +fn parse_json_malformed() { + let err = parse_json_persona(b"not json at all").unwrap_err(); + assert!(err.contains("Invalid JSON")); +} + +#[test] +fn parse_md_persona_preserves_app_avatar_ref() { + let md = br#"--- +name: goosey +display_name: Goosey +description: Goose internal agent. +avatar: app-avatar:gloopies-19 +model: anthropic:claude-sonnet-4 +runtime: goose +--- +You are Goosey. +"#; + let result = parse_md_persona(md).unwrap(); + assert_eq!(result.display_name, "Goosey"); + assert_eq!(result.avatar_data_url, None); + assert_eq!(result.avatar_ref.as_deref(), Some("app-avatar:gloopies-19")); + assert_eq!(result.model.as_deref(), Some("claude-sonnet-4")); + assert_eq!(result.provider.as_deref(), Some("anthropic")); + assert_eq!(result.runtime.as_deref(), Some("goose")); +} + +#[test] +fn parse_lenient_md_persona_preserves_model_provider_prefix() { + let md = r#"--- +display_name: Lenient Agent +model: databricks:gpt-5 +runtime: goose +--- +You are lenient. +"#; + + let result = parse_lenient_md_persona(md).unwrap(); + assert_eq!(result.display_name, "Lenient Agent"); + assert_eq!(result.model.as_deref(), Some("gpt-5")); + assert_eq!(result.provider.as_deref(), Some("databricks")); + assert_eq!(result.runtime.as_deref(), Some("goose")); +} + +#[test] +fn parse_md_persona_accepts_goose_internal_frontmatter() { + let md = br#"--- +name: block.md +description: Opinionated guide to Block's intelligence operating model. +avatar: app-avatar:gloopies-19 +metadata: + gooseInternalBundled: true +--- +You are block.md. +"#; + let result = parse_md_persona(md).unwrap(); + assert_eq!(result.display_name, "block.md"); + assert_eq!(result.avatar_ref.as_deref(), Some("app-avatar:gloopies-19")); + assert_eq!(result.system_prompt, "You are block.md.\n"); +} + +#[test] +fn parse_zip_with_json() { + let j1 = encode_persona_json("Alice", "Prompt A", None, None, None, None, &[]).unwrap(); + let j2 = encode_persona_json("Bob", "Prompt B", None, None, None, None, &[]).unwrap(); + let zip = make_test_zip(&[("alice.persona.json", &j1), ("bob.persona.json", &j2)]); + let result = parse_zip_personas(&zip).unwrap(); + assert_eq!(result.personas.len(), 2); + assert!(result.skipped.is_empty()); + assert_eq!(result.personas[0].display_name, "Alice"); + assert_eq!(result.personas[1].display_name, "Bob"); +} + +#[test] +fn parse_zip_mixed_png_and_json() { + let png = make_test_persona_png("PngPersona", "PNG prompt"); + let json = + encode_persona_json("JsonPersona", "JSON prompt", None, None, None, None, &[]).unwrap(); + let zip = make_test_zip(&[ + ("persona.png", &png), + ("persona.json", &json), + ("readme.txt", b"hello"), + ]); + let result = parse_zip_personas(&zip).unwrap(); + assert_eq!(result.personas.len(), 2); + // readme.txt should be skipped + assert_eq!(result.skipped.len(), 1); + assert!(result.skipped[0] + .reason + .contains("Not a .png, .json, or .md file")); +} + +#[test] +fn parse_zip_with_plain_md_persona_preserves_avatar_ref() { + let md = br#"--- +name: fizz +display_name: Fizz +description: Engineering agent. +avatar: app-avatar:pollies-12 +runtime: goose +model: anthropic:claude-sonnet-4 +--- +You are Fizz. +"#; + let zip = make_test_zip(&[("fizz.md", md)]); + let result = parse_zip_personas(&zip).unwrap(); + assert_eq!(result.personas.len(), 1); + assert!(result.skipped.is_empty()); + assert_eq!(result.personas[0].display_name, "Fizz"); + assert_eq!( + result.personas[0].avatar_ref.as_deref(), + Some("app-avatar:pollies-12") + ); + assert_eq!(result.personas[0].source_file, "fizz.md"); +} + +#[test] +fn parse_zip_ignores_macos_resource_forks() { + let j1 = encode_persona_json("Frank", "You are Frank.", None, None, None, None, &[]).unwrap(); + let j2 = encode_persona_json("Jackie", "You are Jackie.", None, None, None, None, &[]).unwrap(); + let zip = make_test_zip(&[ + ("frank-costanza.persona.json", &j1), + ("jackie-chiles.persona.json", &j2), + ("__MACOSX/._frank-costanza.persona.json", b"\x00\x05\x16"), + ("__MACOSX/._jackie-chiles.persona.json", b"\x00\x05\x16"), + ]); + let result = parse_zip_personas(&zip).unwrap(); + assert_eq!(result.personas.len(), 2); + // macOS resource forks should be silently ignored, not skipped with errors + assert!(result.skipped.is_empty()); +} diff --git a/desktop/src-tauri/src/managed_agents/relay_mesh.rs b/desktop/src-tauri/src/managed_agents/relay_mesh.rs index 808546aa4..6924e5187 100644 --- a/desktop/src-tauri/src/managed_agents/relay_mesh.rs +++ b/desktop/src-tauri/src/managed_agents/relay_mesh.rs @@ -69,6 +69,7 @@ mod tests { auth_tag: Some("tag".into()), relay_url: "ws://localhost:3000".into(), avatar_url: None, + avatar_url_cleared: false, acp_command: "buzz-acp".into(), agent_command: "goose".into(), agent_args: vec![], diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 3469dc80d..410f0ed62 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -210,6 +210,7 @@ pub async fn restore_managed_agents_on_launch( name: record.name.clone(), relay_url: record.relay_url.clone(), avatar_url: record.avatar_url.clone(), + avatar_url_cleared: record.avatar_url_cleared, auth_tag: record.auth_tag.clone(), pubkey: record.pubkey.clone(), agent_command: record.agent_command.clone(), diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index b0470bd70..5f1370509 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1367,6 +1367,7 @@ pub fn build_managed_agent_summary( max_turn_duration_seconds: record.max_turn_duration_seconds, parallelism: record.parallelism, system_prompt: effective_prompt, + avatar_url: record.avatar_url.clone(), model: effective_model, mcp_toolsets: record.mcp_toolsets.clone(), env_vars: record.env_vars.clone(), @@ -1522,18 +1523,6 @@ pub fn spawn_agent_child( .map(|p| p.display().to_string()) .unwrap_or_else(|| record.agent_command.clone()); - // The agent's effective relay drives both the child's relay connection - // (BUZZ_RELAY_URL) and git credential-helper URL: an explicit per-agent - // relay wins; an empty one falls back to the active workspace relay. - let effective_relay_url = { - use tauri::Manager; - let state = app.state::(); - crate::relay::effective_agent_relay_url( - &record.relay_url, - &crate::relay::relay_ws_url_with_override(&state), - ) - }; - // Augment PATH for DMG launches so child processes can find: // - bundled CLI via ~/.local/bin symlink // - bundled sidecars (buzz, buzz-acp, etc.) via exe parent (Contents/MacOS/) @@ -1558,7 +1547,7 @@ pub fn spawn_agent_child( } command.env("RUST_LOG", child_rust_log_filter()); command.env("BUZZ_PRIVATE_KEY", &record.private_key_nsec); - command.env("BUZZ_RELAY_URL", &effective_relay_url); + command.env("BUZZ_RELAY_URL", &record.relay_url); command.env("BUZZ_ACP_AGENT_COMMAND", &resolved_agent_command); command.env("BUZZ_ACP_AGENT_ARGS", agent_args.join(",")); match &resolved_mcp_command { @@ -1681,7 +1670,7 @@ pub fn spawn_agent_child( // // NOSTR_PRIVATE_KEY mirrors BUZZ_PRIVATE_KEY — keep in sync. if let Some(cred_helper) = resolve_command("git-credential-nostr") { - let relay_http_url = crate::relay::relay_http_base_url(&effective_relay_url); + let relay_http_url = crate::relay::relay_http_base_url(&record.relay_url); command.env("NOSTR_PRIVATE_KEY", &record.private_key_nsec); command.env("GIT_TERMINAL_PROMPT", "0"); diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index f8a922da5..6be96057c 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -130,6 +130,7 @@ fn fixture( auth_tag, relay_url: "ws://localhost:3000".into(), avatar_url: None, + avatar_url_cleared: false, acp_command: "buzz-acp".into(), agent_command: "goose".into(), agent_args: vec![], diff --git a/desktop/src-tauri/src/managed_agents/team_repair.rs b/desktop/src-tauri/src/managed_agents/team_repair.rs index d9e272066..002b0dfd7 100644 --- a/desktop/src-tauri/src/managed_agents/team_repair.rs +++ b/desktop/src-tauri/src/managed_agents/team_repair.rs @@ -279,6 +279,7 @@ mod tests { auth_tag: None, relay_url: String::new(), avatar_url: None, + avatar_url_cleared: false, acp_command: String::new(), agent_command: String::new(), agent_args: vec![], diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 51761a0f9..db6246020 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -104,6 +104,9 @@ pub struct ManagedAgentRecord { /// `#[serde(default)]` so pre-existing records deserialize as `None`. #[serde(default)] pub avatar_url: Option, + /// True when `avatar_url: None` came from an explicit user clear. + #[serde(default)] + pub avatar_url_cleared: bool, pub acp_command: String, pub agent_command: String, pub agent_args: Vec, @@ -230,6 +233,7 @@ pub struct ManagedAgentSummary { pub max_turn_duration_seconds: Option, pub parallelism: u32, pub system_prompt: Option, + pub avatar_url: Option, pub model: Option, pub mcp_toolsets: Option, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] @@ -424,6 +428,9 @@ pub struct UpdateManagedAgentRequest { /// Absent = don't touch. Present = rename the agent. #[serde(default)] pub name: Option, + /// Absent = don't touch. null = clear. "url" = set. + #[serde(default)] + pub avatar_url: Option>, /// Absent = don't touch. null = clear to agent default. "id" = set. #[serde(default)] pub model: Option>, @@ -685,7 +692,13 @@ mod tests { assert_eq!(record.auth_tag, None); assert_eq!(record.avatar_url, None); + assert!(!record.avatar_url_cleared); assert_eq!(record.pubkey, "abcd1234"); + + let mut value = serde_json::to_value(&record).expect("should serialize"); + value["avatar_url_cleared"] = true.into(); + let cleared: ManagedAgentRecord = serde_json::from_value(value).unwrap(); + assert!(cleared.avatar_url_cleared); } /// Agent records WITH an auth_tag round-trip correctly through serde. diff --git a/desktop/src/features/agents/ui/personaDialogState.ts b/desktop/src/features/agents/ui/personaDialogState.ts index d1631dea5..1963584f2 100644 --- a/desktop/src/features/agents/ui/personaDialogState.ts +++ b/desktop/src/features/agents/ui/personaDialogState.ts @@ -1,4 +1,5 @@ import type { ParsePersonaFilesResult } from "@/shared/api/tauriPersonas"; +import { resolveImportedPersonaAvatarUrl } from "@/shared/avatars/gooseAppAvatarRefs"; import type { AgentPersona, CreatePersonaInput, @@ -90,7 +91,7 @@ export function importPersonaDialogState( submitLabel: "Create persona", initialValues: { displayName: persona.displayName, - avatarUrl: persona.avatarDataUrl ?? "", + avatarUrl: resolveImportedPersonaAvatarUrl(persona) ?? "", systemPrompt: persona.systemPrompt, runtime: persona.runtime ?? undefined, model: persona.model ?? undefined, diff --git a/desktop/src/features/profile/hooks.ts b/desktop/src/features/profile/hooks.ts index 4c4f86143..e7072f011 100644 --- a/desktop/src/features/profile/hooks.ts +++ b/desktop/src/features/profile/hooks.ts @@ -22,6 +22,7 @@ import type { UsersBatchResponse, } from "@/shared/api/types"; import { useIdentityQuery } from "@/shared/api/hooks"; +import { isGooseAppAvatarRef } from "@/shared/avatars/gooseAppAvatarRefs"; import { getAvatarSnapshotUrl } from "@/shared/lib/animatedAvatar"; import { rewriteRelayUrl } from "@/shared/lib/mediaUrl"; import { @@ -55,7 +56,9 @@ async function persistSelfProfile( profile: Profile, ): Promise { const existing = readSelfProfileCache(relayUrl, pubkey); - const avatarSnapshotUrl = getAvatarSnapshotUrl(profile.avatarUrl); + const avatarSnapshotUrl = isGooseAppAvatarRef(profile.avatarUrl) + ? null + : getAvatarSnapshotUrl(profile.avatarUrl); const fetched = shouldFetchAvatar(profile.avatarUrl, existing) && avatarSnapshotUrl !== null ? await fetchAvatarDataUrl(rewriteRelayUrl(avatarSnapshotUrl)) diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 11e25da4d..35d76d2bc 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -213,6 +213,7 @@ export type RawManagedAgent = { max_turn_duration_seconds: number | null; parallelism: number; system_prompt: string | null; + avatar_url?: string | null; model: string | null; mcp_toolsets: string | null; env_vars?: Record; @@ -228,8 +229,7 @@ export type RawManagedAgent = { start_on_app_launch: boolean; backend: ManagedAgentBackend; backend_agent_id: string | null; - // Optional: pre-feature mock fixtures may omit these. Mapped to - // `"owner-only"` / `[]` in `fromRawManagedAgent`. + // Optional in pre-feature mock fixtures; mapped in fromRawManagedAgent. respond_to?: ManagedAgent["respondTo"]; respond_to_allowlist?: string[]; }; @@ -868,6 +868,7 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent { maxTurnDurationSeconds: agent.max_turn_duration_seconds, parallelism: agent.parallelism, systemPrompt: agent.system_prompt, + avatarUrl: agent.avatar_url ?? null, model: agent.model, mcpToolsets: agent.mcp_toolsets, envVars: agent.env_vars ?? {}, @@ -883,8 +884,7 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent { startOnAppLaunch: agent.start_on_app_launch, backend: agent.backend, backendAgentId: agent.backend_agent_id, - // Fallbacks for pre-feature mocks/fixtures that don't carry these fields. - // Real agent records always include them (defaulted server-side). + // Fallbacks for pre-feature mocks; real records default server-side. respondTo: agent.respond_to ?? "owner-only", respondToAllowlist: agent.respond_to_allowlist ?? [], }; diff --git a/desktop/src/shared/api/tauriPersonas.ts b/desktop/src/shared/api/tauriPersonas.ts index 56ba16879..ca9957f6f 100644 --- a/desktop/src/shared/api/tauriPersonas.ts +++ b/desktop/src/shared/api/tauriPersonas.ts @@ -10,6 +10,7 @@ type RawParsedPersonaPreview = { display_name: string; system_prompt: string; avatar_data_url: string | null; + avatar_ref: string | null; runtime: string | null; model: string | null; provider: string | null; @@ -32,6 +33,7 @@ export type ParsedPersonaPreview = { displayName: string; systemPrompt: string; avatarDataUrl: string | null; + avatarRef: string | null; runtime: string | null; model: string | null; provider: string | null; @@ -60,6 +62,7 @@ type RawPersona = { name_pool?: string[]; is_builtin: boolean; is_active?: boolean; + source_team?: string | null; env_vars?: Record; created_at: string; updated_at: string; @@ -77,6 +80,7 @@ function fromRawPersona(persona: RawPersona): AgentPersona { namePool: persona.name_pool ?? [], isBuiltIn: persona.is_builtin, isActive: persona.is_active ?? true, + sourceTeam: persona.source_team ?? null, envVars: persona.env_vars ?? {}, createdAt: persona.created_at, updatedAt: persona.updated_at, @@ -155,6 +159,7 @@ export async function parsePersonaFiles( displayName: p.display_name, systemPrompt: p.system_prompt, avatarDataUrl: p.avatar_data_url, + avatarRef: p.avatar_ref, runtime: p.runtime, model: p.model, provider: p.provider, diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 653a1be99..c3d0f47f9 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -282,6 +282,7 @@ export type ManagedAgent = { maxTurnDurationSeconds: number | null; parallelism: number; systemPrompt: string | null; + avatarUrl: string | null; model: string | null; mcpToolsets: string | null; /** Per-agent env vars. Layered on top of persona envVars. */ @@ -446,6 +447,7 @@ export type AgentModelInfo = { export type UpdateManagedAgentInput = { pubkey: string; name?: string; + avatarUrl?: string | null; model?: string | null; systemPrompt?: string | null; mcpToolsets?: string | null; diff --git a/desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs b/desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs new file mode 100644 index 000000000..fe1b10db0 --- /dev/null +++ b/desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs @@ -0,0 +1,77 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + resolveImportedPersonaAvatarUrl, + toGooseAppAvatarRef, +} from "./gooseAppAvatarRefs.ts"; + +test("toGooseAppAvatarRef canonicalizes app-avatar refs", () => { + assert.equal( + toGooseAppAvatarRef("app-avatar:gloopies-19"), + "app-avatar:gloopies-19", + ); +}); + +test("toGooseAppAvatarRef ignores Goose-looking paths by default", () => { + assert.equal(toGooseAppAvatarRef("./avatars/pollies_2.png"), null); +}); + +test("toGooseAppAvatarRef detects Goose avatar ids in paths during import", () => { + assert.equal( + toGooseAppAvatarRef("./avatars/pollies_2.png", { + allowFilenameFallback: true, + }), + "app-avatar:pollies-2", + ); +}); + +test("resolveImportedPersonaAvatarUrl prefers app-avatar refs over data URLs", () => { + assert.equal( + resolveImportedPersonaAvatarUrl({ + avatarDataUrl: "https://example.com/avatar.png", + avatarRef: "app-avatar:fuzzies-1", + }), + "app-avatar:fuzzies-1", + ); +}); + +test("resolveImportedPersonaAvatarUrl preserves ordinary image URLs", () => { + assert.equal( + resolveImportedPersonaAvatarUrl({ + avatarDataUrl: "https://example.com/avatar.png", + avatarRef: null, + }), + "https://example.com/avatar.png", + ); +}); + +test("resolveImportedPersonaAvatarUrl does not rewrite Goose-looking remote URLs", () => { + assert.equal( + resolveImportedPersonaAvatarUrl({ + avatarDataUrl: "https://cdn.example.com/avatars/pollies_2.png", + avatarRef: null, + }), + "https://cdn.example.com/avatars/pollies_2.png", + ); +}); + +test("resolveImportedPersonaAvatarUrl preserves URL avatar refs", () => { + assert.equal( + resolveImportedPersonaAvatarUrl({ + avatarDataUrl: null, + avatarRef: " https://example.com/persona-avatar.png ", + }), + "https://example.com/persona-avatar.png", + ); +}); + +test("resolveImportedPersonaAvatarUrl preserves Goose-looking URL avatar refs", () => { + assert.equal( + resolveImportedPersonaAvatarUrl({ + avatarDataUrl: null, + avatarRef: " https://cdn.example.com/avatars/pollies_2.png ", + }), + "https://cdn.example.com/avatars/pollies_2.png", + ); +}); diff --git a/desktop/src/shared/avatars/gooseAppAvatarRefs.ts b/desktop/src/shared/avatars/gooseAppAvatarRefs.ts new file mode 100644 index 000000000..ab1f146c0 --- /dev/null +++ b/desktop/src/shared/avatars/gooseAppAvatarRefs.ts @@ -0,0 +1,91 @@ +export const GOOSE_APP_AVATAR_REF_PREFIX = "app-avatar:" as const; + +const APP_AVATAR_ID_PATTERN = /^[a-z0-9][a-z0-9_-]{0,63}$/; +const GOOSE_COLLECTION_ID_PATTERN = /^(fuzzies|gloopies|pollies)[-_](\d+)$/; + +type ParseGooseAppAvatarOptions = { + allowFilenameFallback?: boolean; +}; + +function cleanAvatarCandidate(value: string): string { + const basename = value + .trim() + .split(/[?#]/)[0] + ?.split(/[\\/]/) + .pop() + ?.replace(/\.(?:apng|gif|heic|heif|jpeg|jpg|mp4|png|webm)$/i, ""); + return (basename ?? "").toLowerCase().replace(/_/g, "-").trim(); +} + +export function parseGooseAppAvatarId( + value: string | null | undefined, + options: ParseGooseAppAvatarOptions = {}, +): string | null { + const trimmed = value?.trim(); + if (!trimmed) { + return null; + } + + const refIndex = trimmed.indexOf(GOOSE_APP_AVATAR_REF_PREFIX); + if (refIndex >= 0) { + const rawId = trimmed.slice(refIndex + GOOSE_APP_AVATAR_REF_PREFIX.length); + const id = cleanAvatarCandidate(rawId); + return APP_AVATAR_ID_PATTERN.test(id) ? id : null; + } + + if (options.allowFilenameFallback) { + const candidate = cleanAvatarCandidate(trimmed); + const collectionMatch = GOOSE_COLLECTION_ID_PATTERN.exec(candidate); + if (collectionMatch) { + return `${collectionMatch[1]}-${collectionMatch[2]}`; + } + } + + return null; +} + +export function toGooseAppAvatarRef( + value: string | null | undefined, + options: ParseGooseAppAvatarOptions = {}, +): string | null { + const id = parseGooseAppAvatarId(value, options); + return id ? `${GOOSE_APP_AVATAR_REF_PREFIX}${id}` : null; +} + +export function isGooseAppAvatarRef(value: string | null | undefined): boolean { + return toGooseAppAvatarRef(value) !== null; +} + +function isPersistableAvatarUrl(value: string): boolean { + return /^(?:https?:|data:image\/|blob:)/i.test(value); +} + +export function resolveImportedPersonaAvatarUrl({ + avatarDataUrl, + avatarRef, +}: { + avatarDataUrl?: string | null; + avatarRef?: string | null; +}): string | null { + const trimmedAvatarRef = avatarRef?.trim(); + const avatarRefFileFallback = + trimmedAvatarRef && !isPersistableAvatarUrl(trimmedAvatarRef) + ? toGooseAppAvatarRef(trimmedAvatarRef, { allowFilenameFallback: true }) + : null; + const gooseRef = + toGooseAppAvatarRef(avatarRef) ?? + avatarRefFileFallback ?? + toGooseAppAvatarRef(avatarDataUrl); + if (gooseRef) { + return gooseRef; + } + + for (const candidate of [avatarRef, avatarDataUrl]) { + const trimmedAvatarUrl = candidate?.trim(); + if (trimmedAvatarUrl && isPersistableAvatarUrl(trimmedAvatarUrl)) { + return trimmedAvatarUrl; + } + } + + return null; +} diff --git a/desktop/src/shared/lib/runtimeAvatar.ts b/desktop/src/shared/lib/runtimeAvatar.ts new file mode 100644 index 000000000..3be2f3412 --- /dev/null +++ b/desktop/src/shared/lib/runtimeAvatar.ts @@ -0,0 +1,13 @@ +const RUNTIME_AVATAR_URLS = new Set([ + "https://goose-docs.ai/img/logo_dark.png", + "https://anthropic.gallerycdn.vsassets.io/extensions/anthropic/claude-code/2.1.77/1773707456892/Microsoft.VisualStudio.Services.Icons.Default", + "https://openai.gallerycdn.vsassets.io/extensions/openai/chatgpt/26.5313.41514/1773706730621/Microsoft.VisualStudio.Services.Icons.Default", + "https://raw.githubusercontent.com/block/buzz/refs/heads/main/crates/buzz-agent/buzz-agent.png", +]); + +export function isKnownRuntimeAvatarUrl( + avatarUrl: string | null | undefined, +): boolean { + const trimmed = avatarUrl?.trim(); + return trimmed ? RUNTIME_AVATAR_URLS.has(trimmed) : false; +} From 0807f6efac99f57fe2324453f52d627fefb0b4a8 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 23 Jun 2026 17:40:04 -0700 Subject: [PATCH 2/5] refactor(desktop): drop dead goose-set avatar refs (#1228) Signed-off-by: Taylor Ho Co-authored-by: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> --- desktop/src-tauri/src/commands/personas.rs | 4 +- .../src/managed_agents/persona_card_tests.rs | 12 +++--- .../avatars/gooseAppAvatarRefs.test.mjs | 37 +++++-------------- .../src/shared/avatars/gooseAppAvatarRefs.ts | 26 +------------ 4 files changed, 19 insertions(+), 60 deletions(-) diff --git a/desktop/src-tauri/src/commands/personas.rs b/desktop/src-tauri/src/commands/personas.rs index db8df524e..aa1259487 100644 --- a/desktop/src-tauri/src/commands/personas.rs +++ b/desktop/src-tauri/src/commands/personas.rs @@ -375,7 +375,7 @@ mod tests { let md = br#"--- name: fizz display_name: Fizz -avatar: app-avatar:pollies-12 +avatar: app-avatar:persona-12 runtime: goose --- You are Fizz. @@ -388,7 +388,7 @@ You are Fizz. assert_eq!(result.personas[0].display_name, "Fizz"); assert_eq!( result.personas[0].avatar_ref.as_deref(), - Some("app-avatar:pollies-12") + Some("app-avatar:persona-12") ); assert_eq!(result.personas[0].source_file, "fizz.md"); } diff --git a/desktop/src-tauri/src/managed_agents/persona_card_tests.rs b/desktop/src-tauri/src/managed_agents/persona_card_tests.rs index 6bcf92481..e36ce6a36 100644 --- a/desktop/src-tauri/src/managed_agents/persona_card_tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_card_tests.rs @@ -409,7 +409,7 @@ fn parse_md_persona_preserves_app_avatar_ref() { name: goosey display_name: Goosey description: Goose internal agent. -avatar: app-avatar:gloopies-19 +avatar: app-avatar:persona-19 model: anthropic:claude-sonnet-4 runtime: goose --- @@ -418,7 +418,7 @@ You are Goosey. let result = parse_md_persona(md).unwrap(); assert_eq!(result.display_name, "Goosey"); assert_eq!(result.avatar_data_url, None); - assert_eq!(result.avatar_ref.as_deref(), Some("app-avatar:gloopies-19")); + assert_eq!(result.avatar_ref.as_deref(), Some("app-avatar:persona-19")); assert_eq!(result.model.as_deref(), Some("claude-sonnet-4")); assert_eq!(result.provider.as_deref(), Some("anthropic")); assert_eq!(result.runtime.as_deref(), Some("goose")); @@ -446,7 +446,7 @@ fn parse_md_persona_accepts_goose_internal_frontmatter() { let md = br#"--- name: block.md description: Opinionated guide to Block's intelligence operating model. -avatar: app-avatar:gloopies-19 +avatar: app-avatar:persona-19 metadata: gooseInternalBundled: true --- @@ -454,7 +454,7 @@ You are block.md. "#; let result = parse_md_persona(md).unwrap(); assert_eq!(result.display_name, "block.md"); - assert_eq!(result.avatar_ref.as_deref(), Some("app-avatar:gloopies-19")); + assert_eq!(result.avatar_ref.as_deref(), Some("app-avatar:persona-19")); assert_eq!(result.system_prompt, "You are block.md.\n"); } @@ -495,7 +495,7 @@ fn parse_zip_with_plain_md_persona_preserves_avatar_ref() { name: fizz display_name: Fizz description: Engineering agent. -avatar: app-avatar:pollies-12 +avatar: app-avatar:persona-12 runtime: goose model: anthropic:claude-sonnet-4 --- @@ -508,7 +508,7 @@ You are Fizz. assert_eq!(result.personas[0].display_name, "Fizz"); assert_eq!( result.personas[0].avatar_ref.as_deref(), - Some("app-avatar:pollies-12") + Some("app-avatar:persona-12") ); assert_eq!(result.personas[0].source_file, "fizz.md"); } diff --git a/desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs b/desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs index fe1b10db0..ef4b71467 100644 --- a/desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs +++ b/desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs @@ -8,31 +8,22 @@ import { test("toGooseAppAvatarRef canonicalizes app-avatar refs", () => { assert.equal( - toGooseAppAvatarRef("app-avatar:gloopies-19"), - "app-avatar:gloopies-19", + toGooseAppAvatarRef("app-avatar:persona-19"), + "app-avatar:persona-19", ); }); -test("toGooseAppAvatarRef ignores Goose-looking paths by default", () => { - assert.equal(toGooseAppAvatarRef("./avatars/pollies_2.png"), null); -}); - -test("toGooseAppAvatarRef detects Goose avatar ids in paths during import", () => { - assert.equal( - toGooseAppAvatarRef("./avatars/pollies_2.png", { - allowFilenameFallback: true, - }), - "app-avatar:pollies-2", - ); +test("toGooseAppAvatarRef ignores filesystem-looking paths", () => { + assert.equal(toGooseAppAvatarRef("./avatars/persona_2.png"), null); }); test("resolveImportedPersonaAvatarUrl prefers app-avatar refs over data URLs", () => { assert.equal( resolveImportedPersonaAvatarUrl({ avatarDataUrl: "https://example.com/avatar.png", - avatarRef: "app-avatar:fuzzies-1", + avatarRef: "app-avatar:persona-1", }), - "app-avatar:fuzzies-1", + "app-avatar:persona-1", ); }); @@ -46,13 +37,13 @@ test("resolveImportedPersonaAvatarUrl preserves ordinary image URLs", () => { ); }); -test("resolveImportedPersonaAvatarUrl does not rewrite Goose-looking remote URLs", () => { +test("resolveImportedPersonaAvatarUrl does not rewrite remote image URLs", () => { assert.equal( resolveImportedPersonaAvatarUrl({ - avatarDataUrl: "https://cdn.example.com/avatars/pollies_2.png", + avatarDataUrl: "https://cdn.example.com/avatars/persona_2.png", avatarRef: null, }), - "https://cdn.example.com/avatars/pollies_2.png", + "https://cdn.example.com/avatars/persona_2.png", ); }); @@ -65,13 +56,3 @@ test("resolveImportedPersonaAvatarUrl preserves URL avatar refs", () => { "https://example.com/persona-avatar.png", ); }); - -test("resolveImportedPersonaAvatarUrl preserves Goose-looking URL avatar refs", () => { - assert.equal( - resolveImportedPersonaAvatarUrl({ - avatarDataUrl: null, - avatarRef: " https://cdn.example.com/avatars/pollies_2.png ", - }), - "https://cdn.example.com/avatars/pollies_2.png", - ); -}); diff --git a/desktop/src/shared/avatars/gooseAppAvatarRefs.ts b/desktop/src/shared/avatars/gooseAppAvatarRefs.ts index ab1f146c0..6afef0587 100644 --- a/desktop/src/shared/avatars/gooseAppAvatarRefs.ts +++ b/desktop/src/shared/avatars/gooseAppAvatarRefs.ts @@ -1,11 +1,6 @@ export const GOOSE_APP_AVATAR_REF_PREFIX = "app-avatar:" as const; const APP_AVATAR_ID_PATTERN = /^[a-z0-9][a-z0-9_-]{0,63}$/; -const GOOSE_COLLECTION_ID_PATTERN = /^(fuzzies|gloopies|pollies)[-_](\d+)$/; - -type ParseGooseAppAvatarOptions = { - allowFilenameFallback?: boolean; -}; function cleanAvatarCandidate(value: string): string { const basename = value @@ -19,7 +14,6 @@ function cleanAvatarCandidate(value: string): string { export function parseGooseAppAvatarId( value: string | null | undefined, - options: ParseGooseAppAvatarOptions = {}, ): string | null { const trimmed = value?.trim(); if (!trimmed) { @@ -33,22 +27,13 @@ export function parseGooseAppAvatarId( return APP_AVATAR_ID_PATTERN.test(id) ? id : null; } - if (options.allowFilenameFallback) { - const candidate = cleanAvatarCandidate(trimmed); - const collectionMatch = GOOSE_COLLECTION_ID_PATTERN.exec(candidate); - if (collectionMatch) { - return `${collectionMatch[1]}-${collectionMatch[2]}`; - } - } - return null; } export function toGooseAppAvatarRef( value: string | null | undefined, - options: ParseGooseAppAvatarOptions = {}, ): string | null { - const id = parseGooseAppAvatarId(value, options); + const id = parseGooseAppAvatarId(value); return id ? `${GOOSE_APP_AVATAR_REF_PREFIX}${id}` : null; } @@ -67,15 +52,8 @@ export function resolveImportedPersonaAvatarUrl({ avatarDataUrl?: string | null; avatarRef?: string | null; }): string | null { - const trimmedAvatarRef = avatarRef?.trim(); - const avatarRefFileFallback = - trimmedAvatarRef && !isPersistableAvatarUrl(trimmedAvatarRef) - ? toGooseAppAvatarRef(trimmedAvatarRef, { allowFilenameFallback: true }) - : null; const gooseRef = - toGooseAppAvatarRef(avatarRef) ?? - avatarRefFileFallback ?? - toGooseAppAvatarRef(avatarDataUrl); + toGooseAppAvatarRef(avatarRef) ?? toGooseAppAvatarRef(avatarDataUrl); if (gooseRef) { return gooseRef; } From 1d01d883831948ab7489fe219a4f35c1674c5c89 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 23 Jun 2026 23:12:16 -0700 Subject: [PATCH 3/5] refactor(agents): derive avatar once at create, drop reconcile-time backfill (#1233) Signed-off-by: Taylor Ho --- desktop/scripts/check-file-sizes.mjs | 7 +- .../src-tauri/src/commands/agent_models.rs | 40 +--- desktop/src-tauri/src/commands/agents.rs | 212 ++---------------- .../src-tauri/src/commands/agents_tests.rs | 124 ---------- desktop/src-tauri/src/managed_agents/nest.rs | 1 - .../src/managed_agents/relay_mesh.rs | 1 - .../src-tauri/src/managed_agents/restore.rs | 9 +- .../src/managed_agents/runtime/tests.rs | 1 - .../src/managed_agents/team_repair.rs | 1 - desktop/src-tauri/src/managed_agents/types.rs | 9 - 10 files changed, 22 insertions(+), 383 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 4a6ddb436..261ee3db9 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", 1294], + ["src-tauri/src/commands/agents.rs", 1110], // Residual repos_dir integration in ensure_nest_at: REPOS is provisioned // outside NEST_DIRS (it may be a symlink), so it needs its own create + // chmod-only-when-real-dir handling plus integration test coverage. The @@ -41,11 +41,6 @@ const overrides = new Map([ ["src-tauri/src/managed_agents/runtime.rs", 1953], ["src-tauri/src/managed_agents/personas.rs", 1080], ["src-tauri/src/managed_agents/persona_card.rs", 1050], - // avatar_url_cleared flag + app-avatar ref plumbing pushed this over the - // 1000-line cap once the branch caught up with main — a small overage from - // load-bearing avatar persistence plumbing, not generic debt growth. - // Approved override; still queued to split with the rest of this list. - ["src-tauri/src/managed_agents/types.rs", 1002], // applyWorkspace reposDir parameter threaded through the Tauri invoke for // configurable repos_dir — a 3-line overage from load-bearing parameter // plumbing, not generic debt growth. Approved override; still queued to split. diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 261f501a6..cf17d241c 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -7,11 +7,10 @@ use crate::{ app_state::AppState, managed_agents::{ build_managed_agent_summary, default_agent_workdir, find_managed_agent_mut, - known_acp_runtime, load_managed_agents, load_personas, managed_agent_avatar_url, - missing_command_message, normalize_agent_args, resolve_command, - resolve_effective_prompt_model_provider, save_managed_agents, sync_managed_agent_processes, - try_regenerate_nest, AgentModelInfo, AgentModelsResponse, ManagedAgentRecord, - UpdateManagedAgentRequest, UpdateManagedAgentResponse, + known_acp_runtime, load_managed_agents, load_personas, missing_command_message, + normalize_agent_args, resolve_command, resolve_effective_prompt_model_provider, + save_managed_agents, sync_managed_agent_processes, try_regenerate_nest, AgentModelInfo, + AgentModelsResponse, UpdateManagedAgentRequest, UpdateManagedAgentResponse, }, relay::{relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, @@ -23,27 +22,6 @@ fn trim_optional(value: Option) -> Option { .filter(|s| !s.is_empty()) } -fn is_persona_runtime_avatar(record: &ManagedAgentRecord, avatar_url: &str) -> bool { - record.persona_id.is_some() - && managed_agent_avatar_url(&record.agent_command) - .as_deref() - .is_some_and(|runtime_avatar_url| runtime_avatar_url == avatar_url.trim()) -} - -fn profile_sync_avatar_url(record: &ManagedAgentRecord) -> Option { - record - .avatar_url - .clone() - .filter(|avatar_url| !is_persona_runtime_avatar(record, avatar_url)) - .or_else(|| { - if record.persona_id.is_none() { - managed_agent_avatar_url(&record.agent_command) - } else { - None - } - }) -} - /// Query available models from an agent via `buzz-acp models --json`. /// /// Spawns a short-lived subprocess (no relay connection needed). The subprocess @@ -200,10 +178,8 @@ pub async fn update_managed_agent( } if let Some(avatar_update) = input.avatar_url { let normalized = trim_optional(avatar_update); - let avatar_url_cleared = normalized.is_none(); - if normalized != record.avatar_url || avatar_url_cleared != record.avatar_url_cleared { + if normalized != record.avatar_url { record.avatar_url = normalized; - record.avatar_url_cleared = avatar_url_cleared; avatar_changed = true; } } @@ -288,11 +264,7 @@ pub async fn update_managed_agent( .map_err(|e| format!("failed to parse agent keys: {e}"))?; let relay_url = record.relay_url.clone(); let display_name = record.name.clone(); - let avatar_url = if avatar_changed || record.avatar_url_cleared { - record.avatar_url.clone() - } else { - profile_sync_avatar_url(record) - }; + let avatar_url = record.avatar_url.clone(); let auth_tag = record.auth_tag.clone(); Some((agent_keys, relay_url, display_name, avatar_url, auth_tag)) } else { diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 8d65b3ea8..aec952bd4 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -78,28 +78,6 @@ fn resolve_created_avatar_url( }) } -fn is_retired_fizz_data_url(persona_id: Option<&str>, avatar_url: &str) -> bool { - persona_id == Some("builtin:fizz") && avatar_url.trim_start().starts_with("data:image/") -} - -fn is_command_avatar_for_persona( - persona_id: Option<&str>, - agent_command: &str, - avatar_url: &str, -) -> bool { - persona_id.is_some() - && managed_agent_avatar_url(agent_command) - .as_deref() - .is_some_and(|command_avatar_url| command_avatar_url == avatar_url.trim()) -} - -fn filter_retired_fizz_avatar( - persona_id: Option<&str>, - avatar_url: Option, -) -> Option { - avatar_url.filter(|url| !is_retired_fizz_data_url(persona_id, url)) -} - #[cfg(feature = "mesh-llm")] async fn ensure_relay_mesh_for_record( app: &AppHandle, @@ -551,7 +529,6 @@ pub async fn create_managed_agent( auth_tag: auth_tag.clone(), relay_url: resolved_relay_url.clone(), avatar_url: resolved_avatar_url.clone(), - avatar_url_cleared: false, acp_command: input .acp_command .as_deref() @@ -763,22 +740,10 @@ 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` can mean either a - /// legacy missing value or an explicit clear; `avatar_url_cleared` - /// disambiguates those cases. + /// Expected avatar URL for the published profile. Derived once at creation + /// and stored verbatim; reconciliation republishes it as-is. pub(crate) avatar_url: Option, - pub(crate) avatar_url_cleared: bool, 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] @@ -823,11 +788,7 @@ pub async fn start_managed_agent( name: record.name.clone(), relay_url: record.relay_url.clone(), avatar_url: record.avatar_url.clone(), - avatar_url_cleared: record.avatar_url_cleared, 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 { @@ -888,8 +849,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. For legacy records (pre-PR-921) - // with no persisted avatar, this also backfills the avatar from the relay. + // profile sync at creation time failed silently. The avatar derived once at + // creation is published verbatim — there is no reconcile-time backfill. if result.is_ok() { let reconcile_pubkey = pubkey.clone(); let reconcile_app = app.clone(); @@ -897,8 +858,7 @@ pub async fn start_managed_agent( use tauri::Manager; let state = reconcile_app.state::(); if let Err(e) = - reconcile_agent_profile(&state, &reconcile_app, &reconcile_pubkey, &reconcile_data) - .await + reconcile_agent_profile(&state, &reconcile_pubkey, &reconcile_data).await { eprintln!( "buzz-desktop: profile reconciliation failed for agent {reconcile_pubkey}: {e}" @@ -910,59 +870,19 @@ 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, - use_command_fallback: bool, -) -> String { - persona_avatar - .or(relay_picture) - .or_else(|| { - if use_command_fallback { - managed_agent_avatar_url(agent_command) - } else { - None - } - }) - .unwrap_or_default() -} - -fn should_skip_legacy_command_avatar( - stored_avatar_was_retired_fizz: bool, - relay_picture_was_retired_fizz: bool, - persona_avatar: Option<&str>, - relay_picture: Option<&str>, -) -> bool { - (stored_avatar_was_retired_fizz || relay_picture_was_retired_fizz) - && persona_avatar.is_none() - && relay_picture.is_none() -} - /// 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. 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 -/// 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. +/// The avatar is derived once at creation and stored on the record; reconcile +/// publishes that stored value verbatim with no backfill. 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. pub(crate) async fn reconcile_agent_profile( state: &AppState, - app: &AppHandle, agent_pubkey: &str, data: &ProfileReconcileData, ) -> Result<(), String> { @@ -971,114 +891,10 @@ pub(crate) async fn reconcile_agent_profile( // Query the relay for the agent's existing kind:0 profile. let existing = query_agent_profile(state, &data.relay_url, agent_pubkey).await?; - // Resolve the expected avatar. A user-initiated clear is intentionally - // `None` and must not be backfilled from persona/relay/runtime defaults. - // For legacy records that have no stored avatar_url yet, `None` still means - // backfill from the best available historical source. - let stored_avatar = - filter_retired_fizz_avatar(data.persona_id.as_deref(), data.avatar_url.clone()); - let stored_avatar_was_retired_fizz = data - .avatar_url - .as_deref() - .is_some_and(|url| is_retired_fizz_data_url(data.persona_id.as_deref(), url)); - let stored_avatar_was_command_fallback = stored_avatar.as_deref().is_some_and(|url| { - is_command_avatar_for_persona(data.persona_id.as_deref(), &data.agent_command, url) - }); - let stored_avatar = stored_avatar.filter(|url| { - !is_command_avatar_for_persona(data.persona_id.as_deref(), &data.agent_command, url) - }); - let expected_avatar = if data.avatar_url_cleared && stored_avatar.is_none() { - None - } else { - match stored_avatar { - Some(url) => Some(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 = filter_retired_fizz_avatar( - data.persona_id.as_deref(), - data.persona_id.as_ref().and_then(|pid| { - load_personas(app) - .ok()? - .into_iter() - .find(|p| p.id == *pid)? - .avatar_url - }), - ); - let relay_picture_raw = existing.as_ref().and_then(|info| info.picture.clone()); - let relay_picture_was_retired_fizz = relay_picture_raw - .as_deref() - .is_some_and(|url| is_retired_fizz_data_url(data.persona_id.as_deref(), url)); - let relay_picture = - filter_retired_fizz_avatar(data.persona_id.as_deref(), relay_picture_raw); - let relay_picture_was_command_fallback = - relay_picture.as_deref().is_some_and(|url| { - is_command_avatar_for_persona( - data.persona_id.as_deref(), - &data.agent_command, - url, - ) - }); - let relay_picture = relay_picture.filter(|url| { - !is_command_avatar_for_persona( - data.persona_id.as_deref(), - &data.agent_command, - url, - ) - }); - - let skip_command_fallback = should_skip_legacy_command_avatar( - stored_avatar_was_retired_fizz, - relay_picture_was_retired_fizz, - persona_avatar.as_deref(), - relay_picture.as_deref(), - ); - let backfilled = if skip_command_fallback { - String::new() - } else { - resolve_legacy_avatar( - persona_avatar, - relay_picture, - &data.agent_command, - data.persona_id.is_none(), - ) - }; - - // Persist the backfilled avatar so this migration only runs once, - // or clear the retired built-in Fizz data URL if there is no - // current profile image to backfill. - let should_persist_avatar = stored_avatar_was_retired_fizz - || relay_picture_was_retired_fizz - || stored_avatar_was_command_fallback - || relay_picture_was_command_fallback - || (!backfilled.is_empty() - && data.avatar_url.as_deref() != Some(backfilled.as_str())); - if should_persist_avatar { - 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 = if backfilled.is_empty() { - None - } else { - Some(backfilled.clone()) - }; - record.avatar_url_cleared = backfilled.is_empty(); - save_managed_agents(app, &records)?; - } - } - - if backfilled.is_empty() { - None - } else { - Some(backfilled) - } - } - } - }; + // Republish the avatar exactly as derived once at creation. There is no + // reconcile-time backfill: a stored `None` (including an explicit clear) + // is published verbatim. + let expected_avatar = data.avatar_url.clone(); if !profile_needs_sync(existing.as_ref(), &data.name, expected_avatar.as_deref()) { return Ok(()); diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index a13be10ff..22bed61fc 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -82,33 +82,6 @@ fn created_persona_avatar_does_not_use_command_fallback() { assert_eq!(resolved, None); } -#[test] -fn retired_fizz_data_url_is_treated_as_absent() { - assert_eq!( - filter_retired_fizz_avatar( - Some("builtin:fizz"), - Some("data:image/png;base64,old-demo".to_string()), - ), - None, - ); - assert_eq!( - filter_retired_fizz_avatar( - Some("custom:fizz"), - Some("data:image/png;base64,user-avatar".to_string()), - ) - .as_deref(), - Some("data:image/png;base64,user-avatar"), - ); - assert_eq!( - filter_retired_fizz_avatar( - Some("builtin:fizz"), - Some("https://relay.example/avatar.png".to_string()), - ) - .as_deref(), - Some("https://relay.example/avatar.png"), - ); -} - fn profile(name: Option<&str>, picture: Option<&str>) -> crate::relay::AgentProfileInfo { crate::relay::AgentProfileInfo { display_name: name.map(str::to_string), @@ -172,100 +145,3 @@ fn profile_needs_sync_when_expected_avatar_absent_but_published() { let existing = profile(Some("Duncan"), Some("https://x/a.png")); assert!(profile_needs_sync(Some(&existing), "Duncan", None)); } - -#[test] -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", - false, - ); - - 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", - false, - ); - - 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", true); - - assert_eq!(resolved, managed_agent_avatar_url("goose").unwrap()); -} - -#[test] -fn legacy_avatar_empty_when_nothing_resolves() { - let resolved = resolve_legacy_avatar(None, None, "totally-unknown-command", true); - - assert!(resolved.is_empty()); -} - -#[test] -fn legacy_persona_avatar_does_not_use_command_fallback() { - let resolved = resolve_legacy_avatar(None, None, "goose", false); - - assert!(resolved.is_empty()); -} - -#[test] -fn detects_command_avatar_for_persona_agents() { - let command_avatar = crate::managed_agents::managed_agent_avatar_url("goose") - .expect("goose avatar should resolve"); - - assert!(is_command_avatar_for_persona( - Some("builtin:fizz"), - "goose", - &command_avatar, - )); - assert!(!is_command_avatar_for_persona( - None, - "goose", - &command_avatar, - )); - assert!(!is_command_avatar_for_persona( - Some("builtin:fizz"), - "goose", - "https://x/fizz.png", - )); -} - -#[test] -fn legacy_avatar_skips_command_icon_for_retired_stored_fizz_avatar() { - assert!(should_skip_legacy_command_avatar(true, false, None, None)); -} - -#[test] -fn legacy_avatar_skips_command_icon_for_retired_relay_fizz_avatar() { - assert!(should_skip_legacy_command_avatar(false, true, None, None)); -} - -#[test] -fn legacy_avatar_keeps_command_icon_when_retired_fizz_has_current_avatar_source() { - assert!(!should_skip_legacy_command_avatar( - false, - true, - Some("https://x/persona.png"), - None, - )); - assert!(!should_skip_legacy_command_avatar( - false, - true, - None, - Some("https://x/relay.png"), - )); -} diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index b822e3d41..9fec60d6d 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -993,7 +993,6 @@ mod tests { auth_tag: None, relay_url: String::new(), avatar_url: None, - avatar_url_cleared: false, acp_command: String::new(), agent_command: String::new(), agent_args: vec![], diff --git a/desktop/src-tauri/src/managed_agents/relay_mesh.rs b/desktop/src-tauri/src/managed_agents/relay_mesh.rs index 6924e5187..808546aa4 100644 --- a/desktop/src-tauri/src/managed_agents/relay_mesh.rs +++ b/desktop/src-tauri/src/managed_agents/relay_mesh.rs @@ -69,7 +69,6 @@ mod tests { auth_tag: Some("tag".into()), relay_url: "ws://localhost:3000".into(), avatar_url: None, - avatar_url_cleared: false, acp_command: "buzz-acp".into(), agent_command: "goose".into(), agent_args: vec![], diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 410f0ed62..0e4f72375 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -210,11 +210,7 @@ pub async fn restore_managed_agents_on_launch( name: record.name.clone(), relay_url: record.relay_url.clone(), avatar_url: record.avatar_url.clone(), - avatar_url_cleared: record.avatar_url_cleared, auth_tag: record.auth_tag.clone(), - pubkey: record.pubkey.clone(), - agent_command: record.agent_command.clone(), - persona_id: record.persona_id.clone(), }, )) }) @@ -229,10 +225,7 @@ pub async fn restore_managed_agents_on_launch( 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 - { + if let Err(e) = crate::commands::reconcile_agent_profile(&state, &pubkey, &data).await { eprintln!("buzz-desktop: profile reconciliation failed for agent {pubkey}: {e}"); } }); diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 6be96057c..f8a922da5 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -130,7 +130,6 @@ fn fixture( auth_tag, relay_url: "ws://localhost:3000".into(), avatar_url: None, - avatar_url_cleared: false, acp_command: "buzz-acp".into(), agent_command: "goose".into(), agent_args: vec![], diff --git a/desktop/src-tauri/src/managed_agents/team_repair.rs b/desktop/src-tauri/src/managed_agents/team_repair.rs index 002b0dfd7..d9e272066 100644 --- a/desktop/src-tauri/src/managed_agents/team_repair.rs +++ b/desktop/src-tauri/src/managed_agents/team_repair.rs @@ -279,7 +279,6 @@ mod tests { auth_tag: None, relay_url: String::new(), avatar_url: None, - avatar_url_cleared: false, acp_command: String::new(), agent_command: String::new(), agent_args: vec![], diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 3b894d8ec..e3372cdbe 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -107,9 +107,6 @@ pub struct ManagedAgentRecord { /// `#[serde(default)]` so pre-existing records deserialize as `None`. #[serde(default)] pub avatar_url: Option, - /// True when `avatar_url: None` came from an explicit user clear. - #[serde(default)] - pub avatar_url_cleared: bool, pub acp_command: String, pub agent_command: String, pub agent_args: Vec, @@ -695,13 +692,7 @@ mod tests { assert_eq!(record.auth_tag, None); assert_eq!(record.avatar_url, None); - assert!(!record.avatar_url_cleared); assert_eq!(record.pubkey, "abcd1234"); - - let mut value = serde_json::to_value(&record).expect("should serialize"); - value["avatar_url_cleared"] = true.into(); - let cleared: ManagedAgentRecord = serde_json::from_value(value).unwrap(); - assert!(cleared.avatar_url_cleared); } /// Agent records WITH an auth_tag round-trip correctly through serde. From 9a19c2d197efe529b0f72a447298772489fb5038 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 23 Jun 2026 23:50:51 -0700 Subject: [PATCH 4/5] refactor(avatars): drop orphaned app-avatar: symbolic-ref machinery (#1235) Signed-off-by: Taylor Ho Co-authored-by: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> --- desktop/src-tauri/src/commands/personas.rs | 6 +- .../src/managed_agents/persona_card_tests.rs | 20 ++++-- .../features/agents/ui/personaDialogState.ts | 27 +++++++- desktop/src/features/profile/hooks.ts | 5 +- .../avatars/gooseAppAvatarRefs.test.mjs | 58 ---------------- .../src/shared/avatars/gooseAppAvatarRefs.ts | 69 ------------------- 6 files changed, 43 insertions(+), 142 deletions(-) delete mode 100644 desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs delete mode 100644 desktop/src/shared/avatars/gooseAppAvatarRefs.ts diff --git a/desktop/src-tauri/src/commands/personas.rs b/desktop/src-tauri/src/commands/personas.rs index aa1259487..09b033636 100644 --- a/desktop/src-tauri/src/commands/personas.rs +++ b/desktop/src-tauri/src/commands/personas.rs @@ -371,11 +371,11 @@ mod tests { use super::*; #[test] - fn parse_persona_files_accepts_plain_md_with_avatar_ref() { + fn parse_persona_files_carries_opaque_avatar_ref() { let md = br#"--- name: fizz display_name: Fizz -avatar: app-avatar:persona-12 +avatar: https://example.com/avatars/fizz.png runtime: goose --- You are Fizz. @@ -388,7 +388,7 @@ You are Fizz. assert_eq!(result.personas[0].display_name, "Fizz"); assert_eq!( result.personas[0].avatar_ref.as_deref(), - Some("app-avatar:persona-12") + Some("https://example.com/avatars/fizz.png") ); assert_eq!(result.personas[0].source_file, "fizz.md"); } diff --git a/desktop/src-tauri/src/managed_agents/persona_card_tests.rs b/desktop/src-tauri/src/managed_agents/persona_card_tests.rs index e36ce6a36..e6500d449 100644 --- a/desktop/src-tauri/src/managed_agents/persona_card_tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_card_tests.rs @@ -404,12 +404,12 @@ fn parse_json_malformed() { } #[test] -fn parse_md_persona_preserves_app_avatar_ref() { +fn parse_md_persona_preserves_avatar_ref() { let md = br#"--- name: goosey display_name: Goosey description: Goose internal agent. -avatar: app-avatar:persona-19 +avatar: https://example.com/avatars/goosey.png model: anthropic:claude-sonnet-4 runtime: goose --- @@ -418,7 +418,10 @@ You are Goosey. let result = parse_md_persona(md).unwrap(); assert_eq!(result.display_name, "Goosey"); assert_eq!(result.avatar_data_url, None); - assert_eq!(result.avatar_ref.as_deref(), Some("app-avatar:persona-19")); + assert_eq!( + result.avatar_ref.as_deref(), + Some("https://example.com/avatars/goosey.png") + ); assert_eq!(result.model.as_deref(), Some("claude-sonnet-4")); assert_eq!(result.provider.as_deref(), Some("anthropic")); assert_eq!(result.runtime.as_deref(), Some("goose")); @@ -446,7 +449,7 @@ fn parse_md_persona_accepts_goose_internal_frontmatter() { let md = br#"--- name: block.md description: Opinionated guide to Block's intelligence operating model. -avatar: app-avatar:persona-19 +avatar: https://avatars.example.com/block-md.png metadata: gooseInternalBundled: true --- @@ -454,7 +457,10 @@ You are block.md. "#; let result = parse_md_persona(md).unwrap(); assert_eq!(result.display_name, "block.md"); - assert_eq!(result.avatar_ref.as_deref(), Some("app-avatar:persona-19")); + assert_eq!( + result.avatar_ref.as_deref(), + Some("https://avatars.example.com/block-md.png") + ); assert_eq!(result.system_prompt, "You are block.md.\n"); } @@ -495,7 +501,7 @@ fn parse_zip_with_plain_md_persona_preserves_avatar_ref() { name: fizz display_name: Fizz description: Engineering agent. -avatar: app-avatar:persona-12 +avatar: https://avatars.example.com/fizz.png runtime: goose model: anthropic:claude-sonnet-4 --- @@ -508,7 +514,7 @@ You are Fizz. assert_eq!(result.personas[0].display_name, "Fizz"); assert_eq!( result.personas[0].avatar_ref.as_deref(), - Some("app-avatar:persona-12") + Some("https://avatars.example.com/fizz.png") ); assert_eq!(result.personas[0].source_file, "fizz.md"); } diff --git a/desktop/src/features/agents/ui/personaDialogState.ts b/desktop/src/features/agents/ui/personaDialogState.ts index 1963584f2..34777387b 100644 --- a/desktop/src/features/agents/ui/personaDialogState.ts +++ b/desktop/src/features/agents/ui/personaDialogState.ts @@ -1,11 +1,36 @@ import type { ParsePersonaFilesResult } from "@/shared/api/tauriPersonas"; -import { resolveImportedPersonaAvatarUrl } from "@/shared/avatars/gooseAppAvatarRefs"; import type { AgentPersona, CreatePersonaInput, UpdatePersonaInput, } from "@/shared/api/types"; +function isPersistableAvatarUrl(value: string): boolean { + return /^(?:https?:|data:image\/|blob:)/i.test(value); +} + +/** + * Picks an avatar URL to seed the import dialog with. Only persistable + * http(s)/data/blob URLs survive; anything else (e.g. a bare filename or an + * unrenderable symbolic ref) is dropped so the dialog starts blank rather than + * carrying a value the app can't display. + */ +function resolveImportedPersonaAvatarUrl({ + avatarDataUrl, + avatarRef, +}: { + avatarDataUrl?: string | null; + avatarRef?: string | null; +}): string | null { + for (const candidate of [avatarRef, avatarDataUrl]) { + const trimmed = candidate?.trim(); + if (trimmed && isPersistableAvatarUrl(trimmed)) { + return trimmed; + } + } + return null; +} + export type PersonaDialogState = { description: string; initialValues: CreatePersonaInput | UpdatePersonaInput; diff --git a/desktop/src/features/profile/hooks.ts b/desktop/src/features/profile/hooks.ts index 911d002d9..9ba5273bd 100644 --- a/desktop/src/features/profile/hooks.ts +++ b/desktop/src/features/profile/hooks.ts @@ -22,7 +22,6 @@ import type { UsersBatchResponse, } from "@/shared/api/types"; import { useIdentityQuery } from "@/shared/api/hooks"; -import { isGooseAppAvatarRef } from "@/shared/avatars/gooseAppAvatarRefs"; import { getAvatarSnapshotUrl } from "@/shared/lib/animatedAvatar"; import { rewriteRelayUrl } from "@/shared/lib/mediaUrl"; import { @@ -56,9 +55,7 @@ async function persistSelfProfile( profile: Profile, ): Promise { const existing = readSelfProfileCache(relayUrl, pubkey); - const avatarSnapshotUrl = isGooseAppAvatarRef(profile.avatarUrl) - ? null - : getAvatarSnapshotUrl(profile.avatarUrl); + const avatarSnapshotUrl = getAvatarSnapshotUrl(profile.avatarUrl); const fetched = shouldFetchAvatar(profile.avatarUrl, existing) && avatarSnapshotUrl !== null ? await fetchAvatarDataUrl(rewriteRelayUrl(avatarSnapshotUrl)) diff --git a/desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs b/desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs deleted file mode 100644 index ef4b71467..000000000 --- a/desktop/src/shared/avatars/gooseAppAvatarRefs.test.mjs +++ /dev/null @@ -1,58 +0,0 @@ -import assert from "node:assert/strict"; -import test from "node:test"; - -import { - resolveImportedPersonaAvatarUrl, - toGooseAppAvatarRef, -} from "./gooseAppAvatarRefs.ts"; - -test("toGooseAppAvatarRef canonicalizes app-avatar refs", () => { - assert.equal( - toGooseAppAvatarRef("app-avatar:persona-19"), - "app-avatar:persona-19", - ); -}); - -test("toGooseAppAvatarRef ignores filesystem-looking paths", () => { - assert.equal(toGooseAppAvatarRef("./avatars/persona_2.png"), null); -}); - -test("resolveImportedPersonaAvatarUrl prefers app-avatar refs over data URLs", () => { - assert.equal( - resolveImportedPersonaAvatarUrl({ - avatarDataUrl: "https://example.com/avatar.png", - avatarRef: "app-avatar:persona-1", - }), - "app-avatar:persona-1", - ); -}); - -test("resolveImportedPersonaAvatarUrl preserves ordinary image URLs", () => { - assert.equal( - resolveImportedPersonaAvatarUrl({ - avatarDataUrl: "https://example.com/avatar.png", - avatarRef: null, - }), - "https://example.com/avatar.png", - ); -}); - -test("resolveImportedPersonaAvatarUrl does not rewrite remote image URLs", () => { - assert.equal( - resolveImportedPersonaAvatarUrl({ - avatarDataUrl: "https://cdn.example.com/avatars/persona_2.png", - avatarRef: null, - }), - "https://cdn.example.com/avatars/persona_2.png", - ); -}); - -test("resolveImportedPersonaAvatarUrl preserves URL avatar refs", () => { - assert.equal( - resolveImportedPersonaAvatarUrl({ - avatarDataUrl: null, - avatarRef: " https://example.com/persona-avatar.png ", - }), - "https://example.com/persona-avatar.png", - ); -}); diff --git a/desktop/src/shared/avatars/gooseAppAvatarRefs.ts b/desktop/src/shared/avatars/gooseAppAvatarRefs.ts deleted file mode 100644 index 6afef0587..000000000 --- a/desktop/src/shared/avatars/gooseAppAvatarRefs.ts +++ /dev/null @@ -1,69 +0,0 @@ -export const GOOSE_APP_AVATAR_REF_PREFIX = "app-avatar:" as const; - -const APP_AVATAR_ID_PATTERN = /^[a-z0-9][a-z0-9_-]{0,63}$/; - -function cleanAvatarCandidate(value: string): string { - const basename = value - .trim() - .split(/[?#]/)[0] - ?.split(/[\\/]/) - .pop() - ?.replace(/\.(?:apng|gif|heic|heif|jpeg|jpg|mp4|png|webm)$/i, ""); - return (basename ?? "").toLowerCase().replace(/_/g, "-").trim(); -} - -export function parseGooseAppAvatarId( - value: string | null | undefined, -): string | null { - const trimmed = value?.trim(); - if (!trimmed) { - return null; - } - - const refIndex = trimmed.indexOf(GOOSE_APP_AVATAR_REF_PREFIX); - if (refIndex >= 0) { - const rawId = trimmed.slice(refIndex + GOOSE_APP_AVATAR_REF_PREFIX.length); - const id = cleanAvatarCandidate(rawId); - return APP_AVATAR_ID_PATTERN.test(id) ? id : null; - } - - return null; -} - -export function toGooseAppAvatarRef( - value: string | null | undefined, -): string | null { - const id = parseGooseAppAvatarId(value); - return id ? `${GOOSE_APP_AVATAR_REF_PREFIX}${id}` : null; -} - -export function isGooseAppAvatarRef(value: string | null | undefined): boolean { - return toGooseAppAvatarRef(value) !== null; -} - -function isPersistableAvatarUrl(value: string): boolean { - return /^(?:https?:|data:image\/|blob:)/i.test(value); -} - -export function resolveImportedPersonaAvatarUrl({ - avatarDataUrl, - avatarRef, -}: { - avatarDataUrl?: string | null; - avatarRef?: string | null; -}): string | null { - const gooseRef = - toGooseAppAvatarRef(avatarRef) ?? toGooseAppAvatarRef(avatarDataUrl); - if (gooseRef) { - return gooseRef; - } - - for (const candidate of [avatarRef, avatarDataUrl]) { - const trimmedAvatarUrl = candidate?.trim(); - if (trimmedAvatarUrl && isPersistableAvatarUrl(trimmedAvatarUrl)) { - return trimmedAvatarUrl; - } - } - - return null; -} From 5cbdf9e21f701b44eea2a1eb4252c7d4046c6d96 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Wed, 24 Jun 2026 00:08:18 -0700 Subject: [PATCH 5/5] refactor(avatars): drop dead runtimeAvatar allowlist (#1236) Signed-off-by: Taylor Ho Co-authored-by: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> --- desktop/src/shared/lib/runtimeAvatar.ts | 13 ------------- 1 file changed, 13 deletions(-) delete mode 100644 desktop/src/shared/lib/runtimeAvatar.ts diff --git a/desktop/src/shared/lib/runtimeAvatar.ts b/desktop/src/shared/lib/runtimeAvatar.ts deleted file mode 100644 index 3be2f3412..000000000 --- a/desktop/src/shared/lib/runtimeAvatar.ts +++ /dev/null @@ -1,13 +0,0 @@ -const RUNTIME_AVATAR_URLS = new Set([ - "https://goose-docs.ai/img/logo_dark.png", - "https://anthropic.gallerycdn.vsassets.io/extensions/anthropic/claude-code/2.1.77/1773707456892/Microsoft.VisualStudio.Services.Icons.Default", - "https://openai.gallerycdn.vsassets.io/extensions/openai/chatgpt/26.5313.41514/1773706730621/Microsoft.VisualStudio.Services.Icons.Default", - "https://raw.githubusercontent.com/block/buzz/refs/heads/main/crates/buzz-agent/buzz-agent.png", -]); - -export function isKnownRuntimeAvatarUrl( - avatarUrl: string | null | undefined, -): boolean { - const trimmed = avatarUrl?.trim(); - return trimmed ? RUNTIME_AVATAR_URLS.has(trimmed) : false; -}