From 1dfc1f303f953cff4835a404aab299871975223b Mon Sep 17 00:00:00 2001 From: JUN Date: Sat, 19 Sep 2026 03:04:37 +0900 Subject: [PATCH 1/2] fix(providers): stop the Antigravity rename migration re-announcing itself The rename migration and the OAuth preset reconciliation disagreed about nine retired Antigravity Flash ids, and startServer runs both, in that order. ANTIGRAVITY_MODEL_CONTEXT_WINDOWS derives a window for every compatibility alias, so the google-antigravity preset carries a modelContextWindows key for each retired id. The migration renamed those keys and persisted, applyOAuthPresetCatalog copied the preset's record back over them in the same boot, and the next start found them again: nine [model-rename-migration] lines on every ocx start, for a user who never selected any of those nine models. Skip a field the registry's own seed still publishes keyed by the retired id. Those entries are deliberate (a request naming gemini-3.6-flash routes to 3.7 and needs a window under the id it asked for), and reconciliation owns that record anyway, so the migration has nothing to repair there. Every field the user owns (models, selectedModels, retainModels, defaultModel, disabledModels) is still renamed and still reported once. Closes #5066 --- src/providers/model-rename-migration.ts | 47 +++++- .../providers/model-rename-migration.test.ts | 140 ++++++++++++++++++ 2 files changed, 186 insertions(+), 1 deletion(-) diff --git a/src/providers/model-rename-migration.ts b/src/providers/model-rename-migration.ts index 89c9b6f577e..764c51b9ff0 100644 --- a/src/providers/model-rename-migration.ts +++ b/src/providers/model-rename-migration.ts @@ -14,9 +14,12 @@ // This migration is deliberately NOT general reconciliation. It rewrites exactly // one thing: an id this file declares retired, on a provider that still carries // the registry's transport, and only when the registry currently seeds the -// replacement. Everything else in the row is left alone. +// replacement — and only in the fields the registry does not itself republish +// keyed by that retired id (see `registrySeededFields`, issue #5066). Everything +// else in the row is left alone. import { PROVIDER_REGISTRY } from "./registry"; +import { providerConfigSeed } from "./derive"; import type { OcxConfig, OcxProviderConfig } from "../types"; export interface ModelRename { @@ -194,6 +197,45 @@ function registrySeedsTarget(rename: ModelRename): boolean { return !!entry?.models?.includes(rename.to); } +/** + * The mirror guard: fields whose saved value the registry writes back with the retired id in it. + * + * `providerConfigSeed` is what both the OAuth preset (`deriveOAuthProviderConfig`) and + * `enrichProviderFromRegistry` contribute to a saved row. A field it publishes keyed by the + * retired id is therefore registry metadata that arrives in the config again on its own — not a + * stale user selection this migration can repair. + * + * Rewriting such a field is worse than a no-op, because it never converges. `startServer` runs + * this migration and then `reconcileOAuthProviders`, and `applyOAuthPresetCatalog` copies the + * preset's record over the saved one whenever the two differ. The rename is undone inside the + * same boot, the config is written twice, and the user is told about a rename that never sticks + * on every single start. Issue #5066 is exactly this loop: `ANTIGRAVITY_MODEL_CONTEXT_WINDOWS` + * derives a window for each alias, which gives `modelContextWindows` a key for all nine retired + * Flash ids — the nine `[model-rename-migration]` lines the reporter saw on every `ocx start`, + * none of which required them to have selected any of those nine models. + * + * Those registry entries are deliberate and stay: a request that still names `gemini-3.6-flash` + * routes to 3.7 and needs a context window under the id it was asked for + * (`tests/adapters/google/gemini-37-flash-migration.test.ts` pins that). So the contradiction is + * resolved on this side, where nothing is lost — the migration keeps repairing every field the + * user owns, and leaves the registry to own the metadata it republishes anyway. + */ +function registrySeededFields(rename: ModelRename): ReadonlySet { + const seeded = new Set(); + const entry = PROVIDER_REGISTRY.find(row => row.id === rename.provider); + if (!entry) return seeded; + const seed = providerConfigSeed(entry) as unknown as Record; + for (const field of [...MODEL_ID_LISTS, ...MODEL_KEYED_RECORDS] as readonly string[]) { + const value = seed[field]; + if (Array.isArray(value)) { + if (value.includes(rename.from)) seeded.add(field); + continue; + } + if (value && typeof value === "object" && rename.from in value) seeded.add(field); + } + return seeded; +} + export interface ModelRenameProjection { config: OcxConfig; changed: boolean; @@ -226,14 +268,17 @@ export function projectModelRenames( // Provider config is a closed interface, so index through one unknown-cast // view rather than casting at each assignment. const row = prov as unknown as Record; + const registrySeeded = registrySeededFields(rename); let touched = false; for (const field of MODEL_ID_LISTS) { + if (registrySeeded.has(field)) continue; const next = renameInList(row[field], rename.from, rename.to); if (!next) continue; row[field] = next; touched = true; } for (const field of MODEL_KEYED_RECORDS) { + if (registrySeeded.has(field)) continue; const next = rename.dropReasoningEffortMap && field === "modelReasoningEffortMap" ? dropFromRecord(row[field], rename.from) : renameInRecord(row[field], rename.from, rename.to); diff --git a/tests/providers/model-rename-migration.test.ts b/tests/providers/model-rename-migration.test.ts index 34801e8b21b..be52a3da6ab 100644 --- a/tests/providers/model-rename-migration.test.ts +++ b/tests/providers/model-rename-migration.test.ts @@ -9,6 +9,12 @@ import { } from "../../src/providers/model-rename-migration"; import { runModelRenameStartupMigration } from "../../src/providers/model-rename-startup"; import { PROVIDER_REGISTRY } from "../../src/providers/registry"; +import { providerConfigSeed } from "../../src/providers/derive"; +import { reconcileOAuthProviders } from "../../src/oauth"; +import { + ANTIGRAVITY_MODELS, + ANTIGRAVITY_MODEL_CONTEXT_WINDOWS, +} from "../../src/providers/antigravity-models"; import { getConfigPath, loadConfig, saveConfig, setPersistedConfigMutationBeforeCommitForTests } from "../../src/config"; import type { OcxConfig } from "../../src/types"; import { removeTreeWithRetry } from "../helpers/remove-tree"; @@ -276,3 +282,137 @@ describe("model rename startup persistence", () => { expect(live.providers.untouched).toBe(liveUntouched); }); }); + +// ── Issue #5066: the migration has to converge, not re-announce itself on every boot ── +// +// The reporter saw the same nine `[model-rename-migration]` lines at every `ocx start`. Nothing +// in the projection, the persistence or the in-memory fallback was broken; the migration was +// losing to the second half of its own boot. `startServer` runs it and then +// `reconcileOAuthProviders`, and the Antigravity OAuth preset carries +// `ANTIGRAVITY_MODEL_CONTEXT_WINDOWS`, which derives a window for every compatibility alias — +// all nine retired Flash ids among them. `applyOAuthPresetCatalog` copies that record over the +// saved one whenever the two differ, so reconciliation restored precisely the keys the rename +// had removed, and the next boot found them and said so again. +// +// It also explains the count: nine messages for a user who never chose nine models, because the +// ids came from the registry rather than from anything they had selected. +describe("registry-seeded metadata does not re-arm the rename (#5066)", () => { + const homes: string[] = []; + const originalHome = process.env.OPENCODEX_HOME; + + function isolate(prefix: string): void { + const home = mkdtempSync(join(tmpdir(), prefix)); + homes.push(home); + process.env.OPENCODEX_HOME = home; + } + + afterEach(() => { + if (originalHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = originalHome; + for (const home of homes.splice(0)) removeTreeWithRetry(home); + }); + + /** + * A saved Antigravity row carrying one genuinely stale user selection. + * + * `modelContextWindows` is deliberately absent. The point of the fixture is to watch + * reconciliation introduce it between the two boots rather than to assume it is there. + */ + function antigravityRow(): Record { + return { + adapter: "google", + baseUrl: "https://daily-cloudcode-pa.googleapis.com", + authMode: "oauth", + googleMode: "cloud-code-assist", + liveModels: true, + defaultModel: "gemini-3.8-flash", + models: [...ANTIGRAVITY_MODELS], + // Reconciliation does not carry `selectedModels`, so this one is the migration’s to fix + // — once. + selectedModels: ["gemini-3.6-flash-high"], + }; + } + + function antigravityConfig(): OcxConfig { + return { + port: 10100, + defaultProvider: "google-antigravity", + providers: { "google-antigravity": antigravityRow() }, + } as unknown as OcxConfig; + } + + /** The rename lines a boot prints, in order. */ + function renameLines(boot: () => void): string[] { + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + boot(); + return warn.mock.calls + .map(([first]) => (typeof first === "string" ? first : "")) + .filter(line => line.startsWith("[model-rename-migration] renamed ")); + } finally { + warn.mockRestore(); + } + } + + // RED on dev, at the last assertion: the second boot reprints nine lines, and so does the + // third, and every one after it. + test("the second boot after reconciliation renames nothing and prints nothing", () => { + isolate("ocx-antigravity-rename-loop-"); + saveConfig(antigravityConfig()); + + // Boot one, in startServer’s order: migrate, then reconcile the OAuth presets. + const first = renameLines(() => { + reconcileOAuthProviders(runModelRenameStartupMigration(loadConfig())); + }); + expect(first).toHaveLength(1); + expect(first.join("\n")).toContain("google-antigravity/gemini-3.6-flash-high"); + + const afterFirstBoot = loadConfig().providers["google-antigravity"]!; + // The writer between the two boots, caught in the act: reconciliation put the registry’s + // own record on disk, retired keys and all. The next boot must not read that as a repair + // the user is still owed. + expect(Object.keys(afterFirstBoot.modelContextWindows ?? {})).toContain("gemini-3.6-flash"); + // What the migration did own stayed fixed. + expect(afterFirstBoot.selectedModels).toEqual(["gemini-3.7-flash"]); + + const second = renameLines(() => { runModelRenameStartupMigration(loadConfig()); }); + expect(second).toEqual([]); + }); + + test("a registry-published id survives in metadata while a user selection is still repaired", () => { + const reconciled = { + ...antigravityRow(), + modelContextWindows: { ...ANTIGRAVITY_MODEL_CONTEXT_WINDOWS }, + }; + const config = { providers: { "google-antigravity": reconciled } } as unknown as OcxConfig; + + const { config: out, changed, warnings } = projectModelRenames(config); + const prov = out.providers["google-antigravity"]!; + + expect(changed).toBe(true); + expect(warnings).toHaveLength(1); + expect(prov.selectedModels).toEqual(["gemini-3.7-flash"]); + // Left alone on purpose: a request that still names the retired id routes to 3.7 and needs + // a window under the id it asked for, and reconciliation would rewrite this record anyway. + expect(prov.modelContextWindows?.["gemini-3.6-flash"]).toBe(1_048_576); + }); + + // The general form of the defect, applied to every rename this file ships: a provider row the + // registry itself produced must never be something the migration wants to rewrite. When it is, + // the two halves of the boot disagree forever and the user pays for it in log noise. + const seedCases: [string, ModelRename][] = MODEL_RENAMES.map( + rename => [`${rename.provider}/${rename.from}`, rename], + ); + test.each(seedCases)("the registry seed for %s is not something the migration rewrites", (_label, rename) => { + const entry = PROVIDER_REGISTRY.find(row => row.id === rename.provider); + expect(entry, `registry entry missing for ${rename.provider}`).toBeDefined(); + const seeded = { + providers: { [rename.provider]: providerConfigSeed(entry!) }, + } as unknown as OcxConfig; + + const { changed, warnings } = projectModelRenames(seeded, [rename]); + + expect(warnings).toEqual([]); + expect(changed).toBe(false); + }); +}); From 952bac63c125b0638813ffefb8f09c51f6e5c8e3 Mon Sep 17 00:00:00 2001 From: JUN Date: Sat, 19 Sep 2026 03:10:07 +0900 Subject: [PATCH 2/2] fix(providers): only skip registry residue, not a value still to carry The first cut skipped any field the registry seed published keyed by the retired id, which also skipped a row that saved only the retired key. That row needs the value carried onto the supported id, and dropping it blanks the record instead of moving it (test 4/4 caught it at gemini-37-flash-migration.test.ts:211). Require both conditions now: the registry seed still publishes the retired id in the field, AND the saved field already carries the supported id, so the rename would only delete a key reconciliation puts straight back. The reporter's post-reconcile row satisfies both for all nine ids; a row with just the retired key does not, and is still migrated and still reported. --- src/providers/model-rename-migration.ts | 83 ++++++++++++++----------- 1 file changed, 46 insertions(+), 37 deletions(-) diff --git a/src/providers/model-rename-migration.ts b/src/providers/model-rename-migration.ts index 764c51b9ff0..0606fe3dcd9 100644 --- a/src/providers/model-rename-migration.ts +++ b/src/providers/model-rename-migration.ts @@ -14,9 +14,9 @@ // This migration is deliberately NOT general reconciliation. It rewrites exactly // one thing: an id this file declares retired, on a provider that still carries // the registry's transport, and only when the registry currently seeds the -// replacement — and only in the fields the registry does not itself republish -// keyed by that retired id (see `registrySeededFields`, issue #5066). Everything -// else in the row is left alone. +// replacement — and it skips a field where the retired entry is registry residue +// with nothing left to carry forward (see `isRegistryResidue`, issue #5066). +// Everything else in the row is left alone. import { PROVIDER_REGISTRY } from "./registry"; import { providerConfigSeed } from "./derive"; @@ -197,43 +197,52 @@ function registrySeedsTarget(rename: ModelRename): boolean { return !!entry?.models?.includes(rename.to); } +/** The row the registry contributes to a saved config, or `undefined` when it dropped the id. */ +function registrySeed(provider: string): Record | undefined { + const entry = PROVIDER_REGISTRY.find(row => row.id === provider); + return entry ? (providerConfigSeed(entry) as unknown as Record) : undefined; +} + +/** Whether a saved list or model-keyed record names this id at all. */ +function publishes(container: unknown, id: string): boolean { + if (Array.isArray(container)) return container.includes(id); + if (!container || typeof container !== "object") return false; + return id in (container as Record); +} + /** - * The mirror guard: fields whose saved value the registry writes back with the retired id in it. + * Whether this field’s retired entry is registry residue rather than something to repair. * - * `providerConfigSeed` is what both the OAuth preset (`deriveOAuthProviderConfig`) and - * `enrichProviderFromRegistry` contribute to a saved row. A field it publishes keyed by the - * retired id is therefore registry metadata that arrives in the config again on its own — not a - * stale user selection this migration can repair. + * Two things have to be true. The registry’s own seed — `providerConfigSeed`, the row both the + * OAuth preset (`deriveOAuthProviderConfig`) and `enrichProviderFromRegistry` contribute — must + * still publish the retired id in this field, so the entry arrives in the config on its own. And + * the saved field must already carry the supported id, so the rename would only DELETE a key, + * with nothing to carry forward. * - * Rewriting such a field is worse than a no-op, because it never converges. `startServer` runs - * this migration and then `reconcileOAuthProviders`, and `applyOAuthPresetCatalog` copies the - * preset's record over the saved one whenever the two differ. The rename is undone inside the - * same boot, the config is written twice, and the user is told about a rename that never sticks - * on every single start. Issue #5066 is exactly this loop: `ANTIGRAVITY_MODEL_CONTEXT_WINDOWS` - * derives a window for each alias, which gives `modelContextWindows` a key for all nine retired - * Flash ids — the nine `[model-rename-migration]` lines the reporter saw on every `ocx start`, - * none of which required them to have selected any of those nine models. + * Under those two conditions the rewrite is worse than a no-op, because it never converges. + * `startServer` runs this migration and then `reconcileOAuthProviders`, and + * `applyOAuthPresetCatalog` copies the preset’s record over the saved one whenever the two + * differ. The deletion is undone inside the same boot, the config is written twice, and the user + * is told about a rename that never sticks on every single start. Issue #5066 is exactly that: + * `ANTIGRAVITY_MODEL_CONTEXT_WINDOWS` derives a window for every compatibility alias, so + * `modelContextWindows` holds a key for all nine retired Flash ids beside the live ones — the + * nine `[model-rename-migration]` lines the reporter saw at every `ocx start`, none of which + * required them to have selected any of those nine models. * * Those registry entries are deliberate and stay: a request that still names `gemini-3.6-flash` - * routes to 3.7 and needs a context window under the id it was asked for - * (`tests/adapters/google/gemini-37-flash-migration.test.ts` pins that). So the contradiction is - * resolved on this side, where nothing is lost — the migration keeps repairing every field the - * user owns, and leaves the registry to own the metadata it republishes anyway. + * routes to 3.7 and needs a context window under the id it asked for. The second condition is + * what keeps this narrow. A row that saved `{ "gemini-3.6-flash": 1048576 }` and nothing else + * still gets the value carried onto the supported id, because dropping it there would blank the + * record instead of moving it (`tests/adapters/google/gemini-37-flash-migration.test.ts`). */ -function registrySeededFields(rename: ModelRename): ReadonlySet { - const seeded = new Set(); - const entry = PROVIDER_REGISTRY.find(row => row.id === rename.provider); - if (!entry) return seeded; - const seed = providerConfigSeed(entry) as unknown as Record; - for (const field of [...MODEL_ID_LISTS, ...MODEL_KEYED_RECORDS] as readonly string[]) { - const value = seed[field]; - if (Array.isArray(value)) { - if (value.includes(rename.from)) seeded.add(field); - continue; - } - if (value && typeof value === "object" && rename.from in value) seeded.add(field); - } - return seeded; +function isRegistryResidue( + seed: Record | undefined, + field: string, + saved: unknown, + rename: ModelRename, +): boolean { + if (!seed) return false; + return publishes(seed[field], rename.from) && publishes(saved, rename.to); } export interface ModelRenameProjection { @@ -268,17 +277,17 @@ export function projectModelRenames( // Provider config is a closed interface, so index through one unknown-cast // view rather than casting at each assignment. const row = prov as unknown as Record; - const registrySeeded = registrySeededFields(rename); + const seed = registrySeed(rename.provider); let touched = false; for (const field of MODEL_ID_LISTS) { - if (registrySeeded.has(field)) continue; + if (isRegistryResidue(seed, field, row[field], rename)) continue; const next = renameInList(row[field], rename.from, rename.to); if (!next) continue; row[field] = next; touched = true; } for (const field of MODEL_KEYED_RECORDS) { - if (registrySeeded.has(field)) continue; + if (isRegistryResidue(seed, field, row[field], rename)) continue; const next = rename.dropReasoningEffortMap && field === "modelReasoningEffortMap" ? dropFromRecord(row[field], rename.from) : renameInRecord(row[field], rename.from, rename.to);