Skip to content

Commit 88a3a2a

Browse files
author
Peter Steinberger
committed
fix(doctor): preserve colliding retired model settings
1 parent ac1bd12 commit 88a3a2a

2 files changed

Lines changed: 217 additions & 116 deletions

File tree

‎src/commands/doctor/shared/legacy-config-migrate.test.ts‎

Lines changed: 139 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2463,134 +2463,204 @@ describe("legacy model compat migrate", () => {
24632463
]);
24642464
});
24652465

2466-
it("merges colliding model ref map keys instead of dropping per-key tuning", () => {
2466+
it("deep-merges colliding retired model refs and reports only unequal fields", () => {
24672467
const res = migrateLegacyConfigForTest({
24682468
agents: {
24692469
defaults: {
24702470
models: {
2471-
"openai/gpt-4o": { params: { reasoning_effort: "high" }, streaming: false },
2472-
"openai/gpt-4": { params: { reasoning_effort: "low" }, alias: "legacy-four" },
2471+
"openai/gpt-4o": {
2472+
params: {
2473+
reasoning: { effort: "high", budget: 100 },
2474+
tags: ["stable"],
2475+
},
2476+
streaming: false,
2477+
},
2478+
"openai/gpt-4": {
2479+
params: {
2480+
reasoning: { effort: "low", summary: "auto" },
2481+
tags: ["stable"],
2482+
},
2483+
alias: "legacy-four",
2484+
},
24732485
},
24742486
},
24752487
},
24762488
});
24772489

24782490
expect(res.config?.agents?.defaults?.models).toEqual({
24792491
"openai/gpt-5.5": {
2480-
params: { reasoning_effort: "high" },
2492+
params: {
2493+
reasoning: { effort: "high", budget: 100, summary: "auto" },
2494+
tags: ["stable"],
2495+
},
24812496
streaming: false,
24822497
alias: "legacy-four",
24832498
},
24842499
});
2485-
const mergeChanges = res.changes.filter((change) =>
2486-
change.includes("config.agents.defaults.models key"),
2487-
);
2488-
expect(
2489-
mergeChanges.some(
2490-
(change) =>
2491-
change.includes("Merged") &&
2492-
change.includes('"openai/gpt-4"') &&
2493-
change.includes('"openai/gpt-5.5"') &&
2494-
change.includes("conflicting fields") &&
2495-
change.includes("params.reasoning_effort"),
2496-
),
2497-
).toBe(true);
2500+
expect(res.changes.filter((change) => change.includes("Merged"))).toEqual([
2501+
'Merged config.agents.defaults.models key "openai/gpt-4" into "openai/gpt-5.5"; kept existing values for conflicting fields: params.reasoning.effort.',
2502+
]);
24982503
});
24992504

2500-
it("merges when the canonical key follows the retired key", () => {
2505+
it("does not report conflicts between model refs that normalize to the same value", () => {
25012506
const res = migrateLegacyConfigForTest({
25022507
agents: {
25032508
defaults: {
25042509
models: {
2505-
"openai/gpt-4": { alias: "legacy-four" },
2506-
"openai/gpt-5.5": { streaming: false },
2510+
"openai/gpt-5.5": { params: { fallbacks: ["openai/gpt-4"] } },
2511+
"openai/gpt-4o": { params: { fallbacks: ["openai/gpt-5.5"] } },
25072512
},
25082513
},
25092514
},
25102515
});
25112516

25122517
expect(res.config?.agents?.defaults?.models).toEqual({
2513-
"openai/gpt-5.5": {
2514-
alias: "legacy-four",
2515-
streaming: false,
2516-
},
2518+
"openai/gpt-5.5": { params: { fallbacks: ["openai/gpt-5.5"] } },
25172519
});
2520+
expect(res.changes.filter((change) => change.includes("Merged"))).toEqual([
2521+
'Merged config.agents.defaults.models key "openai/gpt-4o" into "openai/gpt-5.5".',
2522+
]);
25182523
});
25192524

2520-
it("keeps canonical values when canonical and retired keys collide", () => {
2525+
it.each(["first", "last"] as const)(
2526+
"keeps canonical values when the canonical key appears %s",
2527+
(canonicalPosition) => {
2528+
const canonical = [
2529+
"openai/gpt-5.5",
2530+
{
2531+
alias: "canonical-five",
2532+
params: { reasoning: { effort: "medium", canonicalOnly: true } },
2533+
agentRuntime: { id: "codex" },
2534+
},
2535+
] as const;
2536+
const retired = [
2537+
[
2538+
"openai/gpt-4",
2539+
{
2540+
alias: "legacy-four",
2541+
params: { reasoning: { effort: "low", fourOnly: true } },
2542+
},
2543+
],
2544+
[
2545+
"openai/gpt-4o",
2546+
{
2547+
streaming: false,
2548+
params: { reasoning: { effort: "high", fourOOnly: true } },
2549+
},
2550+
],
2551+
] as const;
2552+
const entries =
2553+
canonicalPosition === "first" ? [canonical, ...retired] : [...retired, canonical];
2554+
const res = migrateLegacyConfigForTest({
2555+
agents: { defaults: { models: Object.fromEntries(entries) } },
2556+
});
2557+
2558+
expect(res.config?.agents?.defaults?.models).toEqual({
2559+
"openai/gpt-5.5": {
2560+
alias: "canonical-five",
2561+
params: {
2562+
reasoning: {
2563+
effort: "medium",
2564+
canonicalOnly: true,
2565+
fourOnly: true,
2566+
fourOOnly: true,
2567+
},
2568+
},
2569+
agentRuntime: { id: "codex" },
2570+
streaming: false,
2571+
},
2572+
});
2573+
expect(res.changes.filter((change) => change.includes("Merged"))).toEqual([
2574+
'Merged config.agents.defaults.models key "openai/gpt-4" into "openai/gpt-5.5"; kept existing values for conflicting fields: alias, params.reasoning.effort.',
2575+
'Merged config.agents.defaults.models key "openai/gpt-4o" into "openai/gpt-5.5"; kept existing values for conflicting fields: params.reasoning.effort.',
2576+
]);
2577+
},
2578+
);
2579+
2580+
it("merges colliding model refs in per-agent model maps", () => {
25212581
const res = migrateLegacyConfigForTest({
25222582
agents: {
2523-
defaults: {
2524-
models: {
2525-
"openai/gpt-4": { alias: "legacy-four", streaming: true },
2526-
"openai/gpt-5.5": { alias: "current-five" },
2583+
list: [
2584+
{
2585+
id: "research",
2586+
models: {
2587+
"openai/gpt-4": { alias: "legacy-four" },
2588+
"openai/gpt-4o": { streaming: false },
2589+
},
25272590
},
2528-
},
2591+
],
25292592
},
25302593
});
25312594

2532-
expect(res.config?.agents?.defaults?.models).toEqual({
2533-
"openai/gpt-5.5": {
2534-
alias: "current-five",
2535-
streaming: true,
2536-
},
2595+
expect(res.config?.agents?.list?.[0]?.models).toEqual({
2596+
"openai/gpt-5.5": { alias: "legacy-four", streaming: false },
25372597
});
2598+
expect(res.changes.filter((change) => change.includes("Merged"))).toEqual([
2599+
'Merged config.agents.list.0.models key "openai/gpt-4o" into "openai/gpt-5.5".',
2600+
]);
25382601
});
25392602

2540-
it("does not pollute prototypes when merging colliding model ref map keys", () => {
2541-
const polluted = JSON.parse(
2542-
'{"agents":{"defaults":{"models":{"openai/gpt-4o":{"streaming":false},"openai/gpt-4":{"__proto__":{"polluted":true},"alias":"legacy-four"}}}}}',
2543-
);
2544-
const res = migrateLegacyConfigForTest(polluted);
2545-
2546-
expect(({} as Record<string, unknown>).polluted).toBeUndefined();
2603+
it.each([
2604+
{
2605+
side: "canonical",
2606+
raw: '{"agents":{"defaults":{"models":{"openai/gpt-5.5":{"__proto__":{"polluted":true},"alias":"canonical-five","params":{"nested":{"__proto__":{"polluted":true},"model":"openai/gpt-4o"}}},"openai/gpt-4":{"streaming":false}}}}}',
2607+
},
2608+
{
2609+
side: "retired",
2610+
raw: '{"agents":{"defaults":{"models":{"openai/gpt-5.5":{"alias":"canonical-five"},"openai/gpt-4":{"__proto__":{"polluted":true},"streaming":false,"params":{"nested":{"__proto__":{"polluted":true},"model":"openai/gpt-4o"}}}}}}}',
2611+
},
2612+
])("filters blocked keys recursively from the $side collision side", ({ raw }) => {
2613+
const res = migrateLegacyConfigForTest(JSON.parse(raw));
25472614
const merged = res.config?.agents?.defaults?.models?.["openai/gpt-5.5"] as Record<
25482615
string,
25492616
unknown
25502617
>;
2551-
expect(Object.getPrototypeOf(merged)).toBe(Object.prototype);
2552-
expect(merged.polluted).toBeUndefined();
2553-
expect(merged).toEqual({ streaming: false, alias: "legacy-four" });
2554-
});
2555-
2556-
it("does not pollute prototypes when the first colliding entry carries a blocked key", () => {
2557-
const polluted = JSON.parse(
2558-
'{"agents":{"defaults":{"models":{"openai/gpt-4o":{"__proto__":{"polluted":true},"fallback":["openai/gpt-4o"],"streaming":false},"openai/gpt-4":{"alias":"legacy-four"}}}}}',
2559-
);
2560-
const res = migrateLegacyConfigForTest(polluted);
2618+
const params = merged.params as Record<string, unknown>;
2619+
const nested = params.nested as Record<string, unknown>;
25612620

2621+
for (const record of [merged, params, nested]) {
2622+
expect(Object.getOwnPropertyNames(record)).not.toContain("__proto__");
2623+
expect(Object.getPrototypeOf(record)).toBe(Object.prototype);
2624+
expect(record.polluted).toBeUndefined();
2625+
}
25622626
expect(({} as Record<string, unknown>).polluted).toBeUndefined();
2563-
const merged = res.config?.agents?.defaults?.models?.["openai/gpt-5.5"] as Record<
2564-
string,
2565-
unknown
2566-
>;
2567-
expect(Object.getOwnPropertyNames(merged)).not.toContain("__proto__");
2568-
expect(Object.getPrototypeOf(merged)).toBe(Object.prototype);
2569-
expect(merged.polluted).toBeUndefined();
25702627
expect(merged).toEqual({
2571-
fallback: ["openai/gpt-5.5"],
2628+
alias: "canonical-five",
2629+
params: { nested: { model: "openai/gpt-5.5" } },
25722630
streaming: false,
2573-
alias: "legacy-four",
25742631
});
25752632
});
25762633

2577-
it("names the retired source key when the canonical key follows the retired key", () => {
2634+
it("does not invoke prototype setters when copying the rewritten config root", () => {
2635+
const raw = JSON.parse(
2636+
'{"__proto__":{"polluted":true},"agents":{"defaults":{"model":"openai/gpt-4"}}}',
2637+
) as Record<string, unknown>;
2638+
const res = migrateLegacyConfigForTest(raw);
2639+
const config = res.config as unknown as Record<string, unknown>;
2640+
2641+
expect(Object.getPrototypeOf(config)).toBe(Object.prototype);
2642+
expect(Object.hasOwn(config, "__proto__")).toBe(true);
2643+
expect(config.polluted).toBeUndefined();
2644+
expect(res.config?.agents?.defaults?.model).toBe("openai/gpt-5.5");
2645+
});
2646+
2647+
it("reports malformed scalar collisions without claiming equal values conflict", () => {
25782648
const res = migrateLegacyConfigForTest({
25792649
agents: {
25802650
defaults: {
25812651
models: {
2582-
"openai/gpt-4": { alias: "legacy-four" },
2583-
"openai/gpt-5.5": { streaming: false },
2652+
"openai/gpt-5.5": false,
2653+
"openai/gpt-4": true,
2654+
"openai/gpt-4o": false,
25842655
},
25852656
},
25862657
},
25872658
});
25882659

2589-
const mergeChanges = res.changes.filter(
2590-
(change) => change.includes("config.agents.defaults.models key") && change.includes("Merged"),
2591-
);
2592-
expect(mergeChanges).toEqual([
2593-
'Merged config.agents.defaults.models key "openai/gpt-4" into "openai/gpt-5.5".',
2660+
expect(res.config?.agents?.defaults?.models?.["openai/gpt-5.5"]).toBe(false);
2661+
expect(res.changes.filter((change) => change.includes("Merged"))).toEqual([
2662+
'Merged config.agents.defaults.models key "openai/gpt-4" into "openai/gpt-5.5"; kept existing values for conflicting fields: value.',
2663+
'Merged config.agents.defaults.models key "openai/gpt-4o" into "openai/gpt-5.5".',
25942664
]);
25952665
});
25962666

0 commit comments

Comments
 (0)