From aab2f3ae195a249e021f778fd1681a27233f6c8b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 8 Oct 2026 21:45:56 -0700 Subject: [PATCH] feat(config): warn on ignored provider fields --- src/config/settings.test.ts | 108 ++++++++++++++++++++++++++++++++++++ src/config/settings.ts | 50 +++++++++++++++++ 2 files changed, 158 insertions(+) diff --git a/src/config/settings.test.ts b/src/config/settings.test.ts index 5a23cf5ab..15ea2cdbf 100644 --- a/src/config/settings.test.ts +++ b/src/config/settings.test.ts @@ -13,6 +13,7 @@ import { loadConfig } from "./index.js"; import { resetPricingMetadataRefreshForTests } from "../cost/pricing-metadata.js"; import { setProviderContextWindowOverrides } from "../provider/context-window.js"; import { withMockedHomedir } from "../../testkit/mock-module.js"; +import { captureStderr } from "../../testkit/capture-stderr.js"; import { isSettings, loadSettings, saveGlobalSettings } from "./settings.js"; test.each([ @@ -680,3 +681,110 @@ test("--config composes with OAuth profile auth instead of suppressing it", asyn await rm(cwd, { recursive: true, force: true }); } }); + +// CL-10226 (Part 1): some external tools' provider blocks carry fields Corbits +// does not honor. They pass ProviderSettingsSchema (which stays fail-open on +// unknown keys) and are silently dropped. The settings load path must warn on +// stderr while still loading successfully. +test("settings load warns and still loads when a provider carries ignored fields", async () => { + const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-ignored-fields-")); + const path = join(cwd, "settings.json"); + const stderr = captureStderr(); + try { + await writeFile( + path, + JSON.stringify({ + providers: { + external: { + baseURL: "https://external.example/v1", + keyless: true, + models: ["test-model"], + supportsThinkingTokenBudget: true, + thinkingLevelMap: { low: 1 }, + samplingParams: { temperature: 0.5 }, + compat: { supportsReasoningEffort: true }, + }, + }, + }), + ); + const loaded = await loadSettings(path); + expect(loaded).not.toBeNull(); + expect(loaded?.providers.external).toBeDefined(); + expect(stderr.output()).toContain( + '"external" sets fields that are not honored by Corbits and will be ignored', + ); + expect(stderr.output()).toContain("supportsThinkingTokenBudget"); + expect(stderr.output()).toContain("thinkingLevelMap"); + expect(stderr.output()).toContain("samplingParams"); + expect(stderr.output()).toContain("compat → supportsReasoningEffort"); + expect(stderr.output()).toContain( + "These fields are not yet supported; configure effort/thinking via provider/model-level settings.", + ); + } finally { + stderr.restore(); + await rm(cwd, { recursive: true, force: true }); + } +}); + +test("settings load does not warn for a normal provider block", async () => { + const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-no-ignored-")); + const path = join(cwd, "settings.json"); + const stderr = captureStderr(); + try { + await writeFile( + path, + JSON.stringify({ + providers: { + external: { + baseURL: "https://external.example/v1", + keyless: true, + models: ["test-model"], + }, + }, + }), + ); + const loaded = await loadSettings(path); + expect(loaded).not.toBeNull(); + expect(stderr.output()).not.toContain("not honored by Corbits"); + expect(stderr.output()).not.toContain("supportsThinkingTokenBudget"); + } finally { + stderr.restore(); + await rm(cwd, { recursive: true, force: true }); + } +}); + +// The same warn must surface on the config path (--config / global settings), +// which routes through loadSettings... → normalizeParsedSettings. +test("loadConfig warns and still loads when a provider carries ignored fields", async () => { + const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-ignored-config-")); + const globalSettingsPath = join(cwd, "global.json"); + const stderr = captureStderr(); + try { + await writeFile( + globalSettingsPath, + JSON.stringify({ + defaultProvider: "external", + providers: { + external: { + baseURL: "https://external.example/v1", + apiKey: "test-key", + models: ["test-model"], + samplingParams: { temperature: 0.5 }, + }, + }, + }), + ); + const { impl } = offlineFetch(); + const config = await loadConfig(["--cwd", cwd, "do something"], { + globalSettingsPath, + pricing: { fetchImpl: impl }, + }); + expect(config.configured).toBe(true); + if (config.configured) expect(config.providerName).toBe("external"); + expect(stderr.output()).toContain("not honored by Corbits"); + expect(stderr.output()).toContain("samplingParams"); + } finally { + stderr.restore(); + await rm(cwd, { recursive: true, force: true }); + } +}); diff --git a/src/config/settings.ts b/src/config/settings.ts index 7e5cbb3a6..1c8f1ca8f 100644 --- a/src/config/settings.ts +++ b/src/config/settings.ts @@ -1036,11 +1036,61 @@ function settingsSchemaError(path: string): Error { ); } +// Provider blocks from some external tools carry fields Corbits does not +// honor: compat.supportsReasoningEffort, supportsThinkingTokenBudget, +// thinkingLevelMap, samplingParams. ProviderSettingsSchema does not reject +// unknown keys, so they pass validation and are silently dropped. Warn when +// any are present so the ignore is never silent. +const IGNORED_PROVIDER_FIELDS: { + display: string; + path: (p: Record) => unknown; +}[] = [ + { + display: "supportsThinkingTokenBudget", + path: (p) => p.supportsThinkingTokenBudget, + }, + { + display: "thinkingLevelMap", + path: (p) => p.thinkingLevelMap, + }, + { + display: "samplingParams", + path: (p) => p.samplingParams, + }, + { + // Rendered as the nested arrow form so it is not mistaken for a bare + // provider-level key (the JSON nests it under the provider's "compat"). + display: "compat → supportsReasoningEffort", + path: (p) => { + const compat = p.compat as Record | undefined; + return compat?.supportsReasoningEffort; + }, + }, +]; + +function warnOnIgnoredProviderFields( + path: string, + providers: Record, +): void { + for (const [name, provider] of Object.entries(providers)) { + const p = provider as unknown as Record; + const present = IGNORED_PROVIDER_FIELDS.filter( + (field) => field.path(p) !== undefined, + ).map((field) => field.display); + if (present.length > 0) { + process.stderr.write( + `settings: ${path}: provider "${name}" sets fields that are not honored by Corbits and will be ignored: ${present.join(", ")}. These fields are not yet supported; configure effort/thinking via provider/model-level settings.\n`, + ); + } + } +} + function normalizeParsedSettings(path: string, parsed: unknown): Settings { if (!isSettings(parsed)) { throw settingsSchemaError(path); } const s = parsed as unknown as Record; + warnOnIgnoredProviderFields(path, parsed.providers); // These keys were removed when plugins moved to discovery; they are now // silently dropped on the next save. Warn so a user who relied on them knows // to re-enable the equivalent plugins in /plugins instead of losing the