fix(desktop): prune the warm cache when a tile is parked, not only on publish (#77311)
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.
This commit is contained in:
@@ -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(<Harness activeSessionId={null} onReady={value => (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(<Harness activeSessionId={null} onReady={value => (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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<string | null>(activeSessionId)
|
||||
const selectedStoredSessionIdRef = useRef<string | null>(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 => {
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user