From 12e093483db67304596df404f5be4175d9d96fe6 Mon Sep 17 00:00:00 2001 From: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Date: Tue, 23 Jun 2026 23:43:32 -0700 Subject: [PATCH] refactor(avatars): drop orphaned app-avatar: symbolic-ref machinery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app-avatar: symbolic pointer was meant to power curated, selectable avatar sets — users pick an agent avatar by symbolic ID instead of baking in one-off URLs. But the resolver half was never built / was removed in #1132 (the goose-set collections). What survived was only the parsing + plumbing half: parseGooseAppAvatarId was only ever called by its own validators, and no id->image map existed anywhere. The machinery stored and round-tripped an ID that nothing could render. This removes that orphaned plumbing: - delete the gooseAppAvatarRefs module (parser + validators + tests) - collapse the profile/hooks.ts skip-fetch guard to a plain snapshot - inline personaDialogState.ts import resolution to keep persistable http(s):/data:/blob: URLs only (byte-for-byte identical branch; symbolic refs now drop instead of round-trip — intended, nothing renders them) - retire the dead app-avatar: vocabulary in Rust test fixtures The Rust avatar_ref field stays: it is a generic opaque carrier of the avatar: frontmatter value (it holds real URLs), not specific to app-avatar:. See #1132 for the breadcrumb to re-add selectable avatar sets — that is where the resolver + picker UI should be rebuilt alongside this plumbing. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- 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; -}