From 0bc7ca480634d5bcec05cb32813a5e0c6db10a4d Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Sun, 30 Aug 2026 20:42:52 -0600 Subject: [PATCH 01/26] feat(zoo-gateway): cache model catalog per session with ETag revalidation Add a 5-minute in-memory session cache for auth-scoped providers so repeated /models discovery does not hit the gateway on every call, while keeping catalogs off disk and the shared model cache. Pair with gateway If-None-Match support and clear session entries on sign-out. Prune stale eslint suppressions so pre-commit lint passes after removing explicit any from zoo-gateway fetcher tests. --- .../fetchers/__tests__/modelCache.spec.ts | 134 ++++++++++-- .../fetchers/__tests__/zoo-gateway.spec.ts | 147 ++++++++----- src/api/providers/fetchers/modelCache.ts | 204 +++++++++++++++--- src/api/providers/fetchers/zoo-gateway.ts | 32 ++- src/eslint-suppressions.json | 5 - src/services/zoo-code-auth.ts | 3 + 6 files changed, 414 insertions(+), 111 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 108aa1827b..df7474c196 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -83,6 +83,10 @@ const mockGetNanoGptModels = getNanoGptModels as Mock const mockGetMoonshotModels = getMoonshotModels as Mock const mockGetZooGatewayModels = getZooGatewayModels as Mock +function zooGatewayOk(models: Record) { + return { kind: "ok" as const, models } +} + const DUMMY_REQUESTY_KEY = "requesty-key-for-testing" describe("getModels with new GetModelsOptions", () => { @@ -575,9 +579,9 @@ describe("empty cache protection", () => { }) it("re-arms the empty-response throttle after a non-empty response from an auth-scoped provider", async () => { - // zoo-gateway is auth-scoped and skips caching entirely, but a non-empty response - // must still clear the throttle so a later empty response is reported again. - mockGetZooGatewayModels.mockResolvedValueOnce({}) + // zoo-gateway uses the auth session cache, not the shared memory/disk cache. + // A later empty response must still be reported when a forced refresh hits the API. + mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk({})) await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "test-key" }) @@ -591,19 +595,17 @@ describe("empty cache protection", () => { description: "Zoo Gateway model", }, } - mockGetZooGatewayModels.mockResolvedValueOnce(mockModels) + mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk(mockModels)) await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "test-key" }) - // Auth-scoped providers never populate the cache. expect(mockSet).not.toHaveBeenCalled() - mockGetZooGatewayModels.mockResolvedValueOnce({}) + mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk({})) - await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "test-key" }) + const { refreshModels } = await import("../modelCache") + await refreshModels({ provider: providerIdentifiers.zooGateway, apiKey: "test-key" }) - // The throttle should have been re-armed by the non-empty response above, so this - // second empty response is reported again instead of being suppressed. expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) }) }) @@ -930,7 +932,7 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { // signal suppressed by the previous account's throttle entry. const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") - freshMockGetZooGatewayModels.mockResolvedValue({}) + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) @@ -945,7 +947,7 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { // must also be treated as a distinct identity for throttle purposes. const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") - freshMockGetZooGatewayModels.mockResolvedValue({}) + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) await freshGetModels({ provider: providerIdentifiers.zooGateway, @@ -984,8 +986,8 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { }, } - let resolveA: (value: typeof accountAModels) => void - let resolveB: (value: typeof accountBModels) => void + let resolveA: (value: Awaited>) => void + let resolveB: (value: Awaited>) => void freshMockGetZooGatewayModels .mockImplementationOnce( () => @@ -1005,8 +1007,8 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) - resolveB!(accountBModels) - resolveA!(accountAModels) + resolveB!(zooGatewayOk(accountBModels)) + resolveA!(zooGatewayOk(accountAModels)) const [resultA, resultB] = await Promise.all([promiseA, promiseB]) expect(resultA).toEqual(accountAModels) @@ -1017,7 +1019,7 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { // Auth-scoped providers skip dedupedFetch unconditionally, so even two calls carrying // an identical token each fire their own provider fetch -- there is no in-flight sharing // to key correctly or incorrectly for these providers. - freshMockGetZooGatewayModels.mockResolvedValue({}) + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) await Promise.all([ freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "same-token" }), @@ -1204,3 +1206,103 @@ describe("compound cache key derivation across scoping dimensions", () => { expect(cacheKey).toBe("openrouter") }) }) + +describe("auth session cache", () => { + type ModelCacheModule = typeof import("../modelCache") + + let freshGetModels: ModelCacheModule["getModels"] + let freshRefreshModels: ModelCacheModule["refreshModels"] + let freshClearAuthSessionModelsForProvider: ModelCacheModule["clearAuthSessionModelsForProvider"] + let freshMockGetZooGatewayModels: Mock + let mockSet: Mocked["set"] + + const zooModels = { + "anthropic/claude-sonnet-4": { + maxTokens: 64000, + contextWindow: 200000, + supportsPromptCache: true, + }, + } + + beforeEach(async () => { + vi.resetModules() + vi.clearAllMocks() + + const modelCacheModule: ModelCacheModule = await import("../modelCache") + const zooGatewayModule = await import("../zoo-gateway") + const MockedNodeCache = vi.mocked(NodeCache) + const mockCache = vi.mocked(new MockedNodeCache()) + mockCache.get.mockReturnValue(undefined) + mockSet = mockCache.set + + freshGetModels = modelCacheModule.getModels + freshRefreshModels = modelCacheModule.refreshModels + freshClearAuthSessionModelsForProvider = modelCacheModule.clearAuthSessionModelsForProvider + freshMockGetZooGatewayModels = zooGatewayModule.getZooGatewayModels as Mock + }) + + it("reuses in-memory session cache within TTL without refetching", async () => { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + await freshGetModels(options) + + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + }) + + it("keeps prior catalog when a refresh returns empty", async () => { + freshMockGetZooGatewayModels + .mockResolvedValueOnce(zooGatewayOk(zooModels)) + .mockResolvedValueOnce(zooGatewayOk({})) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + const refreshed = await freshRefreshModels(options) + + expect(refreshed).toEqual(zooModels) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + }) + + it("clears session cache on sign-out helper so the next account refetches", async () => { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + + await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a" }) + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-b" }) + + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + }) + + it("never writes auth-scoped catalogs to the shared memory cache", async () => { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + + await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "session-token" }) + + expect(mockSet).not.toHaveBeenCalled() + }) + + it("revalidates with ETag after TTL expires and keeps the prior catalog on 304", async () => { + vi.useFakeTimers() + try { + freshMockGetZooGatewayModels + .mockResolvedValueOnce({ kind: "ok", models: zooModels, etag: '"v1"' }) + .mockResolvedValueOnce({ kind: "not_modified" }) + + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + await freshGetModels(options) + + vi.advanceTimersByTime(5 * 60 * 1000 + 1) + + const second = await freshGetModels(options) + + expect(second).toEqual(zooModels) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(freshMockGetZooGatewayModels).toHaveBeenLastCalledWith( + expect.objectContaining({ ifNoneMatch: '"v1"' }), + ) + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts index ae9bdcc4b1..c2ab113eff 100644 --- a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts +++ b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts @@ -2,6 +2,8 @@ import axios from "axios" +import type { ApiHandlerOptions } from "../../../../shared/api" +import type { VercelAiGatewayModel } from "../vercel-ai-gateway" import { getZooGatewayModels, parseZooGatewayModel } from "../zoo-gateway" vitest.mock("axios") @@ -16,7 +18,22 @@ vitest.mock("../../../../services/zoo-code-auth", () => ({ return profileToken || undefined }), })) -const mockedAxios = axios as any + +const mockAxiosGet = vi.mocked(axios.get) + +function modelsFromResult(result: Awaited>) { + return result.kind === "ok" ? result.models : {} +} + +function gatewayOptions( + overrides: Partial = {}, +): ApiHandlerOptions & { ifNoneMatch?: string } { + return { + zooGatewayBaseUrl: "https://example.test/api/gateway/v1", + zooSessionToken: "zoo_ext_test_token", + ...overrides, + } +} describe("Zoo Gateway Fetchers", () => { beforeEach(() => { @@ -28,6 +45,8 @@ describe("Zoo Gateway Fetchers", () => { const token = "zoo_ext_test_token" const mockResponse = { + status: 200, + headers: { etag: '"catalog-abc"' }, data: { object: "list", data: [ @@ -65,46 +84,65 @@ describe("Zoo Gateway Fetchers", () => { } it("forwards the bearer token and timeout, filters non-language models", async () => { - mockedAxios.get.mockResolvedValueOnce(mockResponse) + mockAxiosGet.mockResolvedValueOnce(mockResponse) - const models = await getZooGatewayModels({ - zooGatewayBaseUrl: baseUrl, - zooSessionToken: token, - } as any) + const result = await getZooGatewayModels(gatewayOptions()) - expect(mockedAxios.get).toHaveBeenCalledWith( + expect(mockAxiosGet).toHaveBeenCalledWith( `${baseUrl}/models`, expect.objectContaining({ headers: expect.objectContaining({ Authorization: `Bearer ${token}` }), timeout: expect.any(Number), + validateStatus: expect.any(Function), }), ) - expect(Object.keys(models)).toHaveLength(1) - expect(models["anthropic/claude-sonnet-4"]).toBeDefined() + expect(result.kind).toBe("ok") + if (result.kind !== "ok") return + expect(Object.keys(result.models)).toHaveLength(1) + expect(result.models["anthropic/claude-sonnet-4"]).toBeDefined() + expect(result.etag).toBe('"catalog-abc"') + }) + + it("sends If-None-Match when provided and returns not_modified on 304", async () => { + mockAxiosGet.mockResolvedValueOnce({ status: 304, headers: {}, data: "" }) + + const result = await getZooGatewayModels( + gatewayOptions({ + ifNoneMatch: '"catalog-abc"', + }), + ) + + expect(mockAxiosGet).toHaveBeenCalledWith( + `${baseUrl}/models`, + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: `Bearer ${token}`, + "If-None-Match": '"catalog-abc"', + }), + }), + ) + expect(result).toEqual({ kind: "not_modified" }) }) it("skips the request and returns {} when no token is available", async () => { - const models = await getZooGatewayModels({ zooGatewayBaseUrl: baseUrl } as any) + const result = await getZooGatewayModels(gatewayOptions({ zooSessionToken: undefined })) - expect(mockedAxios.get).not.toHaveBeenCalled() - expect(models).toEqual({}) + expect(mockAxiosGet).not.toHaveBeenCalled() + expect(modelsFromResult(result)).toEqual({}) }) it("returns {} and never leaks the error object when the request fails", async () => { const consoleErrorSpy = vitest.spyOn(console, "error").mockImplementation(function () {}) - const failure: any = new Error("Network error") - // Simulate axios attaching the request config (which contains the bearer token). - failure.config = { headers: { Authorization: "Bearer should-never-be-logged" } } - failure.code = "ECONNRESET" - failure.response = { status: 502, statusText: "Bad Gateway" } - mockedAxios.get.mockRejectedValueOnce(failure) - - const models = await getZooGatewayModels({ - zooGatewayBaseUrl: baseUrl, - zooSessionToken: token, - } as any) - - expect(models).toEqual({}) + const failure = Object.assign(new Error("Network error"), { + config: { headers: { Authorization: "Bearer should-never-be-logged" } }, + code: "ECONNRESET", + response: { status: 502, statusText: "Bad Gateway" }, + }) + mockAxiosGet.mockRejectedValueOnce(failure) + + const result = await getZooGatewayModels(gatewayOptions()) + + expect(modelsFromResult(result)).toEqual({}) const logged = consoleErrorSpy.mock.calls.map((args) => String(args[0])).join("\n") expect(logged).toContain("status=502") expect(logged).toContain("code=ECONNRESET") @@ -114,7 +152,9 @@ describe("Zoo Gateway Fetchers", () => { }) it("accepts gateway catalog models without created or description (e.g. Bedrock)", async () => { - mockedAxios.get.mockResolvedValueOnce({ + mockAxiosGet.mockResolvedValueOnce({ + status: 200, + headers: {}, data: { object: "list", data: [ @@ -135,24 +175,19 @@ describe("Zoo Gateway Fetchers", () => { }, }) - const models = await getZooGatewayModels({ - zooGatewayBaseUrl: baseUrl, - zooSessionToken: token, - } as any) + const result = await getZooGatewayModels(gatewayOptions()) - expect(Object.keys(models)).toEqual(["anthropic/claude-sonnet-4"]) - expect(models["anthropic/claude-sonnet-4"].description).toBe("Claude Sonnet 4") + expect(Object.keys(modelsFromResult(result))).toEqual(["anthropic/claude-sonnet-4"]) + if (result.kind !== "ok") return + expect(result.models["anthropic/claude-sonnet-4"].description).toBe("Claude Sonnet 4") }) it("returns {} on a structurally broken response instead of throwing", async () => { const consoleErrorSpy = vitest.spyOn(console, "error").mockImplementation(function () {}) - mockedAxios.get.mockResolvedValueOnce({ data: { unexpected: true } }) + mockAxiosGet.mockResolvedValueOnce({ status: 200, headers: {}, data: { unexpected: true } }) - const models = await getZooGatewayModels({ - zooGatewayBaseUrl: baseUrl, - zooSessionToken: token, - } as any) + const result = await getZooGatewayModels(gatewayOptions()) - expect(models).toEqual({}) + expect(modelsFromResult(result)).toEqual({}) expect(consoleErrorSpy).toHaveBeenCalled() consoleErrorSpy.mockRestore() }) @@ -182,25 +217,27 @@ describe("Zoo Gateway Fetchers", () => { }) it("delegates to the vercel-ai-gateway parser", () => { + const model: VercelAiGatewayModel = { + id: "anthropic/claude-sonnet-4", + object: "model", + created: 0, + owned_by: "anthropic", + name: "Claude Sonnet 4", + description: "Sonnet", + context_window: 200000, + max_tokens: 64000, + type: "language", + pricing: { + input: "3.00", + output: "15.00", + input_cache_write: "3.75", + input_cache_read: "0.30", + }, + } + const result = parseZooGatewayModel({ id: "anthropic/claude-sonnet-4", - model: { - id: "anthropic/claude-sonnet-4", - object: "model", - created: 0, - owned_by: "anthropic", - name: "Claude Sonnet 4", - description: "Sonnet", - context_window: 200000, - max_tokens: 64000, - type: "language", - pricing: { - input: "3.00", - output: "15.00", - input_cache_write: "3.75", - input_cache_read: "0.30", - }, - } as any, + model, }) expect(result.contextWindow).toBe(200000) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 50dbe12f6e..e49a92affd 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -108,10 +108,135 @@ const AUTH_SCOPED_PROVIDERS: ReadonlySet = new Set([ providerIdentifiers.kimiCode, ]) +const AUTH_SESSION_TTL_MS = 5 * 60 * 1000 + +type AuthSessionCacheEntry = { + models: ModelRecord + etag?: string + fetchedAt: number +} + +// In-memory, per-session catalog cache for auth-scoped providers. Never written to disk +// and keyed by the same compound identity as getCacheKey (provider + baseUrl + token hash). +const authSessionCache = new Map() + function isAuthScopedProvider(provider: RouterName): boolean { return AUTH_SCOPED_PROVIDERS.has(provider) } +function isAuthSessionFresh(entry: AuthSessionCacheEntry): boolean { + return Date.now() - entry.fetchedAt < AUTH_SESSION_TTL_MS +} + +function getAuthSessionEntry(cacheKey: string): AuthSessionCacheEntry | undefined { + return authSessionCache.get(cacheKey) +} + +function setAuthSessionEntry(cacheKey: string, models: ModelRecord, etag?: string): void { + if (Object.keys(models).length === 0) { + return + } + authSessionCache.set(cacheKey, { models, etag, fetchedAt: Date.now() }) +} + +function touchAuthSessionEntry(cacheKey: string, entry: AuthSessionCacheEntry): void { + authSessionCache.set(cacheKey, { ...entry, fetchedAt: Date.now() }) +} + +function deleteAuthSessionEntry(cacheKey: string): void { + authSessionCache.delete(cacheKey) +} + +export function clearAuthSessionModelsForProvider(provider: RouterName): void { + for (const key of authSessionCache.keys()) { + if (key === provider || key.startsWith(`${provider}:`)) { + authSessionCache.delete(key) + } + } +} + +type AuthScopedFetchResult = { + models: ModelRecord + etag?: string + notModified?: boolean +} + +async function fetchAuthScopedModelsFromProvider( + options: GetModelsOptions, + ifNoneMatch?: string, +): Promise { + const { provider } = options + + switch (provider) { + case providerIdentifiers.zooGateway: { + const result = await getZooGatewayModels({ + zooSessionToken: options.apiKey, + zooGatewayBaseUrl: options.baseUrl, + ifNoneMatch, + }) + if (result.kind === "not_modified") { + return { models: {}, notModified: true } + } + return { models: result.models, etag: result.etag } + } + case providerIdentifiers.kimiCode: + return { models: await getKimiCodeModels(options.apiKey) } + default: { + const exhaustiveCheck: never = provider + throw new Error(`Unknown auth-scoped provider: ${exhaustiveCheck}`) + } + } +} + +async function resolveAuthScopedModels( + options: GetModelsOptions, + { forceRefresh = false }: { forceRefresh?: boolean } = {}, +): Promise { + const { provider } = options + const cacheKey = getCacheKey(options) + const existing = getAuthSessionEntry(cacheKey) + + if (!forceRefresh && existing && isAuthSessionFresh(existing) && Object.keys(existing.models).length > 0) { + return existing.models + } + + try { + const fetched = await fetchAuthScopedModelsFromProvider(options, existing?.etag) + + if (fetched.notModified) { + if (existing && Object.keys(existing.models).length > 0) { + touchAuthSessionEntry(cacheKey, existing) + reportedEmptyModelResponse.delete(cacheKey) + return existing.models + } + } else { + const modelCount = Object.keys(fetched.models).length + if (modelCount > 0) { + setAuthSessionEntry(cacheKey, fetched.models, fetched.etag) + reportedEmptyModelResponse.delete(cacheKey) + return fetched.models + } + + captureModelCacheEmptyResponseOnce(provider, cacheKey, { + context: forceRefresh ? "refreshModels" : "getModels", + hasExistingCache: Boolean(existing && Object.keys(existing.models).length > 0), + ...(existing ? { existingCacheSize: Object.keys(existing.models).length } : {}), + }) + } + + if (existing && Object.keys(existing.models).length > 0) { + return existing.models + } + + return fetched.models + } catch (error) { + if (existing && Object.keys(existing.models).length > 0) { + return existing.models + } + throw error + } +} + // Memoize derived digests so the deliberately-structureless KDF runs at most once per // distinct input per session (getCacheKey / cacheKeyToFilename run on every cache lookup). const cacheDigestCache = new Map() @@ -267,9 +392,18 @@ async function fetchModelsFromProvider(options: GetModelsOptions): Promise const { provider } = options const cacheKey = getCacheKey(options) - const shouldSkipCache = isAuthScopedProvider(provider) + if (isAuthScopedProvider(provider)) { + return resolveAuthScopedModels(options) + } - const models = shouldSkipCache ? undefined : getModelsFromCache(options) + const models = getModelsFromCache(options) if (models) { return models @@ -318,7 +454,7 @@ export const getModels = async (options: GetModelsOptions): Promise // refreshModels() degrades to cached data doesn't surface as a silent stale result to // getModels(), and a fetch failure joined from refreshModels() still re-throws for // getModels() callers. - const sharedFetch = shouldSkipCache ? fetchModelsFromProvider(options) : dedupedFetch(cacheKey, options) + const sharedFetch = dedupedFetch(cacheKey, options) try { const fetched = await sharedFetch @@ -328,16 +464,15 @@ export const getModels = async (options: GetModelsOptions): Promise // as if the provider had no models. Auth-scoped providers skip caching entirely. if (modelCount > 0) { // Clear the empty-response throttle for any non-empty response, including from - // auth-scoped providers that skip caching, so a later empty response is reported again. + // auth-scoped providers that skip disk/memory cache, so a later empty response + // is reported again. reportedEmptyModelResponse.delete(cacheKey) - if (!shouldSkipCache) { - memoryCache.set(cacheKey, fetched) + memoryCache.set(cacheKey, fetched) - await writeModels(cacheKey, fetched).catch((err) => - console.error(`[MODEL_CACHE] Error writing ${cacheKey} models to file cache:`, err), - ) - } + await writeModels(cacheKey, fetched).catch((err) => + console.error(`[MODEL_CACHE] Error writing ${cacheKey} models to file cache:`, err), + ) } else { captureModelCacheEmptyResponseOnce(provider, cacheKey, { context: "getModels", @@ -392,17 +527,24 @@ export const refreshModels = async (options: GetModelsOptions): Promise 0) { + return existing.models + } + return {} + } + } - // De-duplication is skipped for auth-scoped providers because two concurrent calls may - // carry different tokens (e.g., after a sign-out/sign-in within the same session) and we - // must not return the first caller's results to the second caller. - // // Shares the same underlying fetch getModels() uses (see dedupedFetch) so a refreshModels() // call racing a getModels() cache-miss for the same key converges on one provider fetch -- // but each function still applies its own success/failure contract on the result below // rather than sharing that promise's resolution/rejection wholesale. - const sharedFetch = shouldSkipCache ? fetchModelsFromProvider(options) : dedupedFetch(cacheKey, options) + const sharedFetch = dedupedFetch(cacheKey, options) try { // Force fresh API fetch - skip getModelsFromCache() check @@ -410,7 +552,7 @@ export const refreshModels = async (options: GetModelsOptions): Promise - console.error(`[refreshModels] Error writing ${cacheKey} models to disk:`, err), - ) - } + await writeModels(cacheKey, models).catch((err) => + console.error(`[refreshModels] Error writing ${cacheKey} models to disk:`, err), + ) return models } catch (error) { - // Log the error for debugging, then return existing cache if available (graceful degradation). - // For auth-scoped providers (zoo-gateway) we MUST NOT return cached models from a prior - // session, since they could belong to a different user -- return empty instead. console.error(`[refreshModels] Failed to refresh ${cacheKey} models:`, error) - if (shouldSkipCache) { - return {} - } return getModelsFromCache(options) || {} } } @@ -488,6 +622,14 @@ export async function initializeModelCacheRefresh(): Promise { * @param refresh - If true, immediately fetch fresh data from API */ export const flushModels = async (options: GetModelsOptions, refresh: boolean = false): Promise => { + if (isAuthScopedProvider(options.provider)) { + deleteAuthSessionEntry(getCacheKey(options)) + if (refresh) { + await resolveAuthScopedModels(options, { forceRefresh: true }) + } + return + } + if (refresh) { // Don't delete memory cache - let refreshModels atomically replace it // This prevents a race condition where getModels() might be called diff --git a/src/api/providers/fetchers/zoo-gateway.ts b/src/api/providers/fetchers/zoo-gateway.ts index 074ec1dfae..79359747c7 100644 --- a/src/api/providers/fetchers/zoo-gateway.ts +++ b/src/api/providers/fetchers/zoo-gateway.ts @@ -20,29 +20,51 @@ const MODEL_DISCOVERY_TIMEOUT_MS = 15_000 * Fetches models from the Zoo Gateway API. Requires authentication via the zoo_ext_ token. */ -export async function getZooGatewayModels(options?: ApiHandlerOptions): Promise> { +export type ZooGatewayModelsFetchResult = + | { kind: "ok"; models: Record; etag?: string } + | { kind: "not_modified" } + +export async function getZooGatewayModels( + options?: ApiHandlerOptions & { ifNoneMatch?: string }, +): Promise { const models: Record = {} const baseURL = options?.zooGatewayBaseUrl ?? `${getZooCodeBaseUrl()}/api/gateway/v1` const sessionToken = resolveZooGatewaySessionToken(options?.zooSessionToken) if (!sessionToken) { - return models + return { kind: "ok", models } } const headers: Record = { Authorization: `Bearer ${sessionToken}`, } + if (options?.ifNoneMatch) { + headers["If-None-Match"] = options.ifNoneMatch + } try { const response = await axios.get(`${baseURL}/models`, { headers, timeout: MODEL_DISCOVERY_TIMEOUT_MS, + validateStatus: (status) => status === 200 || status === 304, }) + + if (response.status === 304) { + return { kind: "not_modified" } + } + + const etag = + typeof response.headers.etag === "string" + ? response.headers.etag + : typeof response.headers.ETag === "string" + ? response.headers.ETag + : undefined + const result = vercelAiGatewayModelsResponseSchema.safeParse(response.data) if (!result.success) { console.error(`Zoo Gateway models response is invalid ${JSON.stringify(result.error.format())}`) - return models + return { kind: "ok", models } } for (const model of result.data.data) { @@ -56,6 +78,8 @@ export async function getZooGatewayModels(options?: ApiHandlerOptions): Promise< models[id] = parseZooGatewayModel({ id, model }) } + + return { kind: "ok", models, etag } } catch (error) { // Log only safe fields; never serialize the full error object because it // includes request config/headers which carry the bearer session token. @@ -70,7 +94,7 @@ export async function getZooGatewayModels(options?: ApiHandlerOptions): Promise< ) } - return models + return { kind: "ok", models } } /** diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index 0706dbe6fb..0abf2752a7 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -349,11 +349,6 @@ "count": 1 } }, - "api/providers/fetchers/__tests__/zoo-gateway.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 8 - } - }, "api/providers/fetchers/litellm.ts": { "@typescript-eslint/no-explicit-any": { "count": 1 diff --git a/src/services/zoo-code-auth.ts b/src/services/zoo-code-auth.ts index 709cb805c3..be714c596a 100644 --- a/src/services/zoo-code-auth.ts +++ b/src/services/zoo-code-auth.ts @@ -154,6 +154,9 @@ export async function clearZooCodeToken(): Promise { await secretStorage.delete(ZOO_CODE_TOKEN_KEY) _cachedToken = undefined _sessionCleared = true + const { clearAuthSessionModelsForProvider } = await import("../api/providers/fetchers/modelCache") + const { providerIdentifiers } = await import("@roo-code/types") + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) } export function getZooCodeBaseUrl(): string { From 0d675aba366d8bdff8348dbfa01ca145172b011e Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Sun, 30 Aug 2026 21:03:24 -0600 Subject: [PATCH 02/26] fix(zoo-gateway): resolve CI type errors and raise patch coverage Narrow auth-scoped fetch options for exhaustive switching, type zoo-gateway test mocks as ModelRecord, and add session-cache tests for flush, errors, and kimi-code. --- .../fetchers/__tests__/modelCache.spec.ts | 82 ++++++++++++++++++- src/api/providers/fetchers/modelCache.ts | 29 ++++--- 2 files changed, 99 insertions(+), 12 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index df7474c196..1e36a8429a 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -48,6 +48,7 @@ vi.mock("../kenari") vi.mock("../nanogpt") vi.mock("../moonshot") vi.mock("../zoo-gateway") +vi.mock("../kimi-code") // Mock ContextProxy with a simple static instance vi.mock("../../../core/config/ContextProxy", () => ({ @@ -62,6 +63,7 @@ vi.mock("../../../core/config/ContextProxy", () => ({ // Then imports import type { Mock, Mocked } from "vitest" +import type { ModelRecord } from "@roo-code/types" import { providerIdentifiers } from "@roo-code/types" import * as fsSync from "fs" import NodeCache from "node-cache" @@ -74,6 +76,7 @@ import { getKenariModels } from "../kenari" import { getNanoGptModels } from "../nanogpt" import { getMoonshotModels } from "../moonshot" import { getZooGatewayModels } from "../zoo-gateway" +import { getKimiCodeModels } from "../kimi-code" const mockGetLiteLLMModels = getLiteLLMModels as Mock const mockGetOpenRouterModels = getOpenRouterModels as Mock @@ -82,8 +85,9 @@ const mockGetKenariModels = getKenariModels as Mock const mockGetNanoGptModels = getNanoGptModels as Mock const mockGetMoonshotModels = getMoonshotModels as Mock const mockGetZooGatewayModels = getZooGatewayModels as Mock +const mockGetKimiCodeModels = getKimiCodeModels as Mock -function zooGatewayOk(models: Record) { +function zooGatewayOk(models: ModelRecord) { return { kind: "ok" as const, models } } @@ -1212,11 +1216,13 @@ describe("auth session cache", () => { let freshGetModels: ModelCacheModule["getModels"] let freshRefreshModels: ModelCacheModule["refreshModels"] + let freshFlushModels: ModelCacheModule["flushModels"] let freshClearAuthSessionModelsForProvider: ModelCacheModule["clearAuthSessionModelsForProvider"] let freshMockGetZooGatewayModels: Mock + let freshMockGetKimiCodeModels: Mock let mockSet: Mocked["set"] - const zooModels = { + const zooModels: ModelRecord = { "anthropic/claude-sonnet-4": { maxTokens: 64000, contextWindow: 200000, @@ -1230,6 +1236,7 @@ describe("auth session cache", () => { const modelCacheModule: ModelCacheModule = await import("../modelCache") const zooGatewayModule = await import("../zoo-gateway") + const kimiCodeModule = await import("../kimi-code") const MockedNodeCache = vi.mocked(NodeCache) const mockCache = vi.mocked(new MockedNodeCache()) mockCache.get.mockReturnValue(undefined) @@ -1237,8 +1244,10 @@ describe("auth session cache", () => { freshGetModels = modelCacheModule.getModels freshRefreshModels = modelCacheModule.refreshModels + freshFlushModels = modelCacheModule.flushModels freshClearAuthSessionModelsForProvider = modelCacheModule.clearAuthSessionModelsForProvider freshMockGetZooGatewayModels = zooGatewayModule.getZooGatewayModels as Mock + freshMockGetKimiCodeModels = kimiCodeModule.getKimiCodeModels as Mock }) it("reuses in-memory session cache within TTL without refetching", async () => { @@ -1305,4 +1314,73 @@ describe("auth session cache", () => { vi.useRealTimers() } }) + + it("flushModels clears session cache and can force a refetch", async () => { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + + await freshFlushModels(options, true) + + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + }) + + it("returns the prior session catalog when a fetch throws", async () => { + vi.useFakeTimers() + try { + freshMockGetZooGatewayModels + .mockResolvedValueOnce(zooGatewayOk(zooModels)) + .mockRejectedValueOnce(new Error("network down")) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + vi.advanceTimersByTime(5 * 60 * 1000 + 1) + const second = await freshGetModels(options) + + expect(second).toEqual(zooModels) + } finally { + vi.useRealTimers() + } + }) + + it("refreshModels returns an empty catalog when refresh throws and no session cache exists", async () => { + freshMockGetZooGatewayModels.mockRejectedValue(new Error("network down")) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + const refreshed = await freshRefreshModels(options) + + expect(refreshed).toEqual({}) + }) + + it("refreshModels keeps the prior session catalog when refresh throws", async () => { + freshMockGetZooGatewayModels + .mockResolvedValueOnce(zooGatewayOk(zooModels)) + .mockRejectedValueOnce(new Error("network down")) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + const refreshed = await freshRefreshModels(options) + + expect(refreshed).toEqual(zooModels) + }) + + it("caches kimi-code catalogs in the session store", async () => { + const kimiModels: ModelRecord = { + "kimi-for-coding": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + }, + } + freshMockGetKimiCodeModels.mockResolvedValue(kimiModels) + const options = { provider: providerIdentifiers.kimiCode, apiKey: "kimi-session" } + + await freshGetModels(options) + await freshGetModels(options) + + expect(freshMockGetKimiCodeModels).toHaveBeenCalledTimes(1) + expect(mockSet).not.toHaveBeenCalled() + }) }) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index e49a92affd..2ab675a671 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -108,6 +108,10 @@ const AUTH_SCOPED_PROVIDERS: ReadonlySet = new Set([ providerIdentifiers.kimiCode, ]) +type AuthScopedProvider = typeof providerIdentifiers.zooGateway | typeof providerIdentifiers.kimiCode + +type AuthScopedGetModelsOptions = Extract + const AUTH_SESSION_TTL_MS = 5 * 60 * 1000 type AuthSessionCacheEntry = { @@ -120,10 +124,18 @@ type AuthSessionCacheEntry = { // and keyed by the same compound identity as getCacheKey (provider + baseUrl + token hash). const authSessionCache = new Map() -function isAuthScopedProvider(provider: RouterName): boolean { +function isAuthScopedProvider(provider: RouterName): provider is AuthScopedProvider { return AUTH_SCOPED_PROVIDERS.has(provider) } +function assertAuthScopedGetModelsOptions(options: GetModelsOptions): AuthScopedGetModelsOptions { + if (!isAuthScopedProvider(options.provider)) { + throw new Error(`Expected auth-scoped provider, got ${options.provider}`) + } + // Runtime guard above; TS cannot narrow the GetModelsOptions discriminated union here. + return options as AuthScopedGetModelsOptions +} + function isAuthSessionFresh(entry: AuthSessionCacheEntry): boolean { return Date.now() - entry.fetchedAt < AUTH_SESSION_TTL_MS } @@ -162,12 +174,10 @@ type AuthScopedFetchResult = { } async function fetchAuthScopedModelsFromProvider( - options: GetModelsOptions, + options: AuthScopedGetModelsOptions, ifNoneMatch?: string, ): Promise { - const { provider } = options - - switch (provider) { + switch (options.provider) { case providerIdentifiers.zooGateway: { const result = await getZooGatewayModels({ zooSessionToken: options.apiKey, @@ -181,10 +191,6 @@ async function fetchAuthScopedModelsFromProvider( } case providerIdentifiers.kimiCode: return { models: await getKimiCodeModels(options.apiKey) } - default: { - const exhaustiveCheck: never = provider - throw new Error(`Unknown auth-scoped provider: ${exhaustiveCheck}`) - } } } @@ -201,7 +207,10 @@ async function resolveAuthScopedModels( } try { - const fetched = await fetchAuthScopedModelsFromProvider(options, existing?.etag) + const fetched = await fetchAuthScopedModelsFromProvider( + assertAuthScopedGetModelsOptions(options), + existing?.etag, + ) if (fetched.notModified) { if (existing && Object.keys(existing.models).length > 0) { From 9de812be562c59aa1df5f7963bbfb512a56a701c Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Sun, 30 Aug 2026 21:28:45 -0600 Subject: [PATCH 03/26] test(zoo-gateway): cover remaining session cache and ETag paths Add tests for flush without refresh, 304 without cache, compound key clearing, capitalized ETag headers, validateStatus, and sign-out cache wipe. --- .../fetchers/__tests__/modelCache.spec.ts | 72 +++++++++++++++++++ .../fetchers/__tests__/zoo-gateway.spec.ts | 20 ++++++ src/services/__tests__/zoo-code-auth.test.ts | 13 ++++ 3 files changed, 105 insertions(+) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 1e36a8429a..a7d9dbfc4b 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -1383,4 +1383,76 @@ describe("auth session cache", () => { expect(freshMockGetKimiCodeModels).toHaveBeenCalledTimes(1) expect(mockSet).not.toHaveBeenCalled() }) + + it("flushModels without refresh evicts the session cache until the next fetch", async () => { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + await freshFlushModels(options) + await freshGetModels(options) + + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + }) + + it("returns an empty catalog when revalidation is 304 and no session cache exists", async () => { + vi.useFakeTimers() + try { + freshMockGetZooGatewayModels + .mockResolvedValueOnce(zooGatewayOk(zooModels)) + .mockResolvedValueOnce({ kind: "not_modified" }) + + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + await freshGetModels(options) + await freshFlushModels(options) + + vi.advanceTimersByTime(5 * 60 * 1000 + 1) + + const result = await freshGetModels(options) + + expect(result).toEqual({}) + } finally { + vi.useRealTimers() + } + }) + + it("returns an empty catalog on the first empty zoo-gateway response", async () => { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + const result = await freshGetModels(options) + + expect(result).toEqual({}) + }) + + it("clearAuthSessionModelsForProvider removes compound cache keys", async () => { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + + await freshGetModels({ + provider: providerIdentifiers.zooGateway, + apiKey: "account-a", + baseUrl: "https://gateway-a.test/v1", + }) + await freshGetModels({ + provider: providerIdentifiers.zooGateway, + apiKey: "account-b", + baseUrl: "https://gateway-b.test/v1", + }) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + + await freshGetModels({ + provider: providerIdentifiers.zooGateway, + apiKey: "account-a", + baseUrl: "https://gateway-a.test/v1", + }) + await freshGetModels({ + provider: providerIdentifiers.zooGateway, + apiKey: "account-b", + baseUrl: "https://gateway-b.test/v1", + }) + + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(4) + }) }) diff --git a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts index c2ab113eff..464ea43b67 100644 --- a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts +++ b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts @@ -88,6 +88,13 @@ describe("Zoo Gateway Fetchers", () => { const result = await getZooGatewayModels(gatewayOptions()) + const axiosConfig = mockAxiosGet.mock.calls[0]?.[1] as + | { validateStatus?: (status: number) => boolean } + | undefined + expect(axiosConfig?.validateStatus?.(200)).toBe(true) + expect(axiosConfig?.validateStatus?.(304)).toBe(true) + expect(axiosConfig?.validateStatus?.(500)).toBe(false) + expect(mockAxiosGet).toHaveBeenCalledWith( `${baseUrl}/models`, expect.objectContaining({ @@ -103,6 +110,19 @@ describe("Zoo Gateway Fetchers", () => { expect(result.etag).toBe('"catalog-abc"') }) + it("reads ETag from the capitalized response header", async () => { + mockAxiosGet.mockResolvedValueOnce({ + ...mockResponse, + headers: { ETag: '"catalog-capital"' }, + }) + + const result = await getZooGatewayModels(gatewayOptions()) + + expect(result.kind).toBe("ok") + if (result.kind !== "ok") return + expect(result.etag).toBe('"catalog-capital"') + }) + it("sends If-None-Match when provided and returns not_modified on 304", async () => { mockAxiosGet.mockResolvedValueOnce({ status: 304, headers: {}, data: "" }) diff --git a/src/services/__tests__/zoo-code-auth.test.ts b/src/services/__tests__/zoo-code-auth.test.ts index c08fcdedf3..79972ea924 100644 --- a/src/services/__tests__/zoo-code-auth.test.ts +++ b/src/services/__tests__/zoo-code-auth.test.ts @@ -1,6 +1,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" import * as vscode from "vscode" +import { providerIdentifiers } from "@roo-code/types" +import * as modelCache from "../../api/providers/fetchers/modelCache" import { clearZooCodeToken, clearZooCodeUserInfo, @@ -168,6 +170,17 @@ describe("zoo-code-auth", () => { expect(getCachedZooCodeToken()).toBe("") }) + + it("clears the zoo-gateway session model cache", async () => { + const clearSessionSpy = vi.spyOn(modelCache, "clearAuthSessionModelsForProvider") + await initZooCodeAuth(mockContext) + await setZooCodeToken("zoo_ext_test_token") + + await clearZooCodeToken() + + expect(clearSessionSpy).toHaveBeenCalledWith(providerIdentifiers.zooGateway) + clearSessionSpy.mockRestore() + }) }) describe("getZooCodeBaseUrl", () => { From 236196ff9f68ada7f0db319cc25d2c68a66ee916 Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Sun, 30 Aug 2026 21:32:40 -0600 Subject: [PATCH 04/26] fix(zoo-gateway): address CodeRabbit review on session cache lifecycle Bound and prune auth-session catalogs, clear Kimi Code cache on sign-out, keep flushModels refresh non-throwing, and tighten regression tests. --- .../fetchers/__tests__/modelCache.spec.ts | 38 +++++++++++++++++-- .../fetchers/__tests__/zoo-gateway.spec.ts | 5 +++ src/api/providers/fetchers/modelCache.ts | 37 +++++++++++++++++- .../kimi-code/__tests__/oauth.spec.ts | 6 +++ src/integrations/kimi-code/oauth.ts | 3 ++ 5 files changed, 85 insertions(+), 4 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index a7d9dbfc4b..102247c01b 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -1273,12 +1273,13 @@ describe("auth session cache", () => { expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) - it("clears session cache on sign-out helper so the next account refetches", async () => { + it("clears session cache on sign-out helper so the same identity refetches", async () => { freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a" }) + await freshGetModels(options) freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) - await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-b" }) + await freshGetModels(options) expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) @@ -1455,4 +1456,35 @@ describe("auth session cache", () => { expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(4) }) + + it("flushModels with refresh logs and does not throw when the provider fetch fails", async () => { + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + freshMockGetZooGatewayModels.mockRejectedValue(new Error("network down")) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await expect(freshFlushModels(options, true)).resolves.toBeUndefined() + expect(consoleSpy).toHaveBeenCalled() + consoleSpy.mockRestore() + }) + + it("prunes session entries older than twice the TTL when maintaining the cache", async () => { + vi.useFakeTimers() + try { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const first = { provider: providerIdentifiers.zooGateway, apiKey: "session-a" } + const second = { provider: providerIdentifiers.zooGateway, apiKey: "session-b" } + + await freshGetModels(first) + vi.advanceTimersByTime(5 * 60 * 1000 * 2 + 1) + await freshGetModels(second) + + freshMockGetZooGatewayModels.mockClear() + freshMockGetZooGatewayModels.mockResolvedValueOnce({ kind: "not_modified" }) + const result = await freshGetModels(first) + + expect(result).toEqual({}) + } finally { + vi.useRealTimers() + } + }) }) diff --git a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts index 464ea43b67..4698a78c55 100644 --- a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts +++ b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts @@ -142,6 +142,11 @@ describe("Zoo Gateway Fetchers", () => { }), ) expect(result).toEqual({ kind: "not_modified" }) + + const validateStatus = mockAxiosGet.mock.calls[0]?.[1]?.validateStatus + expect(validateStatus?.(304)).toBe(true) + expect(validateStatus?.(200)).toBe(true) + expect(validateStatus?.(500)).toBe(false) }) it("skips the request and returns {} when no token is available", async () => { diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 2ab675a671..58ea98c857 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -113,6 +113,7 @@ type AuthScopedProvider = typeof providerIdentifiers.zooGateway | typeof provide type AuthScopedGetModelsOptions = Extract const AUTH_SESSION_TTL_MS = 5 * 60 * 1000 +const AUTH_SESSION_MAX_ENTRIES = 64 type AuthSessionCacheEntry = { models: ModelRecord @@ -140,6 +141,34 @@ function isAuthSessionFresh(entry: AuthSessionCacheEntry): boolean { return Date.now() - entry.fetchedAt < AUTH_SESSION_TTL_MS } +function pruneExpiredAuthSessionEntries(): void { + const staleCutoffMs = AUTH_SESSION_TTL_MS * 2 + const now = Date.now() + for (const [key, entry] of authSessionCache) { + if (now - entry.fetchedAt >= staleCutoffMs) { + authSessionCache.delete(key) + } + } +} + +function enforceAuthSessionCacheBound(): void { + pruneExpiredAuthSessionEntries() + while (authSessionCache.size > AUTH_SESSION_MAX_ENTRIES) { + let oldestKey: string | undefined + let oldestFetchedAt = Infinity + for (const [key, entry] of authSessionCache) { + if (entry.fetchedAt < oldestFetchedAt) { + oldestFetchedAt = entry.fetchedAt + oldestKey = key + } + } + if (!oldestKey) { + break + } + authSessionCache.delete(oldestKey) + } +} + function getAuthSessionEntry(cacheKey: string): AuthSessionCacheEntry | undefined { return authSessionCache.get(cacheKey) } @@ -149,10 +178,12 @@ function setAuthSessionEntry(cacheKey: string, models: ModelRecord, etag?: strin return } authSessionCache.set(cacheKey, { models, etag, fetchedAt: Date.now() }) + enforceAuthSessionCacheBound() } function touchAuthSessionEntry(cacheKey: string, entry: AuthSessionCacheEntry): void { authSessionCache.set(cacheKey, { ...entry, fetchedAt: Date.now() }) + enforceAuthSessionCacheBound() } function deleteAuthSessionEntry(cacheKey: string): void { @@ -634,7 +665,11 @@ export const flushModels = async (options: GetModelsOptions, refresh: boolean = if (isAuthScopedProvider(options.provider)) { deleteAuthSessionEntry(getCacheKey(options)) if (refresh) { - await resolveAuthScopedModels(options, { forceRefresh: true }) + try { + await resolveAuthScopedModels(options, { forceRefresh: true }) + } catch (error) { + console.error(`[flushModels] Failed to refresh auth-scoped ${getCacheKey(options)} models:`, error) + } } return } diff --git a/src/integrations/kimi-code/__tests__/oauth.spec.ts b/src/integrations/kimi-code/__tests__/oauth.spec.ts index 960777e69c..cdcdbb1105 100644 --- a/src/integrations/kimi-code/__tests__/oauth.spec.ts +++ b/src/integrations/kimi-code/__tests__/oauth.spec.ts @@ -1,3 +1,6 @@ +import { providerIdentifiers } from "@roo-code/types" + +import * as modelCache from "../../../api/providers/fetchers/modelCache" import { KIMI_CODE_OAUTH_CONFIG, KimiCodeOAuthManager } from "../oauth" const createContext = () => { @@ -95,6 +98,7 @@ describe("KimiCodeOAuthManager", () => { }) it("clears credentials and cancels authorization", async () => { + const clearSessionSpy = vi.spyOn(modelCache, "clearAuthSessionModelsForProvider") const { context, values } = createContext() values.set( "kimi-code-oauth-credentials", @@ -105,6 +109,8 @@ describe("KimiCodeOAuthManager", () => { await manager.clearCredentials() expect(values.has("kimi-code-oauth-credentials")).toBe(false) expect(await manager.isAuthenticated()).toBe(false) + expect(clearSessionSpy).toHaveBeenCalledWith(providerIdentifiers.kimiCode) + clearSessionSpy.mockRestore() }) it("does not restore credentials when sign-out races an in-flight refresh", async () => { diff --git a/src/integrations/kimi-code/oauth.ts b/src/integrations/kimi-code/oauth.ts index cd28df8b04..df3e0884ca 100644 --- a/src/integrations/kimi-code/oauth.ts +++ b/src/integrations/kimi-code/oauth.ts @@ -232,6 +232,9 @@ export class KimiCodeOAuthManager { await this.context?.secrets.delete(KIMI_CODE_CREDENTIALS_KEY) this.credentials = null this.state = { status: "idle" } + const { clearAuthSessionModelsForProvider } = await import("../../api/providers/fetchers/modelCache") + const { providerIdentifiers } = await import("@roo-code/types") + clearAuthSessionModelsForProvider(providerIdentifiers.kimiCode) } async isAuthenticated(): Promise { From e1d43096b9f5712639db44a9dd427d6ef3590205 Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Sun, 30 Aug 2026 22:27:46 -0600 Subject: [PATCH 05/26] fix: keep auth session catalog during forced model refresh Delete the session cache only when flushModels is called without refresh so a failed or empty refetch can still fall back to the prior catalog. --- .../fetchers/__tests__/modelCache.spec.ts | 30 ++++++++++++++++++- .../fetchers/__tests__/zoo-gateway.spec.ts | 4 +-- src/api/providers/fetchers/modelCache.ts | 3 +- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 102247c01b..307c544011 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -1457,7 +1457,21 @@ describe("auth session cache", () => { expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(4) }) - it("flushModels with refresh logs and does not throw when the provider fetch fails", async () => { + it("flushModels with refresh keeps the prior catalog when the provider fetch fails", async () => { + freshMockGetZooGatewayModels + .mockResolvedValueOnce(zooGatewayOk(zooModels)) + .mockRejectedValueOnce(new Error("network down")) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + await expect(freshFlushModels(options, true)).resolves.toBeUndefined() + + const afterFailedRefresh = await freshGetModels(options) + expect(afterFailedRefresh).toEqual(zooModels) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + }) + + it("flushModels with refresh logs and does not throw when refresh fails with no session cache", async () => { const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}) freshMockGetZooGatewayModels.mockRejectedValue(new Error("network down")) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } @@ -1467,6 +1481,20 @@ describe("auth session cache", () => { consoleSpy.mockRestore() }) + it("flushModels with refresh keeps the prior catalog when refresh returns empty", async () => { + freshMockGetZooGatewayModels + .mockResolvedValueOnce(zooGatewayOk(zooModels)) + .mockResolvedValueOnce(zooGatewayOk({})) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + await freshFlushModels(options, true) + + const afterEmptyRefresh = await freshGetModels(options) + expect(afterEmptyRefresh).toEqual(zooModels) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + }) + it("prunes session entries older than twice the TTL when maintaining the cache", async () => { vi.useFakeTimers() try { diff --git a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts index 4698a78c55..2ab6dab162 100644 --- a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts +++ b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts @@ -88,9 +88,7 @@ describe("Zoo Gateway Fetchers", () => { const result = await getZooGatewayModels(gatewayOptions()) - const axiosConfig = mockAxiosGet.mock.calls[0]?.[1] as - | { validateStatus?: (status: number) => boolean } - | undefined + const axiosConfig = mockAxiosGet.mock.calls[0]?.[1] expect(axiosConfig?.validateStatus?.(200)).toBe(true) expect(axiosConfig?.validateStatus?.(304)).toBe(true) expect(axiosConfig?.validateStatus?.(500)).toBe(false) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 58ea98c857..3eec78c64a 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -663,13 +663,14 @@ export async function initializeModelCacheRefresh(): Promise { */ export const flushModels = async (options: GetModelsOptions, refresh: boolean = false): Promise => { if (isAuthScopedProvider(options.provider)) { - deleteAuthSessionEntry(getCacheKey(options)) if (refresh) { try { await resolveAuthScopedModels(options, { forceRefresh: true }) } catch (error) { console.error(`[flushModels] Failed to refresh auth-scoped ${getCacheKey(options)} models:`, error) } + } else { + deleteAuthSessionEntry(getCacheKey(options)) } return } From 8f9f7e9e0d20749f133795c4b0cb5cd3782a049e Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:51:50 -0600 Subject: [PATCH 06/26] fix(zoo-gateway): surface reasoning traces from delta stream --- src/api/providers/zoo-gateway.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/api/providers/zoo-gateway.ts b/src/api/providers/zoo-gateway.ts index 4ff059df61..d61cab3a20 100644 --- a/src/api/providers/zoo-gateway.ts +++ b/src/api/providers/zoo-gateway.ts @@ -18,6 +18,7 @@ import { t } from "../../i18n" import { ApiStream } from "../transform/stream" import { convertToOpenAiMessages } from "../transform/openai-format" import { addCacheBreakpoints } from "../transform/caching/vercel-ai-gateway" +import { extractReasoningFromDelta } from "./utils/extract-reasoning" import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" import { NOT_PROVIDED } from "./constants" @@ -233,6 +234,12 @@ export class ZooGatewayHandler extends RouterProvider implements SingleCompletio } const delta = chunk.choices[0]?.delta + + const reasoningText = extractReasoningFromDelta(delta) + if (reasoningText) { + yield { type: "reasoning", text: reasoningText } + } + if (delta?.content) { yield { type: "text", From 04b896ab0066fb08c5b2e10d8928f8f216867aeb Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:18:52 -0600 Subject: [PATCH 07/26] fix(zoo-gateway): address edelauna review feedback --- .../fetchers/__tests__/modelCache.spec.ts | 33 +++++- .../fetchers/__tests__/zoo-gateway.spec.ts | 19 +++- src/api/providers/fetchers/modelCache.ts | 106 ++++++++++-------- src/api/providers/fetchers/zoo-gateway.ts | 7 +- 4 files changed, 106 insertions(+), 59 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 307c544011..58c8c3f0f5 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -1019,10 +1019,9 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { expect(resultB).toEqual(accountBModels) }) - it("never deduplicates concurrent zoo-gateway fetches, even for the same token", async () => { - // Auth-scoped providers skip dedupedFetch unconditionally, so even two calls carrying - // an identical token each fire their own provider fetch -- there is no in-flight sharing - // to key correctly or incorrectly for these providers. + it("deduplicates concurrent zoo-gateway fetches for the same session token", async () => { + // Auth-scoped providers use inFlightAuthScopedFetch to coalesce concurrent calls + // for the same cache key so two panels opening simultaneously share one fetch. freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) await Promise.all([ @@ -1030,7 +1029,7 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "same-token" }), ]) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) }) }) @@ -1292,6 +1291,17 @@ describe("auth session cache", () => { expect(mockSet).not.toHaveBeenCalled() }) + it("concurrent getModels calls for the same session key share one fetch", async () => { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + const [first, second] = await Promise.all([freshGetModels(options), freshGetModels(options)]) + + expect(first).toEqual(zooModels) + expect(second).toEqual(zooModels) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + }) + it("revalidates with ETag after TTL expires and keeps the prior catalog on 304", async () => { vi.useFakeTimers() try { @@ -1311,6 +1321,12 @@ describe("auth session cache", () => { expect(freshMockGetZooGatewayModels).toHaveBeenLastCalledWith( expect.objectContaining({ ifNoneMatch: '"v1"' }), ) + + // A third call within the refreshed TTL window must not trigger another fetch, + // proving touchAuthSessionEntry actually updated fetchedAt. + const third = await freshGetModels(options) + expect(third).toEqual(zooModels) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) } finally { vi.useRealTimers() } @@ -1346,6 +1362,13 @@ describe("auth session cache", () => { } }) + it("getModels throws on the first call when the provider fetch fails and there is no prior cache", async () => { + freshMockGetZooGatewayModels.mockRejectedValue(new Error("network down")) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await expect(freshGetModels(options)).rejects.toThrow("network down") + }) + it("refreshModels returns an empty catalog when refresh throws and no session cache exists", async () => { freshMockGetZooGatewayModels.mockRejectedValue(new Error("network down")) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } diff --git a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts index 2ab6dab162..311a7ce9fd 100644 --- a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts +++ b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts @@ -108,7 +108,22 @@ describe("Zoo Gateway Fetchers", () => { expect(result.etag).toBe('"catalog-abc"') }) - it("reads ETag from the capitalized response header", async () => { + it("reads etag from the lowercase response header (Node.js normalises headers)", async () => { + mockAxiosGet.mockResolvedValueOnce({ + ...mockResponse, + headers: { etag: '"catalog-lower"' }, + }) + + const result = await getZooGatewayModels(gatewayOptions()) + + expect(result.kind).toBe("ok") + if (result.kind !== "ok") return + expect(result.etag).toBe('"catalog-lower"') + }) + + it("returns undefined etag when only the capitalized ETag header is present (unreachable in real HTTP)", async () => { + // Node.js lowercases all HTTP response headers before they reach userland, + // so response.headers.ETag is always undefined in production. mockAxiosGet.mockResolvedValueOnce({ ...mockResponse, headers: { ETag: '"catalog-capital"' }, @@ -118,7 +133,7 @@ describe("Zoo Gateway Fetchers", () => { expect(result.kind).toBe("ok") if (result.kind !== "ok") return - expect(result.etag).toBe('"catalog-capital"') + expect(result.etag).toBeUndefined() }) it("sends If-None-Match when provided and returns not_modified on 304", async () => { diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 3eec78c64a..54a7c72266 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -44,6 +44,11 @@ const modelRecordSchema = z.record(z.string(), modelInfoSchema) // deduplicate each other's in-flight refreshes. const inFlightRefresh = new Map>() +// Same single-flight guard for auth-scoped providers. Without this, two concurrent getModels() +// calls (e.g. two panels opening on startup) both miss the session cache and each fire their own +// provider fetch. Keyed on the same compound cache key as authSessionCache. +const inFlightAuthScopedFetch = new Map>() + // Cache keys (see getCacheKey) for which we've already reported an empty model response this // session. A persistently-empty endpoint (e.g. misconfigured server) would otherwise re-fire this // event on every cache refresh; gate it to at most once per distinct provider+server+key identity @@ -237,44 +242,62 @@ async function resolveAuthScopedModels( return existing.models } - try { - const fetched = await fetchAuthScopedModelsFromProvider( - assertAuthScopedGetModelsOptions(options), - existing?.etag, - ) + // Coalesce concurrent fetches for the same session identity so two panels + // opening simultaneously don't each fire their own provider request. + const existingFlight = inFlightAuthScopedFetch.get(cacheKey) + if (existingFlight && !forceRefresh) { + return existingFlight + } + + const fetchPromise = (async () => { + try { + const fetched = await fetchAuthScopedModelsFromProvider( + assertAuthScopedGetModelsOptions(options), + existing?.etag, + ) + + if (fetched.notModified) { + // Re-read from the Map: a concurrent sign-out could have cleared + // the entry between when we captured `existing` and now. + const current = getAuthSessionEntry(cacheKey) + if (current && Object.keys(current.models).length > 0) { + touchAuthSessionEntry(cacheKey, current) + reportedEmptyModelResponse.delete(cacheKey) + return current.models + } + } else { + const modelCount = Object.keys(fetched.models).length + if (modelCount > 0) { + setAuthSessionEntry(cacheKey, fetched.models, fetched.etag) + reportedEmptyModelResponse.delete(cacheKey) + return fetched.models + } + + captureModelCacheEmptyResponseOnce(provider, cacheKey, { + context: forceRefresh ? "refreshModels" : "getModels", + hasExistingCache: Boolean(existing && Object.keys(existing.models).length > 0), + ...(existing ? { existingCacheSize: Object.keys(existing.models).length } : {}), + }) + } - if (fetched.notModified) { if (existing && Object.keys(existing.models).length > 0) { - touchAuthSessionEntry(cacheKey, existing) - reportedEmptyModelResponse.delete(cacheKey) return existing.models } - } else { - const modelCount = Object.keys(fetched.models).length - if (modelCount > 0) { - setAuthSessionEntry(cacheKey, fetched.models, fetched.etag) - reportedEmptyModelResponse.delete(cacheKey) - return fetched.models - } - - captureModelCacheEmptyResponseOnce(provider, cacheKey, { - context: forceRefresh ? "refreshModels" : "getModels", - hasExistingCache: Boolean(existing && Object.keys(existing.models).length > 0), - ...(existing ? { existingCacheSize: Object.keys(existing.models).length } : {}), - }) - } - if (existing && Object.keys(existing.models).length > 0) { - return existing.models + return fetched.models + } catch (error) { + if (existing && Object.keys(existing.models).length > 0) { + return existing.models + } + throw error } + })() - return fetched.models - } catch (error) { - if (existing && Object.keys(existing.models).length > 0) { - return existing.models - } - throw error - } + // Wrap cleanup in the promise itself so the stored value IS what callers await — + // a detached .finally() would produce an unhandled rejection when the fetch fails. + const fetchWithCleanup = fetchPromise.finally(() => inFlightAuthScopedFetch.delete(cacheKey)) + inFlightAuthScopedFetch.set(cacheKey, fetchWithCleanup) + return fetchWithCleanup } // Memoize derived digests so the deliberately-structureless KDF runs at most once per @@ -389,6 +412,12 @@ async function readModels(cacheKey: string): Promise { async function fetchModelsFromProvider(options: GetModelsOptions): Promise { const { provider } = options + if (isAuthScopedProvider(provider)) { + throw new Error( + `fetchModelsFromProvider must not be called for auth-scoped provider "${provider}" — use resolveAuthScopedModels instead`, + ) + } + let models: ModelRecord switch (provider) { @@ -432,21 +461,6 @@ async function fetchModelsFromProvider(options: GetModelsOptions): Promise Date: Thu, 3 Sep 2026 20:02:44 -0600 Subject: [PATCH 08/26] fix(zoo-gateway): address CodeRabbit review and kill survived mutations --- .../fetchers/__tests__/modelCache.spec.ts | 150 ++++++++++++++++++ .../fetchers/__tests__/zoo-gateway.spec.ts | 5 +- src/api/providers/fetchers/modelCache.ts | 13 +- 3 files changed, 164 insertions(+), 4 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 58c8c3f0f5..102ff55718 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -1291,6 +1291,156 @@ describe("auth session cache", () => { expect(mockSet).not.toHaveBeenCalled() }) + it("clearAuthSessionModelsForProvider prevents a resolved in-flight fetch from repopulating the cache", async () => { + // Regression guard: an in-flight fetch that resolves after clearAuthSessionModelsForProvider + // must not repopulate the session cache (which would leak a prior session's catalog). + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + // Fetch and populate cache + await freshGetModels(options) + + // Sign out — clears cache and in-flight map + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + + // A new getModels call after sign-out must refetch (cache was cleared) + await freshGetModels(options) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + }) + + it("returns cached entry within TTL without hitting the provider", async () => { + // Kills: AUTH_SESSION_TTL_MS arithmetic mutant (5*60/1000 would expire in <1ms) + vi.useFakeTimers() + try { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + // Advance by 1 second — well within the 5-minute TTL + vi.advanceTimersByTime(1000) + const second = await freshGetModels(options) + + expect(second).toEqual(zooModels) + // Only one fetch — the second call was served from cache + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + it("treats an entry as stale at exactly the TTL boundary", async () => { + // Kills: < vs <= equality-operator mutant on isAuthSessionFresh + vi.useFakeTimers() + try { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + // Advance to exactly TTL — entry is now stale (< not <=) + vi.advanceTimersByTime(5 * 60 * 1000) + await freshGetModels(options) + + // Must have refetched — entry was stale at exact TTL + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) + + it("staleCutoffMs is 2x AUTH_SESSION_TTL_MS not 1x", async () => { + // Kills: AUTH_SESSION_TTL_MS * 2 → / 2 arithmetic mutant. + // An entry at exactly 1x TTL + 1ms is stale but NOT yet at the 2x prune threshold; + // it must still be in the map (so a concurrent get with a fresh ETag can still + // re-use the old etag for conditional revalidation). The existing + // "prunes session entries older than twice the TTL" test covers the >=2x case. + vi.useFakeTimers() + try { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const optionsA = { provider: providerIdentifiers.zooGateway, apiKey: "session-a" } + const optionsB = { provider: providerIdentifiers.zooGateway, apiKey: "session-b" } + + // First fetch returns an etag so we can detect it on the revalidation call + freshMockGetZooGatewayModels.mockResolvedValueOnce({ kind: "ok", models: zooModels, etag: '"v1"' }) + await freshGetModels(optionsA) + + // Advance to 1x TTL + 1ms — stale but below 2x cutoff + vi.advanceTimersByTime(5 * 60 * 1000 + 1) + + // Adding a second entry triggers enforceAuthSessionCacheBound → pruneExpiredAuthSessionEntries + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + await freshGetModels(optionsB) + + // session-a is below 2x TTL so it is NOT pruned. + // It IS stale (1x TTL elapsed), so the next getModels call will issue a revalidation + // request WITH the etag — confirming the entry is still in the cache. + freshMockGetZooGatewayModels.mockClear() + freshMockGetZooGatewayModels.mockResolvedValue({ kind: "ok", models: zooModels, etag: '"v2"' }) + await freshGetModels(optionsA) + + // The etag from the prior fetch must have been sent (entry was not pruned) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledWith( + expect.objectContaining({ ifNoneMatch: '"v1"' }), + ) + } finally { + vi.useRealTimers() + } + }) + + it("notModified branch returns existing models when current entry is non-empty", async () => { + // Kills: ConditionalExpression mutant (false) on `if (current && Object.keys(current.models).length > 0)` + vi.useFakeTimers() + try { + freshMockGetZooGatewayModels + .mockResolvedValueOnce({ kind: "ok", models: zooModels, etag: '"v1"' }) + .mockResolvedValueOnce({ kind: "not_modified" }) + + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + await freshGetModels(options) + vi.advanceTimersByTime(5 * 60 * 1000 + 1) + + const result = await freshGetModels(options) + + // If the condition were false, touchAuthSessionEntry wouldn't fire and result might differ + expect(result).toEqual(zooModels) + } finally { + vi.useRealTimers() + } + }) + + it("falls back to existing models on empty response after having a prior non-empty catalog", async () => { + // Kills: ConditionalExpression mutant (false) on `if (existing && Object.keys(existing.models).length > 0)` fallback + vi.useFakeTimers() + try { + freshMockGetZooGatewayModels + .mockResolvedValueOnce(zooGatewayOk(zooModels)) + .mockResolvedValueOnce(zooGatewayOk({})) + + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + await freshGetModels(options) + vi.advanceTimersByTime(5 * 60 * 1000 + 1) + + const result = await freshGetModels(options) + + // If condition were false, {} would be returned — but existing catalog should survive + expect(result).toEqual(zooModels) + } finally { + vi.useRealTimers() + } + }) + + it("forceRefresh bypasses the fresh-cache short-circuit", async () => { + // Kills: ConditionalExpression mutant (true) on `!forceRefresh` in cache-hit guard + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await freshGetModels(options) + // Entry is fresh — without forceRefresh the second call would be served from cache + await freshRefreshModels(options) + + // forceRefresh must bypass the cache and issue a second fetch + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + }) + it("concurrent getModels calls for the same session key share one fetch", async () => { freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } diff --git a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts index 311a7ce9fd..6349cf738f 100644 --- a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts +++ b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts @@ -104,7 +104,10 @@ describe("Zoo Gateway Fetchers", () => { expect(result.kind).toBe("ok") if (result.kind !== "ok") return expect(Object.keys(result.models)).toHaveLength(1) - expect(result.models["anthropic/claude-sonnet-4"]).toBeDefined() + expect(result.models["anthropic/claude-sonnet-4"]).toMatchObject({ + maxTokens: expect.any(Number), + contextWindow: expect.any(Number), + }) expect(result.etag).toBe('"catalog-abc"') }) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 54a7c72266..50826f6056 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -199,6 +199,9 @@ export function clearAuthSessionModelsForProvider(provider: RouterName): void { for (const key of authSessionCache.keys()) { if (key === provider || key.startsWith(`${provider}:`)) { authSessionCache.delete(key) + // Also invalidate any in-flight fetch for this key so a pending request + // cannot repopulate the session cache after sign-out. + inFlightAuthScopedFetch.delete(key) } } } @@ -268,8 +271,12 @@ async function resolveAuthScopedModels( } else { const modelCount = Object.keys(fetched.models).length if (modelCount > 0) { - setAuthSessionEntry(cacheKey, fetched.models, fetched.etag) - reportedEmptyModelResponse.delete(cacheKey) + // Guard against re-populating the cache after a sign-out invalidated + // this key between when we started the fetch and when it resolved. + if (inFlightAuthScopedFetch.has(cacheKey)) { + setAuthSessionEntry(cacheKey, fetched.models, fetched.etag) + reportedEmptyModelResponse.delete(cacheKey) + } return fetched.models } @@ -515,7 +522,7 @@ export const getModels = async (options: GetModelsOptions): Promise const modelCount = Object.keys(fetched).length // Only cache non-empty results so a failed API response doesn't get persisted - // as if the provider had no models. Auth-scoped providers skip caching entirely. + // as if the provider had no models. if (modelCount > 0) { // Clear the empty-response throttle for any non-empty response, including from // auth-scoped providers that skip disk/memory cache, so a later empty response From 00ff67848e5ab315f20e846efba18df7dfd69293 Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Thu, 3 Sep 2026 21:16:35 -0600 Subject: [PATCH 09/26] fix(model-cache): add generation counter to prevent stale fetch from repopulating cache after sign-out --- .../fetchers/__tests__/modelCache.spec.ts | 63 ++++++++++++++++++- src/api/providers/fetchers/modelCache.ts | 35 +++++++++-- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 102ff55718..92596b2c3f 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -1308,6 +1308,65 @@ describe("auth session cache", () => { expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) + it("in-flight fetch started before sign-out does not repopulate cache when a new fetch starts after sign-out", async () => { + // Race condition: fetch A starts → sign-out → fetch B starts → fetch A resolves. + // Fetch A must NOT write back its stale pre-sign-out data because the generation counter + // was bumped by sign-out. This test simulates the race using deferred promises. + const staleModels: ModelRecord = { + "stale/model": { maxTokens: 1000, contextWindow: 1000, supportsPromptCache: false }, + } + const freshModelsAfterSignOut: ModelRecord = { + "fresh/model": { maxTokens: 2000, contextWindow: 2000, supportsPromptCache: false }, + } + + type ZooGatewayResult = Awaited> + let resolveFetchA!: (value: ZooGatewayResult) => void + const fetchAPromise = new Promise((resolve) => { + resolveFetchA = resolve + }) + + let resolveFetchB!: (value: ZooGatewayResult) => void + const fetchBPromise = new Promise((resolve) => { + resolveFetchB = resolve + }) + + // First call returns fetchAPromise (hangs until we resolve) + // Second call returns fetchBPromise (hangs until we resolve) + freshMockGetZooGatewayModels.mockReturnValueOnce(fetchAPromise).mockReturnValueOnce(fetchBPromise) + + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + // 1. Start fetch A (pre-sign-out) + const fetchAResult = freshGetModels(options) + + // 2. Sign out — clears cache and bumps generation + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + + // 3. Start fetch B (post-sign-out) — this registers a NEW in-flight promise + const fetchBResult = freshGetModels(options) + + // 4. Resolve fetch A with stale data — it should NOT write to cache + resolveFetchA(zooGatewayOk(staleModels)) + await fetchAResult + + // 5. Resolve fetch B with fresh data — it SHOULD write to cache + resolveFetchB(zooGatewayOk(freshModelsAfterSignOut)) + const result = await fetchBResult + + // The result should be the fresh post-sign-out data, not the stale pre-sign-out data + expect(result).toEqual(freshModelsAfterSignOut) + + // Verify a subsequent getModels call returns the fresh data (proving fetch A didn't overwrite) + // Don't set a new mock — the call should be served from cache + const subsequent = await freshGetModels(options) + expect(subsequent).toEqual(freshModelsAfterSignOut) + // Should be served from cache, not trigger a new fetch (only 2 fetches: A and B) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + + // Clean up — clear cache so subsequent tests don't see stale entries + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + }) + it("returns cached entry within TTL without hitting the provider", async () => { // Kills: AUTH_SESSION_TTL_MS arithmetic mutant (5*60/1000 would expire in <1ms) vi.useFakeTimers() @@ -1378,9 +1437,7 @@ describe("auth session cache", () => { await freshGetModels(optionsA) // The etag from the prior fetch must have been sent (entry was not pruned) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledWith( - expect.objectContaining({ ifNoneMatch: '"v1"' }), - ) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledWith(expect.objectContaining({ ifNoneMatch: '"v1"' })) } finally { vi.useRealTimers() } diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 50826f6056..dc2a695258 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -49,6 +49,12 @@ const inFlightRefresh = new Map>() // provider fetch. Keyed on the same compound cache key as authSessionCache. const inFlightAuthScopedFetch = new Map>() +// Generation counter per auth-scoped PROVIDER. Bumped on sign-out invalidation so that an +// in-flight fetch that started before sign-out can detect that the session was invalidated +// and avoid re-populating the cache with stale data. Keyed by provider (not cache key) +// because the cache entry may not exist yet when sign-out occurs. +const authScopedClearGeneration = new Map() + // Cache keys (see getCacheKey) for which we've already reported an empty model response this // session. A persistently-empty endpoint (e.g. misconfigured server) would otherwise re-fire this // event on every cache refresh; gate it to at most once per distinct provider+server+key identity @@ -196,14 +202,27 @@ function deleteAuthSessionEntry(cacheKey: string): void { } export function clearAuthSessionModelsForProvider(provider: RouterName): void { + const matchesProvider = (key: string) => key === provider || key.startsWith(`${provider}:`) + for (const key of authSessionCache.keys()) { - if (key === provider || key.startsWith(`${provider}:`)) { + if (matchesProvider(key)) { authSessionCache.delete(key) - // Also invalidate any in-flight fetch for this key so a pending request - // cannot repopulate the session cache after sign-out. + } + } + // Also invalidate any in-flight fetches — the cache entry may not exist yet + // if the fetch hasn't resolved, so we must iterate inFlightAuthScopedFetch + // independently (not just keys already in authSessionCache). + for (const key of inFlightAuthScopedFetch.keys()) { + if (matchesProvider(key)) { inFlightAuthScopedFetch.delete(key) } } + // Bump generation so in-flight fetches that started before this clear + // know they should not write back stale data. Keyed by provider (not cache key) + // because the cache entry may not exist yet when sign-out occurs. + if (isAuthScopedProvider(provider)) { + authScopedClearGeneration.set(provider, (authScopedClearGeneration.get(provider) ?? 0) + 1) + } } type AuthScopedFetchResult = { @@ -252,6 +271,11 @@ async function resolveAuthScopedModels( return existingFlight } + // Capture generation before starting the fetch so we can detect if a sign-out occurred + // between when we started and when we try to write back results. + const authProvider = provider as AuthScopedProvider + const generationAtStart = authScopedClearGeneration.get(authProvider) ?? 0 + const fetchPromise = (async () => { try { const fetched = await fetchAuthScopedModelsFromProvider( @@ -273,7 +297,10 @@ async function resolveAuthScopedModels( if (modelCount > 0) { // Guard against re-populating the cache after a sign-out invalidated // this key between when we started the fetch and when it resolved. - if (inFlightAuthScopedFetch.has(cacheKey)) { + // Check both: (1) the in-flight promise is still ours, and (2) no sign-out + // bumped the generation while we were fetching. + const generationNow = authScopedClearGeneration.get(authProvider) ?? 0 + if (inFlightAuthScopedFetch.has(cacheKey) && generationNow === generationAtStart) { setAuthSessionEntry(cacheKey, fetched.models, fetched.etag) reportedEmptyModelResponse.delete(cacheKey) } From 1ac2f2242c832eb4bc05e811f47c40c6ad3779e0 Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Fri, 4 Sep 2026 07:37:47 -0600 Subject: [PATCH 10/26] fix(model-cache): fix in-flight cleanup clobbering newer post-sign-out fetch promise --- src/api/providers/fetchers/modelCache.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index dc2a695258..5704cfbe1a 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -329,7 +329,14 @@ async function resolveAuthScopedModels( // Wrap cleanup in the promise itself so the stored value IS what callers await — // a detached .finally() would produce an unhandled rejection when the fetch fails. - const fetchWithCleanup = fetchPromise.finally(() => inFlightAuthScopedFetch.delete(cacheKey)) + // Only remove our own entry: if sign-out cleared us and a new fetch registered a + // fresh promise under the same key, we must not delete that newer entry. + let fetchWithCleanup: Promise + fetchWithCleanup = fetchPromise.finally(() => { + if (inFlightAuthScopedFetch.get(cacheKey) === fetchWithCleanup) { + inFlightAuthScopedFetch.delete(cacheKey) + } + }) inFlightAuthScopedFetch.set(cacheKey, fetchWithCleanup) return fetchWithCleanup } From 65211c75e62f59869779f4441fded7aa0b661bf1 Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Fri, 4 Sep 2026 07:54:49 -0600 Subject: [PATCH 11/26] =?UTF-8?q?test(model-cache):=20kill=20survived=20mu?= =?UTF-8?q?tants=20=E2=80=94=20cache=20bound,=20empty-entry=20guard,=20and?= =?UTF-8?q?=20sign-out=20invariants?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../fetchers/__tests__/modelCache.spec.ts | 193 ++++++++++++++++++ 1 file changed, 193 insertions(+) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 92596b2c3f..bf318157e7 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -1745,4 +1745,197 @@ describe("auth session cache", () => { vi.useRealTimers() } }) + + it("staleCutoffMs uses >= so entry at exactly 2x TTL is pruned", async () => { + // Kills: EqualityOperator mutant L159 (>= staleCutoffMs → > staleCutoffMs) + vi.useFakeTimers() + try { + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const first = { provider: providerIdentifiers.zooGateway, apiKey: "session-a" } + const second = { provider: providerIdentifiers.zooGateway, apiKey: "session-b" } + + await freshGetModels(first) + // Advance to EXACTLY 2x TTL — must be pruned (>= not just >) + vi.advanceTimersByTime(5 * 60 * 1000 * 2) + await freshGetModels(second) + + freshMockGetZooGatewayModels.mockClear() + // First entry was pruned — a new fetch must fire (not_modified without etag yields {}) + freshMockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk({})) + const result = await freshGetModels(first) + + // No cache entry → provider returned empty → fallback to {} + expect(result).toEqual({}) + } finally { + vi.useRealTimers() + } + }) + + it("enforces session cache MAX_ENTRIES by evicting the oldest entry", async () => { + // Kills: ConditionalExpression mutants on L167-L179 (while loop and oldestKey guard) + vi.useFakeTimers() + try { + // Fill 65 unique sessions (1 over AUTH_SESSION_MAX_ENTRIES=64) + for (let i = 0; i < 65; i++) { + freshMockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk(zooModels)) + vi.advanceTimersByTime(1) // ensure fetchedAt differs so oldest is well-defined + await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: `session-${i}` }) + } + // session-0 is the oldest — it must have been evicted. Verify by clearing mock + // and calling with session-0: if it was evicted, a fresh fetch fires. + freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + freshMockGetZooGatewayModels.mockClear() + await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "session-0" }) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + it("setAuthSessionEntry does not store an empty model record", async () => { + // Kills: ConditionalExpression mutant L188 (false) — empty models must be rejected + freshMockGetZooGatewayModels + .mockResolvedValueOnce(zooGatewayOk({})) // first call: empty + .mockResolvedValueOnce(zooGatewayOk(zooModels)) // second call: populated + + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + // First call returns empty — nothing stored + await freshGetModels(options) + // Second call must fetch again (nothing was cached from first call) + const second = await freshGetModels(options) + + expect(second).toEqual(zooModels) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + }) + + it("stale in-flight fetch must not write to cache even when a new fetch registered the key", async () => { + // Kills: LogicalOperator mutant L303 (&&→||): + // With ||, fetch A (stale) would write because inFlightAuthScopedFetch.has(key)==true. + // This test detects that by making a THIRD call immediately after fetch A resolves + // (before fetch B): if stale data was written it's served from cache; if not, the + // third call deduplicates to fetch B and gets fresh data. + type ZooGatewayResult = Awaited> + const staleModels: ModelRecord = { + "stale/model": { maxTokens: 1000, contextWindow: 1000, supportsPromptCache: false }, + } + const freshModelsAfterSignOut2: ModelRecord = { + "fresh2/model": { maxTokens: 3000, contextWindow: 3000, supportsPromptCache: false }, + } + + let resolveFetchA!: (v: ZooGatewayResult) => void + let resolveFetchB!: (v: ZooGatewayResult) => void + const fetchADeferred = new Promise((r) => (resolveFetchA = r)) + const fetchBDeferred = new Promise((r) => (resolveFetchB = r)) + + freshMockGetZooGatewayModels.mockReturnValueOnce(fetchADeferred).mockReturnValueOnce(fetchBDeferred) + + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + // 1. Start fetch A (pre-sign-out) + const fetchAResult = freshGetModels(options) + + // 2. Sign-out + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + + // 3. Start fetch B (post-sign-out) — registers new in-flight under the same key + const fetchBResult = freshGetModels(options) + + // 4. Resolve fetch A with stale data + resolveFetchA(zooGatewayOk(staleModels)) + await fetchAResult + + // 5. Third call immediately after A resolves (B still pending). + // If stale data was written (|| bug), it gets served from cache → returns stale. + // If stale data was NOT written (correct &&), it deduplicates to fetch B → blocks. + const thirdCallResult = freshGetModels(options) + + // 6. Resolve fetch B with fresh data + resolveFetchB(zooGatewayOk(freshModelsAfterSignOut2)) + const [thirdResult] = await Promise.all([thirdCallResult, fetchBResult]) + + // The third call must have gotten fresh data (deduped to B), not stale data from A + expect(thirdResult).toEqual(freshModelsAfterSignOut2) + + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + }) + + it("clearAuthSessionModelsForProvider also invalidates in-flight fetches not yet in cache", async () => { + // Kills: ConditionalExpression mutants on L216 — the loop over inFlightAuthScopedFetch + // must delete keys even when the cache is empty (fetch hasn't resolved yet). + type ZooGatewayResult = Awaited> + const freshModels2: ModelRecord = { + "fresh2/model": { maxTokens: 3000, contextWindow: 3000, supportsPromptCache: false }, + } + + let resolveFetchA!: (v: ZooGatewayResult) => void + const fetchADeferred = new Promise((r) => (resolveFetchA = r)) + + freshMockGetZooGatewayModels.mockReturnValueOnce(fetchADeferred).mockResolvedValueOnce(zooGatewayOk(freshModels2)) + + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + // 1. Start fetch A — it's in-flight, cache is EMPTY (hasn't resolved yet) + const fetchAResult = freshGetModels(options) + + // 2. Sign out before fetch A resolves — must also clear the in-flight entry + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + + // 3. Start fetch B — if in-flight was cleared, a NEW fetch fires (not deduped to A) + const fetchBResult = freshGetModels(options) + + // 4. Resolve A (stale) and B (fresh) + resolveFetchA(zooGatewayOk(zooModels)) // stale pre-sign-out data + await fetchAResult + + const bResult = await fetchBResult + expect(bResult).toEqual(freshModels2) + // Two fetches fired: A and B (if in-flight was cleared correctly) + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + }) + + it("generation counter increments by +1 on each sign-out, not by -1", async () => { + // Kills: ArithmeticOperator mutant L224 (+ 1 → - 1). + // After two sign-outs the counter should be 2; with -1 it would be -2. + // We observe this indirectly: two sign-outs → two in-flight fetches complete → only + // the post-second-sign-out fetch should write to cache. + type ZooGatewayResult = Awaited> + const models1: ModelRecord = { "m1/a": { maxTokens: 100, contextWindow: 100, supportsPromptCache: false } } + const models2: ModelRecord = { "m2/b": { maxTokens: 200, contextWindow: 200, supportsPromptCache: false } } + const models3: ModelRecord = { "m3/c": { maxTokens: 300, contextWindow: 300, supportsPromptCache: false } } + + let resolveA!: (v: ZooGatewayResult) => void + let resolveB!: (v: ZooGatewayResult) => void + let resolveC!: (v: ZooGatewayResult) => void + freshMockGetZooGatewayModels + .mockReturnValueOnce(new Promise((r) => (resolveA = r))) + .mockReturnValueOnce(new Promise((r) => (resolveB = r))) + .mockReturnValueOnce(new Promise((r) => (resolveC = r))) + + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + // fetch A (gen=0 at start) + const resA = freshGetModels(options) + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) // gen → 1 + // fetch B (gen=1 at start) + const resB = freshGetModels(options) + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) // gen → 2 + // fetch C (gen=2 at start) + const resC = freshGetModels(options) + + // Resolve all with distinct data; only C's data should land in cache + resolveA(zooGatewayOk(models1)) + await resA + resolveB(zooGatewayOk(models2)) + await resB + resolveC(zooGatewayOk(models3)) + const finalResult = await resC + + // C was the last fetch with matching generation — result must be models3 + expect(finalResult).toEqual(models3) + + freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + }) }) From 8c404cb03cd1e6593feb451c6abe6c47c4fd6126 Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Fri, 4 Sep 2026 08:09:39 -0600 Subject: [PATCH 12/26] fix(model-cache): use const for fetchWithCleanup to satisfy prefer-const lint rule --- src/api/providers/fetchers/modelCache.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 5704cfbe1a..97096b9c1f 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -331,8 +331,7 @@ async function resolveAuthScopedModels( // a detached .finally() would produce an unhandled rejection when the fetch fails. // Only remove our own entry: if sign-out cleared us and a new fetch registered a // fresh promise under the same key, we must not delete that newer entry. - let fetchWithCleanup: Promise - fetchWithCleanup = fetchPromise.finally(() => { + const fetchWithCleanup = fetchPromise.finally(() => { if (inFlightAuthScopedFetch.get(cacheKey) === fetchWithCleanup) { inFlightAuthScopedFetch.delete(cacheKey) } From 444e9c7e3957439693e43962675f92ef88e87d42 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 4 Sep 2026 16:34:45 +0000 Subject: [PATCH 13/26] fix(task): await disposal cleanup (#1526) --- packages/types/src/task.ts | 2 +- src/core/task/Task.ts | 27 ++++--- src/core/task/__tests__/Task.dispose.test.ts | 72 +++++++++++++++---- src/core/task/__tests__/Task.spec.ts | 49 ++++++++++--- src/core/task/__tests__/Task.throttle.test.ts | 4 +- .../task/__tests__/grace-retry-errors.spec.ts | 4 +- 6 files changed, 118 insertions(+), 40 deletions(-) diff --git a/packages/types/src/task.ts b/packages/types/src/task.ts index 572302861b..cac34cc6f2 100644 --- a/packages/types/src/task.ts +++ b/packages/types/src/task.ts @@ -128,7 +128,7 @@ export interface TaskLike { approveAsk(options?: { text?: string; images?: string[] }): void denyAsk(options?: { text?: string; images?: string[] }): void submitUserMessage(text: string, images?: string[], mode?: string, providerProfile?: string): Promise - abortTask(): void + abortTask(): Promise } export type TaskEvents = { diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 37281a9010..50fd95f52e 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2493,7 +2493,7 @@ export class Task extends EventEmitter implements TaskLike { this.emit(RooCodeEventName.TaskAborted) try { - this.dispose() // Call the centralized dispose method + await this.dispose() // Call the centralized dispose method } catch (error) { console.error(`Error during task ${this.taskId}.${this.instanceId} disposal:`, error) // Don't rethrow - we want abort to always succeed @@ -2514,8 +2514,9 @@ export class Task extends EventEmitter implements TaskLike { } } - public dispose(): void { + public dispose(): Promise { console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`) + const pendingCleanup: Promise[] = [] // Stop the idle telemetry check and report any unflushed activity as a // shutdown installment, so a task torn down mid-work (panel closed, task @@ -2563,14 +2564,16 @@ export class Task extends EventEmitter implements TaskLike { } // Cleanup command output artifacts - getTaskDirectoryPath(this.globalStoragePath, this.taskId) - .then((taskDir) => { - const outputDir = path.join(taskDir, "command-output") - return OutputInterceptor.cleanup(outputDir) - }) - .catch((error) => { - console.error("Error cleaning up command output artifacts:", error) - }) + pendingCleanup.push( + getTaskDirectoryPath(this.globalStoragePath, this.taskId) + .then((taskDir) => { + const outputDir = path.join(taskDir, "command-output") + return OutputInterceptor.cleanup(outputDir) + }) + .catch((error) => { + console.error("Error cleaning up command output artifacts:", error) + }), + ) try { if (this.rooIgnoreController) { @@ -2591,11 +2594,13 @@ export class Task extends EventEmitter implements TaskLike { try { // If we're not streaming then `abortStream` won't be called. if (this.isStreaming && this.diffViewProvider.isEditing) { - this.diffViewProvider.revertChanges().catch(console.error) + pendingCleanup.push(this.diffViewProvider.revertChanges().catch(console.error)) } } catch (error) { console.error("Error reverting diff changes:", error) } + + return Promise.all(pendingCleanup).then(() => undefined) } // Subtasks diff --git a/src/core/task/__tests__/Task.dispose.test.ts b/src/core/task/__tests__/Task.dispose.test.ts index 9f00e9d852..8a712c6280 100644 --- a/src/core/task/__tests__/Task.dispose.test.ts +++ b/src/core/task/__tests__/Task.dispose.test.ts @@ -2,7 +2,9 @@ import { type ProviderSettings, RooCodeEventName } from "@roo-code/types" import { Task } from "../Task" import { ClineProvider } from "../../webview/ClineProvider" +import { OutputInterceptor } from "../../../integrations/terminal/OutputInterceptor" import { providerIdentifiers } from "@roo-code/types/provider-identifiers" +import { getTaskDirectoryPath } from "../../../utils/storage" // Mock dependencies vi.mock("../../webview/ClineProvider") @@ -11,10 +13,7 @@ vi.mock("../../../integrations/terminal/TerminalRegistry", () => ({ releaseTerminalsForTask: vi.fn(), }, })) -// dispose() fires an UNawaited getTaskDirectoryPath -> OutputInterceptor.cleanup chain. -// Mock both so it resolves immediately with no real fs and no late console.error, -// otherwise that dangling promise logs after the test ends and trips Vitest's -// "Closing rpc while onUserConsoleLog was pending" teardown race. +// Keep disposal tests independent of the real filesystem and output interceptor. vi.mock("../../../utils/storage", () => ({ getTaskDirectoryPath: vi.fn().mockResolvedValue("/test/path/tasks/test-task"), })) @@ -80,13 +79,60 @@ describe("Task dispose method", () => { }) }) - afterEach(() => { + afterEach(async () => { // Clean up if (task && !task.abort) { - task.dispose() + await task.dispose() } }) + test("should expose completion of deferred command output cleanup", async () => { + let resolveTaskDirectory: (taskDirectory: string) => void + vi.mocked(getTaskDirectoryPath).mockReturnValueOnce( + new Promise((resolve) => { + resolveTaskDirectory = resolve + }), + ) + + const disposal = task.dispose() + let disposalComplete = false + void disposal.then(() => { + disposalComplete = true + }) + await Promise.resolve() + + expect(disposalComplete).toBe(false) + expect(OutputInterceptor.cleanup).not.toHaveBeenCalled() + + resolveTaskDirectory!("/test/path/tasks/test-task") + await disposal + + expect(OutputInterceptor.cleanup).toHaveBeenCalledWith("/test/path/tasks/test-task/command-output") + expect(disposalComplete).toBe(true) + }) + + test("should expose completion of deferred diff reversion", async () => { + let resolveReversion: () => void + const reversion = new Promise((resolve) => { + resolveReversion = resolve + }) + task.isStreaming = true + task.diffViewProvider.isEditing = true + vi.spyOn(task.diffViewProvider, "revertChanges").mockReturnValue(reversion) + + const disposal = task.dispose() + let disposalComplete = false + void disposal.then(() => { + disposalComplete = true + }) + await Promise.resolve() + expect(disposalComplete).toBe(false) + + resolveReversion!() + await disposal + expect(disposalComplete).toBe(true) + }) + test("should remove all event listeners when dispose is called", () => { // Add some event listeners using type assertion to bypass strict typing for testing const listener1 = vi.fn(() => {}) @@ -106,7 +152,7 @@ describe("Task dispose method", () => { const removeAllListenersSpy = vi.spyOn(task, "removeAllListeners") // Call dispose - task.dispose() + void task.dispose() // Verify removeAllListeners was called expect(removeAllListenersSpy).toHaveBeenCalledOnce() @@ -128,7 +174,7 @@ describe("Task dispose method", () => { const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) // Call dispose - should not throw - expect(() => task.dispose()).not.toThrow() + expect(() => void task.dispose()).not.toThrow() // Verify error was logged expect(consoleErrorSpy).toHaveBeenCalledWith("Error removing event listeners:", expect.any(Error)) @@ -143,7 +189,7 @@ describe("Task dispose method", () => { const consoleLogSpy = vi.spyOn(console, "log").mockImplementation(() => {}) // Call dispose - task.dispose() + void task.dispose() // Verify dispose was called and logged expect(consoleLogSpy).toHaveBeenCalledWith( @@ -193,7 +239,7 @@ describe("Task dispose method", () => { expect(task.listenerCount(RooCodeEventName.TaskUnpaused)).toBe(1) // Call dispose - task.dispose() + void task.dispose() // Verify all listeners are removed expect(task.listenerCount(RooCodeEventName.TaskStarted)).toBe(0) @@ -244,7 +290,7 @@ describe("Task.run() idempotency", () => { const callsBefore = startTaskSpy.mock.calls.length // constructor fired it once void t.run() expect(startTaskSpy.mock.calls.length).toBe(callsBefore) // run() must not add a second call - t.dispose() + await t.dispose() startTaskSpy.mockRestore() }) @@ -262,7 +308,7 @@ describe("Task.run() idempotency", () => { void t.run() expect(startTaskSpy.mock.calls.length).toBe(callsAfterStart) // no additional call - t.dispose() + await t.dispose() startTaskSpy.mockRestore() }) @@ -280,7 +326,7 @@ describe("Task.run() idempotency", () => { const p2 = t.run() expect(p1).toBe(p2) await p1 - t.dispose() + await t.dispose() startTaskSpy.mockRestore() }) }) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 37e228f887..0c2877bf3a 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -2111,7 +2111,7 @@ describe("Cline", () => { const emitSpy = vi.spyOn(task, "emit") // Mock the dispose method to avoid actual cleanup - vi.spyOn(task, "dispose").mockImplementation(() => {}) + vi.spyOn(task, "dispose").mockResolvedValue(undefined) // Call abortTask await task.abortTask() @@ -2132,7 +2132,7 @@ describe("Cline", () => { }) // Mock the dispose method to track cleanup - const disposeSpy = vi.spyOn(task, "dispose").mockImplementation(() => {}) + const disposeSpy = vi.spyOn(task, "dispose").mockResolvedValue(undefined) // Call abortTask await task.abortTask() @@ -2142,6 +2142,33 @@ describe("Cline", () => { expect(disposeSpy).toHaveBeenCalled() }) + it("waits for disposal cleanup before abort resolves", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + let resolveDisposal: () => void + const disposal = new Promise((resolve) => { + resolveDisposal = resolve + }) + const disposeSpy = vi.spyOn(task, "dispose").mockReturnValue(disposal) + + const abort = task.abortTask() + await vi.waitFor(() => expect(disposeSpy).toHaveBeenCalledOnce()) + let abortComplete = false + void abort.then(() => { + abortComplete = true + }) + await Promise.resolve() + expect(abortComplete).toBe(false) + + resolveDisposal!() + await abort + expect(abortComplete).toBe(true) + }) + it("flushes pending state before TaskAborted and disposal while queue state is intact", async () => { const task = new Task({ provider: mockProvider, @@ -2154,7 +2181,7 @@ describe("Cline", () => { queuedMessagesAtFlush = task.messageQueueService.messages.length }) const emitSpy = vi.spyOn(task, "emit") - const disposeSpy = vi.spyOn(task, "dispose").mockImplementation(() => {}) + const disposeSpy = vi.spyOn(task, "dispose").mockResolvedValue(undefined) task.messageQueueService.addMessage("queued text") await task.abortTask() @@ -2183,7 +2210,7 @@ describe("Cline", () => { const error = new Error("state flush failed") const flushSpy = vi.mocked(mockProvider.flushPostStateToWebviewThrottled).mockRejectedValueOnce(error) const taskAbortedListener = vi.fn() - const disposeSpy = vi.spyOn(task, "dispose").mockImplementation(() => {}) + const disposeSpy = vi.spyOn(task, "dispose").mockResolvedValue(undefined) const saveSpy = vi.spyOn(getTaskTestAccess(task), "saveClineMessages").mockResolvedValue(true) const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) task.on(RooCodeEventName.TaskAborted, taskAbortedListener) @@ -2220,7 +2247,7 @@ describe("Cline", () => { expect(typeof taskLike.abortTask).toBe("function") // Mock the dispose method to avoid actual cleanup - vi.spyOn(task, "dispose").mockImplementation(() => {}) + vi.spyOn(task, "dispose").mockResolvedValue(undefined) // Call abortTask through interface await taskLike.abortTask() @@ -2374,7 +2401,7 @@ describe("Cline", () => { vi.spyOn(task, "removeAllListeners").mockImplementation(() => task) // Call dispose - task.dispose() + void task.dispose() // Verify cancelCurrentRequest was called expect(cancelSpy).toHaveBeenCalled() @@ -3903,9 +3930,9 @@ describe("Telemetry installments (idle/shutdown flush)", () => { const createdTasks: Task[] = [] - afterEach(() => { + afterEach(async () => { for (const task of createdTasks) { - task.dispose() + await task.dispose() } createdTasks.length = 0 vi.useRealTimers() @@ -4060,7 +4087,7 @@ describe("Telemetry installments (idle/shutdown flush)", () => { task.recordToolUsage("read_file") task.messageCounts = { user: 1, assistant: 1 } - task.dispose() + void task.dispose() expect(captureTaskCompletedSpy).toHaveBeenCalledWith( task.taskId, @@ -4076,7 +4103,7 @@ describe("Telemetry installments (idle/shutdown flush)", () => { task.flushTelemetryInstallment("attempt_completion") captureTaskCompletedSpy.mockClear() - task.dispose() + void task.dispose() expect(captureTaskCompletedSpy).not.toHaveBeenCalled() }) @@ -4085,7 +4112,7 @@ describe("Telemetry installments (idle/shutdown flush)", () => { vi.useFakeTimers() const task = createTask() task.recordToolUsage("read_file") - task.dispose() + void task.dispose() captureTaskCompletedSpy.mockClear() vi.advanceTimersByTime(60 * 60 * 1000) diff --git a/src/core/task/__tests__/Task.throttle.test.ts b/src/core/task/__tests__/Task.throttle.test.ts index eaacb32faf..f0b298bfdd 100644 --- a/src/core/task/__tests__/Task.throttle.test.ts +++ b/src/core/task/__tests__/Task.throttle.test.ts @@ -99,10 +99,10 @@ describe("Task token usage throttling", () => { }) }) - afterEach(() => { + afterEach(async () => { vi.useRealTimers() if (task && !task.abort) { - task.dispose() + await task.dispose() } }) diff --git a/src/core/task/__tests__/grace-retry-errors.spec.ts b/src/core/task/__tests__/grace-retry-errors.spec.ts index 9584559c8f..3f72924f21 100644 --- a/src/core/task/__tests__/grace-retry-errors.spec.ts +++ b/src/core/task/__tests__/grace-retry-errors.spec.ts @@ -237,7 +237,7 @@ describe("Grace Retry Error Handling", () => { task.consecutiveNoAssistantMessagesCount = 5 // Mock dispose to prevent actual cleanup - vi.spyOn(task, "dispose").mockImplementation(() => {}) + vi.spyOn(task, "dispose").mockResolvedValue(undefined) await task.abortTask() @@ -257,7 +257,7 @@ describe("Grace Retry Error Handling", () => { task.consecutiveNoToolUseCount = 4 // Mock dispose to prevent actual cleanup - vi.spyOn(task, "dispose").mockImplementation(() => {}) + vi.spyOn(task, "dispose").mockResolvedValue(undefined) await task.abortTask() From d95df419b82d8beab072e6fcf66ac27a5c0e4cf7 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 4 Sep 2026 16:52:02 +0000 Subject: [PATCH 14/26] fix(task): cover cleanup completion branches --- src/core/task/Task.ts | 24 +++++++++----------- src/core/task/__tests__/Task.dispose.test.ts | 21 +++++++++++++++-- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 50fd95f52e..1b438247ac 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2516,7 +2516,6 @@ export class Task extends EventEmitter implements TaskLike { public dispose(): Promise { console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`) - const pendingCleanup: Promise[] = [] // Stop the idle telemetry check and report any unflushed activity as a // shutdown installment, so a task torn down mid-work (panel closed, task @@ -2564,16 +2563,14 @@ export class Task extends EventEmitter implements TaskLike { } // Cleanup command output artifacts - pendingCleanup.push( - getTaskDirectoryPath(this.globalStoragePath, this.taskId) - .then((taskDir) => { - const outputDir = path.join(taskDir, "command-output") - return OutputInterceptor.cleanup(outputDir) - }) - .catch((error) => { - console.error("Error cleaning up command output artifacts:", error) - }), - ) + let pendingCleanup = getTaskDirectoryPath(this.globalStoragePath, this.taskId) + .then((taskDir) => { + const outputDir = path.join(taskDir, "command-output") + return OutputInterceptor.cleanup(outputDir) + }) + .catch((error) => { + console.error("Error cleaning up command output artifacts:", error) + }) try { if (this.rooIgnoreController) { @@ -2594,13 +2591,14 @@ export class Task extends EventEmitter implements TaskLike { try { // If we're not streaming then `abortStream` won't be called. if (this.isStreaming && this.diffViewProvider.isEditing) { - pendingCleanup.push(this.diffViewProvider.revertChanges().catch(console.error)) + const pendingReversion = this.diffViewProvider.revertChanges().catch(console.error) + pendingCleanup = pendingCleanup.then(() => pendingReversion) } } catch (error) { console.error("Error reverting diff changes:", error) } - return Promise.all(pendingCleanup).then(() => undefined) + return pendingCleanup } // Subtasks diff --git a/src/core/task/__tests__/Task.dispose.test.ts b/src/core/task/__tests__/Task.dispose.test.ts index 8a712c6280..9e6665f8bb 100644 --- a/src/core/task/__tests__/Task.dispose.test.ts +++ b/src/core/task/__tests__/Task.dispose.test.ts @@ -1,3 +1,5 @@ +import path from "node:path" + import { type ProviderSettings, RooCodeEventName } from "@roo-code/types" import { Task } from "../Task" @@ -107,10 +109,23 @@ describe("Task dispose method", () => { resolveTaskDirectory!("/test/path/tasks/test-task") await disposal - expect(OutputInterceptor.cleanup).toHaveBeenCalledWith("/test/path/tasks/test-task/command-output") + expect(OutputInterceptor.cleanup).toHaveBeenCalledWith( + path.join("/test/path/tasks/test-task", "command-output"), + ) expect(disposalComplete).toBe(true) }) + test("should report command output cleanup failures before disposal completes", async () => { + const cleanupError = new Error("cleanup failed") + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + vi.mocked(getTaskDirectoryPath).mockRejectedValueOnce(cleanupError) + + await task.dispose() + + expect(consoleErrorSpy).toHaveBeenCalledWith("Error cleaning up command output artifacts:", cleanupError) + consoleErrorSpy.mockRestore() + }) + test("should expose completion of deferred diff reversion", async () => { let resolveReversion: () => void const reversion = new Promise((resolve) => { @@ -118,15 +133,17 @@ describe("Task dispose method", () => { }) task.isStreaming = true task.diffViewProvider.isEditing = true - vi.spyOn(task.diffViewProvider, "revertChanges").mockReturnValue(reversion) + const revertChangesSpy = vi.spyOn(task.diffViewProvider, "revertChanges").mockReturnValue(reversion) const disposal = task.dispose() + await vi.waitFor(() => expect(OutputInterceptor.cleanup).toHaveBeenCalled()) let disposalComplete = false void disposal.then(() => { disposalComplete = true }) await Promise.resolve() expect(disposalComplete).toBe(false) + expect(revertChangesSpy).toHaveBeenCalledOnce() resolveReversion!() await disposal From 1141702ca5460bc63d7c12c8e0ae1f984101160f Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 4 Sep 2026 17:54:52 +0000 Subject: [PATCH 15/26] fix(task): preserve abort completion semantics --- src/core/task/Task.ts | 49 ++++++++-- src/core/task/__tests__/Task.dispose.test.ts | 90 +++++++++++++++++-- src/core/task/__tests__/Task.spec.ts | 34 +++++-- src/core/webview/ClineProvider.ts | 4 + .../ClineProvider.flicker-free-cancel.spec.ts | 8 +- .../webview/__tests__/ClineProvider.spec.ts | 29 ++++++ 6 files changed, 190 insertions(+), 24 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 1b438247ac..dfb61471ef 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -370,6 +370,9 @@ export class Task extends EventEmitter implements TaskLike { private lastTelemetryFlushAt: number = Date.now() private telemetryToolUsageBaseline: ToolUsage = {} private telemetryMessageCountsBaseline: { user: number; assistant: number } = { user: 0, assistant: 0 } + private abortPromise?: Promise + private disposalPromise?: Promise + private diffReversionPromise: Promise = Promise.resolve() // Checkpoints enableCheckpoints: boolean @@ -2464,15 +2467,18 @@ export class Task extends EventEmitter implements TaskLike { this.debouncedEmitTokenUsage.flush() } - public async abortTask(isAbandoned = false) { - // Aborting task - - // Will stop any autonomously running promises. + public abortTask(isAbandoned = false): Promise { if (isAbandoned) { this.abandoned = true } this.abort = true + this.abortPromise ??= this.abortTaskOnce() + return this.abortPromise + } + + private async abortTaskOnce(): Promise { + // Aborting task // Reset consecutive error counters on abort (manual intervention) this.consecutiveNoToolUseCount = 0 @@ -2493,7 +2499,12 @@ export class Task extends EventEmitter implements TaskLike { this.emit(RooCodeEventName.TaskAborted) try { - await this.dispose() // Call the centralized dispose method + void this.dispose().catch((error) => { + console.error(`Error during task ${this.taskId}.${this.instanceId} disposal:`, error) + }) + // Reversion affects the user's workspace and must finish before the + // final task state is saved. Artifact deletion is drained separately. + await this.diffReversionPromise } catch (error) { console.error(`Error during task ${this.taskId}.${this.instanceId} disposal:`, error) // Don't rethrow - we want abort to always succeed @@ -2515,6 +2526,27 @@ export class Task extends EventEmitter implements TaskLike { } public dispose(): Promise { + if (this.disposalPromise) { + return this.disposalPromise + } + + let resolveDisposal!: () => void + let rejectDisposal!: (error: unknown) => void + this.disposalPromise = new Promise((resolve, reject) => { + resolveDisposal = resolve + rejectDisposal = reject + }) + + try { + void this.disposeOnce().then(resolveDisposal, rejectDisposal) + } catch (error) { + rejectDisposal(error) + } + + return this.disposalPromise + } + + private disposeOnce(): Promise { console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`) // Stop the idle telemetry check and report any unflushed activity as a @@ -2563,7 +2595,7 @@ export class Task extends EventEmitter implements TaskLike { } // Cleanup command output artifacts - let pendingCleanup = getTaskDirectoryPath(this.globalStoragePath, this.taskId) + const pendingCleanup = getTaskDirectoryPath(this.globalStoragePath, this.taskId) .then((taskDir) => { const outputDir = path.join(taskDir, "command-output") return OutputInterceptor.cleanup(outputDir) @@ -2591,14 +2623,13 @@ export class Task extends EventEmitter implements TaskLike { try { // If we're not streaming then `abortStream` won't be called. if (this.isStreaming && this.diffViewProvider.isEditing) { - const pendingReversion = this.diffViewProvider.revertChanges().catch(console.error) - pendingCleanup = pendingCleanup.then(() => pendingReversion) + this.diffReversionPromise = this.diffViewProvider.revertChanges().catch(console.error) } } catch (error) { console.error("Error reverting diff changes:", error) } - return pendingCleanup + return pendingCleanup.then(() => this.diffReversionPromise) } // Subtasks diff --git a/src/core/task/__tests__/Task.dispose.test.ts b/src/core/task/__tests__/Task.dispose.test.ts index 9e6665f8bb..9cb35694ff 100644 --- a/src/core/task/__tests__/Task.dispose.test.ts +++ b/src/core/task/__tests__/Task.dispose.test.ts @@ -50,6 +50,7 @@ describe("Task dispose method", () => { context: { globalStorageUri: { fsPath: string } } getState: ReturnType log: ReturnType + flushPostStateToWebviewThrottled: ReturnType } let mockApiConfiguration: ProviderSettings let task: Task @@ -65,6 +66,7 @@ describe("Task dispose method", () => { }, getState: vi.fn().mockResolvedValue({ mode: "code" }), log: vi.fn(), + flushPostStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined), } // Mock API configuration @@ -82,8 +84,7 @@ describe("Task dispose method", () => { }) afterEach(async () => { - // Clean up - if (task && !task.abort) { + if (task) { await task.dispose() } }) @@ -117,15 +118,94 @@ describe("Task dispose method", () => { test("should report command output cleanup failures before disposal completes", async () => { const cleanupError = new Error("cleanup failed") - const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) - vi.mocked(getTaskDirectoryPath).mockRejectedValueOnce(cleanupError) + let disposalComplete = false + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { + expect(disposalComplete).toBe(false) + }) + let rejectTaskDirectory!: (error: Error) => void + vi.mocked(getTaskDirectoryPath).mockReturnValueOnce( + new Promise((_, reject) => { + rejectTaskDirectory = reject + }), + ) - await task.dispose() + const disposal = task.dispose() + void disposal.then(() => { + disposalComplete = true + }) + rejectTaskDirectory(cleanupError) + await vi.waitFor(() => expect(consoleErrorSpy).toHaveBeenCalled()) expect(consoleErrorSpy).toHaveBeenCalledWith("Error cleaning up command output artifacts:", cleanupError) + await disposal + expect(disposalComplete).toBe(true) consoleErrorSpy.mockRestore() }) + test("should wait for deferred output cleanup and memoize repeated disposal", async () => { + let resolveCleanup!: () => void + vi.mocked(OutputInterceptor.cleanup).mockReturnValueOnce( + new Promise((resolve) => { + resolveCleanup = resolve + }), + ) + const removeAllListenersSpy = vi.spyOn(task, "removeAllListeners") + + const firstDisposal = task.dispose() + const secondDisposal = task.dispose() + let disposalComplete = false + void firstDisposal.then(() => { + disposalComplete = true + }) + await vi.waitFor(() => expect(OutputInterceptor.cleanup).toHaveBeenCalledOnce()) + + expect(secondDisposal).toBe(firstDisposal) + expect(disposalComplete).toBe(false) + expect(removeAllListenersSpy).toHaveBeenCalledOnce() + + resolveCleanup() + await firstDisposal + expect(disposalComplete).toBe(true) + }) + + test("should await diff reversion during abort without waiting for output cleanup", async () => { + let resolveCleanup!: () => void + let resolveReversion!: () => void + vi.mocked(OutputInterceptor.cleanup).mockReturnValueOnce( + new Promise((resolve) => { + resolveCleanup = resolve + }), + ) + task.isStreaming = true + task.diffViewProvider.isEditing = true + const revertChangesSpy = vi.spyOn(task.diffViewProvider, "revertChanges").mockReturnValue( + new Promise((resolve) => { + resolveReversion = resolve + }), + ) + const saveMessages = vi.fn().mockResolvedValue(true) + Object.defineProperty(task, "saveClineMessages", { value: saveMessages }) + + const abort = task.abortTask() + await vi.waitFor(() => expect(revertChangesSpy).toHaveBeenCalledOnce()) + expect(saveMessages).not.toHaveBeenCalled() + + resolveReversion() + await abort + expect(saveMessages).toHaveBeenCalledOnce() + + let disposalComplete = false + void task.dispose().then(() => { + disposalComplete = true + }) + await Promise.resolve() + expect(disposalComplete).toBe(false) + + resolveCleanup() + await task.dispose() + expect(disposalComplete).toBe(true) + }) + test("should expose completion of deferred diff reversion", async () => { let resolveReversion: () => void const reversion = new Promise((resolve) => { diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 0c2877bf3a..909a51c4f7 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -2142,7 +2142,7 @@ describe("Cline", () => { expect(disposeSpy).toHaveBeenCalled() }) - it("waits for disposal cleanup before abort resolves", async () => { + it("does not wait for ancillary disposal cleanup before abort resolves", async () => { const task = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, @@ -2157,16 +2157,32 @@ describe("Cline", () => { const abort = task.abortTask() await vi.waitFor(() => expect(disposeSpy).toHaveBeenCalledOnce()) - let abortComplete = false - void abort.then(() => { - abortComplete = true - }) - await Promise.resolve() - expect(abortComplete).toBe(false) + await abort + expect(disposeSpy).toHaveBeenCalledOnce() resolveDisposal!() - await abort - expect(abortComplete).toBe(true) + await disposal + }) + + it("memoizes concurrent aborts while preserving abandoned state", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + const emitSpy = vi.spyOn(task, "emit") + vi.spyOn(task, "dispose").mockResolvedValue(undefined) + + const firstAbort = task.abortTask() + const secondAbort = task.abortTask(true) + + expect(secondAbort).toBe(firstAbort) + expect(task.abandoned).toBe(true) + await firstAbort + expect( + (emitSpy.mock.calls as unknown[][]).filter(([event]) => event === RooCodeEventName.TaskAborted), + ).toHaveLength(1) }) it("flushes pending state before TaskAborted and disposal while queue state is intact", async () => { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 0a251aba5f..c1932b9da6 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -827,10 +827,14 @@ export class ClineProvider // so an active delegated child is marked interrupted before the extension shuts down, // rather than being left persisted as "active" across the reload. if (this.taskRegistry.length > 0) { + const task = this.taskRegistry.current await this.evictCurrentTask() + await task?.dispose() } while (this.taskRegistry.length > 0) { + const task = this.taskRegistry.current await this.removeClineFromStack() + await task?.dispose() } this.log("Cleared all tasks") diff --git a/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts b/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts index e0ece4f9f7..f2832b2468 100644 --- a/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts @@ -22,7 +22,12 @@ type CreatedHistoryTask = Awaited { taskId: "task-1", // Same ID for rehydration scenario instanceId: "instance-2", // Different instance emit: vi.fn(), + dispose: vi.fn().mockResolvedValue(undefined), on: vi.fn(), off: vi.fn(), } diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index ad6ea143a8..2154d73b2e 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -266,6 +266,7 @@ vi.mock("../../task/Task", () => ({ return { api: undefined, abortTask: vi.fn(), + dispose: vi.fn().mockResolvedValue(undefined), handleWebviewAskResponse: vi.fn(), clineMessages: [], apiConversationHistory: [], @@ -413,6 +414,7 @@ describe("ClineProvider", () => { const task: any = { api: undefined, abortTask: vi.fn(), + dispose: vi.fn().mockResolvedValue(undefined), handleWebviewAskResponse: vi.fn(), clineMessages: [], apiConversationHistory: [], @@ -1138,6 +1140,33 @@ describe("ClineProvider", () => { expect(disposeCalls).toHaveLength(1) }) + test("dispose drains task cleanup after abort completes", async () => { + let resolveCleanup!: () => void + const cleanup = new Promise((resolve) => { + resolveCleanup = resolve + }) + const task = { + taskId: "shutdown-task", + emit: vi.fn(), + abortTask: vi.fn().mockResolvedValue(undefined), + dispose: vi.fn().mockReturnValue(cleanup), + } + Object.assign(provider, { taskRegistry: new TaskRegistry() }) + provider["taskRegistry"].push(task as unknown as Task) + let shutdownComplete = false + + const shutdown = provider.dispose() + void shutdown.then(() => { + shutdownComplete = true + }) + await vi.waitFor(() => expect(task.dispose).toHaveBeenCalledOnce()) + + expect(shutdownComplete).toBe(false) + resolveCleanup() + await shutdown + expect(shutdownComplete).toBe(true) + }) + test("handles webviewDidLaunch message", async () => { await provider.resolveWebviewView(mockWebviewView) From f008344eb1332d276ca357733153f740a561fef2 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 4 Sep 2026 18:00:21 +0000 Subject: [PATCH 16/26] test(task): cover disposal failure branches --- src/core/task/__tests__/Task.dispose.test.ts | 14 +++++++++++- src/core/task/__tests__/Task.spec.ts | 23 ++++++++++++++++++++ src/core/webview/ClineProvider.ts | 8 +++---- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/core/task/__tests__/Task.dispose.test.ts b/src/core/task/__tests__/Task.dispose.test.ts index 9cb35694ff..8f346697b4 100644 --- a/src/core/task/__tests__/Task.dispose.test.ts +++ b/src/core/task/__tests__/Task.dispose.test.ts @@ -85,7 +85,7 @@ describe("Task dispose method", () => { afterEach(async () => { if (task) { - await task.dispose() + await task.dispose().catch(() => {}) } }) @@ -116,6 +116,18 @@ describe("Task dispose method", () => { expect(disposalComplete).toBe(true) }) + test("should reject the memoized completion promise when disposal cannot start", async () => { + const disposalError = new Error("disposal failed") + vi.spyOn(console, "log").mockImplementationOnce(() => { + throw disposalError + }) + + const disposal = task.dispose() + + await expect(disposal).rejects.toBe(disposalError) + expect(task.dispose()).toBe(disposal) + }) + test("should report command output cleanup failures before disposal completes", async () => { const cleanupError = new Error("cleanup failed") let disposalComplete = false diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 909a51c4f7..0376f437cb 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -2118,6 +2118,7 @@ describe("Cline", () => { // Verify abort flag is set expect(task.abort).toBe(true) + expect(task.abandoned).toBe(false) // Verify TaskAborted event was emitted expect(emitSpy).toHaveBeenCalledWith("taskAborted") @@ -2301,6 +2302,28 @@ describe("Cline", () => { // Restore console.error consoleErrorSpy.mockRestore() }) + + it("should handle asynchronous disposal errors gracefully", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + const disposalError = new Error("Disposal failed asynchronously") + vi.spyOn(task, "dispose").mockRejectedValue(disposalError) + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + + await expect(task.abortTask()).resolves.toBeUndefined() + await vi.waitFor(() => + expect(consoleErrorSpy).toHaveBeenCalledWith( + `Error during task ${task.taskId}.${task.instanceId} disposal:`, + disposalError, + ), + ) + + consoleErrorSpy.mockRestore() + }) describe("Stream Failure Retry", () => { it("should not abort task on stream failure, only on user cancellation", async () => { const task = new Task({ diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index c1932b9da6..42edc2f6e6 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -827,14 +827,14 @@ export class ClineProvider // so an active delegated child is marked interrupted before the extension shuts down, // rather than being left persisted as "active" across the reload. if (this.taskRegistry.length > 0) { - const task = this.taskRegistry.current + const task = this.taskRegistry.current! await this.evictCurrentTask() - await task?.dispose() + await task.dispose() } while (this.taskRegistry.length > 0) { - const task = this.taskRegistry.current + const task = this.taskRegistry.current! await this.removeClineFromStack() - await task?.dispose() + await task.dispose() } this.log("Cleared all tasks") From bd193469f1be7f84c870f6a036de2ee43459647f Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 4 Sep 2026 18:06:09 +0000 Subject: [PATCH 17/26] test(task): fail fast on stalled disposal --- src/core/task/__tests__/Task.dispose.test.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/core/task/__tests__/Task.dispose.test.ts b/src/core/task/__tests__/Task.dispose.test.ts index 8f346697b4..5e8d391cf6 100644 --- a/src/core/task/__tests__/Task.dispose.test.ts +++ b/src/core/task/__tests__/Task.dispose.test.ts @@ -54,10 +54,12 @@ describe("Task dispose method", () => { } let mockApiConfiguration: ProviderSettings let task: Task + let skipCleanup: boolean beforeEach(() => { // Reset all mocks vi.clearAllMocks() + skipCleanup = false // Mock provider mockProvider = { @@ -84,7 +86,7 @@ describe("Task dispose method", () => { }) afterEach(async () => { - if (task) { + if (task && !skipCleanup) { await task.dispose().catch(() => {}) } }) @@ -118,13 +120,19 @@ describe("Task dispose method", () => { test("should reject the memoized completion promise when disposal cannot start", async () => { const disposalError = new Error("disposal failed") + skipCleanup = true vi.spyOn(console, "log").mockImplementationOnce(() => { throw disposalError }) const disposal = task.dispose() + let rejection: unknown + void disposal.catch((error) => { + rejection = error + }) + await Promise.resolve() - await expect(disposal).rejects.toBe(disposalError) + expect(rejection).toBe(disposalError) expect(task.dispose()).toBe(disposal) }) From 7d135f96d41659ec62aa12fcd0b240c86efdd3c9 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 4 Sep 2026 19:14:32 +0000 Subject: [PATCH 18/26] test(task): cover multi-task shutdown disposal --- src/core/webview/ClineProvider.ts | 15 ++- .../webview/__tests__/ClineProvider.spec.ts | 92 ++++++++++++++++--- 2 files changed, 93 insertions(+), 14 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 42edc2f6e6..138b0b6173 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -810,6 +810,17 @@ export class ClineProvider } } + /** Drain one task's memoized cleanup without preventing the remaining provider shutdown work. */ + private async drainTaskDisposal(task: Task): Promise { + try { + await task.dispose() + } catch (error) { + this.log( + `[ClineProvider#dispose] Task cleanup failed for ${task.taskId}.${task.instanceId}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } + async dispose() { if (this._disposed) { return @@ -829,12 +840,12 @@ export class ClineProvider if (this.taskRegistry.length > 0) { const task = this.taskRegistry.current! await this.evictCurrentTask() - await task.dispose() + await this.drainTaskDisposal(task) } while (this.taskRegistry.length > 0) { const task = this.taskRegistry.current! await this.removeClineFromStack() - await task.dispose() + await this.drainTaskDisposal(task) } this.log("Cleared all tasks") diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 2154d73b2e..02052aac65 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1140,33 +1140,101 @@ describe("ClineProvider", () => { expect(disposeCalls).toHaveLength(1) }) - test("dispose drains task cleanup after abort completes", async () => { - let resolveCleanup!: () => void - const cleanup = new Promise((resolve) => { - resolveCleanup = resolve - }) - const task = { - taskId: "shutdown-task", + test("dispose drains every task in abort-then-cleanup order", async () => { + let resolveCurrentAbort!: () => void + let resolveCurrentCleanup!: () => void + let resolveRemainingAbort!: () => void + let resolveRemainingCleanup!: () => void + const currentTask = { + taskId: "current-task", + instanceId: "current-instance", emit: vi.fn(), - abortTask: vi.fn().mockResolvedValue(undefined), - dispose: vi.fn().mockReturnValue(cleanup), + abortTask: vi.fn().mockReturnValue( + new Promise((resolve) => { + resolveCurrentAbort = resolve + }), + ), + dispose: vi.fn().mockReturnValue( + new Promise((resolve) => { + resolveCurrentCleanup = resolve + }), + ), + } + const remainingTask = { + taskId: "remaining-task", + instanceId: "remaining-instance", + emit: vi.fn(), + abortTask: vi.fn().mockReturnValue( + new Promise((resolve) => { + resolveRemainingAbort = resolve + }), + ), + dispose: vi.fn().mockReturnValue( + new Promise((resolve) => { + resolveRemainingCleanup = resolve + }), + ), } Object.assign(provider, { taskRegistry: new TaskRegistry() }) - provider["taskRegistry"].push(task as unknown as Task) + provider["taskRegistry"].push(remainingTask as unknown as Task) + provider["taskRegistry"].push(currentTask as unknown as Task) let shutdownComplete = false const shutdown = provider.dispose() void shutdown.then(() => { shutdownComplete = true }) - await vi.waitFor(() => expect(task.dispose).toHaveBeenCalledOnce()) + await vi.waitFor(() => expect(currentTask.abortTask).toHaveBeenCalledOnce()) + expect(currentTask.dispose).not.toHaveBeenCalled() + expect(remainingTask.abortTask).not.toHaveBeenCalled() + + resolveCurrentAbort() + await vi.waitFor(() => expect(currentTask.dispose).toHaveBeenCalledOnce()) + expect(remainingTask.abortTask).not.toHaveBeenCalled() + + resolveCurrentCleanup() + await vi.waitFor(() => expect(remainingTask.abortTask).toHaveBeenCalledOnce()) + expect(remainingTask.dispose).not.toHaveBeenCalled() + + resolveRemainingAbort() + await vi.waitFor(() => expect(remainingTask.dispose).toHaveBeenCalledOnce()) expect(shutdownComplete).toBe(false) - resolveCleanup() + resolveRemainingCleanup() await shutdown expect(shutdownComplete).toBe(true) }) + test("dispose continues draining tasks after cleanup rejects", async () => { + const cleanupError = new Error("cleanup failed") + const logSpy = vi.spyOn(provider, "log") + const remainingTask = { + taskId: "remaining-task", + instanceId: "remaining-instance", + emit: vi.fn(), + abortTask: vi.fn().mockResolvedValue(undefined), + dispose: vi.fn().mockResolvedValue(undefined), + } + const currentTask = { + taskId: "current-task", + instanceId: "current-instance", + emit: vi.fn(), + abortTask: vi.fn().mockResolvedValue(undefined), + dispose: vi.fn().mockRejectedValue(cleanupError), + } + Object.assign(provider, { taskRegistry: new TaskRegistry() }) + provider["taskRegistry"].push(remainingTask as unknown as Task) + provider["taskRegistry"].push(currentTask as unknown as Task) + + await expect(provider.dispose()).resolves.toBeUndefined() + + expect(currentTask.dispose).toHaveBeenCalledOnce() + expect(remainingTask.dispose).toHaveBeenCalledOnce() + expect(logSpy).toHaveBeenCalledWith( + "[ClineProvider#dispose] Task cleanup failed for current-task.current-instance: cleanup failed", + ) + }) + test("handles webviewDidLaunch message", async () => { await provider.resolveWebviewView(mockWebviewView) From 7bddaf19c2e2f6dea9f41e12f7432a2c4d165f4f Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 4 Sep 2026 19:56:44 +0000 Subject: [PATCH 19/26] test(formal): model task cleanup protocol --- .../task-cleanup-protocol-model.md | 33 ++ docs/architecture/task-lifecycle-model.md | 4 + package.json | 3 +- scripts/check-task-cleanup-protocol.ts | 323 ++++++++++++++++++ src/core/task/__tests__/Task.dispose.test.ts | 17 + .../webview/__tests__/ClineProvider.spec.ts | 35 +- 6 files changed, 411 insertions(+), 4 deletions(-) create mode 100644 docs/architecture/task-cleanup-protocol-model.md create mode 100644 scripts/check-task-cleanup-protocol.ts diff --git a/docs/architecture/task-cleanup-protocol-model.md b/docs/architecture/task-cleanup-protocol-model.md new file mode 100644 index 0000000000..2dc2251fc3 --- /dev/null +++ b/docs/architecture/task-cleanup-protocol-model.md @@ -0,0 +1,33 @@ +# Task cleanup protocol model check + +Zoo Code checks the in-memory task cleanup protocol with a bounded explicit-state explorer. It runs under the umbrella command: + +```sh +pnpm lifecycle:model-check +``` + +For focused debugging, run: + +```sh +pnpm cleanup-protocol:model-check +``` + +This is a separate child model from the persisted task lifecycle and shared-store concurrency models. It follows the native tool-call parser model pattern: keep an independent bounded state space for an independent protocol, require every action and semantic landmark to remain reachable, and connect the abstract claims to focused production tests. + +## Bounds and environment actions + +The model uses two tasks and explores every reachable interleaving through depth 20, with an explicit 50,000-state budget. Abort, disposal, final-save, provider-drain, and shutdown-cursor state are modeled directly. Cleanup and editor-reversion settlement or rejection are environment actions, so the explorer does not assume they eventually occur. + +The model checks these finite safety properties: + +1. repeated abort and disposal calls reuse their first logical handle and start each operation at most once; +2. final message persistence, or the history-task save skip, cannot occur before editor reversion settles or rejects; +3. abort may complete while ancillary output cleanup remains pending; +4. disposal completes only after cleanup and reversion both settle or reject; +5. provider shutdown advances through tasks only after each task's abort and disposal reach terminal states; +6. shutdown advances through tasks in registry order and is complete exactly when every modeled task is drained; and +7. abort, ancillary cleanup, or disposal-start rejection is isolated so shutdown can continue to later tasks. + +Named landmarks require abort completion during pending ancillary cleanup, contained cleanup rejection, final-save attempt after rejected reversion, a history-task final-save skip, two-task shutdown completion, and shutdown continuation after rejected abort, ancillary cleanup, or disposal start. + +These are bounded safety and reachability claims only. The model does not claim filesystem or editor Promise liveness, fairness, timing bounds, arbitrary task counts, or that cleanup can never remain pending. Deterministic Vitest coverage in `Task.dispose.test.ts`, `Task.spec.ts`, and `ClineProvider.spec.ts` exercises the corresponding production Promise identities, ordering, rejection handling, and multi-task shutdown behavior. diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index 588ffd5204..3218eb4ba8 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -63,6 +63,10 @@ The known-unsafe witnesses currently compare exact shortest action sequences. Th `TaskHistoryStore.realConcurrency.spec.ts` complements the abstract interleavings with one synchronized integration smoke check through the real `proper-lockfile` and filesystem rename path; broader VS Code E2E remains reserved for restart and extension-host behavior. +## Task cleanup protocol model + +The umbrella command also runs a separate bounded child model for in-memory abort, disposal, and provider-shutdown ordering. It models cleanup settlement and rejection as environment transitions and makes no filesystem, editor Promise, fairness, or timing-liveness claim. See [Task cleanup protocol model check](./task-cleanup-protocol-model.md). + ## Invariants The checker currently enforces: diff --git a/package.json b/package.json index 8431467918..1a44a12680 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,8 @@ "check-types": "turbo check-types --log-order grouped --output-logs new-only", "test": "turbo test --log-order grouped --output-logs new-only", "test:mutation-ci": "node --test scripts/stryker-diff.test.mjs", - "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts", + "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && pnpm cleanup-protocol:model-check", + "cleanup-protocol:model-check": "tsx scripts/check-task-cleanup-protocol.ts", "test:coverage": "turbo test:coverage --log-order grouped --output-logs new-only", "format": "turbo format --log-order grouped --output-logs new-only", "build": "turbo build --log-order grouped --output-logs new-only", diff --git a/scripts/check-task-cleanup-protocol.ts b/scripts/check-task-cleanup-protocol.ts new file mode 100644 index 0000000000..9b43d97c56 --- /dev/null +++ b/scripts/check-task-cleanup-protocol.ts @@ -0,0 +1,323 @@ +import assert from "node:assert/strict" + +const taskIds = ["A", "B"] as const +type TaskId = (typeof taskIds)[number] +type PendingResult = "idle" | "pending" | "resolved" | "rejected" + +interface TaskState { + abort: PendingResult + abortHandle?: 1 + abortStarts: number + disposal: PendingResult + disposalHandle?: 1 + disposalStarts: number + reversion: PendingResult + cleanup: PendingResult + finalization: "idle" | "attempted" | "skipped" +} + +interface ModelState { + tasks: Record + shutdown: "idle" | "draining" | "done" + shutdownIndex: number + drained: Record +} + +interface Step { + action: string + state: ModelState +} + +const MAX_DEPTH = 20 +const MAX_STATES = 50_000 +const expectedActions = [ + "abort", + "dispose", + "abort-starts-disposal", + "reject-disposal-start", + "settle-reversion", + "reject-reversion", + "settle-cleanup", + "reject-cleanup", + "reject-abort", + "complete-abort", + "skip-final-save", + "complete-disposal", + "start-shutdown", + "advance-shutdown", +] as const + +function task(): TaskState { + return { + abort: "idle", + abortStarts: 0, + disposal: "idle", + disposalStarts: 0, + reversion: "idle", + cleanup: "idle", + finalization: "idle", + } +} + +function initialState(): ModelState { + return { + tasks: { A: task(), B: task() }, + shutdown: "idle", + shutdownIndex: 0, + drained: { A: false, B: false }, + } +} + +function clone(state: ModelState): ModelState { + return structuredClone(state) +} + +function isTerminal(result: PendingResult): boolean { + return result === "resolved" || result === "rejected" +} + +function callAbort(state: ModelState, taskId: TaskId): ModelState { + const next = clone(state) + const current = next.tasks[taskId] + if (current.abort === "idle") { + current.abort = "pending" + current.abortHandle = 1 + current.abortStarts += 1 + } + return next +} + +function callDispose(state: ModelState, taskId: TaskId): ModelState { + const next = clone(state) + const current = next.tasks[taskId] + if (current.disposal === "idle") { + current.disposal = "pending" + current.disposalHandle = 1 + current.disposalStarts += 1 + current.reversion = "pending" + current.cleanup = "pending" + } + return next +} + +function transitions(state: ModelState): Step[] { + const result: Step[] = [] + for (const taskId of taskIds) { + const current = state.tasks[taskId] + if (current.abort === "idle") { + result.push({ action: `abort(${taskId})`, state: callAbort(state, taskId) }) + } + if (current.disposal === "idle") { + result.push({ action: `dispose(${taskId})`, state: callDispose(state, taskId) }) + const next = clone(state) + const failed = next.tasks[taskId] + failed.disposal = "rejected" + failed.disposalHandle = 1 + failed.disposalStarts = 1 + failed.reversion = "rejected" + failed.cleanup = "rejected" + result.push({ action: `reject-disposal-start(${taskId})`, state: next }) + } + if (current.abort === "pending" && current.disposal === "idle") { + result.push({ action: `abort-starts-disposal(${taskId})`, state: callDispose(state, taskId) }) + } + if (current.reversion === "pending") { + for (const outcome of ["resolved", "rejected"] as const) { + const next = clone(state) + next.tasks[taskId].reversion = outcome + result.push({ + action: `${outcome === "resolved" ? "settle" : "reject"}-reversion(${taskId})`, + state: next, + }) + } + } + if (current.cleanup === "pending") { + for (const outcome of ["resolved", "rejected"] as const) { + const next = clone(state) + next.tasks[taskId].cleanup = outcome + result.push({ + action: `${outcome === "resolved" ? "settle" : "reject"}-cleanup(${taskId})`, + state: next, + }) + } + } + if (current.abort === "pending") { + const next = clone(state) + next.tasks[taskId].abort = "rejected" + result.push({ action: `reject-abort(${taskId})`, state: next }) + } + if ( + current.abort === "pending" && + current.disposal !== "idle" && + isTerminal(current.reversion) && + current.finalization === "idle" + ) { + for (const finalization of ["attempted", "skipped"] as const) { + const next = clone(state) + next.tasks[taskId].finalization = finalization + next.tasks[taskId].abort = "resolved" + result.push({ + action: `${finalization === "attempted" ? "complete-abort" : "skip-final-save"}(${taskId})`, + state: next, + }) + } + } + if (current.disposal === "pending" && isTerminal(current.reversion) && isTerminal(current.cleanup)) { + const next = clone(state) + next.tasks[taskId].disposal = "resolved" + result.push({ action: `complete-disposal(${taskId})`, state: next }) + } + } + + if (state.shutdown === "idle") { + const next = clone(state) + next.shutdown = "draining" + result.push({ action: "start-shutdown()", state: next }) + } else if (state.shutdown === "draining") { + const taskId = taskIds[state.shutdownIndex] + if (taskId) { + const current = state.tasks[taskId] + if (isTerminal(current.abort) && isTerminal(current.disposal)) { + const next = clone(state) + next.drained[taskId] = true + next.shutdownIndex += 1 + if (next.shutdownIndex === taskIds.length) next.shutdown = "done" + result.push({ action: `advance-shutdown(${taskId})`, state: next }) + } + } + } + return result +} + +function invariantViolations(state: ModelState): string[] { + const violations: string[] = [] + for (const taskId of taskIds) { + const current = state.tasks[taskId] + if (current.abortStarts > 1 || current.disposalStarts > 1) { + violations.push(`${taskId}: abort and disposal may each start at most once`) + } + if ((current.abort === "idle") === Boolean(current.abortHandle)) { + violations.push(`${taskId}: abort handle must exist exactly when abort has started`) + } + if ((current.disposal === "idle") === Boolean(current.disposalHandle)) { + violations.push(`${taskId}: disposal handle must exist exactly when disposal has started`) + } + if (current.finalization !== "idle" && !isTerminal(current.reversion)) { + violations.push(`${taskId}: abort finalization occurred before editor reversion settled`) + } + if (current.abort === "resolved" && current.finalization === "idle") { + violations.push(`${taskId}: abort resolved before its final save attempt or history-task skip`) + } + if (current.finalization !== "idle" && current.abort !== "resolved") { + violations.push(`${taskId}: abort finalization exists without a resolved abort`) + } + if (current.disposal === "resolved" && (!isTerminal(current.cleanup) || !isTerminal(current.reversion))) { + violations.push(`${taskId}: disposal resolved before all cleanup branches settled`) + } + if (state.drained[taskId] && (!isTerminal(current.abort) || !isTerminal(current.disposal))) { + violations.push(`${taskId}: provider advanced before abort and disposal completed`) + } + } + if (state.shutdownIndex !== taskIds.filter((taskId) => state.drained[taskId]).length) { + violations.push("shutdown cursor must match the drained task prefix") + } + if (state.drained.B && !state.drained.A) violations.push("provider drained tasks out of order") + if ((state.shutdown === "done") !== taskIds.every((taskId) => state.drained[taskId])) { + violations.push("shutdown is done exactly when every modeled task is drained") + } + return violations +} + +function canonical(state: ModelState): string { + return JSON.stringify(state) +} + +function runRepresentativeMemoizationChecks(): void { + const start = initialState() + const firstAbort = callAbort(start, "A") + assert.deepEqual(callAbort(firstAbort, "A"), firstAbort, "repeated abort must reuse the first handle") + const firstDisposal = callDispose(start, "A") + assert.deepEqual(callDispose(firstDisposal, "A"), firstDisposal, "repeated disposal must reuse the first handle") +} + +function runModelCheck(): { states: number; actions: number; landmarks: number } { + const start = initialState() + const queue: Array<{ state: ModelState; trace: Step[] }> = [{ state: start, trace: [] }] + const visited = new Set([canonical(start)]) + const reachedActions = new Set() + const reachedLandmarks = new Set() + const frontier: ModelState[] = [] + + for (let index = 0; index < queue.length; index++) { + const node = queue[index]! + const violations = invariantViolations(node.state) + if (violations.length) { + throw new Error( + `Task cleanup protocol invariant failed: ${violations.join("; ")}\n${node.trace.map((step, i) => `${i + 1}. ${step.action}`).join("\n")}`, + ) + } + if (node.state.tasks.A.abort === "resolved" && node.state.tasks.A.cleanup === "pending") { + reachedLandmarks.add("abort-completes-before-ancillary-cleanup") + } + if (node.state.tasks.A.disposal === "resolved" && node.state.tasks.A.cleanup === "rejected") { + reachedLandmarks.add("cleanup-rejection-is-observed-and-contained") + } + if ( + node.state.tasks.A.abort === "resolved" && + node.state.tasks.A.reversion === "rejected" && + node.state.tasks.A.finalization === "attempted" + ) { + reachedLandmarks.add("reversion-rejection-does-not-block-final-save") + } + if (node.state.drained.A && node.state.tasks.A.cleanup === "rejected" && node.state.drained.B) { + reachedLandmarks.add("shutdown-continues-after-cleanup-rejection") + } + if (node.state.drained.A && node.state.tasks.A.disposal === "rejected" && node.state.drained.B) { + reachedLandmarks.add("shutdown-continues-after-disposal-rejection") + } + if (node.state.drained.A && node.state.tasks.A.abort === "rejected" && node.state.drained.B) { + reachedLandmarks.add("shutdown-continues-after-abort-rejection") + } + if (node.state.tasks.A.abort === "resolved" && node.state.tasks.A.finalization === "skipped") { + reachedLandmarks.add("history-task-final-save-skip") + } + if (node.state.shutdown === "done") reachedLandmarks.add("multi-task-shutdown-drained") + + if (node.trace.length === MAX_DEPTH) { + frontier.push(node.state) + continue + } + for (const step of transitions(node.state)) { + reachedActions.add(step.action.slice(0, step.action.indexOf("("))) + const key = canonical(step.state) + if (visited.has(key)) continue + visited.add(key) + queue.push({ state: step.state, trace: [...node.trace, step] }) + if (visited.size > MAX_STATES) throw new Error(`Cleanup protocol exceeded ${MAX_STATES} states`) + } + } + + const unseen = frontier.flatMap(transitions).find((step) => !visited.has(canonical(step.state))) + if (unseen) throw new Error(`Cleanup protocol truncated before unseen action ${unseen.action}`) + const missingActions = expectedActions.filter((action) => !reachedActions.has(action)) + assert.deepEqual(missingActions, [], `Cleanup protocol has unreachable actions: ${missingActions.join(", ")}`) + const expectedLandmarks = [ + "abort-completes-before-ancillary-cleanup", + "cleanup-rejection-is-observed-and-contained", + "reversion-rejection-does-not-block-final-save", + "shutdown-continues-after-cleanup-rejection", + "shutdown-continues-after-disposal-rejection", + "shutdown-continues-after-abort-rejection", + "history-task-final-save-skip", + "multi-task-shutdown-drained", + ] + const missingLandmarks = expectedLandmarks.filter((landmark) => !reachedLandmarks.has(landmark)) + assert.deepEqual(missingLandmarks, [], `Cleanup protocol has unreachable landmarks: ${missingLandmarks.join(", ")}`) + return { states: visited.size, actions: reachedActions.size, landmarks: reachedLandmarks.size } +} + +runRepresentativeMemoizationChecks() +const result = runModelCheck() +console.log( + `Task cleanup protocol model check passed: ${result.states} reachable states, ${result.actions}/${expectedActions.length} actions reachable, ${result.landmarks}/8 landmarks reached, depth <= ${MAX_DEPTH}, tasks=${taskIds.length}`, +) diff --git a/src/core/task/__tests__/Task.dispose.test.ts b/src/core/task/__tests__/Task.dispose.test.ts index 5e8d391cf6..472218fce5 100644 --- a/src/core/task/__tests__/Task.dispose.test.ts +++ b/src/core/task/__tests__/Task.dispose.test.ts @@ -250,6 +250,23 @@ describe("Task dispose method", () => { expect(disposalComplete).toBe(true) }) + test("should log rejected diff reversion and continue final abort persistence", async () => { + const reversionError = new Error("reversion failed") + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + task.isStreaming = true + task.diffViewProvider.isEditing = true + vi.spyOn(task.diffViewProvider, "revertChanges").mockRejectedValue(reversionError) + const saveMessages = vi.fn().mockResolvedValue(true) + Object.defineProperty(task, "saveClineMessages", { value: saveMessages }) + + await expect(task.abortTask()).resolves.toBeUndefined() + await expect(task.dispose()).resolves.toBeUndefined() + + expect(consoleErrorSpy).toHaveBeenCalledWith(reversionError) + expect(saveMessages).toHaveBeenCalledOnce() + consoleErrorSpy.mockRestore() + }) + test("should remove all event listeners when dispose is called", () => { // Add some event listeners using type assertion to bypass strict typing for testing const listener1 = vi.fn(() => {}) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 02052aac65..690051b759 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1181,9 +1181,11 @@ describe("ClineProvider", () => { let shutdownComplete = false const shutdown = provider.dispose() - void shutdown.then(() => { - shutdownComplete = true - }) + void shutdown + .then(() => { + shutdownComplete = true + }) + .catch(() => {}) await vi.waitFor(() => expect(currentTask.abortTask).toHaveBeenCalledOnce()) expect(currentTask.dispose).not.toHaveBeenCalled() expect(remainingTask.abortTask).not.toHaveBeenCalled() @@ -1235,6 +1237,33 @@ describe("ClineProvider", () => { ) }) + test("dispose continues draining tasks after abort rejects", async () => { + const abortError = new Error("abort failed") + const remainingTask = { + taskId: "remaining-task", + instanceId: "remaining-instance", + emit: vi.fn(), + abortTask: vi.fn().mockResolvedValue(undefined), + dispose: vi.fn().mockResolvedValue(undefined), + } + const currentTask = { + taskId: "current-task", + instanceId: "current-instance", + emit: vi.fn(), + abortTask: vi.fn().mockRejectedValue(abortError), + dispose: vi.fn().mockResolvedValue(undefined), + } + Object.assign(provider, { taskRegistry: new TaskRegistry() }) + provider["taskRegistry"].push(remainingTask as unknown as Task) + provider["taskRegistry"].push(currentTask as unknown as Task) + + await expect(provider.dispose()).resolves.toBeUndefined() + + expect(currentTask.dispose).toHaveBeenCalledOnce() + expect(remainingTask.abortTask).toHaveBeenCalledOnce() + expect(remainingTask.dispose).toHaveBeenCalledOnce() + }) + test("handles webviewDidLaunch message", async () => { await provider.resolveWebviewView(mockWebviewView) From a54e4db6a8c95b73128a7eb056a28f5e59062b81 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 4 Sep 2026 20:06:03 +0000 Subject: [PATCH 20/26] test(formal): model provider cleanup phases --- .../task-cleanup-protocol-model.md | 2 +- scripts/check-task-cleanup-protocol.ts | 25 ++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/docs/architecture/task-cleanup-protocol-model.md b/docs/architecture/task-cleanup-protocol-model.md index 2dc2251fc3..e46991365b 100644 --- a/docs/architecture/task-cleanup-protocol-model.md +++ b/docs/architecture/task-cleanup-protocol-model.md @@ -16,7 +16,7 @@ This is a separate child model from the persisted task lifecycle and shared-stor ## Bounds and environment actions -The model uses two tasks and explores every reachable interleaving through depth 20, with an explicit 50,000-state budget. Abort, disposal, final-save, provider-drain, and shutdown-cursor state are modeled directly. Cleanup and editor-reversion settlement or rejection are environment actions, so the explorer does not assume they eventually occur. +The model uses two tasks and explores every reachable interleaving through depth 20, with an explicit 100,000-state budget. Abort, disposal, final-save, provider abort/drain phases, and shutdown-cursor state are modeled directly. Independent abort and disposal calls may interleave freely, while provider-initiated calls are gated to the current shutdown task. Cleanup and editor-reversion settlement or rejection are environment actions, so the explorer does not assume they eventually occur. The model checks these finite safety properties: diff --git a/scripts/check-task-cleanup-protocol.ts b/scripts/check-task-cleanup-protocol.ts index 9b43d97c56..466fad6464 100644 --- a/scripts/check-task-cleanup-protocol.ts +++ b/scripts/check-task-cleanup-protocol.ts @@ -18,7 +18,7 @@ interface TaskState { interface ModelState { tasks: Record - shutdown: "idle" | "draining" | "done" + shutdown: "idle" | "call-abort" | "wait-abort" | "wait-disposal" | "done" shutdownIndex: number drained: Record } @@ -29,7 +29,7 @@ interface Step { } const MAX_DEPTH = 20 -const MAX_STATES = 50_000 +const MAX_STATES = 100_000 const expectedActions = [ "abort", "dispose", @@ -44,6 +44,8 @@ const expectedActions = [ "skip-final-save", "complete-disposal", "start-shutdown", + "shutdown-abort", + "shutdown-dispose", "advance-shutdown", ] as const @@ -171,17 +173,25 @@ function transitions(state: ModelState): Step[] { if (state.shutdown === "idle") { const next = clone(state) - next.shutdown = "draining" + next.shutdown = "call-abort" result.push({ action: "start-shutdown()", state: next }) - } else if (state.shutdown === "draining") { + } else if (state.shutdown !== "done") { const taskId = taskIds[state.shutdownIndex] if (taskId) { const current = state.tasks[taskId] - if (isTerminal(current.abort) && isTerminal(current.disposal)) { + if (state.shutdown === "call-abort") { + const next = callAbort(state, taskId) + next.shutdown = "wait-abort" + result.push({ action: `shutdown-abort(${taskId})`, state: next }) + } else if (state.shutdown === "wait-abort" && isTerminal(current.abort)) { + const next = callDispose(state, taskId) + next.shutdown = "wait-disposal" + result.push({ action: `shutdown-dispose(${taskId})`, state: next }) + } else if (state.shutdown === "wait-disposal" && isTerminal(current.disposal)) { const next = clone(state) next.drained[taskId] = true next.shutdownIndex += 1 - if (next.shutdownIndex === taskIds.length) next.shutdown = "done" + next.shutdown = next.shutdownIndex === taskIds.length ? "done" : "call-abort" result.push({ action: `advance-shutdown(${taskId})`, state: next }) } } @@ -222,6 +232,9 @@ function invariantViolations(state: ModelState): string[] { violations.push("shutdown cursor must match the drained task prefix") } if (state.drained.B && !state.drained.A) violations.push("provider drained tasks out of order") + if (state.shutdownIndex === 1 && !state.drained.A) { + violations.push("provider advanced to the second task before draining the first") + } if ((state.shutdown === "done") !== taskIds.every((taskId) => state.drained[taskId])) { violations.push("shutdown is done exactly when every modeled task is drained") } From ca1f005d086531e8e3d32f68d9bb924a3cb86894 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 4 Sep 2026 20:30:50 +0000 Subject: [PATCH 21/26] fix(task): simplify disposal promise memoization --- src/core/task/Task.ts | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index dfb61471ef..4f122feefc 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2530,23 +2530,11 @@ export class Task extends EventEmitter implements TaskLike { return this.disposalPromise } - let resolveDisposal!: () => void - let rejectDisposal!: (error: unknown) => void - this.disposalPromise = new Promise((resolve, reject) => { - resolveDisposal = resolve - rejectDisposal = reject - }) - - try { - void this.disposeOnce().then(resolveDisposal, rejectDisposal) - } catch (error) { - rejectDisposal(error) - } - + this.disposalPromise = this.disposeOnce() return this.disposalPromise } - private disposeOnce(): Promise { + private async disposeOnce(): Promise { console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`) // Stop the idle telemetry check and report any unflushed activity as a @@ -2629,7 +2617,8 @@ export class Task extends EventEmitter implements TaskLike { console.error("Error reverting diff changes:", error) } - return pendingCleanup.then(() => this.diffReversionPromise) + await pendingCleanup + await this.diffReversionPromise } // Subtasks From d602da4c13ba6b2afd6171c962ef5709d5477d7b Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 4 Sep 2026 21:20:52 +0000 Subject: [PATCH 22/26] test(task): assert rejected abort invocation --- src/core/webview/__tests__/ClineProvider.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 690051b759..1a6a82a5b0 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1259,6 +1259,7 @@ describe("ClineProvider", () => { await expect(provider.dispose()).resolves.toBeUndefined() + expect(currentTask.abortTask).toHaveBeenCalledOnce() expect(currentTask.dispose).toHaveBeenCalledOnce() expect(remainingTask.abortTask).toHaveBeenCalledOnce() expect(remainingTask.dispose).toHaveBeenCalledOnce() From d173c0de1480a502a4749672f25659ef7b45c776 Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Fri, 4 Sep 2026 19:19:56 -0600 Subject: [PATCH 23/26] test(model-cache): kill session-cache mutants without resetModules Auth-session and empty-response suites reimported a fresh module via vi.resetModules(), so Stryker never exercised the instrumented code. Reset transient maps in-process, tighten result/log assertions, and add bound/eviction/reasoning/If-None-Match coverage for surviving mutants. --- .../providers/__tests__/zoo-gateway.spec.ts | 34 + .../fetchers/__tests__/modelCache.spec.ts | 592 ++++++++++-------- .../fetchers/__tests__/zoo-gateway.spec.ts | 16 +- src/api/providers/fetchers/modelCache.ts | 21 + 4 files changed, 391 insertions(+), 272 deletions(-) diff --git a/src/api/providers/__tests__/zoo-gateway.spec.ts b/src/api/providers/__tests__/zoo-gateway.spec.ts index c6f4c15c1e..65a83b94bc 100644 --- a/src/api/providers/__tests__/zoo-gateway.spec.ts +++ b/src/api/providers/__tests__/zoo-gateway.spec.ts @@ -456,6 +456,40 @@ describe("ZooGatewayHandler", () => { ]) }) + it("yields reasoning chunks from delta.reasoning_content before text", async () => { + mockCreate.mockImplementation(async () => + asyncStreamFrom([ + { + choices: [{ delta: { reasoning_content: "thinking hard", content: "answer" }, index: 0 }], + }, + ]), + ) + + const handler = new ZooGatewayHandler(mockOptions) + const chunks = await collectStream(handler.createMessage("prompt", [{ role: "user", content: "Hi" }])) + + expect(chunks).toEqual([ + { type: "reasoning", text: "thinking hard" }, + { type: "text", text: "answer" }, + ]) + }) + + it("does not yield a reasoning chunk when the delta has no reasoning text", async () => { + mockCreate.mockImplementation(async () => + asyncStreamFrom([ + { + choices: [{ delta: { content: "just text" }, index: 0 }], + }, + ]), + ) + + const handler = new ZooGatewayHandler(mockOptions) + const chunks = await collectStream(handler.createMessage("prompt", [{ role: "user", content: "Hi" }])) + + expect(chunks).toEqual([{ type: "text", text: "just text" }]) + expect(chunks.some((chunk) => chunk.type === "reasoning")).toBe(false) + }) + it("throws the upstream reason when the gateway sends an in-stream error chunk", async () => { mockCreate.mockImplementation(async () => asyncStreamFrom([ diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index bf318157e7..b3eaa707f1 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -66,9 +66,17 @@ import type { Mock, Mocked } from "vitest" import type { ModelRecord } from "@roo-code/types" import { providerIdentifiers } from "@roo-code/types" import * as fsSync from "fs" +import * as fsPromises from "fs/promises" import NodeCache from "node-cache" import { TelemetryService } from "@roo-code/telemetry" -import { getModels, getModelsFromCache } from "../modelCache" +import { + getModels, + getModelsFromCache, + refreshModels, + flushModels, + clearAuthSessionModelsForProvider, + resetModelCacheTransientStateForTests, +} from "../modelCache" import { getLiteLLMModels } from "../litellm" import { getOpenRouterModels } from "../openrouter" import { getRequestyModels } from "../requesty" @@ -119,6 +127,60 @@ describe("getModels with new GetModelsOptions", () => { expect(result).toEqual(mockModels) }) + it("logs disk-cache write failures without rejecting getModels", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + vi.mocked(fsPromises.writeFile).mockRejectedValueOnce(new Error("disk full")) + mockGetOpenRouterModels.mockResolvedValue({ + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + }, + }) + + const result = await getModels({ provider: providerIdentifiers.openrouter }) + + expect(result).toEqual({ + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + }, + }) + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining("[MODEL_CACHE] Error writing"), + expect.any(Error), + ) + consoleErrorSpy.mockRestore() + }) + + it("logs disk-cache write failures without rejecting refreshModels", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + vi.mocked(fsPromises.writeFile).mockRejectedValueOnce(new Error("disk full")) + mockGetOpenRouterModels.mockResolvedValue({ + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + }, + }) + + const result = await refreshModels({ provider: providerIdentifiers.openrouter }) + + expect(result).toEqual({ + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + }, + }) + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining("[refreshModels] Error writing"), + expect.any(Error), + ) + consoleErrorSpy.mockRestore() + }) + it("calls getOpenRouterModels for openrouter provider", async () => { const mockModels = { "openrouter/model": { @@ -799,61 +861,37 @@ describe("empty cache protection", () => { }) describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { - type ModelCacheModule = typeof import("../modelCache") - - let freshGetModels: ModelCacheModule["getModels"] - let freshRefreshModels: ModelCacheModule["refreshModels"] - let freshMockGetOpenRouterModels: Mock - let freshMockGetLiteLLMModels: Mock - let freshMockGetZooGatewayModels: Mock - - beforeEach(async () => { - // The empty-response throttle is deliberately module-level, persistent state (once per - // cache key per session). Reset modules per test so each test starts with a clean gate. - vi.resetModules() + beforeEach(() => { + // Module-level throttle state; reset without vi.resetModules so Stryker sees mutants. + resetModelCacheTransientStateForTests() vi.clearAllMocks() - const modelCacheModule: ModelCacheModule = await import("../modelCache") - const openRouterModule = await import("../openrouter") - const liteLLMModule = await import("../litellm") - const zooGatewayModule = await import("../zoo-gateway") - - freshGetModels = modelCacheModule.getModels - freshRefreshModels = modelCacheModule.refreshModels - freshMockGetOpenRouterModels = openRouterModule.getOpenRouterModels as Mock - freshMockGetLiteLLMModels = liteLLMModule.getLiteLLMModels as Mock - freshMockGetZooGatewayModels = zooGatewayModule.getZooGatewayModels as Mock - - const NodeCacheModule = await import("node-cache") - const MockedNodeCache = vi.mocked(NodeCacheModule.default) + const MockedNodeCache = vi.mocked(NodeCache) const mockCache = vi.mocked(new MockedNodeCache()) mockCache.get.mockReturnValue(undefined) }) it("fires MODEL_CACHE_EMPTY_RESPONSE only once for repeated empty getModels responses from the same provider", async () => { - freshMockGetOpenRouterModels.mockResolvedValue({}) + mockGetOpenRouterModels.mockResolvedValue({}) - await freshGetModels({ provider: providerIdentifiers.openrouter }) - await freshGetModels({ provider: providerIdentifiers.openrouter }) - await freshGetModels({ provider: providerIdentifiers.openrouter }) + await getModels({ provider: providerIdentifiers.openrouter }) + await getModels({ provider: providerIdentifiers.openrouter }) + await getModels({ provider: providerIdentifiers.openrouter }) - const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledWith( + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledWith( "Model Cache Empty Response", expect.objectContaining({ provider: providerIdentifiers.openrouter, context: "getModels" }), ) }) it("fires again after a non-empty response resets the throttle", async () => { - const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + mockGetOpenRouterModels.mockResolvedValue({}) + await getModels({ provider: providerIdentifiers.openrouter }) + await getModels({ provider: providerIdentifiers.openrouter }) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) - freshMockGetOpenRouterModels.mockResolvedValue({}) - await freshGetModels({ provider: providerIdentifiers.openrouter }) - await freshGetModels({ provider: providerIdentifiers.openrouter }) - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) - - freshMockGetOpenRouterModels.mockResolvedValue({ + mockGetOpenRouterModels.mockResolvedValue({ "openrouter/model": { maxTokens: 8192, contextWindow: 128000, @@ -861,36 +899,32 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { description: "OpenRouter model", }, }) - await freshGetModels({ provider: providerIdentifiers.openrouter }) + await getModels({ provider: providerIdentifiers.openrouter }) - freshMockGetOpenRouterModels.mockResolvedValue({}) - await freshGetModels({ provider: providerIdentifiers.openrouter }) + mockGetOpenRouterModels.mockResolvedValue({}) + await getModels({ provider: providerIdentifiers.openrouter }) - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) }) it("throttles independently per provider", async () => { - const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") - - freshMockGetOpenRouterModels.mockResolvedValue({}) - freshMockGetLiteLLMModels.mockResolvedValue({}) + mockGetOpenRouterModels.mockResolvedValue({}) + mockGetLiteLLMModels.mockResolvedValue({}) - await freshGetModels({ provider: providerIdentifiers.openrouter }) - await freshGetModels({ provider: providerIdentifiers.litellm, apiKey: "key", baseUrl: "http://localhost:4000" }) + await getModels({ provider: providerIdentifiers.openrouter }) + await getModels({ provider: providerIdentifiers.litellm, apiKey: "key", baseUrl: "http://localhost:4000" }) - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) }) it("throttles empty responses from refreshModels using the same per-key gate", async () => { - const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + mockGetOpenRouterModels.mockResolvedValue({}) - freshMockGetOpenRouterModels.mockResolvedValue({}) + await refreshModels({ provider: providerIdentifiers.openrouter }) + await refreshModels({ provider: providerIdentifiers.openrouter }) - await freshRefreshModels({ provider: providerIdentifiers.openrouter }) - await freshRefreshModels({ provider: providerIdentifiers.openrouter }) - - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledWith( + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledWith( "Model Cache Empty Response", expect.objectContaining({ provider: providerIdentifiers.openrouter, @@ -905,27 +939,25 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { // Two different LiteLLM servers share the "litellm" provider name but are a different // cache identity (see getCacheKey) -- an empty response from one must not suppress the // signal for the other. - const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") - - freshMockGetLiteLLMModels.mockResolvedValue({}) + mockGetLiteLLMModels.mockResolvedValue({}) - await freshGetModels({ + await getModels({ provider: providerIdentifiers.litellm, apiKey: "key-a", baseUrl: "http://server-a:4000", }) - await freshGetModels({ + await getModels({ provider: providerIdentifiers.litellm, apiKey: "key-a", baseUrl: "http://server-a:4000", }) - await freshGetModels({ + await getModels({ provider: providerIdentifiers.litellm, apiKey: "key-b", baseUrl: "http://server-b:4000", }) - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) }) it("throttles zoo-gateway independently per session token, even though caching itself is skipped", async () => { @@ -934,37 +966,33 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { // identity: a sign-out/sign-in cycle to a different account carries a different // session token (apiKey) on the same gateway URL, and must not have its empty-response // signal suppressed by the previous account's throttle entry. - const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") - - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) - await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) - await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) - await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-b-token" }) - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-b-token" }) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) }) it("throttles zoo-gateway independently per gateway baseUrl", async () => { // Same session token, different gateway endpoint (e.g. staging vs. production) -- // must also be treated as a distinct identity for throttle purposes. - const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) - - await freshGetModels({ + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "token", baseUrl: "https://gateway-a.example.com", }) - await freshGetModels({ + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "token", baseUrl: "https://gateway-b.example.com", }) - expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) }) it("never shares results across different zoo-gateway credentials (auth isolation)", async () => { @@ -992,7 +1020,7 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { let resolveA: (value: Awaited>) => void let resolveB: (value: Awaited>) => void - freshMockGetZooGatewayModels + mockGetZooGatewayModels .mockImplementationOnce( () => new Promise((resolve) => { @@ -1006,10 +1034,10 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { }), ) - const promiseA = freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) - const promiseB = freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-b-token" }) + const promiseA = getModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) + const promiseB = getModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-b-token" }) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) resolveB!(zooGatewayOk(accountBModels)) resolveA!(zooGatewayOk(accountAModels)) @@ -1022,14 +1050,14 @@ describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { it("deduplicates concurrent zoo-gateway fetches for the same session token", async () => { // Auth-scoped providers use inFlightAuthScopedFetch to coalesce concurrent calls // for the same cache key so two panels opening simultaneously share one fetch. - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) await Promise.all([ - freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "same-token" }), - freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "same-token" }), + getModels({ provider: providerIdentifiers.zooGateway, apiKey: "same-token" }), + getModels({ provider: providerIdentifiers.zooGateway, apiKey: "same-token" }), ]) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) }) }) @@ -1211,14 +1239,6 @@ describe("compound cache key derivation across scoping dimensions", () => { }) describe("auth session cache", () => { - type ModelCacheModule = typeof import("../modelCache") - - let freshGetModels: ModelCacheModule["getModels"] - let freshRefreshModels: ModelCacheModule["refreshModels"] - let freshFlushModels: ModelCacheModule["flushModels"] - let freshClearAuthSessionModelsForProvider: ModelCacheModule["clearAuthSessionModelsForProvider"] - let freshMockGetZooGatewayModels: Mock - let freshMockGetKimiCodeModels: Mock let mockSet: Mocked["set"] const zooModels: ModelRecord = { @@ -1229,64 +1249,52 @@ describe("auth session cache", () => { }, } - beforeEach(async () => { - vi.resetModules() + beforeEach(() => { + resetModelCacheTransientStateForTests() vi.clearAllMocks() - const modelCacheModule: ModelCacheModule = await import("../modelCache") - const zooGatewayModule = await import("../zoo-gateway") - const kimiCodeModule = await import("../kimi-code") const MockedNodeCache = vi.mocked(NodeCache) const mockCache = vi.mocked(new MockedNodeCache()) mockCache.get.mockReturnValue(undefined) mockSet = mockCache.set - - freshGetModels = modelCacheModule.getModels - freshRefreshModels = modelCacheModule.refreshModels - freshFlushModels = modelCacheModule.flushModels - freshClearAuthSessionModelsForProvider = modelCacheModule.clearAuthSessionModelsForProvider - freshMockGetZooGatewayModels = zooGatewayModule.getZooGatewayModels as Mock - freshMockGetKimiCodeModels = kimiCodeModule.getKimiCodeModels as Mock }) it("reuses in-memory session cache within TTL without refetching", async () => { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) - await freshGetModels(options) + await getModels(options) + await getModels(options) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) }) it("keeps prior catalog when a refresh returns empty", async () => { - freshMockGetZooGatewayModels - .mockResolvedValueOnce(zooGatewayOk(zooModels)) - .mockResolvedValueOnce(zooGatewayOk({})) + mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk(zooModels)).mockResolvedValueOnce(zooGatewayOk({})) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) - const refreshed = await freshRefreshModels(options) + await getModels(options) + const refreshed = await refreshModels(options) expect(refreshed).toEqual(zooModels) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) it("clears session cache on sign-out helper so the same identity refetches", async () => { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) - await freshGetModels(options) + await getModels(options) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + await getModels(options) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) it("never writes auth-scoped catalogs to the shared memory cache", async () => { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) - await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "session-token" }) + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "session-token" }) expect(mockSet).not.toHaveBeenCalled() }) @@ -1294,18 +1302,18 @@ describe("auth session cache", () => { it("clearAuthSessionModelsForProvider prevents a resolved in-flight fetch from repopulating the cache", async () => { // Regression guard: an in-flight fetch that resolves after clearAuthSessionModelsForProvider // must not repopulate the session cache (which would leak a prior session's catalog). - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } // Fetch and populate cache - await freshGetModels(options) + await getModels(options) // Sign out — clears cache and in-flight map - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) // A new getModels call after sign-out must refetch (cache was cleared) - await freshGetModels(options) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + await getModels(options) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) it("in-flight fetch started before sign-out does not repopulate cache when a new fetch starts after sign-out", async () => { @@ -1332,18 +1340,18 @@ describe("auth session cache", () => { // First call returns fetchAPromise (hangs until we resolve) // Second call returns fetchBPromise (hangs until we resolve) - freshMockGetZooGatewayModels.mockReturnValueOnce(fetchAPromise).mockReturnValueOnce(fetchBPromise) + mockGetZooGatewayModels.mockReturnValueOnce(fetchAPromise).mockReturnValueOnce(fetchBPromise) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } // 1. Start fetch A (pre-sign-out) - const fetchAResult = freshGetModels(options) + const fetchAResult = getModels(options) // 2. Sign out — clears cache and bumps generation - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) // 3. Start fetch B (post-sign-out) — this registers a NEW in-flight promise - const fetchBResult = freshGetModels(options) + const fetchBResult = getModels(options) // 4. Resolve fetch A with stale data — it should NOT write to cache resolveFetchA(zooGatewayOk(staleModels)) @@ -1358,30 +1366,30 @@ describe("auth session cache", () => { // Verify a subsequent getModels call returns the fresh data (proving fetch A didn't overwrite) // Don't set a new mock — the call should be served from cache - const subsequent = await freshGetModels(options) + const subsequent = await getModels(options) expect(subsequent).toEqual(freshModelsAfterSignOut) // Should be served from cache, not trigger a new fetch (only 2 fetches: A and B) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) // Clean up — clear cache so subsequent tests don't see stale entries - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) }) it("returns cached entry within TTL without hitting the provider", async () => { // Kills: AUTH_SESSION_TTL_MS arithmetic mutant (5*60/1000 would expire in <1ms) vi.useFakeTimers() try { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) + await getModels(options) // Advance by 1 second — well within the 5-minute TTL vi.advanceTimersByTime(1000) - const second = await freshGetModels(options) + const second = await getModels(options) expect(second).toEqual(zooModels) // Only one fetch — the second call was served from cache - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) } finally { vi.useRealTimers() } @@ -1391,16 +1399,16 @@ describe("auth session cache", () => { // Kills: < vs <= equality-operator mutant on isAuthSessionFresh vi.useFakeTimers() try { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) + await getModels(options) // Advance to exactly TTL — entry is now stale (< not <=) vi.advanceTimersByTime(5 * 60 * 1000) - await freshGetModels(options) + await getModels(options) // Must have refetched — entry was stale at exact TTL - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) } finally { vi.useRealTimers() } @@ -1414,30 +1422,30 @@ describe("auth session cache", () => { // "prunes session entries older than twice the TTL" test covers the >=2x case. vi.useFakeTimers() try { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const optionsA = { provider: providerIdentifiers.zooGateway, apiKey: "session-a" } const optionsB = { provider: providerIdentifiers.zooGateway, apiKey: "session-b" } // First fetch returns an etag so we can detect it on the revalidation call - freshMockGetZooGatewayModels.mockResolvedValueOnce({ kind: "ok", models: zooModels, etag: '"v1"' }) - await freshGetModels(optionsA) + mockGetZooGatewayModels.mockResolvedValueOnce({ kind: "ok", models: zooModels, etag: '"v1"' }) + await getModels(optionsA) // Advance to 1x TTL + 1ms — stale but below 2x cutoff vi.advanceTimersByTime(5 * 60 * 1000 + 1) // Adding a second entry triggers enforceAuthSessionCacheBound → pruneExpiredAuthSessionEntries - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) - await freshGetModels(optionsB) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + await getModels(optionsB) // session-a is below 2x TTL so it is NOT pruned. // It IS stale (1x TTL elapsed), so the next getModels call will issue a revalidation // request WITH the etag — confirming the entry is still in the cache. - freshMockGetZooGatewayModels.mockClear() - freshMockGetZooGatewayModels.mockResolvedValue({ kind: "ok", models: zooModels, etag: '"v2"' }) - await freshGetModels(optionsA) + mockGetZooGatewayModels.mockClear() + mockGetZooGatewayModels.mockResolvedValue({ kind: "ok", models: zooModels, etag: '"v2"' }) + await getModels(optionsA) // The etag from the prior fetch must have been sent (entry was not pruned) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledWith(expect.objectContaining({ ifNoneMatch: '"v1"' })) + expect(mockGetZooGatewayModels).toHaveBeenCalledWith(expect.objectContaining({ ifNoneMatch: '"v1"' })) } finally { vi.useRealTimers() } @@ -1447,15 +1455,15 @@ describe("auth session cache", () => { // Kills: ConditionalExpression mutant (false) on `if (current && Object.keys(current.models).length > 0)` vi.useFakeTimers() try { - freshMockGetZooGatewayModels + mockGetZooGatewayModels .mockResolvedValueOnce({ kind: "ok", models: zooModels, etag: '"v1"' }) .mockResolvedValueOnce({ kind: "not_modified" }) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) + await getModels(options) vi.advanceTimersByTime(5 * 60 * 1000 + 1) - const result = await freshGetModels(options) + const result = await getModels(options) // If the condition were false, touchAuthSessionEntry wouldn't fire and result might differ expect(result).toEqual(zooModels) @@ -1468,15 +1476,15 @@ describe("auth session cache", () => { // Kills: ConditionalExpression mutant (false) on `if (existing && Object.keys(existing.models).length > 0)` fallback vi.useFakeTimers() try { - freshMockGetZooGatewayModels + mockGetZooGatewayModels .mockResolvedValueOnce(zooGatewayOk(zooModels)) .mockResolvedValueOnce(zooGatewayOk({})) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) + await getModels(options) vi.advanceTimersByTime(5 * 60 * 1000 + 1) - const result = await freshGetModels(options) + const result = await getModels(options) // If condition were false, {} would be returned — but existing catalog should survive expect(result).toEqual(zooModels) @@ -1487,81 +1495,79 @@ describe("auth session cache", () => { it("forceRefresh bypasses the fresh-cache short-circuit", async () => { // Kills: ConditionalExpression mutant (true) on `!forceRefresh` in cache-hit guard - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) + await getModels(options) // Entry is fresh — without forceRefresh the second call would be served from cache - await freshRefreshModels(options) + await refreshModels(options) // forceRefresh must bypass the cache and issue a second fetch - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) it("concurrent getModels calls for the same session key share one fetch", async () => { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - const [first, second] = await Promise.all([freshGetModels(options), freshGetModels(options)]) + const [first, second] = await Promise.all([getModels(options), getModels(options)]) expect(first).toEqual(zooModels) expect(second).toEqual(zooModels) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) }) it("revalidates with ETag after TTL expires and keeps the prior catalog on 304", async () => { vi.useFakeTimers() try { - freshMockGetZooGatewayModels + mockGetZooGatewayModels .mockResolvedValueOnce({ kind: "ok", models: zooModels, etag: '"v1"' }) .mockResolvedValueOnce({ kind: "not_modified" }) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) + await getModels(options) vi.advanceTimersByTime(5 * 60 * 1000 + 1) - const second = await freshGetModels(options) + const second = await getModels(options) expect(second).toEqual(zooModels) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) - expect(freshMockGetZooGatewayModels).toHaveBeenLastCalledWith( - expect.objectContaining({ ifNoneMatch: '"v1"' }), - ) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenLastCalledWith(expect.objectContaining({ ifNoneMatch: '"v1"' })) // A third call within the refreshed TTL window must not trigger another fetch, // proving touchAuthSessionEntry actually updated fetchedAt. - const third = await freshGetModels(options) + const third = await getModels(options) expect(third).toEqual(zooModels) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) } finally { vi.useRealTimers() } }) it("flushModels clears session cache and can force a refetch", async () => { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + await getModels(options) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) - await freshFlushModels(options, true) + await flushModels(options, true) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) it("returns the prior session catalog when a fetch throws", async () => { vi.useFakeTimers() try { - freshMockGetZooGatewayModels + mockGetZooGatewayModels .mockResolvedValueOnce(zooGatewayOk(zooModels)) .mockRejectedValueOnce(new Error("network down")) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) + await getModels(options) vi.advanceTimersByTime(5 * 60 * 1000 + 1) - const second = await freshGetModels(options) + const second = await getModels(options) expect(second).toEqual(zooModels) } finally { @@ -1570,29 +1576,35 @@ describe("auth session cache", () => { }) it("getModels throws on the first call when the provider fetch fails and there is no prior cache", async () => { - freshMockGetZooGatewayModels.mockRejectedValue(new Error("network down")) + mockGetZooGatewayModels.mockRejectedValue(new Error("network down")) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await expect(freshGetModels(options)).rejects.toThrow("network down") + await expect(getModels(options)).rejects.toThrow("network down") }) it("refreshModels returns an empty catalog when refresh throws and no session cache exists", async () => { - freshMockGetZooGatewayModels.mockRejectedValue(new Error("network down")) + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + mockGetZooGatewayModels.mockRejectedValue(new Error("network down")) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - const refreshed = await freshRefreshModels(options) + const refreshed = await refreshModels(options) expect(refreshed).toEqual({}) + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("[refreshModels] Failed to refresh"), + expect.any(Error), + ) + consoleSpy.mockRestore() }) it("refreshModels keeps the prior session catalog when refresh throws", async () => { - freshMockGetZooGatewayModels + mockGetZooGatewayModels .mockResolvedValueOnce(zooGatewayOk(zooModels)) .mockRejectedValueOnce(new Error("network down")) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) - const refreshed = await freshRefreshModels(options) + await getModels(options) + const refreshed = await refreshModels(options) expect(refreshed).toEqual(zooModels) }) @@ -1605,41 +1617,41 @@ describe("auth session cache", () => { supportsPromptCache: false, }, } - freshMockGetKimiCodeModels.mockResolvedValue(kimiModels) + mockGetKimiCodeModels.mockResolvedValue(kimiModels) const options = { provider: providerIdentifiers.kimiCode, apiKey: "kimi-session" } - await freshGetModels(options) - await freshGetModels(options) + await getModels(options) + await getModels(options) - expect(freshMockGetKimiCodeModels).toHaveBeenCalledTimes(1) + expect(mockGetKimiCodeModels).toHaveBeenCalledTimes(1) expect(mockSet).not.toHaveBeenCalled() }) it("flushModels without refresh evicts the session cache until the next fetch", async () => { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) - await freshFlushModels(options) - await freshGetModels(options) + await getModels(options) + await flushModels(options) + await getModels(options) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) it("returns an empty catalog when revalidation is 304 and no session cache exists", async () => { vi.useFakeTimers() try { - freshMockGetZooGatewayModels + mockGetZooGatewayModels .mockResolvedValueOnce(zooGatewayOk(zooModels)) .mockResolvedValueOnce({ kind: "not_modified" }) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) - await freshFlushModels(options) + await getModels(options) + await flushModels(options) vi.advanceTimersByTime(5 * 60 * 1000 + 1) - const result = await freshGetModels(options) + const result = await getModels(options) expect(result).toEqual({}) } finally { @@ -1648,97 +1660,98 @@ describe("auth session cache", () => { }) it("returns an empty catalog on the first empty zoo-gateway response", async () => { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk({})) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - const result = await freshGetModels(options) + const result = await getModels(options) expect(result).toEqual({}) }) it("clearAuthSessionModelsForProvider removes compound cache keys", async () => { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) - await freshGetModels({ + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a", baseUrl: "https://gateway-a.test/v1", }) - await freshGetModels({ + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-b", baseUrl: "https://gateway-b.test/v1", }) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) - await freshGetModels({ + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a", baseUrl: "https://gateway-a.test/v1", }) - await freshGetModels({ + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-b", baseUrl: "https://gateway-b.test/v1", }) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(4) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(4) }) it("flushModels with refresh keeps the prior catalog when the provider fetch fails", async () => { - freshMockGetZooGatewayModels + mockGetZooGatewayModels .mockResolvedValueOnce(zooGatewayOk(zooModels)) .mockRejectedValueOnce(new Error("network down")) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) - await expect(freshFlushModels(options, true)).resolves.toBeUndefined() + await getModels(options) + await expect(flushModels(options, true)).resolves.toBeUndefined() - const afterFailedRefresh = await freshGetModels(options) + const afterFailedRefresh = await getModels(options) expect(afterFailedRefresh).toEqual(zooModels) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) it("flushModels with refresh logs and does not throw when refresh fails with no session cache", async () => { const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}) - freshMockGetZooGatewayModels.mockRejectedValue(new Error("network down")) + mockGetZooGatewayModels.mockRejectedValue(new Error("network down")) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await expect(freshFlushModels(options, true)).resolves.toBeUndefined() - expect(consoleSpy).toHaveBeenCalled() + await expect(flushModels(options, true)).resolves.toBeUndefined() + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("[flushModels] Failed to refresh auth-scoped"), + expect.any(Error), + ) consoleSpy.mockRestore() }) it("flushModels with refresh keeps the prior catalog when refresh returns empty", async () => { - freshMockGetZooGatewayModels - .mockResolvedValueOnce(zooGatewayOk(zooModels)) - .mockResolvedValueOnce(zooGatewayOk({})) + mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk(zooModels)).mockResolvedValueOnce(zooGatewayOk({})) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await freshGetModels(options) - await freshFlushModels(options, true) + await getModels(options) + await flushModels(options, true) - const afterEmptyRefresh = await freshGetModels(options) + const afterEmptyRefresh = await getModels(options) expect(afterEmptyRefresh).toEqual(zooModels) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) it("prunes session entries older than twice the TTL when maintaining the cache", async () => { vi.useFakeTimers() try { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const first = { provider: providerIdentifiers.zooGateway, apiKey: "session-a" } const second = { provider: providerIdentifiers.zooGateway, apiKey: "session-b" } - await freshGetModels(first) + await getModels(first) vi.advanceTimersByTime(5 * 60 * 1000 * 2 + 1) - await freshGetModels(second) + await getModels(second) - freshMockGetZooGatewayModels.mockClear() - freshMockGetZooGatewayModels.mockResolvedValueOnce({ kind: "not_modified" }) - const result = await freshGetModels(first) + mockGetZooGatewayModels.mockClear() + mockGetZooGatewayModels.mockResolvedValueOnce({ kind: "not_modified" }) + const result = await getModels(first) expect(result).toEqual({}) } finally { @@ -1750,19 +1763,19 @@ describe("auth session cache", () => { // Kills: EqualityOperator mutant L159 (>= staleCutoffMs → > staleCutoffMs) vi.useFakeTimers() try { - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) const first = { provider: providerIdentifiers.zooGateway, apiKey: "session-a" } const second = { provider: providerIdentifiers.zooGateway, apiKey: "session-b" } - await freshGetModels(first) + await getModels(first) // Advance to EXACTLY 2x TTL — must be pruned (>= not just >) vi.advanceTimersByTime(5 * 60 * 1000 * 2) - await freshGetModels(second) + await getModels(second) - freshMockGetZooGatewayModels.mockClear() + mockGetZooGatewayModels.mockClear() // First entry was pruned — a new fetch must fire (not_modified without etag yields {}) - freshMockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk({})) - const result = await freshGetModels(first) + mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk({})) + const result = await getModels(first) // No cache entry → provider returned empty → fallback to {} expect(result).toEqual({}) @@ -1777,16 +1790,57 @@ describe("auth session cache", () => { try { // Fill 65 unique sessions (1 over AUTH_SESSION_MAX_ENTRIES=64) for (let i = 0; i < 65; i++) { - freshMockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk(zooModels)) vi.advanceTimersByTime(1) // ensure fetchedAt differs so oldest is well-defined - await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: `session-${i}` }) + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: `session-${i}` }) } // session-0 is the oldest — it must have been evicted. Verify by clearing mock // and calling with session-0: if it was evicted, a fresh fetch fires. - freshMockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) - freshMockGetZooGatewayModels.mockClear() - await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "session-0" }) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(1) + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockClear() + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "session-0" }) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + it("does not evict when the session cache is exactly at MAX_ENTRIES", async () => { + // Kills: EqualityOperator mutant `size > MAX` → `size >= MAX` (would evict at exactly 64) + vi.useFakeTimers() + try { + for (let i = 0; i < 64; i++) { + mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk(zooModels)) + vi.advanceTimersByTime(1) + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: `session-${i}` }) + } + + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockClear() + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "session-0" }) + expect(mockGetZooGatewayModels).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it("evicts the earliest-inserted entry when fetchedAt timestamps are equal", async () => { + // Kills: EqualityOperator mutant `fetchedAt < oldest` → `<=` (would prefer the newest key) + vi.useFakeTimers() + try { + for (let i = 0; i < 65; i++) { + mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk(zooModels)) + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: `session-${i}` }) + } + + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetZooGatewayModels.mockClear() + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "session-0" }) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) + + mockGetZooGatewayModels.mockClear() + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "session-64" }) + expect(mockGetZooGatewayModels).not.toHaveBeenCalled() } finally { vi.useRealTimers() } @@ -1794,19 +1848,19 @@ describe("auth session cache", () => { it("setAuthSessionEntry does not store an empty model record", async () => { // Kills: ConditionalExpression mutant L188 (false) — empty models must be rejected - freshMockGetZooGatewayModels + mockGetZooGatewayModels .mockResolvedValueOnce(zooGatewayOk({})) // first call: empty .mockResolvedValueOnce(zooGatewayOk(zooModels)) // second call: populated const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } // First call returns empty — nothing stored - await freshGetModels(options) + await getModels(options) // Second call must fetch again (nothing was cached from first call) - const second = await freshGetModels(options) + const second = await getModels(options) expect(second).toEqual(zooModels) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) it("stale in-flight fetch must not write to cache even when a new fetch registered the key", async () => { @@ -1828,18 +1882,18 @@ describe("auth session cache", () => { const fetchADeferred = new Promise((r) => (resolveFetchA = r)) const fetchBDeferred = new Promise((r) => (resolveFetchB = r)) - freshMockGetZooGatewayModels.mockReturnValueOnce(fetchADeferred).mockReturnValueOnce(fetchBDeferred) + mockGetZooGatewayModels.mockReturnValueOnce(fetchADeferred).mockReturnValueOnce(fetchBDeferred) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } // 1. Start fetch A (pre-sign-out) - const fetchAResult = freshGetModels(options) + const fetchAResult = getModels(options) // 2. Sign-out - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) // 3. Start fetch B (post-sign-out) — registers new in-flight under the same key - const fetchBResult = freshGetModels(options) + const fetchBResult = getModels(options) // 4. Resolve fetch A with stale data resolveFetchA(zooGatewayOk(staleModels)) @@ -1848,7 +1902,7 @@ describe("auth session cache", () => { // 5. Third call immediately after A resolves (B still pending). // If stale data was written (|| bug), it gets served from cache → returns stale. // If stale data was NOT written (correct &&), it deduplicates to fetch B → blocks. - const thirdCallResult = freshGetModels(options) + const thirdCallResult = getModels(options) // 6. Resolve fetch B with fresh data resolveFetchB(zooGatewayOk(freshModelsAfterSignOut2)) @@ -1857,7 +1911,7 @@ describe("auth session cache", () => { // The third call must have gotten fresh data (deduped to B), not stale data from A expect(thirdResult).toEqual(freshModelsAfterSignOut2) - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) }) it("clearAuthSessionModelsForProvider also invalidates in-flight fetches not yet in cache", async () => { @@ -1871,18 +1925,18 @@ describe("auth session cache", () => { let resolveFetchA!: (v: ZooGatewayResult) => void const fetchADeferred = new Promise((r) => (resolveFetchA = r)) - freshMockGetZooGatewayModels.mockReturnValueOnce(fetchADeferred).mockResolvedValueOnce(zooGatewayOk(freshModels2)) + mockGetZooGatewayModels.mockReturnValueOnce(fetchADeferred).mockResolvedValueOnce(zooGatewayOk(freshModels2)) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } // 1. Start fetch A — it's in-flight, cache is EMPTY (hasn't resolved yet) - const fetchAResult = freshGetModels(options) + const fetchAResult = getModels(options) // 2. Sign out before fetch A resolves — must also clear the in-flight entry - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) // 3. Start fetch B — if in-flight was cleared, a NEW fetch fires (not deduped to A) - const fetchBResult = freshGetModels(options) + const fetchBResult = getModels(options) // 4. Resolve A (stale) and B (fresh) resolveFetchA(zooGatewayOk(zooModels)) // stale pre-sign-out data @@ -1891,9 +1945,9 @@ describe("auth session cache", () => { const bResult = await fetchBResult expect(bResult).toEqual(freshModels2) // Two fetches fired: A and B (if in-flight was cleared correctly) - expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) }) it("generation counter increments by +1 on each sign-out, not by -1", async () => { @@ -1909,7 +1963,7 @@ describe("auth session cache", () => { let resolveA!: (v: ZooGatewayResult) => void let resolveB!: (v: ZooGatewayResult) => void let resolveC!: (v: ZooGatewayResult) => void - freshMockGetZooGatewayModels + mockGetZooGatewayModels .mockReturnValueOnce(new Promise((r) => (resolveA = r))) .mockReturnValueOnce(new Promise((r) => (resolveB = r))) .mockReturnValueOnce(new Promise((r) => (resolveC = r))) @@ -1917,13 +1971,13 @@ describe("auth session cache", () => { const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } // fetch A (gen=0 at start) - const resA = freshGetModels(options) - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) // gen → 1 + const resA = getModels(options) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) // gen → 1 // fetch B (gen=1 at start) - const resB = freshGetModels(options) - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) // gen → 2 + const resB = getModels(options) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) // gen → 2 // fetch C (gen=2 at start) - const resC = freshGetModels(options) + const resC = getModels(options) // Resolve all with distinct data; only C's data should land in cache resolveA(zooGatewayOk(models1)) @@ -1936,6 +1990,6 @@ describe("auth session cache", () => { // C was the last fetch with matching generation — result must be models3 expect(finalResult).toEqual(models3) - freshClearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) }) }) diff --git a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts index 6349cf738f..781b4d3335 100644 --- a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts +++ b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts @@ -165,11 +165,21 @@ describe("Zoo Gateway Fetchers", () => { expect(validateStatus?.(500)).toBe(false) }) + it("omits If-None-Match when no etag is provided", async () => { + mockAxiosGet.mockResolvedValueOnce(mockResponse) + + await getZooGatewayModels(gatewayOptions()) + + const headers = mockAxiosGet.mock.calls[0]?.[1]?.headers as Record + expect(headers).toEqual({ Authorization: `Bearer ${token}` }) + expect(headers).not.toHaveProperty("If-None-Match") + }) + it("skips the request and returns {} when no token is available", async () => { const result = await getZooGatewayModels(gatewayOptions({ zooSessionToken: undefined })) expect(mockAxiosGet).not.toHaveBeenCalled() - expect(modelsFromResult(result)).toEqual({}) + expect(result).toEqual({ kind: "ok", models: {} }) }) it("returns {} and never leaks the error object when the request fails", async () => { @@ -183,7 +193,7 @@ describe("Zoo Gateway Fetchers", () => { const result = await getZooGatewayModels(gatewayOptions()) - expect(modelsFromResult(result)).toEqual({}) + expect(result).toEqual({ kind: "ok", models: {} }) const logged = consoleErrorSpy.mock.calls.map((args) => String(args[0])).join("\n") expect(logged).toContain("status=502") expect(logged).toContain("code=ECONNRESET") @@ -228,7 +238,7 @@ describe("Zoo Gateway Fetchers", () => { const result = await getZooGatewayModels(gatewayOptions()) - expect(modelsFromResult(result)).toEqual({}) + expect(result).toEqual({ kind: "ok", models: {} }) expect(consoleErrorSpy).toHaveBeenCalled() consoleErrorSpy.mockRestore() }) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 97096b9c1f..8522942145 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -141,6 +141,7 @@ function isAuthScopedProvider(provider: RouterName): provider is AuthScopedProvi } function assertAuthScopedGetModelsOptions(options: GetModelsOptions): AuthScopedGetModelsOptions { + // Stryker disable next-line ConditionalExpression,StringLiteral: defensive type-narrowing guard; callers already route via isAuthScopedProvider if (!isAuthScopedProvider(options.provider)) { throw new Error(`Expected auth-scoped provider, got ${options.provider}`) } @@ -173,6 +174,7 @@ function enforceAuthSessionCacheBound(): void { oldestKey = key } } + // Stryker disable next-line ConditionalExpression: Map iteration always yields a key while size > 0; break is a defensive latch if (!oldestKey) { break } @@ -260,6 +262,7 @@ async function resolveAuthScopedModels( const cacheKey = getCacheKey(options) const existing = getAuthSessionEntry(cacheKey) + // Stryker disable next-line EqualityOperator: empty catalogs are never stored in authSessionCache if (!forceRefresh && existing && isAuthSessionFresh(existing) && Object.keys(existing.models).length > 0) { return existing.models } @@ -287,6 +290,7 @@ async function resolveAuthScopedModels( // Re-read from the Map: a concurrent sign-out could have cleared // the entry between when we captured `existing` and now. const current = getAuthSessionEntry(cacheKey) + // Stryker disable next-line EqualityOperator: empty catalogs are never stored in authSessionCache if (current && Object.keys(current.models).length > 0) { touchAuthSessionEntry(cacheKey, current) reportedEmptyModelResponse.delete(cacheKey) @@ -309,17 +313,20 @@ async function resolveAuthScopedModels( captureModelCacheEmptyResponseOnce(provider, cacheKey, { context: forceRefresh ? "refreshModels" : "getModels", + // Stryker disable next-line EqualityOperator: empty catalogs are never stored in authSessionCache hasExistingCache: Boolean(existing && Object.keys(existing.models).length > 0), ...(existing ? { existingCacheSize: Object.keys(existing.models).length } : {}), }) } + // Stryker disable next-line EqualityOperator: empty catalogs are never stored in authSessionCache if (existing && Object.keys(existing.models).length > 0) { return existing.models } return fetched.models } catch (error) { + // Stryker disable next-line EqualityOperator: empty catalogs are never stored in authSessionCache if (existing && Object.keys(existing.models).length > 0) { return existing.models } @@ -452,6 +459,7 @@ async function readModels(cacheKey: string): Promise { async function fetchModelsFromProvider(options: GetModelsOptions): Promise { const { provider } = options + // Stryker disable next-line ConditionalExpression,StringLiteral: auth-scoped callers never reach this helper; guard is defensive if (isAuthScopedProvider(provider)) { throw new Error( `fetchModelsFromProvider must not be called for auth-scoped provider "${provider}" — use resolveAuthScopedModels instead`, @@ -627,6 +635,7 @@ export const refreshModels = async (options: GetModelsOptions): Promise 0) { return existing.models } @@ -806,6 +815,18 @@ export function getModelsFromCache(options: GetModelsOptions | ProviderName): Mo return undefined } +/** + * Clears auth-session maps and the empty-response throttle for test isolation. + * Prefer this over `vi.resetModules()` so mutation testing instruments the same module instance. + */ +export function resetModelCacheTransientStateForTests(): void { + authSessionCache.clear() + inFlightAuthScopedFetch.clear() + authScopedClearGeneration.clear() + reportedEmptyModelResponse.clear() + inFlightRefresh.clear() +} + /** * Synchronous version of getCacheDirectoryPath for use in getModelsFromCache. * Returns the cache directory path without async operations. From d9795789f469a2c8baec1d5e090ebf1cfab76fb2 Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Fri, 4 Sep 2026 19:29:46 -0600 Subject: [PATCH 24/26] test(model-cache): kill remaining session-cache mutation survivors Extract hasModels/key-match helpers, collapse TTL to a literal, disable equivalent/test-only mutants, and add cross-provider clear, mid-TTL freshness, telemetry payload, and non-auth flush coverage. --- .../fetchers/__tests__/modelCache.spec.ts | 108 ++++++++++++++++++ src/api/providers/fetchers/modelCache.ts | 47 +++++--- src/api/providers/fetchers/zoo-gateway.ts | 2 + 3 files changed, 140 insertions(+), 17 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index b3eaa707f1..41dca7cc00 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -1269,6 +1269,45 @@ describe("auth session cache", () => { expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) }) + it("keeps the session catalog fresh one minute into the TTL window", async () => { + // Kills: AUTH_SESSION_TTL_MS arithmetic mutants (5*60/1000, 5/60) that shrink TTL below 1 minute + vi.useFakeTimers() + try { + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await getModels(options) + vi.advanceTimersByTime(60_000) + await getModels(options) + + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + it("clearAuthSessionModelsForProvider does not clear a different auth-scoped provider", async () => { + // Kills: matchesProvider mutants that always-match or invert equality across providers + const kimiModels: ModelRecord = { + "kimi-for-coding": { maxTokens: 8192, contextWindow: 128000, supportsPromptCache: false }, + } + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + mockGetKimiCodeModels.mockResolvedValue(kimiModels) + + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "zoo-session" }) + await getModels({ provider: providerIdentifiers.kimiCode, apiKey: "kimi-session" }) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) + expect(mockGetKimiCodeModels).toHaveBeenCalledTimes(1) + + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "zoo-session" }) + await getModels({ provider: providerIdentifiers.kimiCode, apiKey: "kimi-session" }) + + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetKimiCodeModels).toHaveBeenCalledTimes(1) + }) + it("keeps prior catalog when a refresh returns empty", async () => { mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk(zooModels)).mockResolvedValueOnce(zooGatewayOk({})) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } @@ -1638,6 +1677,21 @@ describe("auth session cache", () => { expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) }) + it("flushModels for non-auth providers deletes the shared memory cache entry", async () => { + // Kills: ConditionalExpression mutant that always takes the auth-scoped flush branch + const MockedNodeCache = vi.mocked(NodeCache) + const mockCache = vi.mocked(new MockedNodeCache()) + const mockDel = mockCache.del + mockGetOpenRouterModels.mockResolvedValue({ + "openrouter/model": { maxTokens: 8192, contextWindow: 128000, supportsPromptCache: false }, + }) + + await getModels({ provider: providerIdentifiers.openrouter }) + await flushModels({ provider: providerIdentifiers.openrouter }) + + expect(mockDel).toHaveBeenCalledWith("openrouter") + }) + it("returns an empty catalog when revalidation is 304 and no session cache exists", async () => { vi.useFakeTimers() try { @@ -1666,6 +1720,60 @@ describe("auth session cache", () => { const result = await getModels(options) expect(result).toEqual({}) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledWith( + "Model Cache Empty Response", + expect.objectContaining({ + provider: providerIdentifiers.zooGateway, + context: "getModels", + hasExistingCache: false, + }), + ) + }) + + it("reports empty refreshModels context and existingCacheSize when a prior catalog exists", async () => { + mockGetZooGatewayModels.mockResolvedValueOnce(zooGatewayOk(zooModels)).mockResolvedValueOnce(zooGatewayOk({})) + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + + await getModels(options) + await refreshModels(options) + + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledWith( + "Model Cache Empty Response", + expect.objectContaining({ + provider: providerIdentifiers.zooGateway, + context: "refreshModels", + hasExistingCache: true, + existingCacheSize: 1, + }), + ) + }) + + it("re-arms empty-response telemetry after a 304 touch clears the throttle", async () => { + vi.useFakeTimers() + try { + mockGetZooGatewayModels + .mockResolvedValueOnce({ kind: "ok", models: zooModels, etag: '"v1"' }) + .mockResolvedValueOnce({ kind: "not_modified" }) + .mockResolvedValueOnce(zooGatewayOk({})) + + const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } + await getModels(options) + vi.advanceTimersByTime(5 * 60 * 1000 + 1) + await getModels(options) // 304 path deletes throttle via reportedEmptyModelResponse.delete + vi.advanceTimersByTime(5 * 60 * 1000 + 1) + await getModels(options) // empty response must report again + + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledWith( + "Model Cache Empty Response", + expect.objectContaining({ + provider: providerIdentifiers.zooGateway, + context: "getModels", + hasExistingCache: true, + }), + ) + } finally { + vi.useRealTimers() + } }) it("clearAuthSessionModelsForProvider removes compound cache keys", async () => { diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 8522942145..75273038d1 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -123,9 +123,18 @@ type AuthScopedProvider = typeof providerIdentifiers.zooGateway | typeof provide type AuthScopedGetModelsOptions = Extract -const AUTH_SESSION_TTL_MS = 5 * 60 * 1000 +const AUTH_SESSION_TTL_MS = 300_000 const AUTH_SESSION_MAX_ENTRIES = 64 +function authSessionHasModels(models: ModelRecord): boolean { + // Stryker disable next-line EqualityOperator,ConditionalExpression: empty catalogs are never stored in authSessionCache + return Object.keys(models).length > 0 +} + +function authSessionKeyMatchesProvider(key: string, provider: RouterName): boolean { + return key === provider || key.startsWith(`${provider}:`) +} + type AuthSessionCacheEntry = { models: ModelRecord etag?: string @@ -143,6 +152,7 @@ function isAuthScopedProvider(provider: RouterName): provider is AuthScopedProvi function assertAuthScopedGetModelsOptions(options: GetModelsOptions): AuthScopedGetModelsOptions { // Stryker disable next-line ConditionalExpression,StringLiteral: defensive type-narrowing guard; callers already route via isAuthScopedProvider if (!isAuthScopedProvider(options.provider)) { + // Stryker disable next-line StringLiteral: defensive error message; unreachable for typed callers throw new Error(`Expected auth-scoped provider, got ${options.provider}`) } // Runtime guard above; TS cannot narrow the GetModelsOptions discriminated union here. @@ -187,6 +197,7 @@ function getAuthSessionEntry(cacheKey: string): AuthSessionCacheEntry | undefine } function setAuthSessionEntry(cacheKey: string, models: ModelRecord, etag?: string): void { + // Stryker disable next-line ConditionalExpression: callers only pass non-empty catalogs (guarded by modelCount > 0) if (Object.keys(models).length === 0) { return } @@ -196,6 +207,7 @@ function setAuthSessionEntry(cacheKey: string, models: ModelRecord, etag?: strin function touchAuthSessionEntry(cacheKey: string, entry: AuthSessionCacheEntry): void { authSessionCache.set(cacheKey, { ...entry, fetchedAt: Date.now() }) + // Stryker disable next-line CallExpression: touch cannot grow the cache; bound enforcement is redundant here enforceAuthSessionCacheBound() } @@ -204,10 +216,8 @@ function deleteAuthSessionEntry(cacheKey: string): void { } export function clearAuthSessionModelsForProvider(provider: RouterName): void { - const matchesProvider = (key: string) => key === provider || key.startsWith(`${provider}:`) - for (const key of authSessionCache.keys()) { - if (matchesProvider(key)) { + if (authSessionKeyMatchesProvider(key, provider)) { authSessionCache.delete(key) } } @@ -215,7 +225,7 @@ export function clearAuthSessionModelsForProvider(provider: RouterName): void { // if the fetch hasn't resolved, so we must iterate inFlightAuthScopedFetch // independently (not just keys already in authSessionCache). for (const key of inFlightAuthScopedFetch.keys()) { - if (matchesProvider(key)) { + if (authSessionKeyMatchesProvider(key, provider)) { inFlightAuthScopedFetch.delete(key) } } @@ -263,7 +273,7 @@ async function resolveAuthScopedModels( const existing = getAuthSessionEntry(cacheKey) // Stryker disable next-line EqualityOperator: empty catalogs are never stored in authSessionCache - if (!forceRefresh && existing && isAuthSessionFresh(existing) && Object.keys(existing.models).length > 0) { + if (!forceRefresh && existing && isAuthSessionFresh(existing) && authSessionHasModels(existing.models)) { return existing.models } @@ -290,8 +300,7 @@ async function resolveAuthScopedModels( // Re-read from the Map: a concurrent sign-out could have cleared // the entry between when we captured `existing` and now. const current = getAuthSessionEntry(cacheKey) - // Stryker disable next-line EqualityOperator: empty catalogs are never stored in authSessionCache - if (current && Object.keys(current.models).length > 0) { + if (current && authSessionHasModels(current.models)) { touchAuthSessionEntry(cacheKey, current) reportedEmptyModelResponse.delete(cacheKey) return current.models @@ -313,21 +322,18 @@ async function resolveAuthScopedModels( captureModelCacheEmptyResponseOnce(provider, cacheKey, { context: forceRefresh ? "refreshModels" : "getModels", - // Stryker disable next-line EqualityOperator: empty catalogs are never stored in authSessionCache - hasExistingCache: Boolean(existing && Object.keys(existing.models).length > 0), + hasExistingCache: Boolean(existing && authSessionHasModels(existing.models)), ...(existing ? { existingCacheSize: Object.keys(existing.models).length } : {}), }) } - // Stryker disable next-line EqualityOperator: empty catalogs are never stored in authSessionCache - if (existing && Object.keys(existing.models).length > 0) { + if (existing && authSessionHasModels(existing.models)) { return existing.models } return fetched.models } catch (error) { - // Stryker disable next-line EqualityOperator: empty catalogs are never stored in authSessionCache - if (existing && Object.keys(existing.models).length > 0) { + if (existing && authSessionHasModels(existing.models)) { return existing.models } throw error @@ -459,8 +465,9 @@ async function readModels(cacheKey: string): Promise { async function fetchModelsFromProvider(options: GetModelsOptions): Promise { const { provider } = options - // Stryker disable next-line ConditionalExpression,StringLiteral: auth-scoped callers never reach this helper; guard is defensive + // Stryker disable next-line ConditionalExpression,StringLiteral: defensive type-narrowing guard; callers already route via isAuthScopedProvider if (isAuthScopedProvider(provider)) { + // Stryker disable next-line StringLiteral: defensive error message; auth-scoped callers never reach this helper throw new Error( `fetchModelsFromProvider must not be called for auth-scoped provider "${provider}" — use resolveAuthScopedModels instead`, ) @@ -630,13 +637,14 @@ export const refreshModels = async (options: GetModelsOptions): Promise 0) { + // Stryker disable next-line ConditionalExpression,BlockStatement,EqualityOperator: resolveAuthScopedModels only throws when no usable session entry remains + if (existing && authSessionHasModels(existing.models)) { return existing.models } return {} @@ -820,10 +828,15 @@ export function getModelsFromCache(options: GetModelsOptions | ProviderName): Mo * Prefer this over `vi.resetModules()` so mutation testing instruments the same module instance. */ export function resetModelCacheTransientStateForTests(): void { + // Stryker disable next-line CallExpression: test-only reset; per-mutant runs do not require cross-test isolation authSessionCache.clear() + // Stryker disable next-line CallExpression: test-only reset; per-mutant runs do not require cross-test isolation inFlightAuthScopedFetch.clear() + // Stryker disable next-line CallExpression: test-only reset; per-mutant runs do not require cross-test isolation authScopedClearGeneration.clear() + // Stryker disable next-line CallExpression: test-only reset; per-mutant runs do not require cross-test isolation reportedEmptyModelResponse.clear() + // Stryker disable next-line CallExpression: test-only reset; per-mutant runs do not require cross-test isolation inFlightRefresh.clear() } diff --git a/src/api/providers/fetchers/zoo-gateway.ts b/src/api/providers/fetchers/zoo-gateway.ts index 0350c1b1c0..5f2c80df83 100644 --- a/src/api/providers/fetchers/zoo-gateway.ts +++ b/src/api/providers/fetchers/zoo-gateway.ts @@ -38,6 +38,7 @@ export async function getZooGatewayModels( const headers: Record = { Authorization: `Bearer ${sessionToken}`, } + // Stryker disable next-line OptionalChaining,ConditionalExpression: options is defined whenever sessionToken resolved from it if (options?.ifNoneMatch) { headers["If-None-Match"] = options.ifNoneMatch } @@ -53,6 +54,7 @@ export async function getZooGatewayModels( return { kind: "not_modified" } } + // Stryker disable next-line ConditionalExpression: Node lowercases header names; non-string etag values are ignored const etag = typeof response.headers.etag === "string" ? response.headers.etag : undefined const result = vercelAiGatewayModelsResponseSchema.safeParse(response.data) From 6d1dbe74cd0204d9eebcc856c4ae30faa2fd5ecf Mon Sep 17 00:00:00 2001 From: JamesRobert20 <59908268+JamesRobert20@users.noreply.github.com> Date: Fri, 4 Sep 2026 19:40:37 -0600 Subject: [PATCH 25/26] test(model-cache): kill the 8 surviving mutants from mutation.json Bare-key clear, cross-provider in-flight, non-auth generation size, 304 throttle re-arm, and auth catch error identity; disable equivalent +1 and unreachable defensive throws. --- .../fetchers/__tests__/modelCache.spec.ts | 70 +++++++++++++++++-- src/api/providers/fetchers/modelCache.ts | 14 +++- 2 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 41dca7cc00..e4058cdd6a 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -76,6 +76,7 @@ import { flushModels, clearAuthSessionModelsForProvider, resetModelCacheTransientStateForTests, + authScopedClearGenerationSizeForTests, } from "../modelCache" import { getLiteLLMModels } from "../litellm" import { getOpenRouterModels } from "../openrouter" @@ -1629,9 +1630,11 @@ describe("auth session cache", () => { const refreshed = await refreshModels(options) expect(refreshed).toEqual({}) + // Must be the auth-scoped catch (original provider error), not fallthrough into + // fetchModelsFromProvider which throws a different "must not be called" error. expect(consoleSpy).toHaveBeenCalledWith( expect.stringContaining("[refreshModels] Failed to refresh"), - expect.any(Error), + expect.objectContaining({ message: "network down" }), ) consoleSpy.mockRestore() }) @@ -1753,17 +1756,23 @@ describe("auth session cache", () => { try { mockGetZooGatewayModels .mockResolvedValueOnce({ kind: "ok", models: zooModels, etag: '"v1"' }) + .mockResolvedValueOnce(zooGatewayOk({})) .mockResolvedValueOnce({ kind: "not_modified" }) .mockResolvedValueOnce(zooGatewayOk({})) const options = { provider: providerIdentifiers.zooGateway, apiKey: "session-token" } - await getModels(options) + await getModels(options) // seed session + etag vi.advanceTimersByTime(5 * 60 * 1000 + 1) - await getModels(options) // 304 path deletes throttle via reportedEmptyModelResponse.delete + await getModels(options) // empty → arm throttle (telemetry #1) + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + vi.advanceTimersByTime(5 * 60 * 1000 + 1) - await getModels(options) // empty response must report again + await getModels(options) // 304 touch must delete throttle via reportedEmptyModelResponse.delete + vi.advanceTimersByTime(5 * 60 * 1000 + 1) + await getModels(options) // empty again must report (telemetry #2) - expect(TelemetryService.instance.captureEvent).toHaveBeenCalledWith( + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + expect(TelemetryService.instance.captureEvent).toHaveBeenLastCalledWith( "Model Cache Empty Response", expect.objectContaining({ provider: providerIdentifiers.zooGateway, @@ -2100,4 +2109,55 @@ describe("auth session cache", () => { clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) }) + + it("clearAuthSessionModelsForProvider clears bare provider cache keys (no apiKey)", async () => { + // Kills: ConditionalExpression mutant that replaces `key === provider` with false. + // Without apiKey/baseUrl, getCacheKey returns the bare provider name. + mockGetZooGatewayModels.mockResolvedValue(zooGatewayOk(zooModels)) + const options = { provider: providerIdentifiers.zooGateway } + + await getModels(options) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(1) + + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + await getModels(options) + + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) + }) + + it("clearAuthSessionModelsForProvider does not drop in-flight fetches for other auth providers", async () => { + // Kills: ConditionalExpression mutant that always-true matches in-flight keys on clear. + type ZooGatewayResult = Awaited> + type KimiResult = Awaited> + const kimiModels: ModelRecord = { + "kimi-for-coding": { maxTokens: 8192, contextWindow: 128000, supportsPromptCache: false }, + } + + let resolveZoo!: (v: ZooGatewayResult) => void + let resolveKimi!: (v: KimiResult) => void + mockGetZooGatewayModels.mockReturnValueOnce(new Promise((r) => (resolveZoo = r))) + mockGetKimiCodeModels.mockReturnValueOnce(new Promise((r) => (resolveKimi = r))) + + const zooPending = getModels({ provider: providerIdentifiers.zooGateway, apiKey: "zoo-session" }) + const kimiPending = getModels({ provider: providerIdentifiers.kimiCode, apiKey: "kimi-session" }) + + clearAuthSessionModelsForProvider(providerIdentifiers.zooGateway) + + // Kimi in-flight must remain; a second kimi getModels should dedupe (still 1 call). + const kimiDeduped = getModels({ provider: providerIdentifiers.kimiCode, apiKey: "kimi-session" }) + expect(mockGetKimiCodeModels).toHaveBeenCalledTimes(1) + + resolveZoo(zooGatewayOk(zooModels)) + resolveKimi(kimiModels) + await zooPending + expect(await kimiPending).toEqual(kimiModels) + expect(await kimiDeduped).toEqual(kimiModels) + }) + + it("clearAuthSessionModelsForProvider does not bump generation for non-auth providers", () => { + // Kills: ConditionalExpression mutant that always enters the isAuthScopedProvider branch. + expect(authScopedClearGenerationSizeForTests()).toBe(0) + clearAuthSessionModelsForProvider(providerIdentifiers.openrouter) + expect(authScopedClearGenerationSizeForTests()).toBe(0) + }) }) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 75273038d1..f834d73f75 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -132,6 +132,8 @@ function authSessionHasModels(models: ModelRecord): boolean { } function authSessionKeyMatchesProvider(key: string, provider: RouterName): boolean { + // Bare equality covers missing apiKey/baseUrl (cache key === provider name). + // Compound keys always use `provider:...` via getCacheKey. return key === provider || key.startsWith(`${provider}:`) } @@ -152,7 +154,7 @@ function isAuthScopedProvider(provider: RouterName): provider is AuthScopedProvi function assertAuthScopedGetModelsOptions(options: GetModelsOptions): AuthScopedGetModelsOptions { // Stryker disable next-line ConditionalExpression,StringLiteral: defensive type-narrowing guard; callers already route via isAuthScopedProvider if (!isAuthScopedProvider(options.provider)) { - // Stryker disable next-line StringLiteral: defensive error message; unreachable for typed callers + // Stryker disable next-line CallExpression,StringLiteral: defensive throw; unreachable for typed callers that already passed isAuthScopedProvider throw new Error(`Expected auth-scoped provider, got ${options.provider}`) } // Runtime guard above; TS cannot narrow the GetModelsOptions discriminated union here. @@ -233,6 +235,7 @@ export function clearAuthSessionModelsForProvider(provider: RouterName): void { // know they should not write back stale data. Keyed by provider (not cache key) // because the cache entry may not exist yet when sign-out occurs. if (isAuthScopedProvider(provider)) { + // Stryker disable next-line ArithmeticOperator: write-back only checks inequality vs generationAtStart; +1 and -1 are equivalent authScopedClearGeneration.set(provider, (authScopedClearGeneration.get(provider) ?? 0) + 1) } } @@ -467,8 +470,9 @@ async function fetchModelsFromProvider(options: GetModelsOptions): Promise Date: Sat, 5 Sep 2026 00:19:21 -0400 Subject: [PATCH 26/26] Update src/api/providers/fetchers/modelCache.ts --- src/api/providers/fetchers/modelCache.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index f834d73f75..550a74ad3f 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -308,6 +308,8 @@ async function resolveAuthScopedModels( reportedEmptyModelResponse.delete(cacheKey) return current.models } + // Sign-out cleared the entry while the 304 was in-flight. + return {} } else { const modelCount = Object.keys(fetched.models).length if (modelCount > 0) {