From d9b969102c625a3955e2d98c78bf349d136b5cc5 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:36:17 -0700 Subject: [PATCH] test(skills): cover repeated runtime cache eviction --- .../installed-agent-skill-discovery.test.ts | 9 ++--- .../hooks/installed-agent-skill-discovery.ts | 12 ++----- .../src/store/slices/runtime-status.ts | 4 +-- ...ill-discovery-environment-eviction.test.ts | 36 ++++++++++++------- 4 files changed, 28 insertions(+), 33 deletions(-) diff --git a/src/renderer/src/hooks/installed-agent-skill-discovery.test.ts b/src/renderer/src/hooks/installed-agent-skill-discovery.test.ts index 29194ac8588..7dfe26664b7 100644 --- a/src/renderer/src/hooks/installed-agent-skill-discovery.test.ts +++ b/src/renderer/src/hooks/installed-agent-skill-discovery.test.ts @@ -271,8 +271,7 @@ describe('installed agent skill discovery lifecycle', () => { }) it('evicts only the retired environments, leaving other entries warm', async () => { - // Why: #11429 — removal must not degrade into a flush, or every removal - // re-fires each mounted consumer's scan (the storm #7670 fixed). + // Why: a flush would re-fire every mounted consumer's scan. const discover = discoverSkillsForRuntimeTarget discover.mockResolvedValueOnce(result(1)) discover.mockResolvedValueOnce(result(2)) @@ -283,7 +282,6 @@ describe('installed agent skill discovery lifecycle', () => { evictSkillDiscoveryForRuntimeEnvironments(['env-a']) - // env-b and local are still served from cache; only env-a rescans. await expect(discoverInstalledAgentSkills(false, undefined, remote('env-b'))).resolves.toEqual( result(2) ) @@ -297,9 +295,7 @@ describe('installed agent skill discovery lifecycle', () => { }) it('does not let a scan in flight at eviction time repopulate the retired entry', async () => { - // Why: an environment removed mid-scan (ephemeral VM teardown) must not - // resurrect its entry when the scan lands — a re-pair reusing the id would - // then be served the retired peer's skill list. + // Why: VM teardown can race a scan already in flight. const staleScan = deferred() const discover = discoverSkillsForRuntimeTarget discover.mockReturnValueOnce(staleScan.promise) @@ -309,7 +305,6 @@ describe('installed agent skill discovery lifecycle', () => { staleScan.resolve(result(1)) await expect(staleRequest).resolves.toEqual(result(1)) - // The next read must rescan rather than serve the retired result. discover.mockResolvedValueOnce(result(2)) await expect(discoverInstalledAgentSkills(false, undefined, remote('env-a'))).resolves.toEqual( result(2) diff --git a/src/renderer/src/hooks/installed-agent-skill-discovery.ts b/src/renderer/src/hooks/installed-agent-skill-discovery.ts index 189d303de3d..cc17546a51b 100644 --- a/src/renderer/src/hooks/installed-agent-skill-discovery.ts +++ b/src/renderer/src/hooks/installed-agent-skill-discovery.ts @@ -39,14 +39,7 @@ export function invalidateInstalledAgentSkillDiscovery(): void { pendingDiscoverySatisfiesForcedRefreshByTarget.clear() } -/** - * Evict the cached and in-flight scans owned by runtime environments that left - * the saved list (removed, or re-paired under the same id). Keyed, not a flush: - * the local/WSL entries and surviving remotes stay warm, so removal cannot - * re-fire every mounted consumer's scan. Called by the runtime-status slice — - * this module must stay store-free (#6887), so the store subscription lives on - * the consuming side. - */ +// Keyed eviction keeps surviving targets warm and this discovery module store-free. export function evictSkillDiscoveryForRuntimeEnvironments(environmentIds: readonly string[]): void { for (const environmentId of environmentIds) { const key = getRuntimeScopedSkillDiscoveryKey({ kind: 'environment', environmentId }, undefined) @@ -124,8 +117,7 @@ function startInstalledAgentSkillDiscovery( const normalizedTarget = normalizeSkillDiscoveryTarget(target) const discovery = discoverSkillsForRuntimeTarget(runtimeTarget, normalizedTarget) .then((result) => { - // Why: only the still-registered scan may populate — a scan whose runtime - // environment was evicted mid-flight must not resurrect the retired entry. + // Why: an evicted in-flight scan must not resurrect a retired entry. if (generation === discoveryGeneration && pendingDiscoveryByTarget.get(key) === discovery) { writeInstalledAgentSkillDiscoveryCache(key, result) } diff --git a/src/renderer/src/store/slices/runtime-status.ts b/src/renderer/src/store/slices/runtime-status.ts index ee5ef9e6d6d..3417d6bb341 100644 --- a/src/renderer/src/store/slices/runtime-status.ts +++ b/src/renderer/src/store/slices/runtime-status.ts @@ -159,9 +159,7 @@ export const createRuntimeStatusSlice: StateCreator 0) { get().purgeStaleRuntimeHostState?.(retiredEnvironmentIds) - // Why: a retired environment's cached skill scan would otherwise be served - // verbatim if its id is re-paired, and ephemeral VMs mint a fresh id per - // start, leaking one retained entry per start until LRU pressure (#11429). + // Why: ephemeral runtimes otherwise leak one cached scan per retired id. evictSkillDiscoveryForRuntimeEnvironments(retiredEnvironmentIds) } }, diff --git a/src/renderer/src/store/slices/skill-discovery-environment-eviction.test.ts b/src/renderer/src/store/slices/skill-discovery-environment-eviction.test.ts index 171bb9686ba..57f1bac2a79 100644 --- a/src/renderer/src/store/slices/skill-discovery-environment-eviction.test.ts +++ b/src/renderer/src/store/slices/skill-discovery-environment-eviction.test.ts @@ -1,15 +1,3 @@ -/** - * Staleness + memory-leak regression (#11429): runtime-scoped skill discovery - * cache entries must be evicted when their runtime environment leaves the - * saved list. - * - * Remote scans cache under `runtime:` (#6887) in the bounded - * LRU (#7670), but nothing evicted them on environment removal. Ephemeral VMs - * mint a fresh environment id per start, so every start left a permanently - * retained entry until LRU pressure, and re-pairing an id served the removed - * peer's skill list. `setRuntimeEnvironments` now evicts retired ids — - * removed and same-id re-paired alike. - */ import { afterEach, describe, expect, it } from 'vitest' import { create } from 'zustand' import type { PublicKnownRuntimeEnvironment } from '../../../../shared/runtime-environments' @@ -19,6 +7,7 @@ import { resetSkillDiscoveryCacheForTests } from '@/hooks/installed-agent-skill-discovery' import { + getInstalledAgentSkillDiscoveryCacheSizeForTests, hasInstalledAgentSkillDiscoveryCacheEntryForTests, writeInstalledAgentSkillDiscoveryCache } from '@/hooks/installed-agent-skill-discovery-cache' @@ -57,7 +46,6 @@ describe('skill discovery cache evicted on environment removal (#11429)', () => store.getState().setRuntimeEnvironments([env('env-keep')]) expect(hasInstalledAgentSkillDiscoveryCacheEntryForTests(runtimeKey('env-drop'))).toBe(false) - // Surviving remote and local entries stay warm — eviction is keyed, not a flush. expect(hasInstalledAgentSkillDiscoveryCacheEntryForTests(runtimeKey('env-keep'))).toBe(true) expect(hasInstalledAgentSkillDiscoveryCacheEntryForTests('host')).toBe(true) }) @@ -71,4 +59,26 @@ describe('skill discovery cache evicted on environment removal (#11429)', () => expect(hasInstalledAgentSkillDiscoveryCacheEntryForTests(runtimeKey('env-a'))).toBe(false) }) + + it('returns to baseline across repeated ephemeral runtime cycles', () => { + const store = createSliceStore() + const keep = env('env-keep') + store.getState().setRuntimeEnvironments([keep]) + writeInstalledAgentSkillDiscoveryCache(runtimeKey(keep.id), discovery(1)) + writeInstalledAgentSkillDiscoveryCache('host', discovery(2)) + const baseline = getInstalledAgentSkillDiscoveryCacheSizeForTests() + + for (let cycle = 0; cycle < 100; cycle += 1) { + const ephemeral = env(`orca-${cycle}`) + store.getState().setRuntimeEnvironments([keep, ephemeral]) + writeInstalledAgentSkillDiscoveryCache(runtimeKey(ephemeral.id), discovery(cycle + 3)) + expect(getInstalledAgentSkillDiscoveryCacheSizeForTests()).toBe(baseline + 1) + + store.getState().setRuntimeEnvironments([keep]) + expect(getInstalledAgentSkillDiscoveryCacheSizeForTests()).toBe(baseline) + } + + expect(hasInstalledAgentSkillDiscoveryCacheEntryForTests(runtimeKey(keep.id))).toBe(true) + expect(hasInstalledAgentSkillDiscoveryCacheEntryForTests('host')).toBe(true) + }) })