diff --git a/src/commands/doctor/shared/legacy-config-migrate.test.ts b/src/commands/doctor/shared/legacy-config-migrate.test.ts index fe6ce1e76d7..2e79239a43b 100644 --- a/src/commands/doctor/shared/legacy-config-migrate.test.ts +++ b/src/commands/doctor/shared/legacy-config-migrate.test.ts @@ -2463,6 +2463,207 @@ describe("legacy model compat migrate", () => { ]); }); + it("deep-merges colliding retired model refs and reports only unequal fields", () => { + const res = migrateLegacyConfigForTest({ + agents: { + defaults: { + models: { + "openai/gpt-4o": { + params: { + reasoning: { effort: "high", budget: 100 }, + tags: ["stable"], + }, + streaming: false, + }, + "openai/gpt-4": { + params: { + reasoning: { effort: "low", summary: "auto" }, + tags: ["stable"], + }, + alias: "legacy-four", + }, + }, + }, + }, + }); + + expect(res.config?.agents?.defaults?.models).toEqual({ + "openai/gpt-5.5": { + params: { + reasoning: { effort: "high", budget: 100, summary: "auto" }, + tags: ["stable"], + }, + streaming: false, + alias: "legacy-four", + }, + }); + expect(res.changes.filter((change) => change.includes("Merged"))).toEqual([ + 'Merged config.agents.defaults.models key "openai/gpt-4" into "openai/gpt-5.5"; kept existing values for conflicting fields: params.reasoning.effort.', + ]); + }); + + it("does not report conflicts between model refs that normalize to the same value", () => { + const res = migrateLegacyConfigForTest({ + agents: { + defaults: { + models: { + "openai/gpt-5.5": { params: { fallbacks: ["openai/gpt-4"] } }, + "openai/gpt-4o": { params: { fallbacks: ["openai/gpt-5.5"] } }, + }, + }, + }, + }); + + expect(res.config?.agents?.defaults?.models).toEqual({ + "openai/gpt-5.5": { params: { fallbacks: ["openai/gpt-5.5"] } }, + }); + expect(res.changes.filter((change) => change.includes("Merged"))).toEqual([ + 'Merged config.agents.defaults.models key "openai/gpt-4o" into "openai/gpt-5.5".', + ]); + }); + + it.each(["first", "last"] as const)( + "keeps canonical values when the canonical key appears %s", + (canonicalPosition) => { + const canonical = [ + "openai/gpt-5.5", + { + alias: "canonical-five", + params: { reasoning: { effort: "medium", canonicalOnly: true } }, + agentRuntime: { id: "codex" }, + }, + ] as const; + const retired = [ + [ + "openai/gpt-4", + { + alias: "legacy-four", + params: { reasoning: { effort: "low", fourOnly: true } }, + }, + ], + [ + "openai/gpt-4o", + { + streaming: false, + params: { reasoning: { effort: "high", fourOOnly: true } }, + }, + ], + ] as const; + const entries = + canonicalPosition === "first" ? [canonical, ...retired] : [...retired, canonical]; + const res = migrateLegacyConfigForTest({ + agents: { defaults: { models: Object.fromEntries(entries) } }, + }); + + expect(res.config?.agents?.defaults?.models).toEqual({ + "openai/gpt-5.5": { + alias: "canonical-five", + params: { + reasoning: { + effort: "medium", + canonicalOnly: true, + fourOnly: true, + fourOOnly: true, + }, + }, + agentRuntime: { id: "codex" }, + streaming: false, + }, + }); + expect(res.changes.filter((change) => change.includes("Merged"))).toEqual([ + 'Merged config.agents.defaults.models key "openai/gpt-4" into "openai/gpt-5.5"; kept existing values for conflicting fields: alias, params.reasoning.effort.', + 'Merged config.agents.defaults.models key "openai/gpt-4o" into "openai/gpt-5.5"; kept existing values for conflicting fields: params.reasoning.effort.', + ]); + }, + ); + + it("merges colliding model refs in per-agent model maps", () => { + const res = migrateLegacyConfigForTest({ + agents: { + list: [ + { + id: "research", + models: { + "openai/gpt-4": { alias: "legacy-four" }, + "openai/gpt-4o": { streaming: false }, + }, + }, + ], + }, + }); + + expect(res.config?.agents?.list?.[0]?.models).toEqual({ + "openai/gpt-5.5": { alias: "legacy-four", streaming: false }, + }); + expect(res.changes.filter((change) => change.includes("Merged"))).toEqual([ + 'Merged config.agents.list.0.models key "openai/gpt-4o" into "openai/gpt-5.5".', + ]); + }); + + it.each([ + { + side: "canonical", + 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}}}}}', + }, + { + side: "retired", + 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"}}}}}}}', + }, + ])("filters blocked keys recursively from the $side collision side", ({ raw }) => { + const res = migrateLegacyConfigForTest(JSON.parse(raw)); + const merged = res.config?.agents?.defaults?.models?.["openai/gpt-5.5"] as Record< + string, + unknown + >; + const params = merged.params as Record; + const nested = params.nested as Record; + + for (const record of [merged, params, nested]) { + expect(Object.getOwnPropertyNames(record)).not.toContain("__proto__"); + expect(Object.getPrototypeOf(record)).toBe(Object.prototype); + expect(record.polluted).toBeUndefined(); + } + expect(({} as Record).polluted).toBeUndefined(); + expect(merged).toEqual({ + alias: "canonical-five", + params: { nested: { model: "openai/gpt-5.5" } }, + streaming: false, + }); + }); + + it("does not invoke prototype setters when copying the rewritten config root", () => { + const raw = JSON.parse( + '{"__proto__":{"polluted":true},"agents":{"defaults":{"model":"openai/gpt-4"}}}', + ) as Record; + const res = migrateLegacyConfigForTest(raw); + const config = res.config as unknown as Record; + + expect(Object.getPrototypeOf(config)).toBe(Object.prototype); + expect(Object.hasOwn(config, "__proto__")).toBe(true); + expect(config.polluted).toBeUndefined(); + expect(res.config?.agents?.defaults?.model).toBe("openai/gpt-5.5"); + }); + + it("reports malformed scalar collisions without claiming equal values conflict", () => { + const res = migrateLegacyConfigForTest({ + agents: { + defaults: { + models: { + "openai/gpt-5.5": false, + "openai/gpt-4": true, + "openai/gpt-4o": false, + }, + }, + }, + }); + + expect(res.config?.agents?.defaults?.models?.["openai/gpt-5.5"]).toBe(false); + expect(res.changes.filter((change) => change.includes("Merged"))).toEqual([ + 'Merged config.agents.defaults.models key "openai/gpt-4" into "openai/gpt-5.5"; kept existing values for conflicting fields: value.', + 'Merged config.agents.defaults.models key "openai/gpt-4o" into "openai/gpt-5.5".', + ]); + }); + it("removes unrecognized model compat thinkingFormat values", () => { const res = migrateLegacyConfigForTest({ models: { diff --git a/src/commands/doctor/shared/legacy-config-migrations.runtime.models.ts b/src/commands/doctor/shared/legacy-config-migrations.runtime.models.ts index 3df07fdbb24..558a326658e 100644 --- a/src/commands/doctor/shared/legacy-config-migrations.runtime.models.ts +++ b/src/commands/doctor/shared/legacy-config-migrations.runtime.models.ts @@ -1,4 +1,5 @@ // Legacy model runtime config migrations for stale model refs, compat fields, and catalog data. +import { isDeepStrictEqual } from "node:util"; import { normalizeProviderId } from "@openclaw/model-catalog-core/provider-id"; import { splitTrailingAuthProfile } from "../../../agents/model-ref-profile.js"; import { @@ -9,6 +10,7 @@ import { type LegacyConfigRule, } from "../../../config/legacy.shared.js"; import { isModelThinkingFormat, type ModelDefinitionConfig } from "../../../config/types.models.js"; +import { isBlockedObjectKey } from "../../../infra/prototype-keys.js"; import { isLegacyModelsAddCodexMetadataModel } from "./legacy-models-add-metadata.js"; const STALE_CONTEXT_WINDOW_FIXES: Record = { @@ -847,6 +849,86 @@ function rewriteModelRefString(value: string, path: string, changes: string[]): return upgraded; } +function setRecordEntry(record: Record, key: string, value: unknown): void { + // Config dictionaries can contain hostile keys; define own properties so + // rebuilding or copying them never invokes Object.prototype setters. + Object.defineProperty(record, key, { + configurable: true, + enumerable: true, + value, + writable: true, + }); +} + +function sanitizeModelRefMapEntry(value: unknown): unknown { + // Collisions combine both entries before recursive ref rewriting, so blocked + // keys must be removed at every depth on both sides of the merge. + if (Array.isArray(value)) { + return value.map(sanitizeModelRefMapEntry); + } + const record = getRecord(value); + if (!record) { + return value; + } + const sanitized: Record = {}; + for (const [field, child] of Object.entries(record)) { + if (!isBlockedObjectKey(field)) { + setRecordEntry(sanitized, field, sanitizeModelRefMapEntry(child)); + } + } + return sanitized; +} + +function modelRefValuesAreEqual(existing: unknown, incoming: unknown, path: string): boolean { + if (isDeepStrictEqual(existing, incoming)) { + return true; + } + const normalizedExisting = rewriteKnownModelRefs(existing, path, []).value; + const normalizedIncoming = rewriteKnownModelRefs(incoming, path, []).value; + return isDeepStrictEqual(normalizedExisting, normalizedIncoming); +} + +function mergeModelRefMapEntries( + existing: unknown, + incoming: unknown, + path: string, +): { value: unknown; conflicts: string[] } { + const existingRecord = getRecord(existing); + const incomingRecord = getRecord(incoming); + if (!existingRecord || !incomingRecord) { + return { + value: sanitizeModelRefMapEntry(existing), + conflicts: modelRefValuesAreEqual(existing, incoming, path) ? [] : ["value"], + }; + } + const merged = sanitizeModelRefMapEntry(existingRecord) as Record; + const conflicts: string[] = []; + for (const [field, incomingValue] of Object.entries(incomingRecord)) { + if (incomingValue === undefined || isBlockedObjectKey(field)) { + continue; + } + if (!hasOwnDefinedProperty(existingRecord, field)) { + setRecordEntry(merged, field, sanitizeModelRefMapEntry(incomingValue)); + continue; + } + const existingValue = existingRecord[field]; + const fieldPath = `${path}.${field}`; + if (modelRefValuesAreEqual(existingValue, incomingValue, fieldPath)) { + continue; + } + const existingField = getRecord(existingValue); + const incomingField = getRecord(incomingValue); + if (existingField && incomingField) { + const nested = mergeModelRefMapEntries(existingField, incomingField, fieldPath); + setRecordEntry(merged, field, nested.value); + conflicts.push(...nested.conflicts.map((c) => `${field}.${c}`)); + continue; + } + conflicts.push(field); + } + return { value: merged, conflicts }; +} + function rewriteModelRefMapKeys( record: Record, path: string, @@ -854,19 +936,40 @@ function rewriteModelRefMapKeys( ): { value: Record; changed: boolean } { let changed = false; const next: Record = {}; + const consumedCanonicalKeys = new Set(); for (const [key, child] of Object.entries(record)) { const upgradedKey = upgradeRetiredModelRef(key); const nextKey = upgradedKey ?? key; + if (!upgradedKey && consumedCanonicalKeys.has(key)) { + continue; + } if (upgradedKey) { changes.push( `Upgraded ${path} key from ${JSON.stringify(key)} to ${JSON.stringify(upgradedKey)}.`, ); changed = true; } - if (nextKey in next && upgradedKey) { + if (upgradedKey && !Object.hasOwn(next, nextKey) && Object.hasOwn(record, nextKey)) { + // Seed the canonical entry before its retired aliases so canonical conflict + // precedence and per-alias change reporting do not depend on authored key order. + setRecordEntry(next, nextKey, record[nextKey]); + consumedCanonicalKeys.add(nextKey); + } + if (Object.hasOwn(next, nextKey)) { + const existing = next[nextKey]; + const { value, conflicts } = mergeModelRefMapEntries(existing, child, `${path}.${nextKey}`); + setRecordEntry(next, nextKey, value); + const sortedConflicts = conflicts.toSorted(); + if (sortedConflicts.length > 0) { + changes.push( + `Merged ${path} key ${JSON.stringify(key)} into ${JSON.stringify(nextKey)}; kept existing values for conflicting fields: ${sortedConflicts.join(", ")}.`, + ); + } else { + changes.push(`Merged ${path} key ${JSON.stringify(key)} into ${JSON.stringify(nextKey)}.`); + } continue; } - next[nextKey] = child; + setRecordEntry(next, nextKey, child); } return { value: changed ? next : record, changed }; } @@ -915,7 +1018,7 @@ function rewriteKnownModelRefs( for (const [childKey, child] of Object.entries(working)) { const rewritten = rewriteKnownModelRefs(child, `${path}.${childKey}`, changes); changed ||= rewritten.changed; - next[childKey] = rewritten.value; + setRecordEntry(next, childKey, rewritten.value); } return { value: changed ? next : value, changed }; } @@ -1333,13 +1436,16 @@ export const LEGACY_CONFIG_MIGRATIONS_RUNTIME_MODELS: LegacyConfigMigrationSpec[ legacyRules: RETIRED_MODEL_REF_RULES, apply: (raw, changes) => { const rewritten = rewriteKnownModelRefs(raw, "config", changes); - if (!rewritten.changed || !getRecord(rewritten.value)) { + const rewrittenRecord = getRecord(rewritten.value); + if (!rewritten.changed || !rewrittenRecord) { return; } for (const key of Object.keys(raw)) { delete raw[key]; } - Object.assign(raw, rewritten.value); + for (const [key, value] of Object.entries(rewrittenRecord)) { + setRecordEntry(raw, key, value); + } }, }), defineLegacyConfigMigration({