Skip to content

Commit 23eeb79

Browse files
committed
feat(config): warn on ignored provider fields
1 parent 33a8605 commit 23eeb79

2 files changed

Lines changed: 158 additions & 0 deletions

File tree

‎src/config/settings.test.ts‎

Lines changed: 105 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,107 @@ 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 are 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+
} finally {
721+
stderr.restore();
722+
await rm(cwd, { recursive: true, force: true });
723+
}
724+
});
725+
726+
test("settings load does not warn for a normal provider block", async () => {
727+
const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-no-ignored-"));
728+
const path = join(cwd, "settings.json");
729+
const stderr = captureStderr();
730+
try {
731+
await writeFile(
732+
path,
733+
JSON.stringify({
734+
providers: {
735+
external: {
736+
baseURL: "https://external.example/v1",
737+
keyless: true,
738+
models: ["test-model"],
739+
},
740+
},
741+
}),
742+
);
743+
const loaded = await loadSettings(path);
744+
expect(loaded).not.toBeNull();
745+
expect(stderr.output()).not.toContain("not honored by Corbits");
746+
expect(stderr.output()).not.toContain("supportsThinkingTokenBudget");
747+
} finally {
748+
stderr.restore();
749+
await rm(cwd, { recursive: true, force: true });
750+
}
751+
});
752+
753+
// CL-10226 (Part 1): the same warn must surface on the config path (--config /
754+
// global settings), which routes through loadSettings... → normalizeParsedSettings.
755+
test("loadConfig warns and still loads when a provider carries ignored fields", async () => {
756+
const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-ignored-config-"));
757+
const globalSettingsPath = join(cwd, "global.json");
758+
const stderr = captureStderr();
759+
try {
760+
await writeFile(
761+
globalSettingsPath,
762+
JSON.stringify({
763+
defaultProvider: "external",
764+
providers: {
765+
external: {
766+
baseURL: "https://external.example/v1",
767+
apiKey: "test-key",
768+
models: ["test-model"],
769+
samplingParams: { temperature: 0.5 },
770+
},
771+
},
772+
}),
773+
);
774+
const { impl } = offlineFetch();
775+
const config = await loadConfig(["--cwd", cwd, "do something"], {
776+
globalSettingsPath,
777+
pricing: { fetchImpl: impl },
778+
});
779+
expect(config.configured).toBe(true);
780+
if (config.configured) expect(config.providerName).toBe("external");
781+
expect(stderr.output()).toContain("not honored by Corbits");
782+
expect(stderr.output()).toContain("samplingParams");
783+
} finally {
784+
stderr.restore();
785+
await rm(cwd, { recursive: true, force: true });
786+
}
787+
});

‎src/config/settings.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,11 +1036,64 @@ 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. CL-10226 (Part 1): the field
1044+
// mappings themselves are separate follow-ups — this only surfaces the warn.
1045+
const IGNORED_PROVIDER_FIELDS: {
1046+
key: string;
1047+
path: (p: Record<string, unknown>) => unknown;
1048+
}[] = [
1049+
{
1050+
key: "supportsThinkingTokenBudget",
1051+
path: (p) => p.supportsThinkingTokenBudget,
1052+
},
1053+
{ key: "thinkingLevelMap", path: (p) => p.thinkingLevelMap },
1054+
{ key: "samplingParams", path: (p) => p.samplingParams },
1055+
{
1056+
key: "compat.supportsReasoningEffort",
1057+
path: (p) =>
1058+
typeof p.compat === "object" &&
1059+
p.compat !== null &&
1060+
(p.compat as Record<string, unknown>).supportsReasoningEffort !==
1061+
undefined
1062+
? (p.compat as Record<string, unknown>).supportsReasoningEffort
1063+
: undefined,
1064+
},
1065+
];
1066+
1067+
function warnOnIgnoredProviderFields(providers: unknown): void {
1068+
if (
1069+
providers === undefined ||
1070+
typeof providers !== "object" ||
1071+
providers === null
1072+
) {
1073+
return;
1074+
}
1075+
for (const [name, provider] of Object.entries(
1076+
providers as Record<string, unknown>,
1077+
)) {
1078+
if (typeof provider !== "object" || provider === null) continue;
1079+
const p = provider as Record<string, unknown>;
1080+
const present = IGNORED_PROVIDER_FIELDS.filter(
1081+
(field) => field.path(p) !== undefined,
1082+
).map((field) => field.key);
1083+
if (present.length > 0) {
1084+
process.stderr.write(
1085+
`settings: provider "${name}" sets fields that are not honored by Corbits and are ignored: ${present.join(", ")}. These will not be applied.\n`,
1086+
);
1087+
}
1088+
}
1089+
}
1090+
10391091
function normalizeParsedSettings(path: string, parsed: unknown): Settings {
10401092
if (!isSettings(parsed)) {
10411093
throw settingsSchemaError(path);
10421094
}
10431095
const s = parsed as unknown as Record<string, unknown>;
1096+
warnOnIgnoredProviderFields(s.providers);
10441097
// These keys were removed when plugins moved to discovery; they are now
10451098
// silently dropped on the next save. Warn so a user who relied on them knows
10461099
// to re-enable the equivalent plugins in /plugins instead of losing the

0 commit comments

Comments
 (0)