From e1e344efec50b54f181a3486c4621894641238d5 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 11:39:41 -0700 Subject: [PATCH 1/2] Derive ProviderCatalogEntry from ProviderSettings instead of duplicating its field list src/config/index.ts:95-130 (ProviderCatalogEntry) and src/config/settings.ts:21-45 (ProviderSettings) independently declared the same provider record, so a field added to one silently never reached the catalog conversion in catalogEntryAsProviderSettings/buildProviderCatalog. ProviderCatalogEntry is now Omit plus its required name and the OAuth-only markers (codexProfile/codexAccountId/xaiProfile) that have no settings.json counterpart. Added a round-trip test in src/config.test.ts that fails if a shared field stops surviving buildProviderCatalog -> catalogEntryAsProviderSettings. --- src/config.test.ts | 27 ++++++++++++++++++++++++++- src/config/index.ts | 36 ++++++++++-------------------------- 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/src/config.test.ts b/src/config.test.ts index 607e2c908..9ae2425b8 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -3,7 +3,7 @@ import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { buildBifrostSource, buildOpenAISource, buildProviderCatalog, KEYLESS_API_KEY, loadConfig, providerCatalogToSettings, runtimeSettingsWithCatalog, SOURCE_MAX_TOKENS } from "./config/index.js"; +import { buildBifrostSource, buildOpenAISource, buildProviderCatalog, catalogEntryAsProviderSettings, KEYLESS_API_KEY, loadConfig, providerCatalogToSettings, runtimeSettingsWithCatalog, SOURCE_MAX_TOKENS } from "./config/index.js"; import type { Config, UnconfiguredConfig } from "./config/index.js"; import { mergeProviderIntoSettings, type ResolvedProvider, type Settings } from "./config/settings.js"; @@ -648,6 +648,31 @@ describe("buildProviderCatalog", () => { }); expect(restOut).toEqual(restExisting); }); + + test("round-trips every ProviderSettings field a catalog entry can carry through buildProviderCatalog and back", () => { + // ProviderCatalogEntry is defined as Omit. + // This exercises every field that relationship carries over, so a field + // added to ProviderSettings and forgotten in the two conversion sites + // below fails here instead of being silently dropped at runtime. + const provider: Settings["providers"][string] = { + baseURL: "https://fp/v1", + apiKey: "fp-key", + models: ["fp-large"], + defaultModel: "fp-large", + free: true, + bifrostVirtualKey: true, + }; + const settings: Settings = { providers: { fp: provider } }; + const catalog = buildProviderCatalog(settings, { + providerName: "fp", + baseURL: provider.baseURL, + apiKey: "fp-key", + model: "fp-large", + } as ResolvedProvider); + const entry = catalog.find((c) => c.name === "fp")!; + const roundTripped = { fp: catalogEntryAsProviderSettings(entry) }; + expect(roundTripped).toEqual({ fp: provider }); + }); }); describe("mergeProviderIntoSettings", () => { diff --git a/src/config/index.ts b/src/config/index.ts index 3310d189f..8c888a575 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -91,24 +91,19 @@ export function buildOpenAISource(fields: { // One configured provider the /agent modal can switch to. Carries credentials // because live switching builds an InferenceSource from it; the modal only ever -// receives fields needed for provider management, never the key. -export type ProviderCatalogEntry = { +// receives fields needed for provider management, never the key. Derived from +// ProviderSettings (the persisted record) so a field added there is never +// silently missing here: `name` becomes required (every catalog entry is +// resolved to a concrete provider id) and `contextWindow` is dropped (it is a +// settings-only override, never surfaced to the /agent modal). The +// OAuth-profile markers below have no ProviderSettings counterpart because +// such entries are never written to settings.json (their credentials live in +// the Codex/xAI auth stores). +export type ProviderCatalogEntry = Omit & { name: string; - baseURL: string; - // Absent for keyless providers (see `keyless`). When present, carries the - // secret key the harness injects as a Bearer credential. - apiKey?: string; - models: string[]; - defaultModel?: string; - // True for local providers that require no authentication (e.g. Ollama). - // When set, `apiKey` is omitted and resolution skips the key check. - keyless?: boolean; - // Manual override suppressing the status-bar dollar cost for this provider. - free?: boolean; // Set when this entry is a Codex OAuth profile rather than an API-key // provider. Holds the profile name; the send path uses it to refresh the - // access token before each turn. Such entries are never written to - // settings.json (their credentials live in the Codex auth store). + // access token before each turn. codexProfile?: string; // ChatGPT account id for a Codex profile, sent as the chatgpt-account-id // header by the Responses adapter. Present only on Codex entries. @@ -116,17 +111,6 @@ export type ProviderCatalogEntry = { // Set when this entry is an xAI/Grok OAuth profile. It still routes through // openai-compatible; the marker only controls token refresh and persistence. xaiProfile?: string; - // When true this provider is backed by a Bifrost virtual key. Inference - // sources for it are built with provider "bifrost" so the adapter can - // inject the x-bf-vk header (in addition to Authorization). The flag is - // also used to enable /models auto-discovery scoped to the key. - bifrostVirtualKey?: boolean; - // Anthropic Messages API (x-api-key). Used by first-class Anthropic and by - // OpenCode Go models that speak the messages protocol. - anthropic?: boolean; - // OpenCode Go multi-protocol provider. Per-model routing picks - // openai-compatible, openai-responses, or anthropic at source-build time. - opencodeGo?: boolean; }; // Build the InferenceSource for a Codex OAuth profile. Routes to the From 65d3629e59bb7aab9dfefe0cb0fc5ea29f5bab2a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:03:10 -0700 Subject: [PATCH 2/2] Tighten the ProviderCatalogEntry round-trip test and its comment The round-trip test omitted keyless, anthropic, and opencodeGo, so it could not actually catch a dropped optional field despite claiming to exercise every field. Split anthropic/opencodeGo into their own cases since both also normalize baseURL in buildProviderCatalog, and cover keyless in the base case. Also correct the ProviderCatalogEntry doc comment: the Omit/intersection only forces literals to supply a newly required field: an optional field can still be silently dropped by a hand-written conversion, which is why the round-trip test exists. --- src/config.test.ts | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/config/index.ts | 18 +++++++++++------- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/config.test.ts b/src/config.test.ts index 9ae2425b8..404210799 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -6,6 +6,7 @@ import { join } from "node:path"; import { buildBifrostSource, buildOpenAISource, buildProviderCatalog, catalogEntryAsProviderSettings, KEYLESS_API_KEY, loadConfig, providerCatalogToSettings, runtimeSettingsWithCatalog, SOURCE_MAX_TOKENS } from "./config/index.js"; import type { Config, UnconfiguredConfig } from "./config/index.js"; import { mergeProviderIntoSettings, type ResolvedProvider, type Settings } from "./config/settings.js"; +import { OPENCODE_GO_BASE_URL } from "../packages/opencode-go/src/index.js"; function assertConfigured(config: Config | UnconfiguredConfig): asserts config is Config { if (config.configured === false) { @@ -654,12 +655,17 @@ describe("buildProviderCatalog", () => { // This exercises every field that relationship carries over, so a field // added to ProviderSettings and forgotten in the two conversion sites // below fails here instead of being silently dropped at runtime. + // `anthropic` and `opencodeGo` are exercised separately below: both are + // protocol markers that also normalize `baseURL` in buildProviderCatalog, + // so a provider combining them with an arbitrary baseURL isn't a real + // round trip (the healing logic rewrites baseURL by design). const provider: Settings["providers"][string] = { baseURL: "https://fp/v1", apiKey: "fp-key", models: ["fp-large"], defaultModel: "fp-large", free: true, + keyless: true, bifrostVirtualKey: true, }; const settings: Settings = { providers: { fp: provider } }; @@ -673,6 +679,44 @@ describe("buildProviderCatalog", () => { const roundTripped = { fp: catalogEntryAsProviderSettings(entry) }; expect(roundTripped).toEqual({ fp: provider }); }); + + test("round-trips the anthropic protocol marker", () => { + const provider: Settings["providers"][string] = { + baseURL: "https://api.anthropic.com/v1", + apiKey: "an-key", + models: ["claude"], + anthropic: true, + }; + const settings: Settings = { providers: { an: provider } }; + const catalog = buildProviderCatalog(settings, { + providerName: "an", + baseURL: provider.baseURL, + apiKey: "an-key", + model: "claude", + } as ResolvedProvider); + const entry = catalog.find((c) => c.name === "an")!; + const roundTripped = { an: catalogEntryAsProviderSettings(entry) }; + expect(roundTripped).toEqual({ an: provider }); + }); + + test("round-trips the opencodeGo protocol marker", () => { + const provider: Settings["providers"][string] = { + baseURL: OPENCODE_GO_BASE_URL, + apiKey: "go-key", + models: ["go-model"], + opencodeGo: true, + }; + const settings: Settings = { providers: { go: provider } }; + const catalog = buildProviderCatalog(settings, { + providerName: "go", + baseURL: provider.baseURL, + apiKey: "go-key", + model: "go-model", + } as ResolvedProvider); + const entry = catalog.find((c) => c.name === "go")!; + const roundTripped = { go: catalogEntryAsProviderSettings(entry) }; + expect(roundTripped).toEqual({ go: provider }); + }); }); describe("mergeProviderIntoSettings", () => { diff --git a/src/config/index.ts b/src/config/index.ts index 8c888a575..ee4cd78d7 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -92,13 +92,17 @@ export function buildOpenAISource(fields: { // One configured provider the /agent modal can switch to. Carries credentials // because live switching builds an InferenceSource from it; the modal only ever // receives fields needed for provider management, never the key. Derived from -// ProviderSettings (the persisted record) so a field added there is never -// silently missing here: `name` becomes required (every catalog entry is -// resolved to a concrete provider id) and `contextWindow` is dropped (it is a -// settings-only override, never surfaced to the /agent modal). The -// OAuth-profile markers below have no ProviderSettings counterpart because -// such entries are never written to settings.json (their credentials live in -// the Codex/xAI auth stores). +// ProviderSettings (the persisted record) so the field *set* stays tied to it: +// a newly required ProviderSettings field forces every catalog-entry literal +// to supply it. `name` becomes required (every catalog entry is resolved to a +// concrete provider id) and `contextWindow` is dropped (it is a settings-only +// override, never surfaced to the /agent modal). The OAuth-profile markers +// below have no ProviderSettings counterpart because such entries are never +// written to settings.json (their credentials live in the Codex/xAI auth +// stores). Optional fields still need the round-trip test in config.test.ts — +// TS does not flag a missing optional property against an explicitly-typed +// object literal, so forwarding of an optional field can only be caught at +// runtime. export type ProviderCatalogEntry = Omit & { name: string; // Set when this entry is a Codex OAuth profile rather than an API-key