From ca8874350c986e384fba54968b726cdfd760f45e Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 20 Sep 2026 18:32:46 -0700 Subject: [PATCH] fix(desktop): prune the warm cache when a tile is parked, not only on publish (#77311) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the parked-tile fix never fired on the case it targets: the prune effect re-ran only on [activeSessionId, selectedStoredSessionId, sessionTiles] plus each publish, and parking is driven by pane-lifecycle when focus moves onto another pane — which changes none of those. On an idle window the parked transcript stayed pinned until an unrelated publish happened to come along. - the effect now subscribes to $parkedTileStoredIds and lists it as a dependency, so parking alone drains the cache. - tests: drive the real hook (and therefore the real isReferenced predicate) instead of the helper's bookkeeping — park a settled tile with NO other state change and assert eviction plus releaseSessionTranscript, and park a busy tile under the same pressure and assert it is retained. The bookkeeping-only store test is replaced by these two. Proven red on the pre-fix dependency array. --- .../hooks/use-session-state-cache.test.tsx | 85 ++++++++++++++++++- .../session/hooks/use-session-state-cache.ts | 7 +- .../src/store/session-states-parked.test.ts | 24 ------ 3 files changed, 90 insertions(+), 26 deletions(-) delete mode 100644 apps/desktop/src/store/session-states-parked.test.ts diff --git a/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx b/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx index f7899d7b71..0f16a7b761 100644 --- a/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx @@ -31,7 +31,8 @@ import { clearAllSessionStates, reconcileBusyStatesOnReconnect, type SessionTileDelegate, - setSessionTileDelegate + setSessionTileDelegate, + setZoneParkedTiles } from '@/store/session-states' import { cachedSessionRow } from './use-session-actions/utils' @@ -807,3 +808,85 @@ describe('useSessionStateCache — reconnect busy reconcile (#93059)', () => { expect($sessionStates.get()['runtime-1']?.busy).toBe(false) }) }) + +// #77311: a tile the pane shell PARKED (bounded keep-alive, pane-lifecycle.ts) +// still exists in $sessionTiles, so the warm cache's isReferenced predicate used +// to count it as visible and pin its transcript forever. Parking is the only +// thing that changes here — no navigation, no publish — which is exactly the +// idle-window case the fix has to cover. +describe('useSessionStateCache — parked tiles release their warm transcript (#77311)', () => { + const runtime = 'parked-runtime' + const stored = 'parked-stored' + + /** Fill the cache to its settled-entry cap with unreferenced sessions, so a + * single additional candidate is enough to force one eviction. */ + const fillToCap = (cache: Cache, count: number) => { + for (let i = 0; i < count; i += 1) { + act(() => { + cache.updateSessionState( + `parked-filler-${i}`, + state => ({ ...state, messages: transcriptForCache(`filler-${i}`) }), + `parked-filler-${i}-stored` + ) + }) + } + } + + beforeEach(() => { + clearAllSessionStates() + setActiveSessionId(null) + $sessionTiles.set([{ storedSessionId: stored }]) + }) + + afterEach(() => { + cleanup() + setZoneParkedTiles('parked-zone', []) + $sessionTiles.set([]) + clearAllSessionStates() + setActiveSessionId(null) + }) + + it('evicts and releases a settled parked tile with no other state change', () => { + let cache!: Cache + render( (cache = value)} selectedStoredSessionId={null} />) + + // Seeded first, so it is the least-recently-touched candidate once parked. + act(() => { + cache.updateSessionState(runtime, state => ({ ...state, messages: transcriptForCache('parked') }), stored) + }) + fillToCap(cache, 24) + + // Still on screen as a tile: referenced, therefore not even a candidate. + expect(cache.sessionStateByRuntimeIdRef.current.has(runtime)).toBe(true) + + act(() => setZoneParkedTiles('parked-zone', [stored])) + + expect(cache.sessionStateByRuntimeIdRef.current.has(runtime)).toBe(false) + expect(cache.runtimeIdByStoredSessionIdRef.current.has(stored)).toBe(false) + // releaseSessionTranscript ran: the cheap status projection survives, the + // transcript bytes do not. + expect($sessionStates.get()[runtime]).toMatchObject({ storedSessionId: stored }) + expect($sessionStates.get()[runtime]?.messages).toEqual([]) + }) + + it('keeps a parked tile whose turn is still running', () => { + let cache!: Cache + render( (cache = value)} selectedStoredSessionId={null} />) + + act(() => { + cache.updateSessionState( + runtime, + state => ({ ...state, busy: true, messages: transcriptForCache('parked-busy') }), + stored + ) + }) + // One past the cap, so a drain definitely runs — the busy entry surviving + // it is the assertion, not an absence of pressure. + fillToCap(cache, 25) + + act(() => setZoneParkedTiles('parked-zone', [stored])) + + expect(cache.sessionStateByRuntimeIdRef.current.has(runtime)).toBe(true) + expect($sessionStates.get()[runtime]?.messages.length).toBe(2) + }) +}) diff --git a/apps/desktop/src/app/session/hooks/use-session-state-cache.ts b/apps/desktop/src/app/session/hooks/use-session-state-cache.ts index eb5a4ce589..d923ef8f62 100644 --- a/apps/desktop/src/app/session/hooks/use-session-state-cache.ts +++ b/apps/desktop/src/app/session/hooks/use-session-state-cache.ts @@ -69,6 +69,11 @@ export function useSessionStateCache({ }: SessionStateCacheOptions) { const busy = useStore(PRIMARY_SESSION_VIEW.$busy) const sessionTiles = useStore($sessionTiles) + // Parking is driven by pane-lifecycle when focus moves off a tile (onto a + // terminal pane, say). That changes neither the active/selected ids nor the + // tile list, so without subscribing here an idle window would keep the parked + // transcript pinned until some unrelated publish happened to re-run prune. + const parkedTileStoredIds = useStore($parkedTileStoredIds) const activeSessionIdRef = useRef(activeSessionId) const selectedStoredSessionIdRef = useRef(selectedStoredSessionId) @@ -389,7 +394,7 @@ export function useSessionStateCache({ useEffect(() => { sessionStateCache.prune() - }, [activeSessionId, selectedStoredSessionId, sessionStateCache, sessionTiles]) + }, [activeSessionId, parkedTileStoredIds, selectedStoredSessionId, sessionStateCache, sessionTiles]) const getRuntimeIdForStoredSession = useCallback( (storedSessionId: string): string | null => { diff --git a/apps/desktop/src/store/session-states-parked.test.ts b/apps/desktop/src/store/session-states-parked.test.ts deleted file mode 100644 index 749b3dab39..0000000000 --- a/apps/desktop/src/store/session-states-parked.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -// #77311: a PARKED session tile (pane-lifecycle unmounts it) must not pin its -// transcript — the warm cache treats it as unreferenced and evicts it, and the -// tile's resume path re-hydrates on unpark. In-flight (busy) states are never -// evicted by the cache regardless. -import { describe, expect, it } from 'vitest' - -import { $parkedTileStoredIds, setZoneParkedTiles } from './session-states' - -describe('parked session tiles', () => { - it('unions parked tiles across zones and clears a zone on unmount', () => { - setZoneParkedTiles('zone-a', ['s1', 's2']) - setZoneParkedTiles('zone-b', ['s3']) - expect([...$parkedTileStoredIds.get()].sort()).toEqual(['s1', 's2', 's3']) - - const before = $parkedTileStoredIds.get() - setZoneParkedTiles('zone-b', ['s3']) - expect($parkedTileStoredIds.get()).toBe(before) // no churn on an identical report - - setZoneParkedTiles('zone-a', []) - expect([...$parkedTileStoredIds.get()]).toEqual(['s3']) - setZoneParkedTiles('zone-b', []) - expect($parkedTileStoredIds.get().size).toBe(0) - }) -})