From 9c021a6bde53dfe925b828c6cdd4e264f14bb050 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:22:27 -0700 Subject: [PATCH] fix(desktop): Bot Mode pet picker selects locally hatched pets via pet.thumb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting a self-generated pet in the Bot avatar picker always failed with "Could not load that pet — try another", and its tile showed only a name. Locally hatched pets have no petdex manifest entry, so pet.gallery reports an empty spritesheetUrl; the picker cropped frame 0 client-side from that URL and bailed on the empty string. The same raw CDN fetch also lacked a User-Agent, which the petdex CDN rejects with 403 (#90465), so manifest pets could fail on the same path. Route tile rendering and selection through the gateway's pet.thumb RPC, which already backs the Settings pet picker: it crops frame 0 server-side from the installed sheet on disk (or the host-validated CDN URL for uninstalled pets) and returns a same-origin PNG data URI. The client cache is keyed by slug, evicts failures so a blip never poisons a tile, and races a 15s deadline so a hung RPC cannot park a pending promise forever. Based on analysis from PR #90931, whose target (plugin.js) has since been decomposed into pet.tsx. Co-authored-by: m1k3s0 <44042865+m1k3s0@users.noreply.github.com> --- .../src/plugins/hermes-bots/pet.test.tsx | 126 ++++++++---------- apps/desktop/src/plugins/hermes-bots/pet.tsx | 106 +++++++-------- 2 files changed, 102 insertions(+), 130 deletions(-) diff --git a/apps/desktop/src/plugins/hermes-bots/pet.test.tsx b/apps/desktop/src/plugins/hermes-bots/pet.test.tsx index 2206c51952..8fc9dd5ff4 100644 --- a/apps/desktop/src/plugins/hermes-bots/pet.test.tsx +++ b/apps/desktop/src/plugins/hermes-bots/pet.test.tsx @@ -1,15 +1,16 @@ /** - * Pet tiles: frame 0 of a petdex spritesheet, extracted once and cached. + * Pet tiles: frame 0 of a petdex spritesheet, cropped server-side by the + * gateway's `pet.thumb` RPC and cached per slug. * - * A "spritesheet" is the FULL animation sheet (1536×1872 webp, ~2MB, an 8×9 - * grid) — using it directly as an downloads megabytes per tile and shows - * the sheet squashed. So each sheet is fetched once, cropped to frame 0, - * downscaled, and the resulting data URL is cached per URL. + * Why the tile goes through the gateway rather than fetching the CDN sheet + * itself: a locally hatched pet has no manifest entry, so `pet.gallery` reports + * an EMPTY spritesheetUrl — a client-side fetch could never render or select + * it ("Could not load that pet"). `pet.thumb` reads the installed sheet off + * disk, so the slug alone is enough. * - * The regression the cache created: a FAILED fetch was left parked in the - * cache as a resolved-null promise, so one network blip poisoned that pet for - * the rest of the session — the tile never recovered, not even on reopen. A - * failure must be evicted; a success must not be refetched. + * The regression the cache created: a FAILED load was left parked in the + * cache as a resolved-null promise, so one blip poisoned that pet for the rest + * of the session. A failure must be evicted; a success must not be re-requested. */ import { fireEvent, render, waitFor } from '@testing-library/react' @@ -47,13 +48,18 @@ vi.mock('./i18n', () => ({ vi.mock('./shared', () => ({ ID: 'hermes-bots' })) const SHEET = 'https://pets.example/a.webp' +const ICON = 'data:image/png;base64,ok' -/** Record every fetch so the cache behaviour is observable. */ -const fetches: Array<{ init?: RequestInit; url: string }> = [] +/** Every `pet.thumb` call, so the cache behaviour is observable. */ +const thumbs: Array<{ slug: string; url: string }> = [] -function stubFetch(handler: () => Promise) { - vi.stubGlobal('fetch', async (url: string, init?: RequestInit) => { - fetches.push({ init, url }) +function stubThumb(handler: () => Promise<{ dataUri?: string; ok: boolean }>) { + hostMock.request.mockImplementation(async (method: string, params: { slug: string; url: string }) => { + if (method !== 'pet.thumb') { + throw new Error(`unexpected RPC ${method}`) + } + + thumbs.push(params) return handler() }) @@ -67,23 +73,17 @@ async function loadPetTab() { beforeEach(() => { vi.clearAllMocks() - fetches.length = 0 + thumbs.length = 0 useQueryMock.mockReturnValue({ data: { pets: [{ displayName: 'Axolotl', slug: 'axolotl', spritesheetUrl: SHEET }] } }) - vi.stubGlobal('createImageBitmap', async () => ({ close: () => undefined })) - vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue({ - drawImage: () => undefined - } as unknown as CanvasRenderingContext2D) - vi.spyOn(HTMLCanvasElement.prototype, 'toDataURL').mockReturnValue('data:image/png;base64,ok') + stubThumb(async () => ({ ok: true, dataUri: ICON })) }) afterEach(() => { - vi.unstubAllGlobals() vi.restoreAllMocks() }) describe('the pet gallery', () => { it('reserves paint space around boundary tiles inside the bounded scroller', async () => { - stubFetch(async () => ({ blob: async () => new Blob() })) const PetTab = await loadPetTab() const view = render() await waitFor(() => expect(view.container.querySelector('img')).toBeTruthy()) @@ -112,13 +112,12 @@ describe('the pet gallery', () => { })) } }) - stubFetch(async () => ({ blob: async () => new Blob() })) const PetTab = await loadPetTab() const onImage = vi.fn() const view = render() const first = view.getByText('Pet 0').closest('button')! fireEvent.click(first) - await waitFor(() => expect(onImage).toHaveBeenCalledWith('data:image/png;base64,ok')) + await waitFor(() => expect(onImage).toHaveBeenCalledWith(ICON)) const scroller = first.parentElement!.parentElement! Object.defineProperties(scroller, { clientHeight: { value: 220 }, @@ -138,75 +137,56 @@ describe('the pet gallery', () => { }) }) -describe('the sprite-frame cache', () => { - it('never leaves a failed fetch parked in the cache', async () => { - stubFetch(async () => { - throw new Error('network') +describe('the pet thumb cache', () => { + it('selects a locally hatched pet that has no spritesheet URL', async () => { + // Generator-hatched pets are absent from the petdex manifest, so the + // gallery reports spritesheetUrl: "". The gateway crops the installed + // sheet off disk — the slug is the identity, not the URL. + useQueryMock.mockReturnValue({ + data: { pets: [{ displayName: 'Mine', installed: true, slug: 'mine', spritesheetUrl: '' }] } + }) + + const PetTab = await loadPetTab() + const onImage = vi.fn() + const view = render() + + fireEvent.click(view.getByText('Mine').closest('button')!) + await waitFor(() => expect(onImage).toHaveBeenCalledWith(ICON)) + expect(thumbs.length).toBeGreaterThan(0) + expect(thumbs.every(call => call.slug === 'mine')).toBe(true) + expect(hostMock.notify).not.toHaveBeenCalled() + }) + + it('never leaves a failed load parked in the cache', async () => { + stubThumb(async () => { + throw new Error('gateway') }) const PetTab = await loadPetTab() const first = render() - await waitFor(() => expect(fetches).toHaveLength(1)) - // The fetch is abortable: a hung sheet must not hold a slot forever. - expect(fetches[0].init?.signal).toBeTruthy() - + await waitFor(() => expect(thumbs).toHaveLength(1)) first.unmount() const second = render() // Reopening retries rather than serving the poisoned null. - await waitFor(() => expect(fetches).toHaveLength(2)) + await waitFor(() => expect(thumbs).toHaveLength(2)) expect(second.container.querySelector('img')).toBeNull() }) - it('fetches a successful sheet once and reuses the extracted frame', async () => { - stubFetch(async () => ({ blob: async () => new Blob() })) - + it('requests a successful thumb once and reuses it across mounts', async () => { const PetTab = await loadPetTab() const first = render() - await waitFor(() => - expect(first.container.querySelector('img')?.getAttribute('src')).toBe('data:image/png;base64,ok') - ) - expect(fetches).toHaveLength(1) + await waitFor(() => expect(first.container.querySelector('img')?.getAttribute('src')).toBe(ICON)) + expect(thumbs).toHaveLength(1) first.unmount() const second = render() - await waitFor(() => - expect(second.container.querySelector('img')?.getAttribute('src')).toBe('data:image/png;base64,ok') - ) - expect(fetches).toHaveLength(1) - }) - - it('shares one fetch between tiles that point at the same sheet', async () => { - useQueryMock.mockReturnValue({ - data: { - pets: [ - { displayName: 'Axolotl', slug: 'axolotl', spritesheetUrl: SHEET }, - { displayName: 'Axolotl (shiny)', slug: 'axolotl-shiny', spritesheetUrl: SHEET } - ] - } - }) - stubFetch(async () => ({ blob: async () => new Blob() })) - - const PetTab = await loadPetTab() - const { container } = render() - - await waitFor(() => expect(container.querySelectorAll('img')).toHaveLength(2)) - expect(fetches).toHaveLength(1) - }) - - it('does not fetch for a pet with no spritesheet', async () => { - useQueryMock.mockReturnValue({ data: { pets: [{ displayName: 'Ghost', slug: 'ghost', spritesheetUrl: null }] } }) - stubFetch(async () => ({ blob: async () => new Blob() })) - - const PetTab = await loadPetTab() - const { findByText } = render() - - await findByText('Ghost') - expect(fetches).toHaveLength(0) + await waitFor(() => expect(second.container.querySelector('img')?.getAttribute('src')).toBe(ICON)) + expect(thumbs).toHaveLength(1) }) }) diff --git a/apps/desktop/src/plugins/hermes-bots/pet.tsx b/apps/desktop/src/plugins/hermes-bots/pet.tsx index b14fded278..dbbfe1bc91 100644 --- a/apps/desktop/src/plugins/hermes-bots/pet.tsx +++ b/apps/desktop/src/plugins/hermes-bots/pet.tsx @@ -12,80 +12,71 @@ import { ID } from './shared' // ── pet tab: attach a petdex companion that lives beside the avatar ───────── // A petdex "spritesheet" is the FULL animation sheet (1536×1872 webp, ~2MB; -// 8×9 grid of 192×208 frames). Using it as an both downloads megabytes -// per tile and shows the whole sheet squashed. Extract frame 0 once per slug -// via canvas, downscale to 96px, and cache the data URL. Concurrency-capped -// so opening the tab doesn't fire dozens of 2MB fetches at once. -const PET_FRAME_W = 192 -const PET_FRAME_H = 208 +// 8×9 grid of 192×208 frames). Cropping frame 0 client-side meant downloading +// megabytes per tile, a UA-less CDN fetch the petdex CDN rejects with 403 +// (#90465), and it could never serve pets hatched locally — those are absent +// from the petdex manifest, so `pet.gallery` reports an EMPTY spritesheetUrl. +// The gateway's `pet.thumb` crops + downsamples frame 0 server-side (installed +// sheet off disk, else the host-validated CDN URL) and returns a small PNG data +// URI: one path for every pet, same-origin through the authenticated gateway. +// // The gallery is 4500+ pets browsed 24 at a time, and each entry is a decoded // PNG data URL — an unbounded cache holds every pet the user ever scrolled // past for the life of the window. Five pages' worth keeps scrolling back up -// instant; past that a revisit pays the fetch and crop again. -const PET_FRAME_CACHE_MAX = 120 -const petFrameCache = new LruCache>(PET_FRAME_CACHE_MAX) -let petFetchActive = 0 -const petFetchQueue: Array<() => Promise> = [] +// instant; past that a revisit pays the RPC again. +const PET_THUMB_CACHE_MAX = 120 +// A hung gateway call must not park a pending promise in the cache forever. +const PET_THUMB_TIMEOUT_MS = 15000 +const petThumbCache = new LruCache>(PET_THUMB_CACHE_MAX) -function pumpPetQueue() { - while (petFetchActive < 4 && petFetchQueue.length) { - const job = petFetchQueue.shift()! - petFetchActive++ - job().finally(() => { - petFetchActive-- - pumpPetQueue() - }) - } +interface PetThumbResult { + dataUri?: string + ok: boolean } -function petFrameIcon(spriteUrl: null | string | undefined): Promise { - if (!spriteUrl) { +function petThumbIcon(slug: string, spriteUrl: null | string | undefined): Promise { + if (!slug) { return Promise.resolve(null) } - if (!petFrameCache.has(spriteUrl)) { - petFrameCache.set( - spriteUrl, - new Promise(resolve => { - petFetchQueue.push(async () => { - try { - const resp = await fetch(spriteUrl, { - signal: AbortSignal.timeout(15000) - }) + if (!petThumbCache.has(slug)) { + const deadline = new Promise(resolve => setTimeout(() => resolve(null), PET_THUMB_TIMEOUT_MS)) - const blob = await resp.blob() - // Crop frame 0 during decode — never materialize the full sheet. - const bitmap = await createImageBitmap(blob, 0, 0, PET_FRAME_W, PET_FRAME_H) - const canvas = document.createElement('canvas') - canvas.width = 96 - canvas.height = 104 - canvas.getContext('2d')!.drawImage(bitmap, 0, 0, 96, 104) - bitmap.close() - resolve(canvas.toDataURL('image/png')) - } catch { - petFrameCache.delete(spriteUrl) - resolve(null) - } - }) - pumpPetQueue() + const pending = Promise.race([ + host + .request('pet.thumb', { slug, url: spriteUrl || '' }) + .then(result => (result?.ok && result.dataUri ? result.dataUri : null)) + .catch(() => null), + deadline + ]) + .then(icon => { + // Never cache a failure: a transient backend error must not poison + // the tile for the rest of the session. + if (!icon) { + petThumbCache.delete(slug) + } + + return icon }) - ) + + petThumbCache.set(slug, pending) } - return petFrameCache.get(spriteUrl)! + return petThumbCache.get(slug)! } interface PetThumbProps { size?: number + slug: string spriteUrl?: null | string } -/** One pet tile image: frame 0 only, resolved lazily through the cache. */ -function PetThumb({ spriteUrl, size = 40 }: PetThumbProps) { +/** One pet tile image: server-cropped frame 0, resolved lazily through the cache. */ +function PetThumb({ slug, spriteUrl, size = 40 }: PetThumbProps) { const [icon, setIcon] = useState(null) useEffect(() => { let alive = true - petFrameIcon(spriteUrl).then(url => { + petThumbIcon(slug, spriteUrl).then(url => { if (alive) { setIcon(url) } @@ -94,7 +85,7 @@ function PetThumb({ spriteUrl, size = 40 }: PetThumbProps) { return () => { alive = false } - }, [spriteUrl]) + }, [slug, spriteUrl]) if (!icon) { return ( @@ -130,7 +121,7 @@ interface PetGalleryEntry { displayName?: string installed?: boolean slug: string - /** Full animation sheet (1536×1872 webp); frame 0 is cropped out of it. */ + /** Full animation sheet (1536×1872 webp); empty for locally hatched pets. */ spritesheetUrl?: null | string } @@ -244,11 +235,12 @@ export function PetTab({ image, onImage }: PetTabProps) { )} key={pet.slug} onClick={() => { - // The pet IS the profile picture: extract frame 0 - // and hand it to the dialog as the avatar image. + // The pet IS the profile picture: the server-cropped frame 0 + // becomes the dialog's avatar image (works for locally + // hatched pets too — they have no spritesheet URL at all). // Persisted when the user hits Save. setSelectedSlug(pet.slug) - void petFrameIcon(pet.spritesheetUrl).then(icon => { + void petThumbIcon(pet.slug, pet.spritesheetUrl).then(icon => { if (icon) { onImage(icon) } else { @@ -261,7 +253,7 @@ export function PetTab({ image, onImage }: PetTabProps) { }) }} > - + {pet.displayName}