Skip to content

Commit 65d3629

Browse files
committed
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.
1 parent e1e344e commit 65d3629

2 files changed

Lines changed: 55 additions & 7 deletions

File tree

src/config.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { join } from "node:path";
66
import { buildBifrostSource, buildOpenAISource, buildProviderCatalog, catalogEntryAsProviderSettings, KEYLESS_API_KEY, loadConfig, providerCatalogToSettings, runtimeSettingsWithCatalog, SOURCE_MAX_TOKENS } from "./config/index.js";
77
import type { Config, UnconfiguredConfig } from "./config/index.js";
88
import { mergeProviderIntoSettings, type ResolvedProvider, type Settings } from "./config/settings.js";
9+
import { OPENCODE_GO_BASE_URL } from "../packages/opencode-go/src/index.js";
910

1011
function assertConfigured(config: Config | UnconfiguredConfig): asserts config is Config {
1112
if (config.configured === false) {
@@ -654,12 +655,17 @@ describe("buildProviderCatalog", () => {
654655
// This exercises every field that relationship carries over, so a field
655656
// added to ProviderSettings and forgotten in the two conversion sites
656657
// below fails here instead of being silently dropped at runtime.
658+
// `anthropic` and `opencodeGo` are exercised separately below: both are
659+
// protocol markers that also normalize `baseURL` in buildProviderCatalog,
660+
// so a provider combining them with an arbitrary baseURL isn't a real
661+
// round trip (the healing logic rewrites baseURL by design).
657662
const provider: Settings["providers"][string] = {
658663
baseURL: "https://fp/v1",
659664
apiKey: "fp-key",
660665
models: ["fp-large"],
661666
defaultModel: "fp-large",
662667
free: true,
668+
keyless: true,
663669
bifrostVirtualKey: true,
664670
};
665671
const settings: Settings = { providers: { fp: provider } };
@@ -673,6 +679,44 @@ describe("buildProviderCatalog", () => {
673679
const roundTripped = { fp: catalogEntryAsProviderSettings(entry) };
674680
expect(roundTripped).toEqual({ fp: provider });
675681
});
682+
683+
test("round-trips the anthropic protocol marker", () => {
684+
const provider: Settings["providers"][string] = {
685+
baseURL: "https://api.anthropic.com/v1",
686+
apiKey: "an-key",
687+
models: ["claude"],
688+
anthropic: true,
689+
};
690+
const settings: Settings = { providers: { an: provider } };
691+
const catalog = buildProviderCatalog(settings, {
692+
providerName: "an",
693+
baseURL: provider.baseURL,
694+
apiKey: "an-key",
695+
model: "claude",
696+
} as ResolvedProvider);
697+
const entry = catalog.find((c) => c.name === "an")!;
698+
const roundTripped = { an: catalogEntryAsProviderSettings(entry) };
699+
expect(roundTripped).toEqual({ an: provider });
700+
});
701+
702+
test("round-trips the opencodeGo protocol marker", () => {
703+
const provider: Settings["providers"][string] = {
704+
baseURL: OPENCODE_GO_BASE_URL,
705+
apiKey: "go-key",
706+
models: ["go-model"],
707+
opencodeGo: true,
708+
};
709+
const settings: Settings = { providers: { go: provider } };
710+
const catalog = buildProviderCatalog(settings, {
711+
providerName: "go",
712+
baseURL: provider.baseURL,
713+
apiKey: "go-key",
714+
model: "go-model",
715+
} as ResolvedProvider);
716+
const entry = catalog.find((c) => c.name === "go")!;
717+
const roundTripped = { go: catalogEntryAsProviderSettings(entry) };
718+
expect(roundTripped).toEqual({ go: provider });
719+
});
676720
});
677721

678722
describe("mergeProviderIntoSettings", () => {

src/config/index.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,17 @@ export function buildOpenAISource(fields: {
9292
// One configured provider the /agent modal can switch to. Carries credentials
9393
// because live switching builds an InferenceSource from it; the modal only ever
9494
// receives fields needed for provider management, never the key. Derived from
95-
// ProviderSettings (the persisted record) so a field added there is never
96-
// silently missing here: `name` becomes required (every catalog entry is
97-
// resolved to a concrete provider id) and `contextWindow` is dropped (it is a
98-
// settings-only override, never surfaced to the /agent modal). The
99-
// OAuth-profile markers below have no ProviderSettings counterpart because
100-
// such entries are never written to settings.json (their credentials live in
101-
// the Codex/xAI auth stores).
95+
// ProviderSettings (the persisted record) so the field *set* stays tied to it:
96+
// a newly required ProviderSettings field forces every catalog-entry literal
97+
// to supply it. `name` becomes required (every catalog entry is resolved to a
98+
// concrete provider id) and `contextWindow` is dropped (it is a settings-only
99+
// override, never surfaced to the /agent modal). The OAuth-profile markers
100+
// below have no ProviderSettings counterpart because such entries are never
101+
// written to settings.json (their credentials live in the Codex/xAI auth
102+
// stores). Optional fields still need the round-trip test in config.test.ts —
103+
// TS does not flag a missing optional property against an explicitly-typed
104+
// object literal, so forwarding of an optional field can only be caught at
105+
// runtime.
102106
export type ProviderCatalogEntry = Omit<ProviderSettings, "name" | "contextWindow"> & {
103107
name: string;
104108
// Set when this entry is a Codex OAuth profile rather than an API-key

0 commit comments

Comments
 (0)