diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index cc6050e87..261ee3db9 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -30,14 +30,14 @@ 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 // self-contained repos_dir functions and their unit tests live in repos.rs; // this is the seam that must stay in nest.rs. Approved override; still queued // to split with the rest of this list. - ["src-tauri/src/managed_agents/nest.rs", 1447], + ["src-tauri/src/managed_agents/nest.rs", 1449], ["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], diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 8a4e8401b..cf17d241c 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -7,16 +7,21 @@ 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, 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, }; +fn trim_optional(value: Option) -> Option { + value + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) +} + /// Query available models from an agent via `buzz-acp models --json`. /// /// Spawns a short-lived subprocess (no relay connection needed). The subprocess @@ -139,7 +144,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 +168,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 +176,13 @@ pub async fn update_managed_agent( name_changed = true; } } + if let Some(avatar_update) = input.avatar_url { + let normalized = trim_optional(avatar_update); + if normalized != record.avatar_url { + record.avatar_url = normalized; + avatar_changed = true; + } + } if let Some(model_update) = input.model { record.model = model_update; } @@ -188,11 +202,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 +259,12 @@ 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 = 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 5112c3ead..aec952bd4 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,13 @@ 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 + } + }) } #[cfg(feature = "mesh-llm")] @@ -145,7 +152,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 +181,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 +391,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 +518,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 { @@ -676,7 +673,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,21 +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` 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. Derived once at creation + /// and stored verbatim; reconciliation republishes it as-is. pub(crate) avatar_url: Option, pub(crate) auth_tag: Option, - /// The agent's pubkey (hex). Needed to update the persisted record during - /// avatar backfill migration. - pub(crate) pubkey: String, - /// The agent's command (e.g. "goose"). Used as fallback when no profile - /// exists on the relay during avatar backfill. - pub(crate) agent_command: String, - /// Persona ID if this agent was created from a persona. Used during avatar - /// backfill to recover the correct avatar from the persona record when the - /// relay profile has been corrupted. - pub(crate) persona_id: Option, } #[tauri::command] @@ -803,9 +789,6 @@ pub async fn start_managed_agent( relay_url: record.relay_url.clone(), avatar_url: record.avatar_url.clone(), auth_tag: record.auth_tag.clone(), - pubkey: record.pubkey.clone(), - agent_command: record.agent_command.clone(), - persona_id: record.persona_id.clone(), }; let target = if record.backend == BackendKind::Local { @@ -814,7 +797,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)?, } }; @@ -866,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(); @@ -875,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}" @@ -888,104 +870,33 @@ pub async fn start_managed_agent( result } -/// Resolve the avatar to backfill for a legacy agent record (pre-PR-921, no -/// stored `avatar_url`). -/// -/// Priority: the persona's avatar wins, because the old reconciliation code -/// could have overwritten the relay's kind:0 `picture` with the command default -/// — making the relay an unreliable source for persona-backed agents. Only fall -/// back to the relay's `picture`, then the command icon, for agents with no -/// persona avatar to recover from. -fn resolve_legacy_avatar( - persona_avatar: Option, - relay_picture: Option, - agent_command: &str, -) -> String { - persona_avatar - .or(relay_picture) - .or_else(|| managed_agent_avatar_url(agent_command)) - .unwrap_or_default() -} - /// Reconcile an agent's kind:0 profile on the relay. /// /// Queries the relay for the agent's existing profile and re-publishes if missing -/// or stale (display_name or picture mismatch). This is fire-and-forget — errors -/// are returned to the caller for logging but never block agent startup. -/// -/// For legacy records (pre-PR-921) where `avatar_url` is `None`, this function -/// backfills via `resolve_legacy_avatar` — preferring the persona record's avatar -/// over the relay's `picture`, since the old code may have corrupted the relay -/// profile — and persists the updated record. After backfill, normal -/// reconciliation proceeds. +/// or stale. This is fire-and-forget — errors are returned to the caller for +/// logging but never block agent startup. /// -/// 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. +/// 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> { 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 existing = query_agent_profile(state, &data.relay_url, agent_pubkey).await?; - let backfilled = resolve_legacy_avatar( - persona_avatar, - existing.as_ref().and_then(|info| info.picture.clone()), - &data.agent_command, - ); - - // Persist the backfilled avatar so this migration only runs once. - if !backfilled.is_empty() { - let _store_guard = state - .managed_agents_store_lock - .lock() - .map_err(|e| e.to_string())?; - let mut records = load_managed_agents(app)?; - if let Some(record) = records.iter_mut().find(|r| r.pubkey == data.pubkey) { - record.avatar_url = Some(backfilled.clone()); - save_managed_agents(app, &records)?; - } - } + // 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(); - 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 +905,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..22bed61fc 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,18 @@ 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); +} + fn profile(name: Option<&str>, picture: Option<&str>) -> crate::relay::AgentProfileInfo { crate::relay::AgentProfileInfo { display_name: name.map(str::to_string), @@ -133,39 +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", - ); - - assert_eq!(resolved, "https://x/persona.png"); -} - -#[test] -fn legacy_avatar_falls_back_to_relay_picture_without_persona() { - let resolved = resolve_legacy_avatar(None, Some("https://x/relay.png".to_string()), "goose"); - - assert_eq!(resolved, "https://x/relay.png"); -} - -#[test] -fn legacy_avatar_falls_back_to_command_icon_when_no_persona_or_relay() { - use crate::managed_agents::managed_agent_avatar_url; - - let resolved = resolve_legacy_avatar(None, None, "goose"); - - assert_eq!(resolved, managed_agent_avatar_url("goose").unwrap()); -} - -#[test] -fn legacy_avatar_empty_when_nothing_resolves() { - let resolved = resolve_legacy_avatar(None, None, "totally-unknown-command"); - - assert!(resolved.is_empty()); -} diff --git a/desktop/src-tauri/src/commands/personas.rs b/desktop/src-tauri/src/commands/personas.rs index 56f6742f4..09b033636 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_carries_opaque_avatar_ref() { + let md = br#"--- +name: fizz +display_name: Fizz +avatar: https://example.com/avatars/fizz.png +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("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.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..e6500d449 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/persona_card_tests.rs @@ -0,0 +1,536 @@ +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_avatar_ref() { + let md = br#"--- +name: goosey +display_name: Goosey +description: Goose internal agent. +avatar: https://example.com/avatars/goosey.png +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("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")); +} + +#[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: https://avatars.example.com/block-md.png +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("https://avatars.example.com/block-md.png") + ); + 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: https://avatars.example.com/fizz.png +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("https://avatars.example.com/fizz.png") + ); + 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/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 3469dc80d..0e4f72375 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -211,9 +211,6 @@ pub async fn restore_managed_agents_on_launch( relay_url: record.relay_url.clone(), avatar_url: record.avatar_url.clone(), auth_tag: record.auth_tag.clone(), - pubkey: record.pubkey.clone(), - agent_command: record.agent_command.clone(), - persona_id: record.persona_id.clone(), }, )) }) @@ -228,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.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/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 82445106c..e3372cdbe 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -233,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")] @@ -427,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>, diff --git a/desktop/src/features/agents/ui/personaDialogState.ts b/desktop/src/features/agents/ui/personaDialogState.ts index d1631dea5..34777387b 100644 --- a/desktop/src/features/agents/ui/personaDialogState.ts +++ b/desktop/src/features/agents/ui/personaDialogState.ts @@ -5,6 +5,32 @@ import type { 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; @@ -90,7 +116,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/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 519f3fd85..f0ce70cf5 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -205,6 +205,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; @@ -220,8 +221,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[]; }; @@ -863,6 +863,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 ?? {}, @@ -878,8 +879,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 1ca74eca3..c4086c2d5 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -285,6 +285,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. */ @@ -449,6 +450,7 @@ export type AgentModelInfo = { export type UpdateManagedAgentInput = { pubkey: string; name?: string; + avatarUrl?: string | null; model?: string | null; systemPrompt?: string | null; mcpToolsets?: string | null;