Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion src/providers/model-rename-migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update the mapped structure contracts

This changes the startup/config-migration contract for the owned src/providers/ area, but the commit updates none of the documents mapped to that area in structure/INDEX.md (runtime.md, subagents.md, transports/inventory.md, providers-and-adapters.md, and providers/xai-grok.md). Update those documents in this change so the maintained architecture description records which registry-owned fields the rename migration intentionally leaves untouched.

AGENTS.md reference: src/AGENTS.md:L10-L11

Useful? React with 👍 / 👎.

// 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 {
Expand Down Expand Up @@ -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<string> {
const seeded = new Set<string>();
const entry = PROVIDER_REGISTRY.find(row => row.id === rename.provider);
if (!entry) return seeded;
const seed = providerConfigSeed(entry) as unknown as Record<string, unknown>;
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;
Expand Down Expand Up @@ -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<string, unknown>;
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"
Comment on lines 289 to 291

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Limit seed skips to rows that OAuth reconciliation owns

When a saved google-antigravity row has a non-oauth or legacy-absent authMode, reconcileOAuthProviders does not republish modelContextWindows, and enrichProviderFromRegistry only supplies that record when it is missing. Nevertheless, this unconditional skip treats the field as registry-owned merely because the seed contains the retired ID. For a row with defaultModel: "gemini-3.6-flash" and a user override such as modelContextWindows: {"gemini-3.6-flash": 262144}, the migration now changes the default to 3.7 but strands the override under 3.6, so routing falls back to the registry's 3.7 window. Restrict the skip to rows actually eligible for OAuth reconciliation, or distinguish the registry seed shape from user-owned values.

AGENTS.md reference: src/AGENTS.md:L7-L10

Useful? React with 👍 / 👎.

? dropFromRecord(row[field], rename.from)
: renameInRecord(row[field], rename.from, rename.to);
Expand Down
140 changes: 140 additions & 0 deletions tests/providers/model-rename-migration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<string, unknown> {
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);
});
});
Loading