From 3d57c70a5c304610450d501e166b35873bc083da Mon Sep 17 00:00:00 2001 From: SB Yoon <44089734+yansigit@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:16:20 -0600 Subject: [PATCH 1/2] fix(oauth): persist startup reconciliation before adoption --- src/oauth/index.ts | 116 +++++++++++---- src/providers/model-rename-startup.ts | 36 +++-- tests/oauth/oauth-provider-reconcile.test.ts | 133 +++++++++++++++++- .../providers/model-rename-migration.test.ts | 34 +++++ 4 files changed, 282 insertions(+), 37 deletions(-) diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 3ae6f4c01f..a79acb474f 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -1,7 +1,7 @@ import type { KiroOAuthMetadata, OAuthController, OAuthCredentials } from "./types"; import { parseCallbackInput } from "./callback-server"; import type { OcxConfig, OcxProviderConfig, RefreshPolicy } from "../types"; -import { ConfigMutationLockError, loadConfig, saveConfig } from "../config"; +import { ConfigMutationLockError, loadConfig, mutatePersistedConfig, saveConfig } from "../config"; import { resolveProviderApiKey } from "../providers/key-store"; import { maskEmail } from "../lib/privacy"; import { KiroTokenRefreshError, environmentKiroRoutingMetadata, loginKiro, refreshKiroToken, settleKiroLoginTransaction } from "./kiro"; @@ -1247,42 +1247,106 @@ function migrateLegacyAntigravityStaticCatalog(config: OcxConfig): boolean { return true; } -export function reconcileOAuthProviders(config: OcxConfig): boolean { - let changed = migrateLegacyAntigravityStaticCatalog(config); - for (const [name, prov] of Object.entries(config.providers)) { +interface OAuthReconcileProjection { + config: OcxConfig; + changed: boolean; + touchedProviders: string[]; + touchedAntigravityVersion: boolean; +} + +function projectOAuthProviderReconciliation(config: OcxConfig): OAuthReconcileProjection { + const projected = structuredClone(config); + const touchedProviders = new Set(); + const beforeAntigravity = JSON.stringify(projected.providers[GOOGLE_ANTIGRAVITY_PROVIDER]); + const beforeAntigravityVersion = projected.googleAntigravityStaticCatalogVersion; + let changed = migrateLegacyAntigravityStaticCatalog(projected); + if (JSON.stringify(projected.providers[GOOGLE_ANTIGRAVITY_PROVIDER]) !== beforeAntigravity) { + touchedProviders.add(GOOGLE_ANTIGRAVITY_PROVIDER); + } + const touchedAntigravityVersion = projected.googleAntigravityStaticCatalogVersion !== beforeAntigravityVersion; + + for (const [name, prov] of Object.entries(projected.providers)) { + const beforeProvider = JSON.stringify(prov); const def = OAUTH_PROVIDERS[name]; if (name === "command-code" && isLegacyCommandCodeStaticCatalog(prov)) { // The former experimental preset was the exact three-model seed above. It was not a user // choice to disable discovery, so promote only that shape to the account live catalog. prov.liveModels = true; - changed = true; } - if (!def || prov.authMode !== "oauth") continue; - const preset = def.providerConfig; - for (const field of OAUTH_RECONCILE_FIELDS) { - if (JSON.stringify(prov[field]) === JSON.stringify(preset[field])) continue; - if (preset[field] !== undefined) { - prov[field] = cloneProviderField(preset[field]) as never; - } else { - delete prov[field]; + if (def && prov.authMode === "oauth") { + const preset = def.providerConfig; + for (const field of OAUTH_RECONCILE_FIELDS) { + if (JSON.stringify(prov[field]) === JSON.stringify(preset[field])) continue; + if (preset[field] !== undefined) { + prov[field] = cloneProviderField(preset[field]) as never; + } else { + delete prov[field]; + } + } + if (prov.liveModels === undefined && preset.liveModels !== undefined) { + prov.liveModels = preset.liveModels; + } + // Heal a defaultModel that no longer exists in the refreshed list (e.g. a deprecated snapshot). + // Skip providers without a static preset `models` list: for live-discovery providers + // (e.g. command-code OAuth) the account-scoped catalog is not enumerable here, so any + // persisted defaultModel is a user selection and must not be overwritten by the seed. + if (prov.defaultModel && preset.defaultModel && preset.models && preset.models.length > 0 && !(prov.models ?? []).includes(prov.defaultModel)) { + prov.defaultModel = preset.defaultModel; } - changed = true; - } - if (prov.liveModels === undefined && preset.liveModels !== undefined) { - prov.liveModels = preset.liveModels; - changed = true; } - // Heal a defaultModel that no longer exists in the refreshed list (e.g. a deprecated snapshot). - // Skip providers without a static preset `models` list: for live-discovery providers - // (e.g. command-code OAuth) the account-scoped catalog is not enumerable here, so any - // persisted defaultModel is a user selection and must not be overwritten by the seed. - if (prov.defaultModel && preset.defaultModel && preset.models && preset.models.length > 0 && !(prov.models ?? []).includes(prov.defaultModel)) { - prov.defaultModel = preset.defaultModel; + if (JSON.stringify(prov) !== beforeProvider) { changed = true; + touchedProviders.add(name); } } - if (changed) saveConfig(config); - return changed; + + return { + config: projected, + changed, + touchedProviders: [...touchedProviders], + touchedAntigravityVersion, + }; +} + +function adoptOAuthReconciliation(config: OcxConfig, projection: OAuthReconcileProjection): void { + for (const name of projection.touchedProviders) { + const provider = projection.config.providers[name]; + if (provider) config.providers[name] = structuredClone(provider); + else delete config.providers[name]; + } + if (projection.touchedAntigravityVersion) { + config.googleAntigravityStaticCatalogVersion = projection.config.googleAntigravityStaticCatalogVersion; + } +} + +function withOAuthReconciliationTouchedKeys( + projection: OAuthReconcileProjection, + required: OAuthReconcileProjection, +): OAuthReconcileProjection { + return { + ...projection, + touchedProviders: [...new Set([...projection.touchedProviders, ...required.touchedProviders])], + touchedAntigravityVersion: projection.touchedAntigravityVersion || required.touchedAntigravityVersion, + }; +} + +export function reconcileOAuthProviders(config: OcxConfig, persist = true): boolean { + const projection = projectOAuthProviderReconciliation(config); + if (!projection.changed) return false; + if (!persist) { + adoptOAuthReconciliation(config, projection); + return true; + } + const outcome = mutatePersistedConfig(fresh => { + const next = projectOAuthProviderReconciliation(fresh); + if (next.changed) adoptOAuthReconciliation(fresh, next); + return { changed: next.changed, value: next }; + }); + if (outcome.status === "unavailable") { + throw new Error(`OAuth provider reconciliation persistence unavailable: ${outcome.reason}`); + } + adoptOAuthReconciliation(config, withOAuthReconciliationTouchedKeys(outcome.value, projection)); + return true; } /** Runtime guards: provider config is intentionally passthrough, so persisted fields may be malformed. */ diff --git a/src/providers/model-rename-startup.ts b/src/providers/model-rename-startup.ts index 1f983a6acb..b97768d42f 100644 --- a/src/providers/model-rename-startup.ts +++ b/src/providers/model-rename-startup.ts @@ -1,10 +1,16 @@ -import { saveConfig } from "../config"; +import { mutatePersistedConfig } from "../config"; import { projectModelRenames } from "./model-rename-migration"; import type { OcxConfig } from "../types"; export interface ModelRenameStartupDeps { project: typeof projectModelRenames; - save: (config: OcxConfig) => void; + save?: (config: OcxConfig) => void; +} + +function adoptConfig(target: OcxConfig, source: OcxConfig): void { + if (target === source) return; + for (const key of Object.keys(target)) delete (target as unknown as Record)[key]; + Object.assign(target, structuredClone(source)); } /** @@ -18,11 +24,25 @@ export interface ModelRenameStartupDeps { */ export function runModelRenameStartupMigration( config: OcxConfig, - deps: ModelRenameStartupDeps = { project: projectModelRenames, save: saveConfig }, + deps: ModelRenameStartupDeps = { project: projectModelRenames }, ): OcxConfig { - const projection = deps.project(config); - for (const warning of projection.warnings) console.warn(`[model-rename-migration] ${warning}`); - if (!projection.changed) return projection.config; - deps.save(projection.config); - return projection.config; + const projection = deps.project(structuredClone(config)); + if (!projection.changed) return config; + if (deps.save) { + deps.save(projection.config); + adoptConfig(config, projection.config); + for (const warning of projection.warnings) console.warn(`[model-rename-migration] ${warning}`); + return config; + } + const outcome = mutatePersistedConfig(fresh => { + const next = deps.project(fresh); + if (next.changed) adoptConfig(fresh, next.config); + return { changed: next.changed, value: next }; + }); + if (outcome.status === "unavailable") { + throw new Error(`model rename startup persistence unavailable: ${outcome.reason}`); + } + adoptConfig(config, outcome.value.config); + for (const warning of outcome.value.warnings) console.warn(`[model-rename-migration] ${warning}`); + return config; } diff --git a/tests/oauth/oauth-provider-reconcile.test.ts b/tests/oauth/oauth-provider-reconcile.test.ts index 4f1ef9768f..e7dc1d7d88 100644 --- a/tests/oauth/oauth-provider-reconcile.test.ts +++ b/tests/oauth/oauth-provider-reconcile.test.ts @@ -1,8 +1,13 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { mkdtempSync} from "node:fs"; +import { mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { loadConfig } from "../../src/config"; +import { + getConfigPath, + loadConfig, + saveConfig, + setPersistedConfigMutationBeforeCommitForTests, +} from "../../src/config"; import { OAUTH_PROVIDERS, reconcileOAuthProviders, upsertOAuthProvider } from "../../src/oauth"; import { getCredential, saveCredential } from "../../src/oauth/store"; import { routeModel } from "../../src/router"; @@ -15,6 +20,7 @@ const originalHome = process.env.OPENCODEX_HOME; const homes: string[] = []; afterEach(() => { + setPersistedConfigMutationBeforeCommitForTests(null); if (originalHome === undefined) delete process.env.OPENCODEX_HOME; else process.env.OPENCODEX_HOME = originalHome; for (const home of homes.splice(0)) removeTreeWithRetry(home); @@ -39,6 +45,7 @@ describe("OAuth provider reconciliation", () => { }, }, } satisfies OcxConfig; + saveConfig(config); expect(reconcileOAuthProviders(config)).toBe(true); expect(config.providers.cursor.noVisionModels).toEqual(preset.noVisionModels); @@ -48,6 +55,66 @@ describe("OAuth provider reconciliation", () => { expect(modelInList(config.providers.cursor.noVisionModels, "composer-2.5")).toBe(true); expect(reconcileOAuthProviders(config)).toBe(false); }); + + test("unavailable persistence leaves live OAuth reconciliation input unchanged", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-oauth-reconcile-unavailable-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + const config = { + port: 10100, + defaultProvider: "google-antigravity", + providers: { + "google-antigravity": { + adapter: "google", + baseUrl: "https://daily-cloudcode-pa.googleapis.com", + authMode: "oauth", + googleMode: "cloud-code-assist", + defaultModel: "gemini-3.5-flash-low", + models: ["gemini-3.5-flash-low", "gemini-3.5-flash-high"], + liveModels: true, + }, + }, + } satisfies OcxConfig; + const before = structuredClone(config); + + expect(() => reconcileOAuthProviders(config)).toThrow( + "OAuth provider reconciliation persistence unavailable: missing", + ); + expect(config).toEqual(before); + }); + + test("rebases startup reconciliation over a concurrent provider edit", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-oauth-reconcile-race-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + const preset = OAUTH_PROVIDERS.cursor.providerConfig; + const config = { + port: 10100, + defaultProvider: "cursor", + providers: { + cursor: { + ...structuredClone(preset), + authMode: "oauth", + noVisionModels: cursorModelIds(CURSOR_STATIC_MODELS), + }, + }, + } satisfies OcxConfig; + saveConfig(config); + setPersistedConfigMutationBeforeCommitForTests(() => { + const concurrent = loadConfig(); + concurrent.providers.cursor.note = "concurrent-operator-edit"; + writeFileSync(getConfigPath(), JSON.stringify(concurrent, null, 2) + "\n"); + }); + + expect(reconcileOAuthProviders(config)).toBe(true); + + expect(config.providers.cursor.noVisionModels).toEqual(preset.noVisionModels); + expect(config.providers.cursor.note).toBe("concurrent-operator-edit"); + const persisted = loadConfig(); + expect(persisted.providers.cursor.noVisionModels).toEqual(preset.noVisionModels); + expect(persisted.providers.cursor.note).toBe("concurrent-operator-edit"); + }); + test("refreshes a saved Antigravity 3.5 preset without touching credentials or user fields", async () => { const home = mkdtempSync(join(tmpdir(), "ocx-gemini-36-reconcile-")); homes.push(home); @@ -76,6 +143,7 @@ describe("OAuth provider reconciliation", () => { }, }, } satisfies OcxConfig; + saveConfig(config); expect(reconcileOAuthProviders(config)).toBe(true); const provider = config.providers["google-antigravity"]; @@ -112,6 +180,9 @@ describe("OAuth provider reconciliation", () => { }); test("migrates the version-1 canonical Antigravity static row to live discovery", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-antigravity-static-reconcile-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; const config = { port: 10100, defaultProvider: "google-antigravity", @@ -134,6 +205,7 @@ describe("OAuth provider reconciliation", () => { }, }, } satisfies OcxConfig; + saveConfig(config); expect(reconcileOAuthProviders(config)).toBe(true); expect(config.providers["google-antigravity"].liveModels).toBe(true); @@ -144,11 +216,64 @@ describe("OAuth provider reconciliation", () => { expect(config.providers["google-antigravity"].models).toHaveLength(7); }); + test("adopts already-reconciled persisted OAuth state into a stale live config", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-antigravity-unchanged-adopt-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + const staleLive = { + port: 10100, + defaultProvider: "google-antigravity", + googleAntigravityStaticCatalogVersion: 1, + providers: { + "google-antigravity": { + ...structuredClone(OAUTH_PROVIDERS["google-antigravity"].providerConfig), + defaultModel: "gemini-3.6-flash", + models: [ + "gemini-3.6-flash", + "gemini-3.1-pro", + "gemini-3.1-flash-image", + "claude-sonnet-4-6", + "claude-opus-4-6-thinking", + "gpt-oss-120b-medium", + ], + liveModels: false, + }, + "local-only": { + adapter: "openai", + baseUrl: "http://127.0.0.1:9999/v1", + allowPrivateNetwork: true, + models: ["local-live"], + note: "live-only", + }, + }, + } satisfies OcxConfig; + const reconciledDisk = structuredClone(staleLive); + reconciledDisk.googleAntigravityStaticCatalogVersion = 2; + reconciledDisk.providers["google-antigravity"].liveModels = true; + reconciledDisk.providers["disk-only"] = { + adapter: "openai", + baseUrl: "http://127.0.0.1:9998/v1", + allowPrivateNetwork: true, + models: ["disk-only"], + }; + delete reconciledDisk.providers["local-only"]; + saveConfig(reconciledDisk); + + expect(reconcileOAuthProviders(staleLive)).toBe(true); + expect(staleLive.googleAntigravityStaticCatalogVersion).toBe(2); + expect(staleLive.providers["google-antigravity"].liveModels).toBe(true); + expect(staleLive.providers["local-only"]?.note).toBe("live-only"); + expect(staleLive.providers["disk-only"]).toBeUndefined(); + }); + test("an explicit 3.7 default survives the 3.8 launch while its capabilities refresh", () => { // The 3.5 case above starts from a RETIRED id, so it only exercises the stale-default // healing branch. This one is the opposite claim, and the one that matters for an // additive rollout: a user who deliberately chose 3.7 must still be on 3.7 afterwards. // Google still serves it, so healing it onto 3.8 would be silently overriding a choice. + const home = mkdtempSync(join(tmpdir(), "ocx-antigravity-explicit-default-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; saveCredential("google-antigravity", { access: "a", refresh: "r", projectId: "p" }); const config = { port: 10100, @@ -165,6 +290,7 @@ describe("OAuth provider reconciliation", () => { }, }, } satisfies OcxConfig; + saveConfig(config); reconcileOAuthProviders(config); const provider = config.providers["google-antigravity"]; @@ -264,7 +390,7 @@ describe("OAuth provider reconciliation", () => { }, } satisfies OcxConfig; - reconcileOAuthProviders(config); + reconcileOAuthProviders(config, false); expect(config.providers.kimi.requiresReasoningPlaceholderModels).toEqual([]); }); @@ -288,6 +414,7 @@ describe("OAuth provider reconciliation", () => { }, }, } satisfies OcxConfig; + saveConfig(config); expect(reconcileOAuthProviders(config)).toBe(true); expect(config.providers.xai.modelReasoningEfforts?.["grok-4.6"]) diff --git a/tests/providers/model-rename-migration.test.ts b/tests/providers/model-rename-migration.test.ts index 40803a4ed1..411b0d12cc 100644 --- a/tests/providers/model-rename-migration.test.ts +++ b/tests/providers/model-rename-migration.test.ts @@ -1,11 +1,16 @@ import { describe, expect, test } from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { MODEL_RENAMES, projectModelRenames, type ModelRename, } from "../../src/providers/model-rename-migration"; +import { runModelRenameStartupMigration } from "../../src/providers/model-rename-startup"; import { PROVIDER_REGISTRY } from "../../src/providers/registry"; import type { OcxConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; const INTL_BASE_URL = "https://token-plan.ap-southeast-1.maas.aliyuncs.com/compatible-mode/v1"; @@ -130,4 +135,33 @@ describe("registry model rename migration (#1610)", () => { expect(entry?.models).not.toContain(rename.from); } }); + + test("startup no-op preserves the live config object identity", () => { + const clean = projectModelRenames(staleConfig(), [RENAME]).config; + const returned = runModelRenameStartupMigration(clean, { + project: config => projectModelRenames(config, [RENAME]), + save: () => { throw new Error("no-op must not save"); }, + }); + + expect(returned).toBe(clean); + }); + + test("startup persistence failure leaves live model-rename input unchanged", () => { + const previousHome = process.env.OPENCODEX_HOME; + const home = mkdtempSync(join(tmpdir(), "ocx-model-rename-unavailable-")); + try { + process.env.OPENCODEX_HOME = home; + const live = staleConfig(); + const before = structuredClone(live); + + expect(() => runModelRenameStartupMigration(live)).toThrow( + "model rename startup persistence unavailable: missing", + ); + expect(live).toEqual(before); + } finally { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(home); + } + }); }); From cf0b3fe0aaa3b72137b88da8b341769871e5845e Mon Sep 17 00:00:00 2001 From: SB Yoon <44089734+yansigit@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:53:28 -0600 Subject: [PATCH 2/2] fix(providers): surface no-op rename warnings --- src/providers/model-rename-startup.ts | 5 ++++- .../providers/model-rename-migration.test.ts | 20 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/providers/model-rename-startup.ts b/src/providers/model-rename-startup.ts index b97768d42f..e084b1d62d 100644 --- a/src/providers/model-rename-startup.ts +++ b/src/providers/model-rename-startup.ts @@ -27,7 +27,10 @@ export function runModelRenameStartupMigration( deps: ModelRenameStartupDeps = { project: projectModelRenames }, ): OcxConfig { const projection = deps.project(structuredClone(config)); - if (!projection.changed) return config; + if (!projection.changed) { + for (const warning of projection.warnings) console.warn(`[model-rename-migration] ${warning}`); + return config; + } if (deps.save) { deps.save(projection.config); adoptConfig(config, projection.config); diff --git a/tests/providers/model-rename-migration.test.ts b/tests/providers/model-rename-migration.test.ts index 411b0d12cc..b28ab23c6c 100644 --- a/tests/providers/model-rename-migration.test.ts +++ b/tests/providers/model-rename-migration.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "bun:test"; +import { describe, expect, spyOn, test } from "bun:test"; import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -146,6 +146,24 @@ describe("registry model rename migration (#1610)", () => { expect(returned).toBe(clean); }); + test("startup no-op still reports projection warnings", () => { + const clean = projectModelRenames(staleConfig(), [RENAME]).config; + const warning = spyOn(console, "warn").mockImplementation(() => {}); + try { + const returned = runModelRenameStartupMigration(clean, { + project: config => ({ config, changed: false, warnings: ["rename target is unavailable"] }), + save: () => { throw new Error("no-op must not save"); }, + }); + + expect(returned).toBe(clean); + expect(warning).toHaveBeenCalledWith( + "[model-rename-migration] rename target is unavailable", + ); + } finally { + warning.mockRestore(); + } + }); + test("startup persistence failure leaves live model-rename input unchanged", () => { const previousHome = process.env.OPENCODEX_HOME; const home = mkdtempSync(join(tmpdir(), "ocx-model-rename-unavailable-"));