From 5493ae226c67b3ff76e865b6ed5c7342fff9b9f3 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 10 Apr 2026 19:01:59 +0900 Subject: [PATCH] fix(migration): persist fullMigrationSet in-config when sidecar write fails Addresses Cubic review: configs without prior _migrations now get the full migration set written to config as fallback when sidecar write fails, preventing migration tracking loss. --- src/shared/migration.test.ts | 33 ++++++++++++++++++++++-- src/shared/migration/config-migration.ts | 12 ++++++--- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/shared/migration.test.ts b/src/shared/migration.test.ts index e6155a2cb..e0b5f2808 100644 --- a/src/shared/migration.test.ts +++ b/src/shared/migration.test.ts @@ -1488,9 +1488,12 @@ describe("migrateConfigFile with migration tracking via sidecar (#3263)", () => // when: Migrate config file (sidecar write will fail) const needsWrite = migrateConfigFile(testConfigPath, rawConfig) - // then: _migrations should be preserved as fallback since sidecar write failed + // then: _migrations should contain full set (existing + new) as fallback expect(needsWrite).toBe(true) - expect(rawConfig._migrations).toEqual(["model-version:openai/gpt-5.3-codex->openai/gpt-5.4"]) + const migrations = rawConfig._migrations as string[] + expect(Array.isArray(migrations)).toBe(true) + expect(migrations).toContain("model-version:openai/gpt-5.3-codex->openai/gpt-5.4") + expect(migrations.length).toBeGreaterThanOrEqual(1) expect((rawConfig.agents as Record>).oracle.model).toBe("anthropic/claude-opus-4-6") // Sidecar should not exist because write failed @@ -1499,4 +1502,30 @@ describe("migrateConfigFile with migration tracking via sidecar (#3263)", () => // cleanup: restore permissions for cleanup fs.chmodSync(workdir, 0o755) }) + + test("writes _migrations into config as fallback when sidecar write fails and no prior _migrations existed", () => { + // given: config WITHOUT _migrations field and a read-only dir + const testConfigPath = tempConfigPath("sidecar-fail-no-prior") + const rawConfig: Record = { + agents: { + oracle: { model: "anthropic/claude-opus-4-5" }, + }, + } + fs.writeFileSync(testConfigPath, JSON.stringify(rawConfig, null, 2)) + const workdir = path.dirname(testConfigPath) + fs.chmodSync(workdir, 0o555) + + // when: migrate runs (sidecar write will fail) + const needsWrite = migrateConfigFile(testConfigPath, rawConfig) + + // then: _migrations should be injected into config as fallback + expect(needsWrite).toBe(true) + expect(rawConfig._migrations).toBeDefined() + expect(Array.isArray(rawConfig._migrations)).toBe(true) + expect((rawConfig._migrations as string[]).length).toBeGreaterThan(0) + expect(fs.existsSync(sidecarPath(testConfigPath))).toBe(false) + + // cleanup + fs.chmodSync(workdir, 0o755) + }) }) diff --git a/src/shared/migration/config-migration.ts b/src/shared/migration/config-migration.ts index f5457e613..3174720bb 100644 --- a/src/shared/migration/config-migration.ts +++ b/src/shared/migration/config-migration.ts @@ -75,11 +75,11 @@ export function migrateConfigFile( // `_migrations` to downstream schema validation. const newMigrationsToRecord = allNewMigrations.filter(mKey => !existingMigrations.has(mKey)) let sidecarWriteSucceeded = false + const fullMigrationSet = new Set([ + ...existingMigrations, + ...newMigrationsToRecord, + ]) if (newMigrationsToRecord.length > 0 || hadLegacyInConfigMigrations) { - const fullMigrationSet = new Set([ - ...existingMigrations, - ...newMigrationsToRecord, - ]) sidecarWriteSucceeded = writeAppliedMigrations(configPath, fullMigrationSet) } if (newMigrationsToRecord.length > 0) { @@ -91,6 +91,10 @@ export function migrateConfigFile( } if (sidecarWriteSucceeded && "_migrations" in copy) { delete copy._migrations + } else if (!sidecarWriteSucceeded && newMigrationsToRecord.length > 0) { + // Sidecar write failed — persist migration tracking in-config as fallback + ;(copy as Record)._migrations = Array.from(fullMigrationSet) + needsWrite = true } if (copy.omo_agent) {