From 0a26789f35d1d46c8b8dbc86536a076a791ba709 Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Wed, 28 Oct 2026 21:57:39 -0700 Subject: [PATCH 1/4] refactor(provider): inject the request and parser into bounded catalogs An authenticated catalog with a different response shape can then reuse the snapshot, in-flight dedupe and bounded read instead of copying them. The "models" discovery state is now a non-empty tuple and the catalog is generic over its seed, so a catalog seeded with a non-empty list always yields a non-empty one without a cast. --- src/provider/bounded-model-catalog.ts | 53 +++++++++++++++++---------- src/provider/model-catalogs.test.ts | 21 +++++++++++ src/provider/model-catalogs.ts | 21 +++++++++-- src/provider/models-endpoint.ts | 19 ++++++---- src/tui/provider-setup.test.ts | 9 +---- 5 files changed, 84 insertions(+), 39 deletions(-) diff --git a/src/provider/bounded-model-catalog.ts b/src/provider/bounded-model-catalog.ts index a3218e43b..f6f0409b1 100644 --- a/src/provider/bounded-model-catalog.ts +++ b/src/provider/bounded-model-catalog.ts @@ -1,8 +1,6 @@ import { endpointErrorMessage, - parseModelsData, type ModelsEndpointDiscoveryState, - requestModelsEndpoint, } from "./models-endpoint.js"; export type CatalogDiscoveryState = ModelsEndpointDiscoveryState; @@ -65,25 +63,39 @@ async function readBoundedCatalogText( } } -export function createBoundedModelCatalog(args: { - baseURL: string; - seedIds: readonly string[]; +type DiscoveredModels = Extract< + CatalogDiscoveryState, + { status: "models" } +>["models"]; + +export function createBoundedModelCatalog< + Seed extends readonly string[], +>(args: { + request: (opts: { + timeoutMs?: number; + signal?: AbortSignal; + }) => Promise; + parse: ( + raw: unknown, + cap: { readonly maxModels: number; readonly oversizeMessage: string }, + ) => CatalogDiscoveryState; + seedIds: Seed; catalogLabel: string; maxBytes: number; maxModels: number; }): { - discoverModels: (args?: { + discoverModels: (opts?: { timeoutMs?: number; signal?: AbortSignal; }) => Promise; - selectableModelIds: () => readonly string[]; - prefetchModels: () => Promise; + selectableModelIds: () => Seed | DiscoveredModels; + prefetchModels: () => Promise; resetDiscoveryForTests: () => void; } { - const { baseURL, seedIds, catalogLabel, maxBytes, maxModels } = args; + const { request, parse, seedIds, catalogLabel, maxBytes, maxModels } = args; - let inflight: Promise | undefined; - let snapshot: readonly string[] | undefined; + let inflight: Promise | undefined; + let snapshot: DiscoveredModels | undefined; function oversizeMessage(kind: "bytes" | "models"): string { if (kind === "bytes") { @@ -111,16 +123,17 @@ export function createBoundedModelCatalog(args: { } } - async function discoverModels(args?: { + async function discoverModels(opts?: { timeoutMs?: number; signal?: AbortSignal; }): Promise { let response: Response; try { - response = await requestModelsEndpoint({ - baseURL, - ...(args?.timeoutMs !== undefined ? { timeoutMs: args.timeoutMs } : {}), - ...(args?.signal !== undefined ? { signal: args.signal } : {}), + // Forward only the transport knobs so a caller's extra keys can never + // reach the request and override its URL or headers. + response = await request({ + ...(opts?.timeoutMs !== undefined ? { timeoutMs: opts.timeoutMs } : {}), + ...(opts?.signal !== undefined ? { signal: opts.signal } : {}), }); } catch (error) { return { @@ -140,17 +153,17 @@ export function createBoundedModelCatalog(args: { if (!body.ok) { return { status: "malformed", message: body.message }; } - return parseModelsData(body.value, { + return parse(body.value, { maxModels, oversizeMessage: oversizeMessage("models"), }); } - function selectableModelIds(): readonly string[] { + function selectableModelIds(): Seed | DiscoveredModels { return snapshot ?? seedIds; } - async function runPrefetch(): Promise { + async function runPrefetch(): Promise { const state = await discoverModels(); // Empty/unavailable/malformed leave the successful snapshot in place: // stale-but-live beats empty; a cold failure falls back to the seed. @@ -160,7 +173,7 @@ export function createBoundedModelCatalog(args: { return selectableModelIds(); } - function prefetchModels(): Promise { + function prefetchModels(): Promise { if (inflight !== undefined) return inflight; const pending = runPrefetch(); diff --git a/src/provider/model-catalogs.test.ts b/src/provider/model-catalogs.test.ts index a4f33e357..7547bd7e9 100644 --- a/src/provider/model-catalogs.test.ts +++ b/src/provider/model-catalogs.test.ts @@ -255,6 +255,27 @@ for (const harness of harnesses) { expect(state.message).toContain(String(maxModels)); expect(selectable()).toEqual(seedIds); }); + + test("counts raw entries against the model-count cap before dedupe", async () => { + const data = Array.from({ length: maxModels + 1 }, () => ({ + id: sampleId, + })); + globalThis.fetch = (async () => + Response.json({ data })) as unknown as typeof fetch; + + expect((await discover()).status).toBe("malformed"); + }); + + test("aborts the request when timeoutMs elapses", async () => { + globalThis.fetch = ((_input: RequestInfo | URL, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => { + reject(init.signal?.reason); + }); + })) as unknown as typeof fetch; + + expect((await discover({ timeoutMs: 20 })).status).toBe("unavailable"); + }); }); describe(prefetchName, () => { diff --git a/src/provider/model-catalogs.ts b/src/provider/model-catalogs.ts index a709f0f2c..46b3e2c6c 100644 --- a/src/provider/model-catalogs.ts +++ b/src/provider/model-catalogs.ts @@ -7,6 +7,7 @@ import { ZEN_MODEL_IDS, } from "../../packages/zen/src/index.js"; import { createBoundedModelCatalog } from "./bounded-model-catalog.js"; +import { parseModelsData, requestModelsEndpoint } from "./models-endpoint.js"; // Bound live /models so a huge or hostile catalog cannot blow process memory. // One shared bound for both catalogs; the per-catalog names stay as aliases. @@ -32,14 +33,26 @@ const MODEL_CATALOG_CONFIGS = { }, } as const; -const goCatalog = createBoundedModelCatalog({ - ...MODEL_CATALOG_CONFIGS.go, +// Public OpenAI-compatible /models catalogs: unauthenticated GET, `{ data }` body. +function createPublicModelCatalog( + config: (typeof MODEL_CATALOG_CONFIGS)[keyof typeof MODEL_CATALOG_CONFIGS], + limits: { maxBytes: number; maxModels: number }, +) { + const { baseURL, ...rest } = config; + return createBoundedModelCatalog({ + ...rest, + ...limits, + request: (opts) => requestModelsEndpoint({ baseURL, ...opts }), + parse: parseModelsData, + }); +} + +const goCatalog = createPublicModelCatalog(MODEL_CATALOG_CONFIGS.go, { maxBytes: MAX_GO_CATALOG_BYTES, maxModels: MAX_GO_CATALOG_MODELS, }); -const zenCatalog = createBoundedModelCatalog({ - ...MODEL_CATALOG_CONFIGS.zen, +const zenCatalog = createPublicModelCatalog(MODEL_CATALOG_CONFIGS.zen, { maxBytes: MAX_ZEN_CATALOG_BYTES, maxModels: MAX_ZEN_CATALOG_MODELS, }); diff --git a/src/provider/models-endpoint.ts b/src/provider/models-endpoint.ts index 5b27953ca..e3a554773 100644 --- a/src/provider/models-endpoint.ts +++ b/src/provider/models-endpoint.ts @@ -11,13 +11,16 @@ export function modelsEndpointURL(baseURL: string): string { } // One /models `{ data: [{ id }] }` shape shared by every discovery importer -// (bounded-model-catalog, ollama) so the parsers cannot drift. +// (model-catalogs, ollama) so the parsers cannot drift. export const ModelsEndpointResponse = type({ data: type({ id: "string" }).array(), }); export type ModelsEndpointDiscoveryState = - | { readonly status: "models"; readonly models: readonly string[] } + | { + readonly status: "models"; + readonly models: readonly [string, ...string[]]; + } | { readonly status: "empty" } | { readonly status: "unavailable"; readonly message: string } | { readonly status: "malformed"; readonly message: string }; @@ -44,12 +47,12 @@ export function parseModelsData( if (cap !== undefined && parsed.data.length > cap.maxModels) { return { status: "malformed", message: cap.oversizeMessage }; } - const models = [ - ...new Set( - parsed.data.map(({ id }) => id.trim()).filter((id) => id.length > 0), - ), - ]; - return models.length > 0 ? { status: "models", models } : { status: "empty" }; + const [first, ...rest] = new Set( + parsed.data.map(({ id }) => id.trim()).filter((id) => id.length > 0), + ); + return first === undefined + ? { status: "empty" } + : { status: "models", models: [first, ...rest] }; } // Single GET against an OpenAI-compatible /models endpoint, so URL diff --git a/src/tui/provider-setup.test.ts b/src/tui/provider-setup.test.ts index 80484c04b..3162f30df 100644 --- a/src/tui/provider-setup.test.ts +++ b/src/tui/provider-setup.test.ts @@ -7,6 +7,7 @@ import { defined } from "../../testkit/defined.js"; import { OAuthProviderScopeError } from "../auth/oauth-scope-check.js"; import { resetGoModelDiscoveryForTests } from "../provider/model-catalogs.js"; +import type { OllamaDiscoveryState } from "../provider/ollama.js"; import { loadLocalSettings, loadSettings, @@ -752,13 +753,7 @@ describe("runProviderSetup Ollama discovery", () => { }); test("distinguishes empty, unavailable, and malformed states and ignores stale retries", async () => { - const pending: (( - state: - | { status: "empty" } - | { status: "unavailable"; message: string } - | { status: "malformed"; message: string } - | { status: "models"; models: string[] }, - ) => void)[] = []; + const pending: ((state: OllamaDiscoveryState) => void)[] = []; const { done, harness } = await mountConfig({ initialProviderId: "ollama", discoverOllamaModels: async () => From ab29f49742ec4b81fd89349314e91047198f09bb Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Wed, 28 Oct 2026 22:05:28 -0700 Subject: [PATCH 2/4] fix(tui): keep the live selection when a catalog prefetch settles The startup refresh after an OpenCode Go or Zen prefetch rebuilt the catalog without the live selection, so a model switched during that rebuild kept the previous model's context window. All four prefetch refresh sites now share one helper that passes it, as the connect path already did. --- src/tui/runner/catalog-refresh.test.ts | 183 +++++++++++++++++++++++++ src/tui/runner/catalog-refresh.ts | 58 ++++++++ src/tui/runner/settings.ts | 45 +----- src/tui/runner/wiring.ts | 76 +--------- 4 files changed, 247 insertions(+), 115 deletions(-) create mode 100644 src/tui/runner/catalog-refresh.test.ts create mode 100644 src/tui/runner/catalog-refresh.ts diff --git a/src/tui/runner/catalog-refresh.test.ts b/src/tui/runner/catalog-refresh.test.ts new file mode 100644 index 000000000..36a0424bb --- /dev/null +++ b/src/tui/runner/catalog-refresh.test.ts @@ -0,0 +1,183 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { defined } from "../../../testkit/defined.js"; +import { withMockedModuleDuring } from "../../../testkit/mock-module.js"; +import { + contextWindowFor, + setModelContextWindows, + setProviderContextWindowOverrides, +} from "../../provider/context-window.js"; +import { refreshCatalogAfterPrefetch } from "./catalog-refresh.js"; +import type { RunnerServices, RunnerState } from "./state.js"; + +const dirs: string[] = []; +afterEach(() => { + setModelContextWindows(undefined); + setProviderContextWindowOverrides(undefined); + for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true }); +}); + +function settingsFile(): string { + const dir = mkdtempSync(join(tmpdir(), "catalog-refresh-")); + dirs.push(dir); + const path = join(dir, "settings.json"); + writeFileSync( + path, + JSON.stringify({ + providers: { + first: { + baseURL: "https://first.example/v1", + keyless: true, + models: ["shared-model"], + contextWindow: 32_000, + }, + second: { + baseURL: "https://second.example/v1", + keyless: true, + models: ["shared-model"], + contextWindow: 64_000, + }, + }, + }), + ); + return path; +} + +function fixture(withHost: boolean) { + const calls: unknown[][] = []; + const host = { + refreshModels: (...args: unknown[]) => { + calls.push(args); + }, + }; + const state = { + trueGlobalSettingsPath: settingsFile(), + config: { + apiKey: "", + baseURL: "https://first.example/v1", + model: "shared-model", + providerName: "first", + keyless: true, + providers: [], + }, + } as unknown as RunnerState; + const services = { + hostHolder: withHost ? { instance: host } : {}, + } as unknown as RunnerServices; + return { state, services, calls }; +} + +function withNoOAuthProfiles(run: () => Promise): Promise { + return withMockedModuleDuring( + import.meta.resolve("../../config/oauth-stores.js"), + (real: typeof import("../../config/oauth-stores.js")) => ({ + ...real, + listCodexProfiles: async () => [], + listXaiProfiles: async () => [], + listMetaProfiles: async () => [], + }), + run, + ); +} + +const flush = () => new Promise((r) => setTimeout(r, 50)); + +describe("refreshCatalogAfterPrefetch", () => { + test("rebuilds the catalog from disk and hands it to the host after the prefetch settles", async () => { + const { state, services, calls } = fixture(true); + const prefetch = Promise.withResolvers(); + await withNoOAuthProfiles(async () => { + refreshCatalogAfterPrefetch( + state, + services, + "go", + () => prefetch.promise, + ); + await flush(); + expect(calls).toHaveLength(0); + prefetch.resolve(undefined); + await flush(); + }); + expect(calls).toHaveLength(1); + expect(defined(calls[0])[2]).toBe(state.config.providers); + expect(state.config.providers.map((p) => p.name).sort()).toEqual([ + "first", + "second", + ]); + expect(state.config.settings).toBeDefined(); + }); + + test("a model switch while the prefetch is in flight is the selection the refresh keeps", async () => { + const { state, services } = fixture(true); + const prefetch = Promise.withResolvers(); + await withNoOAuthProfiles(async () => { + refreshCatalogAfterPrefetch( + state, + services, + "zen", + () => prefetch.promise, + ); + state.config = { ...state.config, providerName: "second" }; + prefetch.resolve(undefined); + await flush(); + }); + expect(contextWindowFor("shared-model")).toBe(64_000); + }); + + test("does nothing when no host is mounted", async () => { + const { state, services } = fixture(false); + const before = state.config; + await withNoOAuthProfiles(async () => { + refreshCatalogAfterPrefetch(state, services, "go", async () => undefined); + await flush(); + }); + expect(state.config).toBe(before); + }); + + test("a failed prefetch leaves state and the host untouched", async () => { + const { state, services, calls } = fixture(true); + const before = state.config; + await withNoOAuthProfiles(async () => { + refreshCatalogAfterPrefetch(state, services, "go", () => + Promise.reject(new Error("offline")), + ); + await flush(); + }); + expect(calls).toHaveLength(0); + expect(state.config).toBe(before); + }); + + test("a model switch during the post-prefetch credential discovery is the selection the refresh keeps", async () => { + const { state, services } = fixture(true); + const discovery = Promise.withResolvers<[]>(); + const discoveryStarted = Promise.withResolvers(); + await withMockedModuleDuring( + import.meta.resolve("../../config/oauth-stores.js"), + (real: typeof import("../../config/oauth-stores.js")) => ({ + ...real, + listCodexProfiles: () => { + discoveryStarted.resolve(undefined); + return discovery.promise; + }, + listXaiProfiles: async () => [], + listMetaProfiles: async () => [], + }), + async () => { + refreshCatalogAfterPrefetch( + state, + services, + "go", + async () => undefined, + ); + await discoveryStarted.promise; + state.config = { ...state.config, providerName: "second" }; + discovery.resolve([]); + await flush(); + }, + ); + expect(contextWindowFor("shared-model")).toBe(64_000); + }); +}); diff --git a/src/tui/runner/catalog-refresh.ts b/src/tui/runner/catalog-refresh.ts new file mode 100644 index 000000000..9247bc0ea --- /dev/null +++ b/src/tui/runner/catalog-refresh.ts @@ -0,0 +1,58 @@ +import { getLogger } from "@intx/log"; +import { LOG_NAMESPACE_ROOT } from "../../branding.js"; +import { refreshLiveProviderCatalog } from "../../config/index.js"; +import { + listFavoriteModels, + listRecentModels, + loadSettings, + type ResolvedProvider, +} from "../../config/settings.js"; +import { caughtDebug } from "../../logging/caught.js"; +import type { RunnerServices, RunnerState } from "./state.js"; + +const tuiLogger = getLogger([LOG_NAMESPACE_ROOT, "tui"]); + +/** + * Once a live model catalog prefetch settles, rebuild the provider catalog + * from disk and hand it to the model picker. The resolved provider is read + * when the prefetch settles, not when it starts, so a model switched in the + * meantime is the one the catalog keeps. + */ +export function refreshCatalogAfterPrefetch( + state: RunnerState, + services: RunnerServices, + catalogName: string, + prefetch: () => Promise, +): void { + void prefetch() + .then(async () => { + const host = services.hostHolder.instance; + if (host === undefined) return; + const onDisk = await loadSettings(state.trueGlobalSettingsPath); + const resolvedForCatalog: ResolvedProvider = { + apiKey: state.config.apiKey, + baseURL: state.config.baseURL, + model: state.config.model, + providerName: state.config.providerName, + ...(state.config.keyless !== undefined + ? { keyless: state.config.keyless } + : {}), + }; + const providers = await refreshLiveProviderCatalog( + onDisk, + resolvedForCatalog, + () => state.config, + ); + state.config = { + ...state.config, + providers, + ...(onDisk !== null ? { settings: onDisk } : {}), + }; + host.refreshModels( + listRecentModels(state.config.settings ?? { providers: {} }), + listFavoriteModels(state.config.settings ?? { providers: {} }), + providers, + ); + }) + .catch(caughtDebug(tuiLogger, `${catalogName} model prefetch failed`)); +} diff --git a/src/tui/runner/settings.ts b/src/tui/runner/settings.ts index 7c37b4dec..a5a613e9f 100644 --- a/src/tui/runner/settings.ts +++ b/src/tui/runner/settings.ts @@ -65,6 +65,7 @@ import { resolveWaitForApproval } from "../../agent/tool-execution-watchdog.js"; import { hostOf, type RunnerServices, type RunnerState } from "./state.js"; import type { ProductHostConnectRequest } from "../product-host.js"; import { LOG_NAMESPACE_ROOT } from "../../branding.js"; +import { refreshCatalogAfterPrefetch } from "./catalog-refresh.js"; const tuiLogger = getLogger([LOG_NAMESPACE_ROOT, "tui"]); @@ -378,50 +379,10 @@ export async function wireSettings( `Connected ${connectedName}. Open /model to pick a model.`, ); if (isOpenCodeGoProvider({ name: providerName })) { - void prefetchGoModels() - .then(async () => { - if (services.hostHolder.instance === undefined) return; - const nextDisk = await loadSettings(trueGlobalSettingsPath); - const nextProviders = await refreshLiveProviderCatalog( - nextDisk, - resolvedForCatalog, - () => state.config, - ); - state.config = { - ...state.config, - providers: nextProviders, - ...(nextDisk !== null ? { settings: nextDisk } : {}), - }; - services.hostHolder.instance.refreshModels( - listRecentModels(state.config.settings ?? { providers: {} }), - listFavoriteModels(state.config.settings ?? { providers: {} }), - nextProviders, - ); - }) - .catch(caughtDebug(tuiLogger, "go model prefetch failed")); + refreshCatalogAfterPrefetch(state, services, "go", prefetchGoModels); } if (isZenProvider({ name: providerName })) { - void prefetchZenModels() - .then(async () => { - if (services.hostHolder.instance === undefined) return; - const nextDisk = await loadSettings(trueGlobalSettingsPath); - const nextProviders = await refreshLiveProviderCatalog( - nextDisk, - resolvedForCatalog, - () => state.config, - ); - state.config = { - ...state.config, - providers: nextProviders, - ...(nextDisk !== null ? { settings: nextDisk } : {}), - }; - services.hostHolder.instance.refreshModels( - listRecentModels(state.config.settings ?? { providers: {} }), - listFavoriteModels(state.config.settings ?? { providers: {} }), - nextProviders, - ); - }) - .catch(caughtDebug(tuiLogger, "zen model prefetch failed")); + refreshCatalogAfterPrefetch(state, services, "zen", prefetchZenModels); } })().catch(caughtDebug(tuiLogger, "provider connect failed")); }; diff --git a/src/tui/runner/wiring.ts b/src/tui/runner/wiring.ts index 440f84088..fd3ca75d7 100644 --- a/src/tui/runner/wiring.ts +++ b/src/tui/runner/wiring.ts @@ -5,13 +5,6 @@ */ import { getLogger } from "@intx/log"; -import { - loadSettings, - listFavoriteModels, - listRecentModels, -} from "../../config/settings.js"; -import { refreshLiveProviderCatalog } from "../../config/index.js"; -import type { ResolvedProvider } from "../../config/settings.js"; import { prefetchGoModels, prefetchZenModels, @@ -85,6 +78,7 @@ import { buildFleetDryContinuationMessage, buildMailboxMailMessage, } from "../../session/runtime-assembly.js"; +import { refreshCatalogAfterPrefetch } from "./catalog-refresh.js"; const tuiLogger = getLogger([LOG_NAMESPACE_ROOT, "tui"]); @@ -246,74 +240,10 @@ export function wirePostStartup( mcpConnectCallbacks: MCPConnectCallbacks, ): void { if (state.config.providers.some((p) => isOpenCodeGoProvider(p))) { - void prefetchGoModels() - .then(async () => { - if (services.hostHolder.instance === undefined) return; - const onDisk = await loadSettings(state.trueGlobalSettingsPath); - const resolvedForCatalog: ResolvedProvider = { - apiKey: state.config.apiKey, - baseURL: state.config.baseURL, - model: state.config.model, - providerName: state.config.providerName, - ...(state.config.keyless !== undefined - ? { keyless: state.config.keyless } - : {}), - }; - const providers = await refreshLiveProviderCatalog( - onDisk, - resolvedForCatalog, - ); - state.config = { - ...state.config, - providers, - ...(onDisk !== null ? { settings: onDisk } : {}), - }; - services.hostHolder.instance.refreshModels( - listRecentModels(state.config.settings ?? { providers: {} }), - listFavoriteModels(state.config.settings ?? { providers: {} }), - providers, - ); - }) - .catch((err: unknown) => { - tuiLogger.debug("go model prefetch failed: {error}", { - error: err instanceof Error ? err.message : String(err), - }); - }); + refreshCatalogAfterPrefetch(state, services, "go", prefetchGoModels); } if (state.config.providers.some((p) => isZenProvider(p))) { - void prefetchZenModels() - .then(async () => { - if (services.hostHolder.instance === undefined) return; - const onDisk = await loadSettings(state.trueGlobalSettingsPath); - const resolvedForCatalog: ResolvedProvider = { - apiKey: state.config.apiKey, - baseURL: state.config.baseURL, - model: state.config.model, - providerName: state.config.providerName, - ...(state.config.keyless !== undefined - ? { keyless: state.config.keyless } - : {}), - }; - const providers = await refreshLiveProviderCatalog( - onDisk, - resolvedForCatalog, - ); - state.config = { - ...state.config, - providers, - ...(onDisk !== null ? { settings: onDisk } : {}), - }; - services.hostHolder.instance.refreshModels( - listRecentModels(state.config.settings ?? { providers: {} }), - listFavoriteModels(state.config.settings ?? { providers: {} }), - providers, - ); - }) - .catch((err: unknown) => { - tuiLogger.debug("zen model prefetch failed: {error}", { - error: err instanceof Error ? err.message : String(err), - }); - }); + refreshCatalogAfterPrefetch(state, services, "zen", prefetchZenModels); } const shutdownRuntime = createRuntimeShutdown({ From 00a44cb48a3ae7cdb76f5d82153f593412425b40 Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Wed, 28 Oct 2026 22:09:40 -0700 Subject: [PATCH 3/4] refactor(config): let an OAuth projection choose models per profile --- src/config/codex-providers.ts | 2 +- src/config/meta-providers.ts | 2 +- src/config/oauth-providers.test.ts | 46 +++++++++++++++++++++++++++++- src/config/oauth-providers.ts | 29 +++++++++++-------- src/config/xai-providers.ts | 2 +- 5 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/config/codex-providers.ts b/src/config/codex-providers.ts index 0bc0e0301..5a5803262 100644 --- a/src/config/codex-providers.ts +++ b/src/config/codex-providers.ts @@ -13,7 +13,7 @@ export const CODEX_PROVIDER_PREFIX = "codex/"; const projection = createOAuthProviderProjection({ prefix: CODEX_PROVIDER_PREFIX, baseURL: CODEX_BASE_URL, - defaultModels: CODEX_DEFAULT_MODELS, + models: () => CODEX_DEFAULT_MODELS, catalogExtras: (profile) => ({ codexProfile: profile.name, ...(profile.tokens.accountId !== undefined diff --git a/src/config/meta-providers.ts b/src/config/meta-providers.ts index 266f80747..d11386161 100644 --- a/src/config/meta-providers.ts +++ b/src/config/meta-providers.ts @@ -7,7 +7,7 @@ export const META_PROVIDER_PREFIX = "meta/"; const projection = createOAuthProviderProjection({ prefix: META_PROVIDER_PREFIX, baseURL: META_BASE_URL, - defaultModels: META_DEFAULT_MODELS, + models: () => META_DEFAULT_MODELS, catalogExtras: (profile) => ({ metaProfile: profile.name }), }); diff --git a/src/config/oauth-providers.test.ts b/src/config/oauth-providers.test.ts index e6d7f23ce..f84f1638d 100644 --- a/src/config/oauth-providers.test.ts +++ b/src/config/oauth-providers.test.ts @@ -3,12 +3,14 @@ import { describe, expect, test } from "bun:test"; import type { CodexProfile } from "../auth/codex/store.js"; import type { XaiProfile } from "../auth/xai/store.js"; import { codexProfilesToCatalogEntries } from "./codex-providers.js"; +import { createOAuthProviderProjection } from "./oauth-providers.js"; import { xaiProfilesToCatalogEntries } from "./xai-providers.js"; // Characterization tests pinning the projection behavior both provider // wrappers must preserve. Name round-trips and settings projection are // characterized in codex-providers.test.ts and xai-providers.test.ts — only -// cross-provider marker isolation lives here. +// cross-provider marker isolation and the per-profile models contract live +// here. const codexProfile: CodexProfile = { name: "work", @@ -50,3 +52,45 @@ describe("catalog projection", () => { expect(entries[0]?.codexAccountId).toBeUndefined(); }); }); + +describe("per-profile models", () => { + const profiles = [ + { name: "a", tokens: { access: "ta" } }, + { name: "b", tokens: { access: "tb" } }, + ]; + const perProfile: Record = { + a: ["m-a1", "m-a2"], + b: ["m-b1"], + }; + const projection = createOAuthProviderProjection({ + prefix: "p/", + baseURL: "https://example.invalid", + models: (profile) => { + const list = perProfile[profile.name]; + if (list === undefined) throw new Error(`no models for ${profile.name}`); + return list; + }, + catalogExtras: () => ({}), + }); + + test("the settings projection lists each profile's models with the first as default", () => { + const settings = projection.providersAsSettings(profiles); + expect(settings["p/a"]?.models).toEqual(["m-a1", "m-a2"]); + expect(settings["p/a"]?.defaultModel).toBe("m-a1"); + expect(settings["p/b"]?.models).toEqual(["m-b1"]); + expect(settings["p/b"]?.defaultModel).toBe("m-b1"); + }); + + test("the catalog projection lists each profile's models with the first as default", () => { + const entries = projection.profilesToCatalogEntries(profiles); + expect(entries.map((e) => [e.name, e.models, e.defaultModel])).toEqual([ + ["p/a", ["m-a1", "m-a2"], "m-a1"], + ["p/b", ["m-b1"], "m-b1"], + ]); + }); + + test("projected lists are copies the caller can mutate without touching the source", () => { + projection.providersAsSettings(profiles)["p/a"]?.models.push("mutated"); + expect(perProfile["a"]).toEqual(["m-a1", "m-a2"]); + }); +}); diff --git a/src/config/oauth-providers.ts b/src/config/oauth-providers.ts index abfb1a451..6554283f6 100644 --- a/src/config/oauth-providers.ts +++ b/src/config/oauth-providers.ts @@ -26,8 +26,9 @@ export interface OAuthProviderProjectionOptions

{ // Provider namespace including the trailing slash (e.g. "codex/"). prefix: string; baseURL: string; - // Non-empty: the first entry doubles as the default model. - defaultModels: readonly [string, ...string[]]; + // Models a profile can select. Non-empty: the first entry doubles as the + // default model. + models: (profile: P) => readonly [string, ...string[]]; // Provider-specific catalog fields (profile marker, account id, …) merged // into each catalog entry so the runtime can route it to the right adapter. catalogExtras: (profile: P) => Partial; @@ -60,12 +61,13 @@ export function createOAuthProviderProjection

( ): Record { const entries: Record = {}; for (const profile of profiles) { + const models = options.models(profile); entries[providerName(profile.name)] = { name: providerName(profile.name), baseURL: options.baseURL, apiKey: profile.tokens.access, - models: [...options.defaultModels], - defaultModel: options.defaultModels[0], + models: [...models], + defaultModel: models[0], }; } return entries; @@ -74,14 +76,17 @@ export function createOAuthProviderProjection

( // these carry the provider's marker fields (via catalogExtras) so the // runtime can route them to the matching adapter and headers. profilesToCatalogEntries(profiles: readonly P[]): ProviderCatalogEntry[] { - return profiles.map((profile) => ({ - name: providerName(profile.name), - baseURL: options.baseURL, - apiKey: profile.tokens.access, - models: [...options.defaultModels], - defaultModel: options.defaultModels[0], - ...options.catalogExtras(profile), - })); + return profiles.map((profile) => { + const models = options.models(profile); + return { + name: providerName(profile.name), + baseURL: options.baseURL, + apiKey: profile.tokens.access, + models: [...models], + defaultModel: models[0], + ...options.catalogExtras(profile), + }; + }); }, }; } diff --git a/src/config/xai-providers.ts b/src/config/xai-providers.ts index ec166c0c8..a5c1831bb 100644 --- a/src/config/xai-providers.ts +++ b/src/config/xai-providers.ts @@ -7,7 +7,7 @@ export const XAI_PROVIDER_PREFIX = "xai/"; const projection = createOAuthProviderProjection({ prefix: XAI_PROVIDER_PREFIX, baseURL: XAI_BASE_URL, - defaultModels: XAI_DEFAULT_MODELS, + models: () => XAI_DEFAULT_MODELS, catalogExtras: (profile) => ({ xaiProfile: profile.name }), }); From bed40379f025861e7a109dfb6efdf018329c82e6 Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Wed, 28 Oct 2026 22:26:08 -0700 Subject: [PATCH 4/4] feat(provider): log why a live model catalog kept its fallback A connection that drops or times out mid-body now counts as unavailable rather than malformed, so routine network trouble stays at debug while an empty or malformed catalog warns. --- src/provider/bounded-model-catalog.ts | 83 +++++++++++++++++--- src/provider/model-catalogs.test.ts | 106 ++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 11 deletions(-) diff --git a/src/provider/bounded-model-catalog.ts b/src/provider/bounded-model-catalog.ts index f6f0409b1..ec643a1c0 100644 --- a/src/provider/bounded-model-catalog.ts +++ b/src/provider/bounded-model-catalog.ts @@ -1,8 +1,12 @@ +import { getLogger } from "@intx/log"; +import { LOG_NAMESPACE_ROOT } from "../branding.js"; import { endpointErrorMessage, type ModelsEndpointDiscoveryState, } from "./models-endpoint.js"; +const catalogLogger = getLogger([LOG_NAMESPACE_ROOT, "provider", "catalog"]); + export type CatalogDiscoveryState = ModelsEndpointDiscoveryState; function declaredCatalogBytes(response: Response): number | undefined { @@ -18,12 +22,16 @@ async function readBoundedCatalogText( args: { readonly maxBytes: number; readonly oversizeMessage: string }, ): Promise< | { readonly ok: true; readonly text: string } - | { readonly ok: false; readonly message: string } + | { + readonly ok: false; + readonly message: string; + readonly transport: boolean; + } > { const declared = declaredCatalogBytes(response); if (declared !== undefined && declared > args.maxBytes) { await response.body?.cancel().catch(() => undefined); - return { ok: false, message: args.oversizeMessage }; + return { ok: false, message: args.oversizeMessage, transport: false }; } try { @@ -31,7 +39,7 @@ async function readBoundedCatalogText( if (body === null) { const text = await response.text(); if (new TextEncoder().encode(text).byteLength > args.maxBytes) { - return { ok: false, message: args.oversizeMessage }; + return { ok: false, message: args.oversizeMessage, transport: false }; } return { ok: true, text }; } @@ -46,7 +54,7 @@ async function readBoundedCatalogText( total += value.byteLength; if (total > args.maxBytes) { await reader.cancel().catch(() => undefined); - return { ok: false, message: args.oversizeMessage }; + return { ok: false, message: args.oversizeMessage, transport: false }; } chunks.push(value); } @@ -59,7 +67,11 @@ async function readBoundedCatalogText( } return { ok: true, text: new TextDecoder().decode(buffer) }; } catch (error) { - return { ok: false, message: endpointErrorMessage(error) }; + return { + ok: false, + message: endpointErrorMessage(error), + transport: true, + }; } } @@ -104,11 +116,13 @@ export function createBoundedModelCatalog< return `${catalogLabel} catalog exceeds ${String(maxModels)} models`; } - async function readCatalogJson( - response: Response, - ): Promise< + async function readCatalogJson(response: Response): Promise< | { readonly ok: true; readonly value: unknown } - | { readonly ok: false; readonly message: string } + | { + readonly ok: false; + readonly message: string; + readonly transport: boolean; + } > { const text = await readBoundedCatalogText(response, { maxBytes, @@ -119,7 +133,11 @@ export function createBoundedModelCatalog< const value: unknown = JSON.parse(text.text); return { ok: true, value }; } catch (error) { - return { ok: false, message: endpointErrorMessage(error) }; + return { + ok: false, + message: endpointErrorMessage(error), + transport: false, + }; } } @@ -151,7 +169,12 @@ export function createBoundedModelCatalog< const body = await readCatalogJson(response); if (!body.ok) { - return { status: "malformed", message: body.message }; + // A connection that drops or times out mid-body never delivered an + // answer; only a body that arrived and fails the contract is malformed. + return { + status: body.transport ? "unavailable" : "malformed", + message: body.message, + }; } return parse(body.value, { maxModels, @@ -169,10 +192,48 @@ export function createBoundedModelCatalog< // stale-but-live beats empty; a cold failure falls back to the seed. if (state.status === "models") { snapshot = state.models; + } else { + logFallback(state); } return selectableModelIds(); } + // An unreachable catalog is routine (offline, transient outage), so it + // stays at debug. An empty or malformed answer means the endpoint's + // contract moved, which only shows up here, so it warns. + function logFallback( + state: Exclude, + ): void { + const fallback = + snapshot === undefined ? "the built-in list" : "the last live list"; + // HTTP and size failures already name the catalog; transport and parse + // errors do not, so the catalog is named exactly once either way. + const reason = (message: string): string => + message.startsWith(catalogLabel) + ? message + : `${catalogLabel}: ${message}`; + switch (state.status) { + case "unavailable": + catalogLogger.debug( + "Model catalog unavailable ({reason}); keeping {fallback}", + { reason: reason(state.message), fallback }, + ); + return; + case "empty": + catalogLogger.warn( + "Model catalog listed no models ({catalog}); keeping {fallback}", + { catalog: catalogLabel, fallback }, + ); + return; + case "malformed": + catalogLogger.warn( + "Model catalog response rejected ({reason}); keeping {fallback}", + { reason: reason(state.message), fallback }, + ); + return; + } + } + function prefetchModels(): Promise { if (inflight !== undefined) return inflight; diff --git a/src/provider/model-catalogs.test.ts b/src/provider/model-catalogs.test.ts index 7547bd7e9..03537ae12 100644 --- a/src/provider/model-catalogs.test.ts +++ b/src/provider/model-catalogs.test.ts @@ -1,4 +1,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { configureSync, getConfig, resetSync } from "@intx/log"; + +import { defined } from "../../testkit/defined.js"; import { OPENCODE_GO_MODEL_IDS } from "../../packages/opencode-go/src/index.js"; import { ZEN_MODEL_IDS } from "../../packages/zen/src/index.js"; @@ -92,6 +95,35 @@ afterEach(() => { resetZenModelDiscoveryForTests(); }); +// Records the catalog's own log lines; restores the previous log config even +// when `run` throws so other files keep their sinks. +async function captureCatalogLogs( + catalogLabel: string, + run: () => Promise, +): Promise<{ level: string; text: string }[]> { + const records: { level: string; text: string }[] = []; + const previous = getConfig(); + configureSync({ + reset: true, + sinks: { + test: (record) => { + records.push({ + level: record.level, + text: record.message.map(String).join(""), + }); + }, + }, + loggers: [{ category: [], lowestLevel: "debug", sinks: ["test"] }], + }); + try { + await run(); + } finally { + if (previous === null) resetSync(); + else configureSync({ ...previous, reset: true }); + } + return records.filter((r) => r.text.includes(catalogLabel)); +} + function oversizedCatalogResponse(byteLength: number): Response { const chunk = new Uint8Array(64 * 1024).fill(0x61); let remaining = byteLength; @@ -324,6 +356,80 @@ for (const harness of harnesses) { expect(selectable()).toEqual(seedIds); }); + test("logs why a prefetch kept the fallback, warning only when the catalog answered badly", async () => { + const records = await captureCatalogLogs(catalogLabel, async () => { + globalThis.fetch = (async () => + new Response("no", { status: 503 })) as unknown as typeof fetch; + await prefetch(); + globalThis.fetch = (async () => + Response.json({ data: [] })) as unknown as typeof fetch; + await prefetch(); + globalThis.fetch = (async () => + Response.json({ models: [] })) as unknown as typeof fetch; + await prefetch(); + }); + + expect(records.map((r) => r.level)).toEqual([ + "debug", + "warning", + "warning", + ]); + for (const record of records) { + expect(record.text).toContain("keeping the built-in list"); + expect(record.text.split(catalogLabel)).toHaveLength(2); + } + }); + + test("a connection that drops mid-body logs as unavailable, not as a contract change", async () => { + const records = await captureCatalogLogs(catalogLabel, async () => { + globalThis.fetch = (async () => { + let sent = false; + const stream = new ReadableStream({ + pull(controller) { + if (!sent) { + sent = true; + controller.enqueue(new TextEncoder().encode('{"data":[')); + return; + } + controller.error(new Error("socket connection was closed")); + }, + }); + return new Response(stream, { status: 200 }); + }) as unknown as typeof fetch; + await prefetch(); + }); + + expect(records.map((r) => r.level)).toEqual(["debug"]); + expect(defined(records[0]).text).toContain("unavailable"); + }); + + test("a failed refresh after a live list says it kept the last live list", async () => { + globalThis.fetch = (async () => + Response.json({ + data: [{ id: liveOnlyId }], + })) as unknown as typeof fetch; + await prefetch(); + + const records = await captureCatalogLogs(catalogLabel, async () => { + globalThis.fetch = (async () => + Response.json({ data: [] })) as unknown as typeof fetch; + await prefetch(); + }); + + expect(records).toHaveLength(1); + expect(defined(records[0]).text).toContain("keeping the last live list"); + }); + + test("concurrent callers of one prefetch run produce one fallback record", async () => { + const records = await captureCatalogLogs(catalogLabel, async () => { + globalThis.fetch = (async () => + Response.json({ data: [] })) as unknown as typeof fetch; + await Promise.all([prefetch(), prefetch(), prefetch()]); + }); + + expect(records).toHaveLength(1); + }); + test("oversized live catalog does not replace the seed with a truncated prefix", async () => { globalThis.fetch = (async () => oversizedCatalogResponse(maxBytes + 1)) as unknown as typeof fetch;