fix(migration): track applied migrations in sidecar so user reverts stick

Users who auto-migrated from `openai/gpt-5.3-codex` to `openai/gpt-5.4`
and then reverted their config back to `gpt-5.3-codex` by hand had the
migration re-apply on every startup in an infinite loop. Discord bug
report pointed at the exact symptom: "i deleted the migrations and they
kept coming back".

The old migration tracking lived on the config body itself as a
`_migrations` string array. The skip-already-applied check relied on
the user not touching that field. But users hit by the unwanted
migration naturally reached for the JSON file to roll their model back,
and the natural human reaction to an incomprehensible internal field
next to their config is to delete it. That wiped the migration memory
and let the same migration re-apply at the next startup.

This PR introduces a sidecar state file that lives next to the config
as `<configPath>.migrations.json` and tracks applied migrations
outside the user's hand-editable config body. The migration pipeline:

  1. Reads applied migrations from BOTH the sidecar AND the legacy
     in-config `_migrations` field, unioning them. This keeps old
     configs that still carry `_migrations` working without forcing
     a reset.

  2. Writes the updated migration set to the sidecar, never to the
     config body.

  3. Strips the legacy `_migrations` field out of the config body on
     the first write after the sidecar takes over. Users stop seeing
     the mystery internal field in their own config from that point
     forward.

If the user also deletes the sidecar (explicit fresh-start gesture)
the migrations run again - that is intentional.

Tests (TDD, all new tests written before implementation):

  - src/shared/migration/migrations-sidecar.test.ts - 11 unit tests
    covering read/write/round-trip, malformed-payload resilience,
    parent-directory creation, sorted output for stable diffs, and
    non-string entry filtering.

  - src/shared/migration.test.ts - 6 new integration tests under the
    "migrateConfigFile with migration tracking via sidecar" block
    covering: no-op path, sidecar-only write, sidecar skip after user
    revert, legacy _migrations mirroring + strip, sidecar + legacy
    union with dedupe, and partial-history append. Existing
    "preserves existing _migrations and appends new ones" test was
    rewritten to assert the new sidecar-based contract.

  - Also fixes a latent test-hygiene bug: the shared
    /tmp/nonexistent-path-for-test.json config path used by many
    migrateConfigFile tests did not clean up its companion sidecar
    between tests, letting state from one test bleed into the next.
    Added afterEach that unlinks the sidecar.

Verified:

  - bun test src/shared/migration/ -> 11 new sidecar tests pass
  - bun test src/shared/migration.test.ts -> 82 pass, 0 fail
  - bun run typecheck -> clean
  - bun run script/run-ci-tests.ts -> 4458 pass, 0 fail (full suite)
This commit is contained in:
YeonGyu-Kim
2026-04-09 10:54:08 +09:00
parent d2bb5d57d1
commit 00a4f318ef
4 changed files with 444 additions and 51 deletions
@@ -0,0 +1,92 @@
import * as fs from "node:fs"
import * as path from "node:path"
import { log } from "../logger"
import { writeFileAtomically } from "../write-file-atomically"
/**
* Sidecar state file that tracks applied config migrations outside the user's
* config file.
*
* Why this exists (#3263): users who revert an auto-migrated value (e.g.
* `gpt-5.4` → `gpt-5.3-codex`) and then delete the `_migrations` field from
* their config would fall into an infinite migration loop — every startup
* re-applied the migration because there was no memory of the previous
* application. The sidecar remembers applied migrations even when the user
* scrubs the config, and only "resets" when the user explicitly deletes both
* the config and the sidecar.
*
* The sidecar lives next to the config file as
* `<configFileName>.migrations.json`. One sidecar per config file. The file
* format is a flat JSON object:
*
* {
* "appliedMigrations": [
* "model-version:openai/gpt-5.3-codex->openai/gpt-5.4",
* "model-version:anthropic/claude-opus-4-5->anthropic/claude-opus-4-6"
* ]
* }
*/
export interface MigrationsSidecar {
appliedMigrations: string[]
}
export function getSidecarPath(configPath: string): string {
return `${configPath}.migrations.json`
}
/**
* Read the set of applied migration keys from the sidecar next to
* `configPath`. Returns an empty set on any read or parse failure so the
* caller can still trust the return value and safely fall back to the
* config's `_migrations` field.
*/
export function readAppliedMigrations(configPath: string): Set<string> {
const sidecarPath = getSidecarPath(configPath)
try {
if (!fs.existsSync(sidecarPath)) {
return new Set()
}
const content = fs.readFileSync(sidecarPath, "utf-8")
const parsed = JSON.parse(content) as unknown
if (
parsed &&
typeof parsed === "object" &&
!Array.isArray(parsed) &&
Array.isArray((parsed as MigrationsSidecar).appliedMigrations)
) {
return new Set((parsed as MigrationsSidecar).appliedMigrations.filter((m): m is string => typeof m === "string"))
}
return new Set()
} catch (err) {
log(`[migration] Failed to read migrations sidecar at ${sidecarPath}`, err)
return new Set()
}
}
/**
* Persist the given set of applied migration keys to the sidecar next to
* `configPath`. The sidecar is written atomically. Returns true on success,
* false if the write failed (the caller can still proceed — the next
* startup will re-run the migration, which is idempotent by design).
*/
export function writeAppliedMigrations(configPath: string, migrations: Set<string>): boolean {
const sidecarPath = getSidecarPath(configPath)
const body: MigrationsSidecar = {
appliedMigrations: Array.from(migrations).sort(),
}
try {
// Ensure the parent directory exists in case the config file was created
// out-of-band. We intentionally do NOT create the sidecar when the migration
// set is empty — there is nothing to remember.
const parentDir = path.dirname(sidecarPath)
if (!fs.existsSync(parentDir)) {
fs.mkdirSync(parentDir, { recursive: true })
}
writeFileAtomically(sidecarPath, JSON.stringify(body, null, 2) + "\n")
return true
} catch (err) {
log(`[migration] Failed to write migrations sidecar at ${sidecarPath}`, err)
return false
}
}