Skip to content

Commit 95d52c3

Browse files
feat(config): warn on ignored provider fields (#1372)
1 parent 209929e commit 95d52c3

2 files changed

Lines changed: 158 additions & 0 deletions

File tree

‎src/config/settings.test.ts‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { loadConfig } from "./index.js";
1313
import { resetPricingMetadataRefreshForTests } from "../cost/pricing-metadata.js";
1414
import { setProviderContextWindowOverrides } from "../provider/context-window.js";
1515
import { withMockedHomedir } from "../../testkit/mock-module.js";
16+
import { captureStderr } from "../../testkit/capture-stderr.js";
1617
import { isSettings, loadSettings, saveGlobalSettings } from "./settings.js";
1718

1819
test.each([
@@ -680,3 +681,110 @@ test("--config composes with OAuth profile auth instead of suppressing it", asyn
680681
await rm(cwd, { recursive: true, force: true });
681682
}
682683
});
684+
685+
// CL-10226 (Part 1): some external tools' provider blocks carry fields Corbits
686+
// does not honor. They pass ProviderSettingsSchema (which stays fail-open on
687+
// unknown keys) and are silently dropped. The settings load path must warn on
688+
// stderr while still loading successfully.
689+
test("settings load warns and still loads when a provider carries ignored fields", async () => {
690+
const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-ignored-fields-"));
691+
const path = join(cwd, "settings.json");
692+
const stderr = captureStderr();
693+
try {
694+
await writeFile(
695+
path,
696+
JSON.stringify({
697+
providers: {
698+
external: {
699+
baseURL: "https://external.example/v1",
700+
keyless: true,
701+
models: ["test-model"],
702+
supportsThinkingTokenBudget: true,
703+
thinkingLevelMap: { low: 1 },
704+
samplingParams: { temperature: 0.5 },
705+
compat: { supportsReasoningEffort: true },
706+
},
707+
},
708+
}),
709+
);
710+
const loaded = await loadSettings(path);
711+
expect(loaded).not.toBeNull();
712+
expect(loaded?.providers.external).toBeDefined();
713+
expect(stderr.output()).toContain(
714+
'"external" sets fields that are not honored by Corbits and will be ignored',
715+
);
716+
expect(stderr.output()).toContain("supportsThinkingTokenBudget");
717+
expect(stderr.output()).toContain("thinkingLevelMap");
718+
expect(stderr.output()).toContain("samplingParams");
719+
expect(stderr.output()).toContain("compat → supportsReasoningEffort");
720+
expect(stderr.output()).toContain(
721+
"These fields are not yet supported; configure effort/thinking via provider/model-level settings.",
722+
);
723+
} finally {
724+
stderr.restore();
725+
await rm(cwd, { recursive: true, force: true });
726+
}
727+
});
728+
729+
test("settings load does not warn for a normal provider block", async () => {
730+
const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-no-ignored-"));
731+
const path = join(cwd, "settings.json");
732+
const stderr = captureStderr();
733+
try {
734+
await writeFile(
735+
path,
736+
JSON.stringify({
737+
providers: {
738+
external: {
739+
baseURL: "https://external.example/v1",
740+
keyless: true,
741+
models: ["test-model"],
742+
},
743+
},
744+
}),
745+
);
746+
const loaded = await loadSettings(path);
747+
expect(loaded).not.toBeNull();
748+
expect(stderr.output()).not.toContain("not honored by Corbits");
749+
expect(stderr.output()).not.toContain("supportsThinkingTokenBudget");
750+
} finally {
751+
stderr.restore();
752+
await rm(cwd, { recursive: true, force: true });
753+
}
754+
});
755+
756+
// The same warn must surface on the config path (--config / global settings),
757+
// which routes through loadSettings... → normalizeParsedSettings.
758+
test("loadConfig warns and still loads when a provider carries ignored fields", async () => {
759+
const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-ignored-config-"));
760+
const globalSettingsPath = join(cwd, "global.json");
761+
const stderr = captureStderr();
762+
try {
763+
await writeFile(
764+
globalSettingsPath,
765+
JSON.stringify({
766+
defaultProvider: "external",
767+
providers: {
768+
external: {
769+
baseURL: "https://external.example/v1",
770+
apiKey: "test-key",
771+
models: ["test-model"],
772+
samplingParams: { temperature: 0.5 },
773+
},
774+
},
775+
}),
776+
);
777+
const { impl } = offlineFetch();
778+
const config = await loadConfig(["--cwd", cwd, "do something"], {
779+
globalSettingsPath,
780+
pricing: { fetchImpl: impl },
781+
});
782+
expect(config.configured).toBe(true);
783+
if (config.configured) expect(config.providerName).toBe("external");
784+
expect(stderr.output()).toContain("not honored by Corbits");
785+
expect(stderr.output()).toContain("samplingParams");
786+
} finally {
787+
stderr.restore();
788+
await rm(cwd, { recursive: true, force: true });
789+
}
790+
});

‎src/config/settings.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,11 +1036,61 @@ function settingsSchemaError(path: string): Error {
10361036
);
10371037
}
10381038

1039+
// Provider blocks from some external tools carry fields Corbits does not
1040+
// honor: compat.supportsReasoningEffort, supportsThinkingTokenBudget,
1041+
// thinkingLevelMap, samplingParams. ProviderSettingsSchema does not reject
1042+
// unknown keys, so they pass validation and are silently dropped. Warn when
1043+
// any are present so the ignore is never silent.
1044+
const IGNORED_PROVIDER_FIELDS: {
1045+
display: string;
1046+
path: (p: Record<string, unknown>) => unknown;
1047+
}[] = [
1048+
{
1049+
display: "supportsThinkingTokenBudget",
1050+
path: (p) => p.supportsThinkingTokenBudget,
1051+
},
1052+
{
1053+
display: "thinkingLevelMap",
1054+
path: (p) => p.thinkingLevelMap,
1055+
},
1056+
{
1057+
display: "samplingParams",
1058+
path: (p) => p.samplingParams,
1059+
},
1060+
{
1061+
// Rendered as the nested arrow form so it is not mistaken for a bare
1062+
// provider-level key (the JSON nests it under the provider's "compat").
1063+
display: "compat → supportsReasoningEffort",
1064+
path: (p) => {
1065+
const compat = p.compat as Record<string, unknown> | undefined;
1066+
return compat?.supportsReasoningEffort;
1067+
},
1068+
},
1069+
];
1070+
1071+
function warnOnIgnoredProviderFields(
1072+
path: string,
1073+
providers: Record<string, ProviderSettings>,
1074+
): void {
1075+
for (const [name, provider] of Object.entries(providers)) {
1076+
const p = provider as unknown as Record<string, unknown>;
1077+
const present = IGNORED_PROVIDER_FIELDS.filter(
1078+
(field) => field.path(p) !== undefined,
1079+
).map((field) => field.display);
1080+
if (present.length > 0) {
1081+
process.stderr.write(
1082+
`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`,
1083+
);
1084+
}
1085+
}
1086+
}
1087+
10391088
function normalizeParsedSettings(path: string, parsed: unknown): Settings {
10401089
if (!isSettings(parsed)) {
10411090
throw settingsSchemaError(path);
10421091
}
10431092
const s = parsed as unknown as Record<string, unknown>;
1093+
warnOnIgnoredProviderFields(path, parsed.providers);
10441094
// These keys were removed when plugins moved to discovery; they are now
10451095
// silently dropped on the next save. Warn so a user who relied on them knows
10461096
// to re-enable the equivalent plugins in /plugins instead of losing the

0 commit comments

Comments
 (0)