From 041e94c43b197370e6d7f8ea32fa973fcdd89a7f Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 11 May 2026 14:40:57 -0400 Subject: [PATCH] fix: roll back installer migration state --- bin/install.js | 11 +++- .../bin/lib/installer-migrations.cjs | 59 +++++++++++++++++++ tests/installer-migrations.test.cjs | 52 ++++++++++++++++ 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/bin/install.js b/bin/install.js index b2b9daae9..ba9fab9a9 100755 --- a/bin/install.js +++ b/bin/install.js @@ -7634,6 +7634,13 @@ function install(isGlobal, runtime = 'claude') { // Track installation failures const failures = []; + let installerMigrationResult = null; + const rollbackInstallerMigrations = () => { + if (!installerMigrationResult || typeof installerMigrationResult.rollback !== 'function') return; + const rollback = installerMigrationResult.rollback; + installerMigrationResult = null; + rollback(); + }; // Save any locally modified GSD files before they get wiped. // The pristine context lets saveLocalPatches populate gsd-pristine/ via @@ -7647,7 +7654,7 @@ function install(isGlobal, runtime = 'claude') { }); // Run manifest-backed cleanup migrations before package materialization. - runInstallerMigrations({ configDir: targetDir }); + installerMigrationResult = runInstallerMigrations({ configDir: targetDir }); // #3245 — Codex idempotent rollback. Capture pre-install state of ALL // directories and files GSD will mutate so that any post-install validation @@ -8439,6 +8446,7 @@ function install(isGlobal, runtime = 'claude') { // #3245 CR finding 2 — any throw in the pre-config install operations (skills copy, // agents copy, VERSION write, manifest write, etc.) triggers the Codex pre-config // rollback so the caller is never left in a partially-installed state. + rollbackInstallerMigrations(); if (_codexPreConfigRollback) { _codexPreConfigRollback(); } @@ -8471,6 +8479,7 @@ function install(isGlobal, runtime = 'claude') { // existence checks. Safe to call before any snapshots are captured (variables // default to empty Set / null). Does NOT touch non-gsd-* user content. const restoreCodexSnapshot = () => { + rollbackInstallerMigrations(); // 1. config.toml if (codexConfigPreInstallSnapshot !== null) { try { fs.writeFileSync(codexConfigPathPreInstall, codexConfigPreInstallSnapshot); } diff --git a/get-shit-done/bin/lib/installer-migrations.cjs b/get-shit-done/bin/lib/installer-migrations.cjs index 966da17f2..e0b3143ce 100644 --- a/get-shit-done/bin/lib/installer-migrations.cjs +++ b/get-shit-done/bin/lib/installer-migrations.cjs @@ -232,6 +232,10 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t actions: [], }; const rollback = []; + const installStatePath = path.join(configDir, INSTALL_STATE_NAME); + const previousInstallStateBytes = fs.existsSync(installStatePath) + ? fs.readFileSync(installStatePath) + : null; try { for (const action of plan.actions) { @@ -285,6 +289,13 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t return { appliedMigrationIds: journal.appliedMigrationIds, journalRelPath, + rollback: () => rollbackAppliedMigrationResult({ + configDir, + journal, + journalPath, + rollbackRoot, + previousInstallStateBytes, + }), }; } catch (error) { const rollbackFailures = []; @@ -311,6 +322,54 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t } } +function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, previousInstallStateBytes }) { + const failures = []; + for (const action of [...journal.actions].reverse()) { + if (!action.rollbackRelPath) continue; + const rollbackPath = path.join(configDir, action.rollbackRelPath); + const dest = path.join(configDir, action.relPath); + try { + if (fs.existsSync(rollbackPath)) { + fs.mkdirSync(path.dirname(dest), { recursive: true }); + fs.copyFileSync(rollbackPath, dest); + } + } catch (error) { + failures.push({ relPath: action.relPath, error: error.message }); + } + if (action.backupRelPath) { + try { + fs.rmSync(path.join(configDir, action.backupRelPath), { force: true }); + } catch { + // backup cleanup is best-effort; preserve restore failures above + } + } + } + + try { + if (previousInstallStateBytes === null) { + fs.rmSync(path.join(configDir, INSTALL_STATE_NAME), { force: true }); + } else { + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, INSTALL_STATE_NAME), previousInstallStateBytes); + } + } catch (error) { + failures.push({ relPath: INSTALL_STATE_NAME, error: error.message }); + } + + try { + fs.rmSync(journalPath, { force: true }); + fs.rmSync(rollbackRoot, { recursive: true, force: true }); + } catch { + // journal cleanup is best-effort; the rollback above is the safety-critical part + } + + if (failures.length > 0) { + const error = new Error('migration rollback incomplete'); + error.rollbackFailures = failures; + throw error; + } +} + function runInstallerMigrations({ configDir, migrationsDir = DEFAULT_MIGRATIONS_DIR, diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index a424a014c..b13de19ff 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -208,6 +208,58 @@ test('applies an unblocked plan with a journal and install-state update', () => } }); +test('rollback handle restores files and install state after a successful apply', () => { + const configDir = createTempInstall(); + try { + writeFile(configDir, 'hooks/old-hook.js', 'managed hook\n'); + writeManifest(configDir, { + 'hooks/old-hook.js': sha256('managed hook\n'), + }); + writeInstallState(configDir, { + schemaVersion: 1, + appliedMigrations: [ + { + id: 'already-applied', + appliedAt: '2026-05-10T00:00:00.000Z', + journal: 'gsd-migration-journal/prior.json', + }, + ], + }); + + const plan = planInstallerMigrations({ + configDir, + migrations: [ + { + id: '2026-05-11-remove-old-hook', + description: 'Remove retired hook', + plan: () => [ + { + type: 'remove-managed', + relPath: 'hooks/old-hook.js', + reason: 'retired hook', + }, + ], + }, + ], + now: () => '2026-05-11T00:00:00.000Z', + }); + + const result = applyInstallerMigrationPlan({ + configDir, + plan, + now: () => '2026-05-11T00:00:01.000Z', + }); + + result.rollback(); + + assert.equal(fs.readFileSync(path.join(configDir, 'hooks/old-hook.js'), 'utf8'), 'managed hook\n'); + assert.deepEqual(readInstallState(configDir).appliedMigrations.map((entry) => entry.id), ['already-applied']); + assert.equal(fs.existsSync(path.join(configDir, result.journalRelPath)), false); + } finally { + cleanup(configDir); + } +}); + test('rolls back touched files and leaves state unchanged when apply fails', () => { const configDir = createTempInstall(); try {