diff --git a/.changeset/fierce-lynx-howl.md b/.changeset/fierce-lynx-howl.md deleted file mode 100644 index 3f4aabe5a..000000000 --- a/.changeset/fierce-lynx-howl.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 674 ---- -Accept published installer migration checksums so Windows users upgrading from 1.2.0 to 1.3.0 do not hit checksum drift failures. diff --git a/.changeset/humble-ibex-jump.md b/.changeset/humble-ibex-jump.md new file mode 100644 index 000000000..1454056cc --- /dev/null +++ b/.changeset/humble-ibex-jump.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 675 +--- +**`npx @opengsd/gsd-core` upgrades no longer abort with "applied migration checksum changed"** — an already-applied installer migration whose recorded checksum drifted (e.g. a shipped body was edited) is now detected and reconciled automatically on the next install, instead of hard-failing the upgrade. Replaces the published-checksum allowlist with general self-healing recovery plus a CI baseline lock. diff --git a/docs/installer-migrations.md b/docs/installer-migrations.md index 124d7bfe0..88f4e96b0 100644 --- a/docs/installer-migrations.md +++ b/docs/installer-migrations.md @@ -121,14 +121,17 @@ Required fields: } ``` -The checksum is calculated from the migration definition. If an applied -migration's checksum changes, the installer must warn and refuse to silently -re-run it. Fix-forward migrations should use a new migration id. +The checksum is calculated from the migration definition. -Migration records may pin a stable `checksum` and list `legacyChecksums` for -checksums already written by published packages. This is only for compatibility -with released install state; new behavior should still ship as a new migration -id rather than mutating an already-applied migration in place. +An already-applied migration is never re-run, so a drifted checksum is +tolerated at runtime: it is collected in `plan.checksumDrift` and reconciled +into install state on the next write, rather than aborting the user's upgrade +(this unblocks upgrades — see issue #670). + +The "shipped migration bodies are immutable" rule is enforced in CI by a +committed checksum-baseline test in `tests/installer-migrations.test.cjs`. +If you need to change the behaviour of a released migration, add a NEW +fix-forward migration id instead of editing the shipped body. ## Migration Record diff --git a/scripts/ci-test-scope.cjs b/scripts/ci-test-scope.cjs index 771aa81be..299491b2b 100644 --- a/scripts/ci-test-scope.cjs +++ b/scripts/ci-test-scope.cjs @@ -64,7 +64,6 @@ const RULES = [ 'tests/install-regressions.test.cjs', 'tests/install-runtime-artifacts.test.cjs', 'tests/install-path-detection.test.cjs', - 'tests/installer-migration-checksum-compat.test.cjs', 'tests/release-tarball-smoke.install.test.cjs', 'tests/runtime-artifact-layout.test.cjs', ], @@ -255,7 +254,7 @@ function classify(files) { let fullMatrix = false; for (const file of files) { - if (['bin/', 'gsd-core/', 'agents/', 'commands/', 'docs/', 'hooks/', 'tests/', 'scripts/', 'src/'].some(p => file.startsWith(p)) || + if (['bin/', 'gsd-core/', 'agents/', 'commands/', 'docs/', 'hooks/', 'tests/', 'scripts/'].some(p => file.startsWith(p)) || file === 'package.json' || file === 'package-lock.json' || (file.startsWith('tsconfig') && file.endsWith('.json')) || file.startsWith('.github/workflows/') || diff --git a/src/installer-migration-authoring.cts b/src/installer-migration-authoring.cts index a09a4fcc5..a859363d7 100644 --- a/src/installer-migration-authoring.cts +++ b/src/installer-migration-authoring.cts @@ -91,7 +91,6 @@ export function validateInstallerMigrationRecord(record: unknown, source?: strin throw new Error(`migration record must declare destructive as a boolean: ${displaySource}`); } validateStringArray(rec, 'runtimes', displaySource); - validateStringArray(rec, 'legacyChecksums', displaySource); requireStringArray(rec, 'scopes', displaySource); if (typeof rec['plan'] !== 'function') { throw new Error(`migration record must include a plan function: ${displaySource}`); diff --git a/src/installer-migrations.cts b/src/installer-migrations.cts index 4133c07ed..95105cd5b 100644 --- a/src/installer-migrations.cts +++ b/src/installer-migrations.cts @@ -208,27 +208,51 @@ function migrationChecksum(migration: MigrationRecord): string { return `sha256:${sha256Text(JSON.stringify(serializable))}`; } -function migrationLegacyChecksums(migration: MigrationRecord): string[] { - return Array.isArray(migration.legacyChecksums) - ? migration.legacyChecksums.filter((checksum): checksum is string => typeof checksum === 'string' && checksum.length > 0) - : []; +// Rewrite the stored checksum of any already-applied entry whose id drifted, so the +// drift is reconciled durably and not re-detected on every subsequent run (issue #670). +// Returns the number of entries actually changed (so callers know whether a write is needed). +function reconcileDriftedChecksums( + appliedEntries: Array>, + checksumDrift: Array<{ id: string; currentChecksum: string }> | undefined +): number { + if (!Array.isArray(checksumDrift) || checksumDrift.length === 0) return 0; + const reconcile = new Map(checksumDrift.map((d) => [d.id, d.currentChecksum])); + let changed = 0; + for (let i = 0; i < appliedEntries.length; i++) { + const existing = appliedEntries[i]; + if (existing && typeof existing.id === 'string' && reconcile.has(existing.id)) { + const next = reconcile.get(existing.id) as string; + if (existing.checksum !== next) { + appliedEntries[i] = { ...existing, checksum: next }; + changed += 1; + } + } + } + return changed; } -function migrationAcceptedChecksums(migration: MigrationRecord): Set { - return new Set([migrationChecksum(migration), ...migrationLegacyChecksums(migration)]); -} - -function assertAppliedMigrationChecksums(applied: Map>, migrations: MigrationRecord[]): void { +function collectAppliedChecksumDrift( + applied: Map>, + migrations: MigrationRecord[] +): Array<{ id: string; storedChecksum: string; currentChecksum: string }> { + const drift: Array<{ id: string; storedChecksum: string; currentChecksum: string }> = []; for (const migration of migrations) { const entry = applied.get(migration.id as string); if (!entry || !entry.checksum) continue; - const accepted = migrationAcceptedChecksums(migration); - if (!accepted.has(entry.checksum as string)) { - throw new Error( - `applied migration checksum changed for ${migration.id as string}; create a new fix-forward migration id` - ); + const currentChecksum = migrationChecksum(migration); + if (entry.checksum !== currentChecksum) { + // An already-applied migration is never re-run (it is filtered out of `pending`), + // so a checksum drift here is functionally inert. A prior release may have edited a + // shipped migration body (see issue #670). Surface it for reconciliation instead of + // hard-aborting the user's upgrade. + drift.push({ + id: migration.id as string, + storedChecksum: entry.checksum as string, + currentChecksum, + }); } } + return drift; } function migrationMatchesContext(migration: MigrationRecord, { runtime, scope }: { runtime: string | null; scope: string | null }): boolean { @@ -461,6 +485,7 @@ interface MigrationPlan { pendingMigrations: MigrationRecord[]; actions: PlannedAction[]; blocked: PlannedAction[]; + checksumDrift: Array<{ id: string; storedChecksum: string; currentChecksum: string }>; } function planInstallerMigrations({ @@ -490,7 +515,7 @@ function planInstallerMigrations({ migrationMatchesContext(migration, { runtime, scope }) ); const applied = appliedMigrationEntries(state); - assertAppliedMigrationChecksums(applied, scopedMigrations); + const checksumDrift = collectAppliedChecksumDrift(applied, scopedMigrations); const pending = scopedMigrations.filter((migration) => !applied.has(migration.id as string)); const actions: PlannedAction[] = []; const blocked: PlannedAction[] = []; @@ -578,6 +603,7 @@ function planInstallerMigrations({ pendingMigrations: pending, actions, blocked, + checksumDrift, }; } @@ -757,6 +783,7 @@ function applyInstallerMigrationPlan({ const state = readInstallState(configDir); const applied = appliedMigrationIds(state); const nextApplied = [...state.appliedMigrations]; + reconcileDriftedChecksums(nextApplied, plan.checksumDrift); const actionsByMigrationId = new Map(); for (const action of plan.actions) { if (action.migrationId && !actionsByMigrationId.has(action.migrationId)) { @@ -819,29 +846,36 @@ function markPendingMigrationsApplied({ plan: MigrationPlan; now?: () => string; }): string[] { - if (!plan || !Array.isArray(plan.pendingMigrationIds) || plan.pendingMigrationIds.length === 0) { - return []; - } + if (!plan) return []; + const hasPending = Array.isArray(plan.pendingMigrationIds) && plan.pendingMigrationIds.length > 0; + const hasDrift = Array.isArray(plan.checksumDrift) && plan.checksumDrift.length > 0; + if (!hasPending && !hasDrift) return []; + const appliedAt = now(); const state = readInstallState(configDir); const applied = appliedMigrationIds(state); - const checksumsByMigrationId = new Map(); - for (const migration of plan.pendingMigrations || []) { - checksumsByMigrationId.set(migration.id as string, migrationChecksum(migration)); - } const nextApplied = [...state.appliedMigrations]; + const reconciledCount = reconcileDriftedChecksums(nextApplied, plan.checksumDrift); + const newlyApplied: string[] = []; - for (const id of plan.pendingMigrationIds) { - if (applied.has(id)) continue; - nextApplied.push({ - id, - appliedAt, - journal: null, - checksum: checksumsByMigrationId.get(id) || null, - }); - newlyApplied.push(id); + if (hasPending) { + const checksumsByMigrationId = new Map(); + for (const migration of plan.pendingMigrations || []) { + checksumsByMigrationId.set(migration.id as string, migrationChecksum(migration)); + } + for (const id of plan.pendingMigrationIds) { + if (applied.has(id)) continue; + nextApplied.push({ + id, + appliedAt, + journal: null, + checksum: checksumsByMigrationId.get(id) || null, + }); + newlyApplied.push(id); + } } - if (newlyApplied.length > 0) { + + if (newlyApplied.length > 0 || reconciledCount > 0) { writeInstallState(configDir, { schemaVersion: 1, appliedMigrations: nextApplied, @@ -933,8 +967,8 @@ export = { acquireInstallMigrationLock, applyInstallerMigrationPlan, classifyArtifact, - computeInstallerMigrationChecksum: migrationChecksum, discoverInstallerMigrations, + migrationChecksum, planInstallerMigrations, readInstallManifest, readInstallState, diff --git a/src/installer-migrations/000-first-time-baseline.cts b/src/installer-migrations/000-first-time-baseline.cts index 0f31541c8..c81f30e6a 100644 --- a/src/installer-migrations/000-first-time-baseline.cts +++ b/src/installer-migrations/000-first-time-baseline.cts @@ -171,8 +171,6 @@ interface InstallerMigration { title: string; description: string; introducedIn: string; - checksum: string; - legacyChecksums?: string[]; scopes: string[]; destructive: boolean; plan: (ctx: PlanContext) => BaselineAction[]; @@ -183,10 +181,6 @@ const migration: InstallerMigration = { title: 'Record first-time installer migration baseline', description: 'Classify existing install surfaces before destructive installer migrations run.', introducedIn: '1.50.0', - checksum: 'sha256:4ec58d35b30dbf39cc56e3972146086d8d31861ecd800cf0b37a7aa94fe74c2a', - legacyChecksums: [ - 'sha256:34608ea4e2f4e1c53b069604892860e603600d8573cc6a5584e4194044b48e67', - ], scopes: ['global', 'local'], destructive: false, plan: ({ configDir, runtime, baselineScan, classifyArtifact }: PlanContext): BaselineAction[] => { diff --git a/src/installer-migrations/001-legacy-orphan-files.cts b/src/installer-migrations/001-legacy-orphan-files.cts index 0f7e7e677..bd3dad449 100644 --- a/src/installer-migrations/001-legacy-orphan-files.cts +++ b/src/installer-migrations/001-legacy-orphan-files.cts @@ -31,8 +31,6 @@ interface InstallerMigration { title: string; description: string; introducedIn: string; - checksum: string; - legacyChecksums?: string[]; scopes: string[]; destructive: boolean; plan: (ctx: MigrationPlanContext) => MigrationAction[]; @@ -48,10 +46,6 @@ const migration: InstallerMigration = { title: 'Remove manifest-managed legacy orphan hook files', description: 'Remove legacy orphan hook files that are still manifest-managed.', introducedIn: '1.50.0', - checksum: 'sha256:e492698748a2436a12a55f0940f539b9bf651d8ffcac6f60cd856a6dabd6788c', - legacyChecksums: [ - 'sha256:4488e38c127a5225b31016918bcbc85ba3fd3139291ad407b94e76c03c0b89d3', - ], scopes: ['global', 'local'], destructive: true, // Retired generated hook files are removed only with manifest-managed diff --git a/src/installer-migrations/002-codex-legacy-hooks-json.cts b/src/installer-migrations/002-codex-legacy-hooks-json.cts index 8032bed09..4e3a8a214 100644 --- a/src/installer-migrations/002-codex-legacy-hooks-json.cts +++ b/src/installer-migrations/002-codex-legacy-hooks-json.cts @@ -48,8 +48,6 @@ interface InstallerMigration { title: string; description: string; introducedIn: string; - checksum: string; - legacyChecksums?: string[]; runtimes: string[]; scopes: string[]; destructive: boolean; @@ -112,10 +110,6 @@ const migration: InstallerMigration = { title: 'Remove legacy Codex hooks.json GSD hook registrations', description: 'Remove legacy Codex hooks.json GSD hook registrations after config.toml migration.', introducedIn: '1.50.0', - checksum: 'sha256:5ce55294aa02f25758f604a569c899a6d2d060299189f5f447f68d8033157058', - legacyChecksums: [ - 'sha256:41f1545704dc72dfc3ab019207677a8652e389b200e8c450d52317df5bc198da', - ], runtimes: ['codex'], scopes: ['global', 'local'], destructive: true, diff --git a/src/installer-migrations/003-rename-get-shit-done-to-gsd-core.cts b/src/installer-migrations/003-rename-get-shit-done-to-gsd-core.cts index 3db761dd9..13729c737 100644 --- a/src/installer-migrations/003-rename-get-shit-done-to-gsd-core.cts +++ b/src/installer-migrations/003-rename-get-shit-done-to-gsd-core.cts @@ -48,7 +48,6 @@ interface InstallerMigration { title: string; description: string; introducedIn: string; - checksum: string; scopes: string[]; destructive: boolean; plan(ctx: MigrationPlanContext): MigrationAction[]; @@ -81,7 +80,6 @@ const migration: InstallerMigration = { 'After the config dir rename from get-shit-done/ to gsd-core/ (#604), remove prior-manifest-managed files ' + // gsd-allow-legacy-name 'from the stale legacy directory during install (framework rollback restores them if install fails). User-added files are preserved.', introducedIn: '1.2.0', - checksum: 'sha256:3a9f1d97f64097fb313203d19c6d93a187a38df61dd299afa5eef73e16124e95', scopes: ['global', 'local'], destructive: true, plan(ctx: MigrationPlanContext): MigrationAction[] { diff --git a/tests/ci-test-scope.test.cjs b/tests/ci-test-scope.test.cjs index be00683ce..2ce8d4f1d 100644 --- a/tests/ci-test-scope.test.cjs +++ b/tests/ci-test-scope.test.cjs @@ -56,18 +56,9 @@ describe('ci-test-scope.cjs', () => { assert.strictEqual(result.code_changed, true); assert.strictEqual(result.full_matrix, true); assert.ok(result.targeted_tests.includes('tests/install.test.cjs')); - assert.ok(result.targeted_tests.includes('tests/installer-migration-checksum-compat.test.cjs')); assert.ok(result.targeted_tests.includes('tests/release-tarball-smoke.install.test.cjs')); }); - test('ADR-457 installer source changes wake scoped CI and checksum compatibility guard', () => { - const result = scopeFor(['src/installer-migrations/000-first-time-baseline.cts']); - assert.strictEqual(result.code_changed, true); - assert.strictEqual(result.full_matrix, true); - assert.ok(result.targeted_tests.includes('tests/installer-migration-checksum-compat.test.cjs')); - assert.ok(result.windows_tests.includes('tests/installer-migration-checksum-compat.test.cjs')); - }); - test('missing required CLI values fail with usage', () => { const r = spawnSync(process.execPath, [SCRIPT, '--files'], { cwd: ROOT, diff --git a/tests/fixtures/installer-migrations/published-checksums.json b/tests/fixtures/installer-migrations/published-checksums.json deleted file mode 100644 index d7784748e..000000000 --- a/tests/fixtures/installer-migrations/published-checksums.json +++ /dev/null @@ -1,49 +0,0 @@ -{ - "_comment": "Published installer migration checksums that must remain accepted by future releases. If an existing migration implementation changes, preserve compatibility here via legacyChecksums or create a new fix-forward migration id.", - "migrations": { - "2026-05-11-first-time-baseline-scan": { - "published": [ - { - "version": "1.2.0", - "checksum": "sha256:34608ea4e2f4e1c53b069604892860e603600d8573cc6a5584e4194044b48e67" - }, - { - "version": "1.3.0", - "checksum": "sha256:4ec58d35b30dbf39cc56e3972146086d8d31861ecd800cf0b37a7aa94fe74c2a" - } - ] - }, - "2026-05-11-legacy-orphan-files": { - "published": [ - { - "version": "1.2.0", - "checksum": "sha256:4488e38c127a5225b31016918bcbc85ba3fd3139291ad407b94e76c03c0b89d3" - }, - { - "version": "1.3.0", - "checksum": "sha256:e492698748a2436a12a55f0940f539b9bf651d8ffcac6f60cd856a6dabd6788c" - } - ] - }, - "2026-05-11-codex-legacy-hooks-json": { - "published": [ - { - "version": "1.2.0", - "checksum": "sha256:41f1545704dc72dfc3ab019207677a8652e389b200e8c450d52317df5bc198da" - }, - { - "version": "1.3.0", - "checksum": "sha256:5ce55294aa02f25758f604a569c899a6d2d060299189f5f447f68d8033157058" - } - ] - }, - "2026-06-02-rename-get-shit-done-to-gsd-core": { - "published": [ - { - "version": "1.3.0", - "checksum": "sha256:3a9f1d97f64097fb313203d19c6d93a187a38df61dd299afa5eef73e16124e95" - } - ] - } - } -} diff --git a/tests/installer-migration-checksum-compat.test.cjs b/tests/installer-migration-checksum-compat.test.cjs deleted file mode 100644 index 790bf208d..000000000 --- a/tests/installer-migration-checksum-compat.test.cjs +++ /dev/null @@ -1,94 +0,0 @@ -'use strict'; - -const test = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('node:fs'); -const os = require('node:os'); -const path = require('node:path'); - -const { - computeInstallerMigrationChecksum, - discoverInstallerMigrations, - planInstallerMigrations, -} = require('../gsd-core/bin/lib/installer-migrations.cjs'); -const { cleanup } = require('./helpers.cjs'); - -const FIXTURE = require('./fixtures/installer-migrations/published-checksums.json'); -const MIGRATIONS_DIR = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'installer-migrations'); - -function createConfigDir() { - const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-migration-checksum-compat-')); - fs.writeFileSync( - path.join(configDir, 'gsd-file-manifest.json'), - JSON.stringify({ version: 'compat-fixture', timestamp: '2026-06-04T00:00:00.000Z', mode: 'full', files: {} }, null, 2), - 'utf8' - ); - return configDir; -} - -function writeAppliedState(configDir, migration, checksum, version) { - fs.writeFileSync( - path.join(configDir, 'gsd-install-state.json'), - JSON.stringify({ - schemaVersion: 1, - appliedMigrations: [ - { - id: migration.id, - checksum, - appliedAt: '2026-06-04T00:00:00.000Z', - packageVersion: version, - journal: 'gsd-migration-journal/published-compat-fixture.json', - }, - ], - }, null, 2), - 'utf8' - ); -} - -function planContextFor(migration) { - return { - runtime: Array.isArray(migration.runtimes) && migration.runtimes.length > 0 ? migration.runtimes[0] : 'claude', - scope: Array.isArray(migration.scopes) && migration.scopes.length > 0 ? migration.scopes[0] : 'global', - }; -} - -test('shipped installer migration checksums remain accepted for upgrade compatibility', () => { - const migrations = discoverInstallerMigrations({ migrationsDir: MIGRATIONS_DIR }); - const byId = new Map(migrations.map((migration) => [migration.id, migration])); - const fixtureIds = new Set(Object.keys(FIXTURE.migrations)); - - for (const migration of migrations) { - assert.ok( - fixtureIds.has(migration.id), - `current migration must be pinned in published-checksums.json: ${migration.id}` - ); - } - - for (const [migrationId, fixture] of Object.entries(FIXTURE.migrations)) { - const migration = byId.get(migrationId); - assert.ok(migration, `published migration fixture no longer exists: ${migrationId}`); - - const currentChecksum = computeInstallerMigrationChecksum(migration); - assert.ok( - fixture.published.some((entry) => entry.checksum === currentChecksum), - `${migrationId} current checksum ${currentChecksum} is not pinned in published-checksums.json` - ); - - for (const { version, checksum } of fixture.published) { - const configDir = createConfigDir(); - try { - writeAppliedState(configDir, migration, checksum, version); - assert.doesNotThrow( - () => planInstallerMigrations({ - configDir, - migrations: [migration], - ...planContextFor(migration), - }), - `${migrationId} must accept checksum ${checksum} from ${version}` - ); - } finally { - cleanup(configDir); - } - } - } -}); diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index 23c436081..4367f8f15 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -10,6 +10,7 @@ const { classifyArtifact, discoverInstallerMigrations, INSTALL_STATE_NAME, + migrationChecksum, planInstallerMigrations, readInstallState, runInstallerMigrations, @@ -482,6 +483,46 @@ test('computes each migration checksum once per planned migration', (t) => { assert.equal(checksumReads, 1); }); +test('tolerates an applied-migration checksum drift instead of aborting the upgrade', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + // A migration that a prior release recorded as applied under a DIFFERENT body, + // so the stored checksum no longer matches the current computed checksum. + const migration = migrationRecord({ id: '2026-05-11-remove-old-hook' }); + writeInstallState(configDir, { + schemaVersion: 1, + appliedMigrations: [ + { + id: '2026-05-11-remove-old-hook', + appliedAt: '2026-01-01T00:00:00.000Z', + journal: null, + checksum: 'sha256:stale-pre-1-3-0-value', + }, + ], + }); + + // Planning must NOT throw, must skip the already-applied migration, and must + // surface the drift on the plan for downstream reconciliation. + let plan; + assert.doesNotThrow(() => { + plan = planInstallerMigrations({ + configDir, + migrations: [migration], + scope: 'global', + now: () => '2026-05-11T00:00:00.000Z', + }); + }); + assert.deepEqual(plan.pendingMigrationIds, []); + assert.equal(plan.actions.length, 0); + assert.ok(Array.isArray(plan.checksumDrift)); + const drift = plan.checksumDrift.find((d) => d.id === '2026-05-11-remove-old-hook'); + assert.ok(drift, 'expected checksum drift to be reported for the applied migration'); + assert.equal(drift.storedChecksum, 'sha256:stale-pre-1-3-0-value'); + assert.match(drift.currentChecksum, /^sha256:/); + assert.notEqual(drift.currentChecksum, drift.storedChecksum); +}); + test('classifies large files without loading the whole file through readFileSync', (t) => { const configDir = createTempInstall(); const originalReadFileSync = fs.readFileSync; @@ -1063,7 +1104,7 @@ test('marks zero-action pending migrations as applied', () => { } }); -test('refuses to plan an already-applied migration whose checksum changed', () => { +test('surfaces checksum drift for an already-applied migration without aborting', () => { const configDir = createTempInstall(); try { writeManifest(configDir, {}); @@ -1079,8 +1120,9 @@ test('refuses to plan an already-applied migration whose checksum changed', () = ], }); - assert.throws( - () => planInstallerMigrations({ + let plan; + assert.doesNotThrow(() => { + plan = planInstallerMigrations({ configDir, migrations: [ migrationRecord({ @@ -1089,9 +1131,14 @@ test('refuses to plan an already-applied migration whose checksum changed', () = }), ], scope: 'global', - }), - /applied migration checksum changed/ - ); + }); + }); + assert.deepEqual(plan.pendingMigrationIds, []); + assert.ok(Array.isArray(plan.checksumDrift)); + const drift = plan.checksumDrift.find((d) => d.id === '2026-05-11-remove-old-hook'); + assert.ok(drift, 'expected drift entry for the applied migration'); + assert.equal(drift.storedChecksum, 'sha256:old-definition'); + assert.equal(drift.currentChecksum, 'sha256:new-definition'); } finally { cleanup(configDir); } @@ -1385,3 +1432,146 @@ test('skips runtime-specific migration records for other runtimes', () => { cleanup(configDir); } }); + +// --------------------------------------------------------------------------- +// Checksum-baseline guardrail (issue #670) +// +// Shipped installer-migration bodies are immutable: editing a released body +// breaks the stored checksum for any user who has already applied that +// migration, which was the root cause of issue #670. +// +// This test locks every shipped migration to its committed checksum so that CI +// catches accidental body edits. When you INTENTIONALLY change the behaviour +// of a migration you must add a NEW fix-forward migration id instead; if for +// some extraordinary reason you truly need to update an existing baseline, add +// the new checksum here with a comment explaining why. +// +// Mechanism: compute each migration's checksum directly via the exported +// migrationChecksum() (scope-independent) and assert it matches the committed +// baseline. This is simpler and more robust than the previous plan()-based +// approach because it doesn't depend on runtime/scope filtering. +// --------------------------------------------------------------------------- +test('shipped installer-migration checksums are locked to a committed baseline (issue #670 guardrail)', () => { + // Committed baseline — update ONLY when adding a new migration or performing an + // extraordinary intentional body change (add a comment explaining why). Editing a + // shipped migration body breaks the stored checksum for everyone who already applied + // it (root cause of #670) — add a NEW fix-forward migration id instead. + const EXPECTED_CHECKSUMS = { + '2026-05-11-first-time-baseline-scan': + 'sha256:4ec58d35b30dbf39cc56e3972146086d8d31861ecd800cf0b37a7aa94fe74c2a', + '2026-05-11-legacy-orphan-files': + 'sha256:e492698748a2436a12a55f0940f539b9bf651d8ffcac6f60cd856a6dabd6788c', + '2026-05-11-codex-legacy-hooks-json': + 'sha256:5ce55294aa02f25758f604a569c899a6d2d060299189f5f447f68d8033157058', + '2026-06-02-rename-get-shit-done-to-gsd-core': + 'sha256:3a9f1d97f64097fb313203d19c6d93a187a38df61dd299afa5eef73e16124e95', + }; + + const { DEFAULT_MIGRATIONS_DIR, migrationChecksum: computeChecksum } = require('../gsd-core/bin/lib/installer-migrations.cjs'); + const migrations = discoverInstallerMigrations({ migrationsDir: DEFAULT_MIGRATIONS_DIR }); + const discoveredIds = new Set(migrations.map((m) => m.id)); + + // No stale baseline entries. + for (const id of Object.keys(EXPECTED_CHECKSUMS)) { + assert.ok(discoveredIds.has(id), + `EXPECTED_CHECKSUMS has a stale entry for '${id}' — that migration no longer exists; remove it`); + } + // Every discovered migration has a committed baseline entry. + for (const id of discoveredIds) { + assert.ok(Object.prototype.hasOwnProperty.call(EXPECTED_CHECKSUMS, id), + `new migration '${id}' has no committed checksum baseline — add it to EXPECTED_CHECKSUMS in tests/installer-migrations.test.cjs`); + } + // Core lock: each shipped migration's current checksum must match its committed baseline, + // computed directly (scope-independent). + for (const m of migrations) { + assert.strictEqual(computeChecksum(m), EXPECTED_CHECKSUMS[m.id], + `'${m.id}' body changed — its checksum drifted from the committed baseline; ` + + `add a NEW fix-forward migration id instead of editing a shipped migration body, ` + + `or intentionally update the baseline in EXPECTED_CHECKSUMS`); + } +}); + +test('reconciles a drifted applied-migration checksum into install state on apply', () => { + const configDir = createTempInstall(); + try { + // Set up: one already-applied migration with a stale checksum, one pending migration + // that will produce an action (so applyInstallerMigrationPlan writes state). + const alreadyAppliedMigration = migrationRecord({ + id: '2026-05-11-already-applied-with-drift', + title: 'Already applied with drift', + description: 'Already applied with drift', + scopes: ['global'], + destructive: false, + plan: () => [], + }); + const pendingMigration = migrationRecord({ + id: '2026-05-11-pending-to-trigger-apply', + title: 'Pending migration', + description: 'Pending migration', + scopes: ['global'], + destructive: true, + plan: () => [ + { + type: 'remove-managed', + relPath: 'hooks/old-hook.js', + reason: 'retiring hook', + ownershipEvidence: 'test fixture manifest-managed hook', + }, + ], + }); + + writeFile(configDir, 'hooks/old-hook.js', 'managed hook\n'); + writeManifest(configDir, { + 'hooks/old-hook.js': sha256('managed hook\n'), + }); + + // Seed install state: alreadyAppliedMigration recorded with a STALE checksum. + writeInstallState(configDir, { + schemaVersion: 1, + appliedMigrations: [ + { + id: alreadyAppliedMigration.id, + appliedAt: '2026-01-01T00:00:00.000Z', + journal: null, + checksum: 'sha256:stale-old', + }, + ], + }); + + const plan = planInstallerMigrations({ + configDir, + migrations: [alreadyAppliedMigration, pendingMigration], + scope: 'global', + now: () => '2026-05-11T00:00:00.000Z', + }); + + // The already-applied migration should appear in checksumDrift. + const drift = plan.checksumDrift.find((d) => d.id === alreadyAppliedMigration.id); + assert.ok(drift, 'expected checksumDrift entry for the already-applied migration'); + assert.equal(drift.storedChecksum, 'sha256:stale-old'); + + // Apply the plan (the pending migration has an action, so this writes state). + applyInstallerMigrationPlan({ + configDir, + plan, + now: () => '2026-05-11T00:00:01.000Z', + }); + + // Re-read install state and assert the stale checksum was reconciled. + const stateAfter = readInstallState(configDir); + const reconciledEntry = stateAfter.appliedMigrations.find( + (entry) => entry.id === alreadyAppliedMigration.id + ); + assert.ok(reconciledEntry, 'expected the already-applied entry to still be in install state'); + const expectedChecksum = migrationChecksum(alreadyAppliedMigration); + assert.strictEqual( + reconciledEntry.checksum, + expectedChecksum, + `expected checksum to be reconciled to current value (${expectedChecksum}), not the stale 'sha256:stale-old'` + ); + assert.notEqual(reconciledEntry.checksum, 'sha256:stale-old', + 'stale checksum must not remain after apply'); + } finally { + cleanup(configDir); + } +});