From 0b621298474963cdb977e1828c3dc6bb1704cfb5 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 11 May 2026 09:11:11 -0400 Subject: [PATCH 1/5] Wire installer migrations into install flow --- bin/install.js | 23 +++ docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 3 +- docs/installer-migrations.md | 15 ++ .../bin/lib/installer-migration-report.cjs | 50 +++++ .../000-first-time-baseline.cjs | 57 +++++- .../001-legacy-orphan-files.cjs | 4 + ...ler-migration-install-integration.test.cjs | 183 ++++++++++++++++++ tests/installer-migration-report.test.cjs | 93 +++++++++ tests/installer-migrations.test.cjs | 50 +++++ 10 files changed, 472 insertions(+), 7 deletions(-) create mode 100644 get-shit-done/bin/lib/installer-migration-report.cjs create mode 100644 tests/installer-migration-install-integration.test.cjs create mode 100644 tests/installer-migration-report.test.cjs diff --git a/bin/install.js b/bin/install.js index 7cc17d601..750b62ed6 100755 --- a/bin/install.js +++ b/bin/install.js @@ -79,6 +79,10 @@ const { const { runInstallerMigrations, } = require(path.join(_gsdLibDir, 'installer-migrations.cjs')); +const { + assertInstallerMigrationsUnblocked, + summarizeInstallerMigrationResult, +} = require(path.join(_gsdLibDir, 'installer-migration-report.cjs')); // Parse args const args = process.argv.slice(2); @@ -7467,6 +7471,17 @@ function reportLocalPatches(configDir, runtime = 'claude') { return meta.files || []; } +function reportInstallerMigrationResult(result) { + const summary = summarizeInstallerMigrationResult(result); + if (!summary.hasReportableActions) return; + + console.log(` ${green}✓${reset} Installer migrations`); + for (const row of summary.rows) { + const reason = row.reason ? ` — ${row.reason}` : ''; + console.log(` ${row.label} ${dim}${row.relPath}${reset}${reason}`); + } +} + function install(isGlobal, runtime = 'claude') { const isOpencode = runtime === 'opencode'; const isGemini = runtime === 'gemini'; @@ -7722,11 +7737,19 @@ function install(isGlobal, runtime = 'claude') { // Run manifest-backed cleanup migrations after rollback snapshots exist and // before package materialization. Codex rollback paths invoke the migration // rollback handle if a later install step fails. + // + // Runtime scope comes from docs/installer-migrations.md#runtime-configuration-contract-registry: + // every supported runtime uses this same planner/apply/report path, while + // individual migration records decide whether a runtime-specific config + // rewrite is allowed by that runtime's documented ownership boundary. installerMigrationResult = runInstallerMigrations({ configDir: targetDir, runtime, scope: isGlobal ? 'global' : 'local', + baselineScan: true, }); + reportInstallerMigrationResult(installerMigrationResult); + assertInstallerMigrationsUnblocked(installerMigrationResult); // #3245 CR finding 2 — wrap the pre-config install operations in a try/catch so // that ANY throw between snapshot capture and the Codex config block triggers rollback. diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 1e3b768e7..c2bcdbc84 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -277,6 +277,7 @@ "init-command-router.cjs", "init.cjs", "install-profiles.cjs", + "installer-migration-report.cjs", "installer-migrations.cjs", "intel.cjs", "learnings.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index f89271737..9a50be43a 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -359,7 +359,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (51 shipped) +## CLI Modules (52 shipped) Full listing: `get-shit-done/bin/lib/*.cjs`. @@ -385,6 +385,7 @@ Full listing: `get-shit-done/bin/lib/*.cjs`. | `init-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools init` | | `init.cjs` | Compound context loading for each workflow type | | `install-profiles.cjs` | Install profile allowlist + skill staging for `--minimal` install (#2762); single source of truth for which `gsd-*` skills/agents land in runtime config dirs | +| `installer-migration-report.cjs` | Installer migration report projection and blocked-action guard for install/update integration | | `installer-migrations.cjs` | Installer migration planning, artifact classification, install-state persistence, journaled apply, and rollback helpers | | `intel.cjs` | Codebase intel store backing `/gsd-map-codebase --query` and `gsd-intel-updater` | | `learnings.cjs` | Cross-phase learnings extraction for `/gsd-extract-learnings` | diff --git a/docs/installer-migrations.md b/docs/installer-migrations.md index 64a39a7b5..8f07804fb 100644 --- a/docs/installer-migrations.md +++ b/docs/installer-migrations.md @@ -254,6 +254,21 @@ The installer runs migrations before materializing the new package payload. 11. Write the new manifest and install state. 12. Report backups, preserved files, removed stale files, and skipped actions. +The Phase 4 install integration wires this flow into the normal install/update +entry point for every supported runtime: Claude Code, Antigravity, Augment, +Cline, CodeBuddy, Codex, Copilot, Cursor, Gemini, Hermes Agent, Kilo, OpenCode, +Qwen Code, Trae, and Windsurf. The installer invokes the same migration runner +with `baselineScan: true`, reports the projected action rows, applies safe +non-interactive actions before materialization, writes install state after a +successful apply, and fails before writing new package files when the runner +returns blocked user-choice actions. + +Phase 1-3 built the planning, apply, rollback, install-state, baseline, and +migration-record mechanics. Those phases did not prove the normal install entry +point across every runtime. Phase 4 owns that guardrail with an all-runtime +install matrix that exercises safe managed cleanup and blocked user-choice +artifacts for each runtime above. + If any apply step fails, the executor uses the journal to restore modified paths where possible. Rollback must never delete files that were not created or modified by the current installer run. diff --git a/get-shit-done/bin/lib/installer-migration-report.cjs b/get-shit-done/bin/lib/installer-migration-report.cjs new file mode 100644 index 000000000..d0ff12275 --- /dev/null +++ b/get-shit-done/bin/lib/installer-migration-report.cjs @@ -0,0 +1,50 @@ +'use strict'; + +function installerMigrationActionLabel(action) { + if (!action || !action.type) return 'skipped'; + if (action.type === 'backup-and-remove') return 'backed up and removed'; + if (action.type === 'remove-managed') return 'removed'; + if (action.type === 'rewrite-json') return action.deleteIfEmpty ? 'rewrote or removed' : 'rewrote'; + if (action.type === 'record-baseline') return 'recorded'; + if (action.type === 'baseline-preserve-user') return 'preserved'; + if (action.type === 'preserve-user') return 'preserved'; + if (action.type === 'prompt-user') return 'blocked'; + return 'skipped'; +} + +function blockedInstallerMigrationActions(result) { + if (result && Array.isArray(result.blocked)) return result.blocked; + const plan = result && result.plan; + if (plan && Array.isArray(plan.blocked)) return plan.blocked; + return []; +} + +function summarizeInstallerMigrationResult(result) { + const plan = result && result.plan; + const actions = plan && Array.isArray(plan.actions) ? plan.actions : []; + const blocked = blockedInstallerMigrationActions(result); + const blockedSet = new Set(blocked); + + return { + hasReportableActions: actions.length > 0 || blocked.length > 0, + blocked, + rows: actions.map((action) => ({ + label: blockedSet.has(action) ? 'blocked' : installerMigrationActionLabel(action), + relPath: action.relPath, + reason: action.reason || '', + action, + })), + }; +} + +function assertInstallerMigrationsUnblocked(result) { + const blocked = blockedInstallerMigrationActions(result); + if (blocked.length === 0) return; + const paths = blocked.map((action) => action.relPath).join(', '); + throw new Error(`installer migration blocked pending user choice: ${paths}`); +} + +module.exports = { + assertInstallerMigrationsUnblocked, + summarizeInstallerMigrationResult, +}; diff --git a/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs b/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs index 56f790b27..8d03a5359 100644 --- a/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs +++ b/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs @@ -2,10 +2,16 @@ const fs = require('fs'); const path = require('path'); -const crypto = require('crypto'); const BASELINE_MIGRATION_ID = '2026-05-11-first-time-baseline-scan'; +// Runtime install surfaces must stay aligned with: +// - docs/installer-migrations.md#runtime-configuration-contract-registry +// - docs/ARCHITECTURE.md#runtime-install-contract-matrix +// +// The registry rows are based on each runtime's upstream loader docs where +// available. Source-limited rows are intentionally conservative: scan generated +// files GSD materializes, but do not infer ownership of undocumented host config. const RUNTIME_SURFACES = { claude: ['get-shit-done', 'commands/gsd', 'skills', 'agents', 'hooks', 'settings.json'], codex: ['get-shit-done', 'skills', 'agents', 'hooks', 'config.toml', 'hooks.json'], @@ -36,10 +42,7 @@ const USER_OWNED_PATHS = new Set([ 'commands/gsd/dev-preferences.md', 'skills/gsd-dev-preferences/SKILL.md', ]); - -function sha256File(filePath) { - return crypto.createHash('sha256').update(fs.readFileSync(filePath)).digest('hex'); -} +let knownGeneratedAgentNames = null; function normalizeRelPath(relPath) { return relPath.replace(/\\/g, '/').replace(/^\/+/, ''); @@ -91,6 +94,36 @@ function isUserOwnedBaselinePath(relPath) { return false; } +function listKnownGeneratedAgentNames() { + if (knownGeneratedAgentNames) return knownGeneratedAgentNames; + + knownGeneratedAgentNames = new Set(); + const agentsDir = path.resolve(__dirname, '..', '..', '..', '..', 'agents'); + try { + for (const entry of fs.readdirSync(agentsDir, { withFileTypes: true })) { + if (entry.isFile() && entry.name.startsWith('gsd-') && entry.name.endsWith('.md')) { + knownGeneratedAgentNames.add(entry.name.replace(/\.md$/, '')); + } + } + } catch { + // If the source agent directory is unavailable, fail closed and treat + // GSD-looking agent files as user-choice artifacts. + } + + return knownGeneratedAgentNames; +} + +function isKnownGeneratedAgentPath(relPath, runtime) { + const parts = relPath.split('/'); + if (parts.length !== 2 || parts[0] !== 'agents') return false; + const fileName = parts[1]; + const extension = path.posix.extname(fileName); + if (extension !== '.md' && !(runtime === 'codex' && extension === '.toml')) return false; + + const agentName = fileName.slice(0, -extension.length); + return listKnownGeneratedAgentNames().has(agentName); +} + function isStaleGsdLookingPath(relPath) { const baseName = path.posix.basename(relPath); if (/^gsd[-_]/.test(baseName)) return true; @@ -129,7 +162,19 @@ module.exports = { continue; } - const currentHash = fs.existsSync(path.join(configDir, relPath)) ? sha256File(path.join(configDir, relPath)) : null; + const currentHash = artifact.currentHash; + if (isKnownGeneratedAgentPath(relPath, runtime)) { + actions.push({ + type: 'record-baseline', + relPath, + reason: 'known installer-generated agent included in first-time migration baseline', + classification: artifact.classification, + originalHash: artifact.originalHash, + currentHash, + }); + continue; + } + if (isUserOwnedBaselinePath(relPath)) { actions.push({ type: 'baseline-preserve-user', diff --git a/get-shit-done/bin/lib/installer-migrations/001-legacy-orphan-files.cjs b/get-shit-done/bin/lib/installer-migrations/001-legacy-orphan-files.cjs index d216bb366..b8868288a 100644 --- a/get-shit-done/bin/lib/installer-migrations/001-legacy-orphan-files.cjs +++ b/get-shit-done/bin/lib/installer-migrations/001-legacy-orphan-files.cjs @@ -12,6 +12,10 @@ module.exports = { introducedIn: '1.50.0', scopes: ['global', 'local'], destructive: true, + // Retired generated hook files are removed only with manifest-managed + // evidence. This follows docs/installer-migrations.md#ownership and avoids + // relying on whether a runtime currently registers host hook config in the + // runtime contract registry. plan: ({ classifyArtifact }) => { const actions = []; for (const relPath of LEGACY_ORPHAN_FILES) { diff --git a/tests/installer-migration-install-integration.test.cjs b/tests/installer-migration-install-integration.test.cjs new file mode 100644 index 000000000..4ff6b82aa --- /dev/null +++ b/tests/installer-migration-install-integration.test.cjs @@ -0,0 +1,183 @@ +/** + * Phase 4 installer migration integration tests. + * + * These exercise the public install() entry point so the migration runner is + * pinned at the install/update seam, not just as a standalone library. + */ + +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { spawnSync } = require('node:child_process'); +const fs = require('node:fs'); +const path = require('node:path'); +const crypto = require('node:crypto'); + +const installModule = require('../bin/install.js'); +const { install } = installModule; +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const installScript = path.join(__dirname, '..', 'bin', 'install.js'); +const SUPPORTED_RUNTIMES = installModule.allRuntimes; + +function sha256(content) { + return crypto.createHash('sha256').update(content).digest('hex'); +} + +function writeFile(root, relPath, content) { + const fullPath = path.join(root, relPath); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, content, 'utf8'); +} + +function writeManifest(root, files) { + fs.writeFileSync( + path.join(root, 'gsd-file-manifest.json'), + JSON.stringify({ + version: '1.49.0', + timestamp: '2026-05-10T00:00:00.000Z', + mode: 'full', + files, + }, null, 2), + 'utf8' + ); +} + +function withEnv(key, value, fn) { + const previous = process.env[key]; + process.env[key] = value; + try { + return fn(); + } finally { + if (previous == null) delete process.env[key]; + else process.env[key] = previous; + } +} + +function captureConsole(fn) { + const originalLog = console.log; + const originalWarn = console.warn; + const lines = []; + console.log = (...args) => { lines.push(args.join(' ')); }; + console.warn = (...args) => { lines.push(args.join(' ')); }; + try { + return { value: fn(), output: lines.join('\n') }; + } finally { + console.log = originalLog; + console.warn = originalWarn; + } +} + +function stripAnsi(value) { + return value.replace(/\x1b\[[0-9;]*m/g, ''); +} + +function runInstallerCli(runtime, targetDir) { + const env = { ...process.env }; + delete env.GSD_TEST_MODE; + + return spawnSync( + process.execPath, + [installScript, `--${runtime}`, '--global', '--config-dir', targetDir, '--minimal', '--no-sdk'], + { + encoding: 'utf8', + env, + } + ); +} + +describe('installer migration install integration', { concurrency: false }, () => { + let tmpRoot; + let codexHome; + + beforeEach(() => { + tmpRoot = createTempDir('gsd-install-migrations-'); + codexHome = path.join(tmpRoot, '.codex'); + fs.mkdirSync(codexHome, { recursive: true }); + }); + + afterEach(() => { + cleanup(tmpRoot); + }); + + test('reports applied migration actions before package materialization', () => { + writeFile(codexHome, 'hooks/statusline.js', 'legacy managed hook\n'); + writeManifest(codexHome, { + 'hooks/statusline.js': sha256('legacy managed hook\n'), + }); + + const { output } = captureConsole(() => + withEnv('CODEX_HOME', codexHome, () => install(true, 'codex')) + ); + + const plainOutput = stripAnsi(output); + assert.match(plainOutput, /Installer migrations/); + assert.match(plainOutput, /removed\s+hooks\/statusline\.js/); + assert.ok( + plainOutput.indexOf('Installer migrations') < plainOutput.indexOf('Installed get-shit-done'), + 'migration report should appear before package materialization' + ); + assert.equal(fs.existsSync(path.join(codexHome, 'hooks/statusline.js')), false); + }); + + test('blocks install before materialization when baseline needs explicit user choice', () => { + writeFile(codexHome, 'hooks/gsd-retired-hook.js', 'old gsd hook\n'); + + assert.throws( + () => captureConsole(() => + withEnv('CODEX_HOME', codexHome, () => install(true, 'codex')) + ), + /installer migration blocked/ + ); + + assert.equal(fs.readFileSync(path.join(codexHome, 'hooks/gsd-retired-hook.js'), 'utf8'), 'old gsd hook\n'); + assert.equal(fs.existsSync(path.join(codexHome, 'skills')), false); + assert.equal(fs.existsSync(path.join(codexHome, 'get-shit-done', 'VERSION')), false); + }); + + for (const runtime of SUPPORTED_RUNTIMES) { + test(`runs managed cleanup migrations for ${runtime}`, () => { + const targetDir = path.join(tmpRoot, `.${runtime}-managed-cleanup`); + fs.mkdirSync(targetDir, { recursive: true }); + writeFile(targetDir, 'hooks/statusline.js', 'legacy managed hook\n'); + writeManifest(targetDir, { + 'hooks/statusline.js': sha256('legacy managed hook\n'), + }); + + const result = runInstallerCli(runtime, targetDir); + + assert.equal(result.status, 0, result.stderr || result.stdout); + const output = stripAnsi(`${result.stdout}\n${result.stderr}`); + assert.match(output, /Installer migrations/); + assert.match(output, /removed\s+hooks\/statusline\.js/); + assert.equal(fs.existsSync(path.join(targetDir, 'hooks/statusline.js')), false); + const installState = JSON.parse(fs.readFileSync(path.join(targetDir, 'gsd-install-state.json'), 'utf8')); + assert.ok( + installState.appliedMigrations.some((entry) => entry.id === '2026-05-11-legacy-orphan-files'), + 'successful install should write install state for the applied cleanup migration' + ); + }); + + test(`blocks ambiguous GSD-looking user-choice artifacts for ${runtime}`, () => { + const targetDir = path.join(tmpRoot, `.${runtime}-blocked`); + fs.mkdirSync(targetDir, { recursive: true }); + writeFile(targetDir, 'get-shit-done/gsd-retired-tool.cjs', 'old ambiguous artifact\n'); + + const result = runInstallerCli(runtime, targetDir); + + assert.notEqual(result.status, 0, 'install should fail before materialization'); + const output = stripAnsi(`${result.stdout}\n${result.stderr}`); + assert.match(output, /Installer migrations/); + assert.match(output, /blocked\s+get-shit-done\/gsd-retired-tool\.cjs/); + assert.match(output, /installer migration blocked/); + assert.equal( + fs.readFileSync(path.join(targetDir, 'get-shit-done/gsd-retired-tool.cjs'), 'utf8'), + 'old ambiguous artifact\n' + ); + assert.equal(fs.existsSync(path.join(targetDir, 'get-shit-done', 'VERSION')), false); + }); + } +}); diff --git a/tests/installer-migration-report.test.cjs b/tests/installer-migration-report.test.cjs new file mode 100644 index 000000000..0770c33ae --- /dev/null +++ b/tests/installer-migration-report.test.cjs @@ -0,0 +1,93 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { + assertInstallerMigrationsUnblocked, + summarizeInstallerMigrationResult, +} = require('../get-shit-done/bin/lib/installer-migration-report.cjs'); + +test('summarizes every installer migration report category', () => { + const blockedAction = { + type: 'prompt-user', + relPath: 'hooks/gsd-retired-hook.js', + reason: 'needs a user choice', + }; + const result = { + blocked: [blockedAction], + plan: { + actions: [ + { + type: 'remove-managed', + relPath: 'hooks/statusline.js', + reason: 'retired hook', + }, + { + type: 'backup-and-remove', + relPath: 'hooks/modified.js', + reason: 'modified managed hook retired', + }, + { + type: 'baseline-preserve-user', + relPath: 'hooks/custom.js', + reason: 'user-owned hook', + }, + { + type: 'unknown-action', + relPath: 'hooks/unknown.js', + reason: 'unsupported in this installer', + }, + blockedAction, + ], + }, + }; + + assert.deepEqual( + summarizeInstallerMigrationResult(result).rows.map((row) => ({ + label: row.label, + relPath: row.relPath, + reason: row.reason, + })), + [ + { + label: 'removed', + relPath: 'hooks/statusline.js', + reason: 'retired hook', + }, + { + label: 'backed up and removed', + relPath: 'hooks/modified.js', + reason: 'modified managed hook retired', + }, + { + label: 'preserved', + relPath: 'hooks/custom.js', + reason: 'user-owned hook', + }, + { + label: 'skipped', + relPath: 'hooks/unknown.js', + reason: 'unsupported in this installer', + }, + { + label: 'blocked', + relPath: 'hooks/gsd-retired-hook.js', + reason: 'needs a user choice', + }, + ] + ); +}); + +test('throws when installer migrations require user choice', () => { + assert.throws( + () => assertInstallerMigrationsUnblocked({ + blocked: [ + { + relPath: 'hooks/gsd-retired-hook.js', + }, + ], + }), + /installer migration blocked pending user choice: hooks\/gsd-retired-hook\.js/ + ); +}); diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index e2fdfae65..a0bd17c7c 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -197,6 +197,56 @@ test('blocks stale GSD-looking baseline artifacts for explicit user choice', () } }); +test('records known generated agent artifacts so profile cleanup can remove them', () => { + const configDir = createTempInstall(); + try { + writeFile(configDir, 'agents/gsd-executor.md', 'old generated agent\n'); + writeFile(configDir, 'agents/gsd-executor.toml', 'old generated agent config\n'); + writeFile(configDir, 'agents/gsd-local-experiment.md', 'user experiment\n'); + writeManifest(configDir, {}); + + const result = runInstallerMigrations({ + configDir, + runtime: 'codex', + scope: 'global', + migrations: [firstTimeBaselineMigration], + baselineScan: true, + now: () => '2026-05-11T00:00:03.000Z', + }); + + assert.deepEqual( + result.plan.actions.map((action) => ({ + type: action.type, + relPath: action.relPath, + classification: action.classification, + })), + [ + { + type: 'record-baseline', + relPath: 'agents/gsd-executor.md', + classification: 'unknown', + }, + { + type: 'record-baseline', + relPath: 'agents/gsd-executor.toml', + classification: 'unknown', + }, + { + type: 'prompt-user', + relPath: 'agents/gsd-local-experiment.md', + classification: 'stale-gsd-looking', + }, + ] + ); + assert.deepEqual(result.blocked.map((action) => action.relPath), ['agents/gsd-local-experiment.md']); + assert.equal(fs.readFileSync(path.join(configDir, 'agents/gsd-executor.md'), 'utf8'), 'old generated agent\n'); + assert.equal(fs.readFileSync(path.join(configDir, 'agents/gsd-executor.toml'), 'utf8'), 'old generated agent config\n'); + assert.equal(fs.readFileSync(path.join(configDir, 'agents/gsd-local-experiment.md'), 'utf8'), 'user experiment\n'); + } finally { + cleanup(configDir); + } +}); + test('plans a pending migration against an unchanged managed file', () => { const configDir = createTempInstall(); try { From c29adb01c0145d08023274f86cc073ec3b0788bc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 11 May 2026 09:13:16 -0400 Subject: [PATCH 2/5] Add changeset for installer migration phase four --- .changeset/vivid-foxes-romp.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/vivid-foxes-romp.md diff --git a/.changeset/vivid-foxes-romp.md b/.changeset/vivid-foxes-romp.md new file mode 100644 index 000000000..b04b1673b --- /dev/null +++ b/.changeset/vivid-foxes-romp.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 3402 +--- +Wire installer migrations into the normal install flow for every supported runtime with baseline scanning, action reporting, blocked-action protection, and install-state persistence before package materialization. From 7fe75c2c3e000de7fe213b3b5b0c34d16f5a192a Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 11 May 2026 10:02:02 -0400 Subject: [PATCH 3/5] Feat(installer): harden phase 4 migration integration --- .changeset/vivid-foxes-romp.md | 2 +- bin/install.js | 89 +++-- .../bin/lib/installer-migration-report.cjs | 56 +++- .../bin/lib/installer-migrations.cjs | 168 ++++++++-- .../000-first-time-baseline.cjs | 5 +- ...ler-migration-install-integration.test.cjs | 65 ++++ tests/installer-migration-report.test.cjs | 70 +++- tests/installer-migrations.test.cjs | 310 +++++++++++++++++- 8 files changed, 693 insertions(+), 72 deletions(-) diff --git a/.changeset/vivid-foxes-romp.md b/.changeset/vivid-foxes-romp.md index b04b1673b..f1357b255 100644 --- a/.changeset/vivid-foxes-romp.md +++ b/.changeset/vivid-foxes-romp.md @@ -2,4 +2,4 @@ type: Added pr: 3402 --- -Wire installer migrations into the normal install flow for every supported runtime with baseline scanning, action reporting, blocked-action protection, and install-state persistence before package materialization. +Wire installer migrations into the normal install flow for every supported runtime with baseline scanning, collapsed action reporting, blocked-action protection, transactional rollback, install-state persistence, and lock-guarded execution before package materialization. diff --git a/bin/install.js b/bin/install.js index 750b62ed6..007bc783c 100755 --- a/bin/install.js +++ b/bin/install.js @@ -77,6 +77,7 @@ const { stageSkillsForMode, } = require(path.join(_gsdLibDir, 'install-profiles.cjs')); const { + discoverInstallerMigrations, runInstallerMigrations, } = require(path.join(_gsdLibDir, 'installer-migrations.cjs')); const { @@ -7482,7 +7483,7 @@ function reportInstallerMigrationResult(result) { } } -function install(isGlobal, runtime = 'claude') { +function install(isGlobal, runtime = 'claude', options = {}) { const isOpencode = runtime === 'opencode'; const isGemini = runtime === 'gemini'; const isKilo = runtime === 'kilo'; @@ -7746,6 +7747,7 @@ function install(isGlobal, runtime = 'claude') { configDir: targetDir, runtime, scope: isGlobal ? 'global' : 'local', + migrations: options.installerMigrations, baselineScan: true, }); reportInstallerMigrationResult(installerMigrationResult); @@ -8377,6 +8379,12 @@ function install(isGlobal, runtime = 'claude') { } } catch (_earlyInstallErr) { + // Installer Migration Module Phase 4: docs/installer-migrations.md + // requires safe migrations to run before package materialization without + // leaving stale state behind when materialization fails. Roll migration + // actions back for every runtime; Codex then layers its broader runtime + // snapshot rollback on top. + rollbackInstallerMigrations(); // #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. @@ -9112,6 +9120,7 @@ function install(isGlobal, runtime = 'claude') { updateBannerCommand, runtime, configDir: targetDir, + rollbackInstallerMigrations, }; } @@ -10573,40 +10582,69 @@ function trySelfLinkGsdSdkWindows(shimSrc) { */ function installAllRuntimes(runtimes, isGlobal, isInteractive) { const results = []; + const installerMigrations = discoverInstallerMigrations({ + migrationsDir: path.join(_gsdLibDir, 'installer-migrations'), + }); for (const runtime of runtimes) { - const result = install(isGlobal, runtime); + const result = install(isGlobal, runtime, { installerMigrations }); results.push(result); } const statuslineRuntimes = ['claude', 'gemini']; const primaryStatuslineResult = results.find(r => statuslineRuntimes.includes(r.runtime)); - const finalize = (shouldInstallStatusline, shouldInstallBanner) => { - // Verify sdk/dist/cli.js is present and executable. The dist is shipped - // prebuilt in the tarball (fix/2441-sdk-decouple); gsd-sdk reaches users via - // the parent package's bin/gsd-sdk.js shim, so no sub-install is needed. - // Skip with --no-sdk. Skip with isLocal (#2678 — local installs don't own global npm). - // #3033: pass forceSdk so --sdk overrides the local-install skip. - installSdkIfNeeded({ isLocal: !isGlobal, forceSdk: hasSdk }); - - const printSummaries = () => { - for (const result of results) { - const useStatusline = statuslineRuntimes.includes(result.runtime) && shouldInstallStatusline; - finishInstall( - result.settingsPath, - result.settings, - result.statuslineCommand, - useStatusline, - result.runtime, - isGlobal, - result.configDir, - { shouldInstallBanner: !!shouldInstallBanner, bannerCommand: result.updateBannerCommand } - ); + const rollbackFinalizedInstallerMigrations = (error) => { + const rollbackFailures = []; + for (const result of [...results].reverse()) { + if (!result || typeof result.rollbackInstallerMigrations !== 'function') continue; + try { + result.rollbackInstallerMigrations(); + } catch (rollbackError) { + rollbackFailures.push({ + runtime: result.runtime, + error: rollbackError.message, + }); } - }; + } + if (rollbackFailures.length > 0) { + error.installerMigrationRollbackFailures = rollbackFailures; + } + }; - printSummaries(); + const finalize = (shouldInstallStatusline, shouldInstallBanner) => { + try { + // Verify sdk/dist/cli.js is present and executable. The dist is shipped + // prebuilt in the tarball (fix/2441-sdk-decouple); gsd-sdk reaches users via + // the parent package's bin/gsd-sdk.js shim, so no sub-install is needed. + // Skip with --no-sdk. Skip with isLocal (#2678 — local installs don't own global npm). + // #3033: pass forceSdk so --sdk overrides the local-install skip. + installSdkIfNeeded({ isLocal: !isGlobal, forceSdk: hasSdk }); + + const printSummaries = () => { + for (const result of results) { + const useStatusline = statuslineRuntimes.includes(result.runtime) && shouldInstallStatusline; + finishInstall( + result.settingsPath, + result.settings, + result.statuslineCommand, + useStatusline, + result.runtime, + isGlobal, + result.configDir, + { shouldInstallBanner: !!shouldInstallBanner, bannerCommand: result.updateBannerCommand } + ); + } + }; + + printSummaries(); + } catch (error) { + // Phase 4 install/update integration requires safe migrations to roll + // back when later package/finalization materialization fails: + // docs/installer-migrations.md#phase-4-installupdate-integration. + rollbackFinalizedInstallerMigrations(error); + throw error; + } }; // Statusline first; if it won't actually be installed (declined, or local @@ -10681,6 +10719,7 @@ if (process.env.GSD_TEST_MODE) { readGsdRuntimeProfileResolver, readGsdEffectiveModelOverrides, install, + installAllRuntimes, uninstall, installSdkIfNeeded, buildSdkFailFastReport, diff --git a/get-shit-done/bin/lib/installer-migration-report.cjs b/get-shit-done/bin/lib/installer-migration-report.cjs index d0ff12275..ff175b968 100644 --- a/get-shit-done/bin/lib/installer-migration-report.cjs +++ b/get-shit-done/bin/lib/installer-migration-report.cjs @@ -19,21 +19,67 @@ function blockedInstallerMigrationActions(result) { return []; } +function baselineSummaryLabel(count, noun) { + return `${count} ${noun}${count === 1 ? '' : 's'}`; +} + +function baselineSummaryRow(type, actions) { + const count = actions.length; + if (type === 'record-baseline') { + return { + label: 'recorded', + relPath: baselineSummaryLabel(count, 'managed baseline file'), + reason: 'first-time baseline scan', + action: { type: 'record-baseline-summary', count, actions }, + }; + } + return { + label: 'preserved', + relPath: baselineSummaryLabel(count, 'user baseline file'), + reason: 'first-time baseline scan', + action: { type: 'baseline-preserve-user-summary', count, actions }, + }; +} + function summarizeInstallerMigrationResult(result) { const plan = result && result.plan; const actions = plan && Array.isArray(plan.actions) ? plan.actions : []; const blocked = blockedInstallerMigrationActions(result); const blockedSet = new Set(blocked); + const rows = []; + const baselineIndexes = new Map(); + const baselineActions = new Map(); - return { - hasReportableActions: actions.length > 0 || blocked.length > 0, - blocked, - rows: actions.map((action) => ({ + for (const action of actions) { + const type = action && action.type; + if (type === 'record-baseline' || type === 'baseline-preserve-user') { + if (!baselineActions.has(type)) { + baselineActions.set(type, []); + baselineIndexes.set(type, rows.length); + rows.push(null); + } + baselineActions.get(type).push(action); + continue; + } + + rows.push({ label: blockedSet.has(action) ? 'blocked' : installerMigrationActionLabel(action), relPath: action.relPath, reason: action.reason || '', action, - })), + }); + } + + // Phase 4 requires action reporting without flooding first-time baseline installs: + // docs/installer-migrations.md#phase-4-installupdate-integration. + for (const [type, baselineRows] of baselineActions) { + rows[baselineIndexes.get(type)] = baselineSummaryRow(type, baselineRows); + } + + return { + hasReportableActions: actions.length > 0 || blocked.length > 0, + blocked, + rows, }; } diff --git a/get-shit-done/bin/lib/installer-migrations.cjs b/get-shit-done/bin/lib/installer-migrations.cjs index b69272995..f497907b4 100644 --- a/get-shit-done/bin/lib/installer-migrations.cjs +++ b/get-shit-done/bin/lib/installer-migrations.cjs @@ -6,10 +6,25 @@ const crypto = require('crypto'); const MANIFEST_NAME = 'gsd-file-manifest.json'; const INSTALL_STATE_NAME = 'gsd-install-state.json'; +const INSTALL_MIGRATION_LOCK_NAME = 'gsd-install-migration.lock'; const DEFAULT_MIGRATIONS_DIR = path.join(__dirname, 'installer-migrations'); +const DEFAULT_LOCK_TIMEOUT_MS = 30_000; +const STRICT_JSON = Symbol('strict-json'); function sha256File(filePath) { - return crypto.createHash('sha256').update(fs.readFileSync(filePath)).digest('hex'); + const hash = crypto.createHash('sha256'); + const buffer = Buffer.allocUnsafe(1024 * 1024); + const fd = fs.openSync(filePath, 'r'); + try { + while (true) { + const bytesRead = fs.readSync(fd, buffer, 0, buffer.length, null); + if (bytesRead === 0) break; + hash.update(buffer.subarray(0, bytesRead)); + } + } finally { + fs.closeSync(fd); + } + return hash.digest('hex'); } function sha256Text(value) { @@ -20,7 +35,10 @@ function readJsonIfPresent(filePath, fallback) { if (!fs.existsSync(filePath)) return fallback; try { return JSON.parse(fs.readFileSync(filePath, 'utf8')); - } catch { + } catch (error) { + if (fallback === STRICT_JSON) { + throw new Error(`invalid installer migration state JSON: ${filePath}: ${error.message}`); + } return fallback; } } @@ -39,7 +57,7 @@ function readInstallManifest(configDir) { } function readInstallState(configDir) { - const state = readJsonIfPresent(path.join(configDir, INSTALL_STATE_NAME), null); + const state = readJsonIfPresent(path.join(configDir, INSTALL_STATE_NAME), STRICT_JSON); if (!state || typeof state !== 'object') { return { schemaVersion: 1, appliedMigrations: [] }; } @@ -114,7 +132,8 @@ function appliedMigrationEntries(state) { } function migrationChecksum(migration) { - if (typeof migration.checksum === 'string' && migration.checksum) return migration.checksum; + const checksum = migration.checksum; + if (typeof checksum === 'string' && checksum) return checksum; const serializable = { id: migration.id, title: migration.title || null, @@ -129,8 +148,7 @@ function migrationChecksum(migration) { return `sha256:${sha256Text(JSON.stringify(serializable))}`; } -function assertAppliedMigrationChecksums(state, migrations) { - const applied = appliedMigrationEntries(state); +function assertAppliedMigrationChecksums(applied, migrations) { for (const migration of migrations) { const entry = applied.get(migration.id); if (!entry || !entry.checksum) continue; @@ -186,6 +204,55 @@ function journalTimestamp(now) { return now().replace(/[:.]/g, '-'); } +function migrationRunId(appliedAt) { + return `${journalTimestamp(() => appliedAt)}-${crypto.randomBytes(8).toString('hex')}`; +} + +function sleepSync(ms) { + const buffer = new SharedArrayBuffer(4); + Atomics.wait(new Int32Array(buffer), 0, 0, ms); +} + +function acquireInstallMigrationLock(configDir, { timeoutMs = DEFAULT_LOCK_TIMEOUT_MS } = {}) { + fs.mkdirSync(configDir, { recursive: true }); + const lockPath = path.join(configDir, INSTALL_MIGRATION_LOCK_NAME); + const started = Date.now(); + + while (true) { + let fd = null; + try { + fd = fs.openSync(lockPath, 'wx'); + fs.writeFileSync(fd, JSON.stringify({ + pid: process.pid, + acquiredAt: new Date().toISOString(), + }) + '\n'); + return () => { + const failures = []; + try { fs.closeSync(fd); } catch (error) { failures.push(error); } + try { fs.rmSync(lockPath, { force: true }); } catch (error) { failures.push(error); } + if (failures.length > 0) { + const releaseError = new Error(`failed to release installer migration lock: ${lockPath}`); + releaseError.failures = failures; + throw releaseError; + } + }; + } catch (error) { + if (fd !== null) { + try { fs.closeSync(fd); } catch { /* best-effort */ } + try { fs.rmSync(lockPath, { force: true }); } catch { /* best-effort */ } + } + if (error && error.code === 'EEXIST') { + if (Date.now() - started >= timeoutMs) { + throw new Error(`installer migration lock is held: ${lockPath}`); + } + sleepSync(Math.min(50, Math.max(1, timeoutMs - (Date.now() - started)))); + continue; + } + throw error; + } + } +} + function ensureInsideConfig(configDir, relPath) { const normalized = normalizeRelPath(relPath); const fullPath = path.resolve(configDir, normalized); @@ -238,8 +305,8 @@ function planInstallerMigrations({ const scopedMigrations = migrations.filter((migration) => migration && migrationMatchesContext(migration, { runtime, scope }) ); - assertAppliedMigrationChecksums(state, scopedMigrations); - const applied = appliedMigrationIds(state); + const applied = appliedMigrationEntries(state); + assertAppliedMigrationChecksums(applied, scopedMigrations); const pending = scopedMigrations.filter((migration) => !applied.has(migration.id)); const actions = []; const blocked = []; @@ -273,6 +340,7 @@ function planInstallerMigrations({ if (!Array.isArray(plannedActions)) { throw new Error(`migration ${migration.id} plan must return an array`); } + const checksum = migrationChecksum(migration); for (const rawAction of plannedActions) { const relPath = normalizeRelPath(rawAction.relPath); const classification = rawAction.classification @@ -291,7 +359,7 @@ function planInstallerMigrations({ } const action = { migrationId: migration.id, - migrationChecksum: migrationChecksum(migration), + migrationChecksum: checksum, type: protectedType, relPath, reason: rawAction.reason || migration.description || '', @@ -303,7 +371,7 @@ function planInstallerMigrations({ action.requestedType = rawAction.type; } if (action.type === 'backup-and-remove') { - action.backupRelPath = path.posix.join('gsd-migration-backups', migration.id, relPath); + action.backupRelPath = null; } if (action.type === 'rewrite-json') { action.value = rawAction.value; @@ -339,7 +407,7 @@ function uniqueActionMigrationIds(actions) { return [...new Set(actions.map((action) => action.migrationId).filter(Boolean))]; } -function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, previousInstallStateBytes }) { +function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, backupRoot, previousInstallStateBytes }) { const failures = []; for (const action of [...journal.actions].reverse()) { if (!action.rollbackRelPath) continue; @@ -376,6 +444,7 @@ function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollb try { fs.rmSync(journalPath, { force: true }); fs.rmSync(rollbackRoot, { recursive: true, force: true }); + fs.rmSync(backupRoot, { recursive: true, force: true }); } catch { // journal cleanup is best-effort; the rollback above is the safety-critical part } @@ -387,6 +456,12 @@ function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollb } } +function cleanupMigrationRunArtifacts(journalPath, rollbackRoot, backupRoot) { + try { fs.rmSync(journalPath, { force: true }); } catch { /* best-effort */ } + try { fs.rmSync(rollbackRoot, { recursive: true, force: true }); } catch { /* best-effort */ } + try { fs.rmSync(backupRoot, { recursive: true, force: true }); } catch { /* best-effort */ } +} + function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().toISOString() }) { if (!configDir) throw new Error('configDir is required'); if (!plan || !Array.isArray(plan.actions)) throw new Error('plan with actions is required'); @@ -395,10 +470,13 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t } const appliedAt = now(); - const journalRelPath = path.posix.join('gsd-migration-journal', `${journalTimestamp(() => appliedAt)}.json`); + const runId = migrationRunId(appliedAt); + const journalRelPath = path.posix.join('gsd-migration-journal', `${runId}.json`); const journalPath = path.join(configDir, journalRelPath); - const rollbackRootRelPath = path.posix.join('gsd-migration-journal', `${journalTimestamp(() => appliedAt)}-rollback`); + const rollbackRootRelPath = path.posix.join('gsd-migration-journal', `${runId}-rollback`); const rollbackRoot = path.join(configDir, rollbackRootRelPath); + const backupRootRelPath = path.posix.join('gsd-migration-journal', `${runId}-backups`); + const backupRoot = path.join(configDir, backupRootRelPath); const journal = { schemaVersion: 1, appliedAt, @@ -455,7 +533,7 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t } if (action.type === 'backup-and-remove') { - const backupRelPath = action.backupRelPath || path.posix.join('gsd-migration-backups', action.migrationId, normalized); + const backupRelPath = action.backupRelPath || path.posix.join(backupRootRelPath, normalized); const backupPath = path.join(configDir, backupRelPath); fs.mkdirSync(path.dirname(backupPath), { recursive: true }); fs.copyFileSync(fullPath, backupPath); @@ -502,7 +580,7 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t return { appliedMigrationIds: journal.appliedMigrationIds, journalRelPath, - rollback: () => rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, previousInstallStateBytes }), + rollback: () => rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, backupRoot, previousInstallStateBytes }), }; } catch (error) { const rollbackFailures = []; @@ -525,6 +603,7 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t rollbackError.rollbackFailures = rollbackFailures; throw rollbackError; } + cleanupMigrationRunArtifacts(journalPath, rollbackRoot, backupRoot); throw error; } } @@ -537,29 +616,54 @@ function runInstallerMigrations({ migrations = discoverInstallerMigrations({ migrationsDir }), baselineScan = false, now = () => new Date().toISOString(), + lockTimeoutMs = DEFAULT_LOCK_TIMEOUT_MS, } = {}) { - const plan = planInstallerMigrations({ configDir, runtime, scope, migrations, baselineScan, now }); - if (plan.actions.length === 0) { - return { - appliedMigrationIds: [], - journalRelPath: null, - plan, - }; + const releaseLock = acquireInstallMigrationLock(configDir, { timeoutMs: lockTimeoutMs }); + let primaryError = null; + let completed = false; + try { + const plan = planInstallerMigrations({ configDir, runtime, scope, migrations, baselineScan, now }); + if (plan.actions.length === 0) { + completed = true; + return { + appliedMigrationIds: [], + journalRelPath: null, + plan, + }; + } + if (plan.blocked.length > 0) { + completed = true; + return { + appliedMigrationIds: [], + journalRelPath: null, + plan, + blocked: plan.blocked, + }; + } + const result = applyInstallerMigrationPlan({ configDir, plan, now }); + completed = true; + return { ...result, plan }; + } catch (error) { + primaryError = error; + throw error; + } finally { + try { + releaseLock(); + } catch (releaseError) { + if (primaryError) { + primaryError.suppressed = [...(primaryError.suppressed || []), releaseError]; + } else if (completed) { + throw releaseError; + } else { + throw releaseError; + } + } } - if (plan.blocked.length > 0) { - return { - appliedMigrationIds: [], - journalRelPath: null, - plan, - blocked: plan.blocked, - }; - } - const result = applyInstallerMigrationPlan({ configDir, plan, now }); - return { ...result, plan }; } module.exports = { DEFAULT_MIGRATIONS_DIR, + INSTALL_MIGRATION_LOCK_NAME, INSTALL_STATE_NAME, MANIFEST_NAME, applyInstallerMigrationPlan, diff --git a/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs b/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs index 8d03a5359..ddb93e4f4 100644 --- a/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs +++ b/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs @@ -56,8 +56,7 @@ function baselineInstallSurfaces(runtime) { function walkFiles(root, relDir, files) { const dir = path.join(root, relDir); if (!fs.existsSync(dir)) return; - const entries = fs.readdirSync(dir, { withFileTypes: true }) - .sort((a, b) => a.name.localeCompare(b.name)); + const entries = fs.readdirSync(dir, { withFileTypes: true }); for (const entry of entries) { const relPath = path.posix.join(relDir, entry.name); if (relDir === '' && INTERNAL_TOP_LEVEL_NAMES.has(entry.name)) continue; @@ -83,7 +82,7 @@ function scanBaselineFiles(configDir, runtime) { relPaths.add(normalized); } } - return [...relPaths].sort(); + return [...relPaths]; } function isUserOwnedBaselinePath(relPath) { diff --git a/tests/installer-migration-install-integration.test.cjs b/tests/installer-migration-install-integration.test.cjs index 4ff6b82aa..6e7b496ed 100644 --- a/tests/installer-migration-install-integration.test.cjs +++ b/tests/installer-migration-install-integration.test.cjs @@ -71,6 +71,21 @@ function captureConsole(fn) { } } +function withWriteFailure(matchPath, fn) { + const originalWriteFileSync = fs.writeFileSync; + fs.writeFileSync = (filePath, ...args) => { + if (path.resolve(String(filePath)) === path.resolve(matchPath)) { + throw new Error(`injected write failure for ${path.basename(matchPath)}`); + } + return originalWriteFileSync.call(fs, filePath, ...args); + }; + try { + return fn(); + } finally { + fs.writeFileSync = originalWriteFileSync; + } +} + function stripAnsi(value) { return value.replace(/\x1b\[[0-9;]*m/g, ''); } @@ -138,6 +153,56 @@ describe('installer migration install integration', { concurrency: false }, () = assert.equal(fs.existsSync(path.join(codexHome, 'get-shit-done', 'VERSION')), false); }); + test('rolls back applied migrations when package materialization fails for non-Codex installs', () => { + const claudeHome = path.join(tmpRoot, '.claude'); + fs.mkdirSync(claudeHome, { recursive: true }); + writeFile(claudeHome, 'hooks/statusline.js', 'legacy managed hook\n'); + writeManifest(claudeHome, { + 'hooks/statusline.js': sha256('legacy managed hook\n'), + }); + + assert.throws( + () => captureConsole(() => + withEnv('CLAUDE_CONFIG_DIR', claudeHome, () => + withWriteFailure(path.join(claudeHome, 'get-shit-done', 'VERSION'), () => install(true, 'claude')) + ) + ), + /injected write failure for VERSION/ + ); + + assert.equal( + fs.readFileSync(path.join(claudeHome, 'hooks/statusline.js'), 'utf8'), + 'legacy managed hook\n' + ); + assert.equal(fs.existsSync(path.join(claudeHome, 'gsd-install-state.json')), false); + }); + + test('rolls back applied migrations when multi-runtime finalization fails', () => { + const claudeHome = path.join(tmpRoot, '.claude'); + fs.mkdirSync(claudeHome, { recursive: true }); + writeFile(claudeHome, 'hooks/statusline.js', 'legacy managed hook\n'); + writeManifest(claudeHome, { + 'hooks/statusline.js': sha256('legacy managed hook\n'), + }); + + assert.throws( + () => captureConsole(() => + withEnv('CLAUDE_CONFIG_DIR', claudeHome, () => + withWriteFailure(path.join(claudeHome, 'settings.json'), () => + installModule.installAllRuntimes(['claude'], true, false) + ) + ) + ), + /injected write failure for settings\.json/ + ); + + assert.equal( + fs.readFileSync(path.join(claudeHome, 'hooks/statusline.js'), 'utf8'), + 'legacy managed hook\n' + ); + assert.equal(fs.existsSync(path.join(claudeHome, 'gsd-install-state.json')), false); + }); + for (const runtime of SUPPORTED_RUNTIMES) { test(`runs managed cleanup migrations for ${runtime}`, () => { const targetDir = path.join(tmpRoot, `.${runtime}-managed-cleanup`); diff --git a/tests/installer-migration-report.test.cjs b/tests/installer-migration-report.test.cjs index 0770c33ae..6ebcccfbe 100644 --- a/tests/installer-migration-report.test.cjs +++ b/tests/installer-migration-report.test.cjs @@ -62,8 +62,8 @@ test('summarizes every installer migration report category', () => { }, { label: 'preserved', - relPath: 'hooks/custom.js', - reason: 'user-owned hook', + relPath: '1 user baseline file', + reason: 'first-time baseline scan', }, { label: 'skipped', @@ -79,6 +79,72 @@ test('summarizes every installer migration report category', () => { ); }); +test('collapses first-time baseline report rows without hiding destructive actions', () => { + const blockedAction = { + type: 'prompt-user', + relPath: 'hooks/gsd-ambiguous.js', + reason: 'needs a user choice', + }; + const result = { + blocked: [blockedAction], + plan: { + actions: [ + { + type: 'record-baseline', + relPath: 'hooks/statusline.js', + reason: 'first-time baseline scan', + }, + { + type: 'record-baseline', + relPath: 'hooks/workflow-guard.js', + reason: 'first-time baseline scan', + }, + { + type: 'baseline-preserve-user', + relPath: 'hooks/custom.js', + reason: 'first-time baseline scan', + }, + { + type: 'remove-managed', + relPath: 'hooks/retired.js', + reason: 'retired hook', + }, + blockedAction, + ], + }, + }; + + assert.deepEqual( + summarizeInstallerMigrationResult(result).rows.map((row) => ({ + label: row.label, + relPath: row.relPath, + reason: row.reason, + })), + [ + { + label: 'recorded', + relPath: '2 managed baseline files', + reason: 'first-time baseline scan', + }, + { + label: 'preserved', + relPath: '1 user baseline file', + reason: 'first-time baseline scan', + }, + { + label: 'removed', + relPath: 'hooks/retired.js', + reason: 'retired hook', + }, + { + label: 'blocked', + relPath: 'hooks/gsd-ambiguous.js', + reason: 'needs a user choice', + }, + ] + ); +}); + test('throws when installer migrations require user choice', () => { assert.throws( () => assertInstallerMigrationsUnblocked({ diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index a0bd17c7c..5a112fd97 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -7,6 +7,7 @@ const crypto = require('crypto'); const { applyInstallerMigrationPlan, + classifyArtifact, discoverInstallerMigrations, INSTALL_STATE_NAME, planInstallerMigrations, @@ -334,7 +335,7 @@ test('plans backup before removal for a modified managed file', () => { assert.equal(plan.actions[0].classification, 'managed-modified'); assert.equal(plan.actions[0].originalHash, sha256('managed hook\n')); assert.equal(plan.actions[0].currentHash, sha256('user changed hook\n')); - assert.equal(plan.actions[0].backupRelPath, 'gsd-migration-backups/2026-05-11-remove-old-hook/hooks/old-hook.js'); + assert.equal(plan.actions[0].backupRelPath, null); } finally { cleanup(configDir); } @@ -374,6 +375,77 @@ test('blocks removal of unknown files by preserving them by default', () => { } }); +test('fails closed when install state JSON is malformed', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + fs.writeFileSync(path.join(configDir, INSTALL_STATE_NAME), '{ not json\n', 'utf8'); + + assert.throws( + () => readInstallState(configDir), + /invalid installer migration state JSON/ + ); +}); + +test('computes each migration checksum once per planned migration', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + writeFile(configDir, 'hooks/first.js', 'first hook\n'); + writeFile(configDir, 'hooks/second.js', 'second hook\n'); + writeManifest(configDir, { + 'hooks/first.js': sha256('first hook\n'), + 'hooks/second.js': sha256('second hook\n'), + }); + let checksumReads = 0; + const migration = { + id: '2026-05-11-remove-two-hooks', + description: 'Remove retired hooks', + get checksum() { + checksumReads += 1; + return 'sha256:precomputed'; + }, + plan: () => [ + { type: 'remove-managed', relPath: 'hooks/first.js', reason: 'retired hook' }, + { type: 'remove-managed', relPath: 'hooks/second.js', reason: 'retired hook' }, + ], + }; + + const plan = planInstallerMigrations({ + configDir, + migrations: [migration], + }); + + assert.equal(plan.actions.length, 2); + assert.equal(checksumReads, 1); +}); + +test('classifies large files without loading the whole file through readFileSync', (t) => { + const configDir = createTempInstall(); + const originalReadFileSync = fs.readFileSync; + t.after(() => { + fs.readFileSync = originalReadFileSync; + cleanup(configDir); + }); + + const relPath = 'skills/gsd-large/SKILL.md'; + const fullPath = path.join(configDir, relPath); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, Buffer.alloc(1024 * 1024 + 1, 'a')); + + fs.readFileSync = (filePath, ...args) => { + if (path.resolve(String(filePath)) === path.resolve(fullPath)) { + throw new Error('large file should be streamed for hashing'); + } + return originalReadFileSync.call(fs, filePath, ...args); + }; + + const artifact = classifyArtifact(configDir, relPath, { files: {} }); + + assert.equal(artifact.classification, 'unknown'); + assert.match(artifact.currentHash, /^[0-9a-f]{64}$/); +}); + test('applies an unblocked plan with a journal and install-state update', () => { const configDir = createTempInstall(); try { @@ -408,7 +480,10 @@ test('applies an unblocked plan with a journal and install-state update', () => assert.equal(fs.existsSync(path.join(configDir, 'hooks/old-hook.js')), false); assert.deepEqual(result.appliedMigrationIds, ['2026-05-11-remove-old-hook']); - assert.equal(result.journalRelPath, 'gsd-migration-journal/2026-05-11T00-00-01-000Z.json'); + assert.match( + result.journalRelPath, + /^gsd-migration-journal\/2026-05-11T00-00-01-000Z-[0-9a-f]+\.json$/ + ); const journal = JSON.parse(fs.readFileSync(path.join(configDir, result.journalRelPath), 'utf8')); assert.deepEqual(journal.appliedMigrationIds, ['2026-05-11-remove-old-hook']); @@ -422,6 +497,174 @@ test('applies an unblocked plan with a journal and install-state update', () => } }); +test('uses unique journal paths for applies that share a timestamp', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + writeFile(configDir, 'hooks/first.js', 'first hook\n'); + writeFile(configDir, 'hooks/second.js', 'second hook\n'); + writeManifest(configDir, { + 'hooks/first.js': sha256('first hook\n'), + 'hooks/second.js': sha256('second hook\n'), + }); + const now = () => '2026-05-11T00:00:09.000Z'; + + const first = applyInstallerMigrationPlan({ + configDir, + plan: { + blocked: [], + actions: [{ + migrationId: 'first-migration', + migrationChecksum: 'sha256:first', + type: 'remove-managed', + relPath: 'hooks/first.js', + reason: 'first', + classification: 'managed-pristine', + originalHash: sha256('first hook\n'), + currentHash: sha256('first hook\n'), + }], + }, + now, + }); + const second = applyInstallerMigrationPlan({ + configDir, + plan: { + blocked: [], + actions: [{ + migrationId: 'second-migration', + migrationChecksum: 'sha256:second', + type: 'remove-managed', + relPath: 'hooks/second.js', + reason: 'second', + classification: 'managed-pristine', + originalHash: sha256('second hook\n'), + currentHash: sha256('second hook\n'), + }], + }, + now, + }); + + assert.notEqual(first.journalRelPath, second.journalRelPath); + assert.equal(fs.existsSync(path.join(configDir, first.journalRelPath)), true); + assert.equal(fs.existsSync(path.join(configDir, second.journalRelPath)), true); +}); + +test('stores modified-file backups under the unique migration run journal', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + writeFile(configDir, 'hooks/old-hook.js', 'user changed hook\n'); + writeManifest(configDir, { + 'hooks/old-hook.js': sha256('managed hook\n'), + }); + + 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:10.000Z', + }); + const journal = JSON.parse(fs.readFileSync(path.join(configDir, result.journalRelPath), 'utf8')); + const backupRelPath = journal.actions[0].backupRelPath; + + assert.match(backupRelPath, /^gsd-migration-journal\/2026-05-11T00-00-10-000Z-[0-9a-f]+-backups\/hooks\/old-hook\.js$/); + assert.equal(fs.readFileSync(path.join(configDir, backupRelPath), 'utf8'), 'user changed hook\n'); +}); + +test('successful migration rollback removes run-scoped backup directories', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + writeFile(configDir, 'hooks/old-hook.js', 'user changed hook\n'); + writeManifest(configDir, { + 'hooks/old-hook.js': sha256('managed hook\n'), + }); + + 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', + }], + }, + ], + }); + + const result = applyInstallerMigrationPlan({ + configDir, + plan, + now: () => '2026-05-11T00:00:11.000Z', + }); + + result.rollback(); + + assert.equal(fs.readFileSync(path.join(configDir, 'hooks/old-hook.js'), 'utf8'), 'user changed hook\n'); + assert.equal(fs.existsSync(path.join(configDir, result.journalRelPath)), false); + assert.equal( + fs.readdirSync(path.join(configDir, 'gsd-migration-journal')).some((name) => name.includes('backups')), + false + ); +}); + +test('refuses to run migrations while another installer owns the migration lock', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + fs.writeFileSync(path.join(configDir, 'gsd-install-migration.lock'), 'held by test\n', 'utf8'); + + assert.throws( + () => runInstallerMigrations({ + configDir, + migrations: [], + lockTimeoutMs: 0, + }), + /installer migration lock is held/ + ); +}); + +test('reports lock release failures after migration work completes', (t) => { + const configDir = createTempInstall(); + const originalRmSync = fs.rmSync; + t.after(() => { + fs.rmSync = originalRmSync; + cleanup(configDir); + }); + + fs.rmSync = (targetPath, ...args) => { + if (path.basename(String(targetPath)) === 'gsd-install-migration.lock') { + throw new Error('simulated lock unlink failure'); + } + return originalRmSync.call(fs, targetPath, ...args); + }; + + assert.throws( + () => runInstallerMigrations({ + configDir, + migrations: [], + }), + /failed to release installer migration lock/ + ); +}); + test('rolls back touched files and leaves state unchanged when apply fails', () => { const configDir = createTempInstall(); try { @@ -466,12 +709,71 @@ test('rolls back touched files and leaves state unchanged when apply fails', () assert.equal(fs.readFileSync(path.join(configDir, 'hooks/old-hook.js'), 'utf8'), 'managed hook\n'); assert.deepEqual(readInstallState(configDir).appliedMigrations, []); - assert.equal(fs.existsSync(path.join(configDir, 'gsd-migration-journal', '2026-05-11T00-00-02-000Z.json')), false); + assert.equal( + fs.existsSync(path.join(configDir, 'gsd-migration-journal')) && + fs.readdirSync(path.join(configDir, 'gsd-migration-journal')).some((name) => + name.startsWith('2026-05-11T00-00-02-000Z') + ), + false + ); } finally { cleanup(configDir); } }); +test('cleans rollback and backup artifacts when migration apply fails', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + writeFile(configDir, 'hooks/old-hook.js', 'user changed hook\n'); + writeManifest(configDir, { + 'hooks/old-hook.js': sha256('managed hook\n'), + }); + const plan = { + blocked: [], + actions: [ + { + migrationId: '2026-05-11-remove-old-hook', + migrationChecksum: 'sha256:remove', + type: 'backup-and-remove', + relPath: 'hooks/old-hook.js', + reason: 'retired hook', + classification: 'managed-modified', + originalHash: sha256('managed hook\n'), + currentHash: sha256('user changed hook\n'), + }, + { + migrationId: '2026-05-11-remove-old-hook', + migrationChecksum: 'sha256:remove', + type: 'unsupported-test-action', + relPath: 'hooks/other.js', + reason: 'force failure', + classification: 'managed-pristine', + originalHash: null, + currentHash: null, + }, + ], + }; + + assert.throws( + () => applyInstallerMigrationPlan({ + configDir, + plan, + now: () => '2026-05-11T00:00:12.000Z', + }), + /unsupported migration action type/ + ); + + assert.equal(fs.readFileSync(path.join(configDir, 'hooks/old-hook.js'), 'utf8'), 'user changed hook\n'); + assert.equal( + fs.existsSync(path.join(configDir, 'gsd-migration-journal')) && + fs.readdirSync(path.join(configDir, 'gsd-migration-journal')).some((name) => + name.startsWith('2026-05-11T00-00-12-000Z') + ), + false + ); +}); + test('reports rollback restore failures instead of swallowing them', () => { const configDir = createTempInstall(); const originalCopyFileSync = fs.copyFileSync; @@ -506,7 +808,7 @@ test('reports rollback restore failures instead of swallowing them', () => { }; fs.copyFileSync = (src, dest) => { - if (String(src).includes('2026-05-11T00-00-04-000Z-rollback')) { + if (/2026-05-11T00-00-04-000Z-[0-9a-f]+-rollback/.test(String(src))) { throw new Error('simulated rollback copy failure'); } return originalCopyFileSync(src, dest); From f5510fe7e2c6d18d81326973ed9a05ec8e2d52dd Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 11 May 2026 10:04:55 -0400 Subject: [PATCH 4/5] Docs(installer): reword Antigravity contract note --- docs/installer-migrations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/installer-migrations.md b/docs/installer-migrations.md index 8f07804fb..2428805e6 100644 --- a/docs/installer-migrations.md +++ b/docs/installer-migrations.md @@ -345,7 +345,7 @@ for the new shape before changing migration behavior. | Gemini CLI | TOML slash commands in `commands/gsd/*.toml`; agents in `agents/gsd-*.md`; `settings.json` feature flag, hooks, and statusline | Global `GEMINI_CONFIG_DIR` or `~/.gemini`; local `./.gemini` | GSD owns generated commands/agents/hooks and only GSD settings entries; local command copy may be skipped when global GSD commands already exist | [Custom commands](https://google-gemini.github.io/gemini-cli/docs/cli/custom-commands.html), [configuration](https://google-gemini.github.io/gemini-cli/docs/cli/configuration.html); docs checked 2026-05-11 | | Codex | Skills in `skills/gsd-*/SKILL.md`; agents as source markdown plus per-agent TOML in `agents/`; `[agents.gsd-*]` and hooks in `config.toml` | Global `CODEX_HOME` or `~/.codex`; local `./.codex` | GSD owns generated skills, generated agent TOML, `agents.gsd-*` config sections, `[features].codex_hooks` when added by GSD, and GSD hook entries | [Codex config schema](https://developers.openai.com/codex/config-schema.json), [Codex developer docs](https://developers.openai.com/codex/); docs not versioned, checked 2026-05-11; installer compatibility sentinel: Codex 0.124.0 agent table shape | | GitHub Copilot | Skills in `skills/gsd-*/SKILL.md`; agents as `.agent.md`; repository instructions in `copilot-instructions.md` | Global `COPILOT_CONFIG_DIR` or `~/.copilot`; local `./.github` | GSD owns generated skill/agent files and GSD-authored instruction files; no hook/statusline ownership | [Repository custom instructions](https://docs.github.com/en/copilot/how-tos/configure-custom-instructions/add-repository-instructions), [Copilot CLI custom instructions](https://docs.github.com/en/copilot/how-tos/copilot-cli/add-custom-instructions); GitHub Docs product docs, checked 2026-05-11 | -| Antigravity | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; Gemini-style `settings.json` hooks when installed by GSD | Global `ANTIGRAVITY_CONFIG_DIR` or `~/.gemini/antigravity`; local `./.agent` | GSD owns generated skills/agents/hooks and GSD settings entries only | Public Antigravity install/config docs for this file layout were not stable or complete as of 2026-05-11; GSD uses the Gemini-compatible settings contract as a compatibility shim | +| Antigravity | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; Gemini-style `settings.json` hooks when installed by GSD | Global `ANTIGRAVITY_CONFIG_DIR` or `~/.gemini/antigravity`; local `./.agent` | GSD owns generated skills/agents/hooks and GSD settings entries only | Public Antigravity install/config docs for this file layout were not stable or complete on 2026-05-11; GSD uses the Gemini-compatible compatibility-shim settings contract | | Cursor | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; rule references under `rules/` | Global `CURSOR_CONFIG_DIR` or `~/.cursor`; local `./.cursor` | GSD owns generated skills/agents and GSD rule files or references; no hook/statusline ownership | [Cursor rules](https://docs.cursor.com/context/rules); docs not versioned, checked 2026-05-11 | | Windsurf | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; rule references under `rules/` | Global `WINDSURF_CONFIG_DIR` or `~/.codeium/windsurf`; local `./.windsurf` | GSD owns generated skills/agents and GSD rule files or references; no hook/statusline ownership | Windsurf public rule docs were source-limited in search results as of 2026-05-11; installer targets the common workspace rules convention `./.windsurf/rules` and must be rechecked before migrations rewrite rules | | Augment Code | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/` | Global `AUGMENT_CONFIG_DIR` or `~/.augment`; local `./.augment` | GSD owns generated skills/agents only; no hook/statusline ownership | [Augment Agent Skills](https://docs.augmentcode.com/cli/skills), [Augment IDE skills](https://docs.augmentcode.com/using-augment/skills); IDE skills public beta in VS Code 0.789.0+, checked 2026-05-11 | From 908a19cd04d3258b6330117399a33e77922050a3 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 11 May 2026 14:36:14 -0400 Subject: [PATCH 5/5] fix: harden installer migration integration --- bin/install.js | 29 +++++--- docs/installer-migrations.md | 4 +- .../bin/lib/installer-migrations.cjs | 51 +++++++++++-- tests/bug-2771-user-profile-manifest.test.cjs | 14 ++-- tests/bug-3033-sdk-flag-wired.test.cjs | 15 ++++ ...ler-migration-install-integration.test.cjs | 72 ++++++++++++++++++- tests/installer-migrations.test.cjs | 58 +++++++++++++++ 7 files changed, 220 insertions(+), 23 deletions(-) diff --git a/bin/install.js b/bin/install.js index 007bc783c..f57d2c124 100755 --- a/bin/install.js +++ b/bin/install.js @@ -9898,6 +9898,12 @@ function installSdkIfNeeded(opts) { if (!fs.existsSync(sdkCliPath)) { const ir = buildSdkFailFastReport(sdkDir, sdkCliPath); renderSdkFailFastReport(ir); + if (opts.throwOnFailure) { + const error = new Error(`GSD SDK prebuilt artifact missing: ${sdkCliPath}`); + error.code = 'GSD_SDK_MISSING_DIST'; + error.exitCode = 1; + throw error; + } process.exit(1); } @@ -10586,14 +10592,6 @@ function installAllRuntimes(runtimes, isGlobal, isInteractive) { migrationsDir: path.join(_gsdLibDir, 'installer-migrations'), }); - for (const runtime of runtimes) { - const result = install(isGlobal, runtime, { installerMigrations }); - results.push(result); - } - - const statuslineRuntimes = ['claude', 'gemini']; - const primaryStatuslineResult = results.find(r => statuslineRuntimes.includes(r.runtime)); - const rollbackFinalizedInstallerMigrations = (error) => { const rollbackFailures = []; for (const result of [...results].reverse()) { @@ -10612,6 +10610,19 @@ function installAllRuntimes(runtimes, isGlobal, isInteractive) { } }; + try { + for (const runtime of runtimes) { + const result = install(isGlobal, runtime, { installerMigrations }); + results.push(result); + } + } catch (error) { + rollbackFinalizedInstallerMigrations(error); + throw error; + } + + const statuslineRuntimes = ['claude', 'gemini']; + const primaryStatuslineResult = results.find(r => statuslineRuntimes.includes(r.runtime)); + const finalize = (shouldInstallStatusline, shouldInstallBanner) => { try { // Verify sdk/dist/cli.js is present and executable. The dist is shipped @@ -10619,7 +10630,7 @@ function installAllRuntimes(runtimes, isGlobal, isInteractive) { // the parent package's bin/gsd-sdk.js shim, so no sub-install is needed. // Skip with --no-sdk. Skip with isLocal (#2678 — local installs don't own global npm). // #3033: pass forceSdk so --sdk overrides the local-install skip. - installSdkIfNeeded({ isLocal: !isGlobal, forceSdk: hasSdk }); + installSdkIfNeeded({ isLocal: !isGlobal, forceSdk: hasSdk, throwOnFailure: true }); const printSummaries = () => { for (const result of results) { diff --git a/docs/installer-migrations.md b/docs/installer-migrations.md index 2428805e6..8c378bec1 100644 --- a/docs/installer-migrations.md +++ b/docs/installer-migrations.md @@ -259,8 +259,8 @@ entry point for every supported runtime: Claude Code, Antigravity, Augment, Cline, CodeBuddy, Codex, Copilot, Cursor, Gemini, Hermes Agent, Kilo, OpenCode, Qwen Code, Trae, and Windsurf. The installer invokes the same migration runner with `baselineScan: true`, reports the projected action rows, applies safe -non-interactive actions before materialization, writes install state after a -successful apply, and fails before writing new package files when the runner +non-interactive actions before materialization, persists install state only after +package materialization and finalization succeed, and fails before writing new package files when the runner returns blocked user-choice actions. Phase 1-3 built the planning, apply, rollback, install-state, baseline, and diff --git a/get-shit-done/bin/lib/installer-migrations.cjs b/get-shit-done/bin/lib/installer-migrations.cjs index f497907b4..8aaa3eeb9 100644 --- a/get-shit-done/bin/lib/installer-migrations.cjs +++ b/get-shit-done/bin/lib/installer-migrations.cjs @@ -90,10 +90,14 @@ function normalizeRelPath(relPath) { throw new Error('migration action relPath must be a non-empty string'); } const normalized = relPath.replace(/\\/g, '/'); - if (normalized.startsWith('/') || normalized.includes('../') || normalized === '..') { + if (path.isAbsolute(normalized) || path.win32.isAbsolute(normalized)) { throw new Error(`migration action relPath must stay inside configDir: ${relPath}`); } - return normalized; + const segments = normalized.split('/'); + if (segments.some((segment) => segment === '' || segment === '.' || segment === '..')) { + throw new Error(`migration action relPath must stay inside configDir: ${relPath}`); + } + return segments.join('/'); } function classifyArtifact(configDir, relPath, manifest) { @@ -192,11 +196,10 @@ function discoverInstallerMigrations({ migrationsDir }) { .sort() .flatMap((fileName) => { const source = path.join(migrationsDir, fileName); - const checksum = `sha256:${sha256File(source)}`; delete require.cache[require.resolve(source)]; const exported = require(source); const records = Array.isArray(exported) ? exported : [exported]; - return records.map((record) => validateMigrationRecord({ ...record, checksum: record.checksum || checksum }, source)); + return records.map((record) => validateMigrationRecord(record, source)); }); } @@ -398,6 +401,7 @@ function planInstallerMigrations({ manifest, state, pendingMigrationIds: pending.map((migration) => migration.id), + pendingMigrations: pending, actions, blocked, }; @@ -490,6 +494,9 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t : null; try { + fs.mkdirSync(path.dirname(journalPath), { recursive: true }); + fs.writeFileSync(journalPath, JSON.stringify(journal, null, 2) + '\n', 'utf8'); + for (const action of plan.actions) { if ( action.type !== 'remove-managed' && @@ -549,7 +556,6 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t fs.rmSync(fullPath, { force: true }); } - fs.mkdirSync(path.dirname(journalPath), { recursive: true }); fs.writeFileSync(journalPath, JSON.stringify(journal, null, 2) + '\n', 'utf8'); const state = readInstallState(configDir); @@ -608,6 +614,38 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t } } +function markPendingMigrationsApplied({ configDir, plan, now = () => new Date().toISOString() }) { + if (!plan || !Array.isArray(plan.pendingMigrationIds) || plan.pendingMigrationIds.length === 0) { + 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, migrationChecksum(migration)); + } + const nextApplied = [...state.appliedMigrations]; + const newlyApplied = []; + 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) { + writeInstallState(configDir, { + schemaVersion: 1, + appliedMigrations: nextApplied, + }); + } + return newlyApplied; +} + function runInstallerMigrations({ configDir, runtime = null, @@ -624,9 +662,10 @@ function runInstallerMigrations({ try { const plan = planInstallerMigrations({ configDir, runtime, scope, migrations, baselineScan, now }); if (plan.actions.length === 0) { + const appliedMigrationIds = markPendingMigrationsApplied({ configDir, plan, now }); completed = true; return { - appliedMigrationIds: [], + appliedMigrationIds, journalRelPath: null, plan, }; diff --git a/tests/bug-2771-user-profile-manifest.test.cjs b/tests/bug-2771-user-profile-manifest.test.cjs index eeda82adc..e2385d953 100644 --- a/tests/bug-2771-user-profile-manifest.test.cjs +++ b/tests/bug-2771-user-profile-manifest.test.cjs @@ -220,9 +220,16 @@ describe('#2771: USER_OWNED_ARTIFACTS is a single source of truth', () => { describe('manifest path safety', () => { let tmpDir; + let outside; - beforeEach(() => { tmpDir = createTempDir('gsd-manifest-path-safety-'); }); - afterEach(() => { cleanup(tmpDir); }); + beforeEach(() => { + tmpDir = createTempDir('gsd-manifest-path-safety-'); + outside = path.join(tmpDir, '..', `outside-managed-file-${path.basename(tmpDir)}.txt`); + }); + afterEach(() => { + if (outside) fs.rmSync(outside, { recursive: true, force: true }); + cleanup(tmpDir); + }); test('saveLocalPatches ignores manifest entries that escape the install root', () => { const origMode = process.env.GSD_TEST_MODE; @@ -236,7 +243,6 @@ describe('manifest path safety', () => { else process.env.GSD_TEST_MODE = origMode; } - const outside = path.join(tmpDir, '..', 'outside-managed-file.txt'); fs.writeFileSync(outside, 'outside user data\n', 'utf8'); fs.writeFileSync( path.join(tmpDir, MANIFEST_NAME), @@ -254,6 +260,6 @@ describe('manifest path safety', () => { assert.deepEqual(modified, []); assert.equal(fs.readFileSync(outside, 'utf8'), 'outside user data\n'); - assert.equal(fs.existsSync(path.join(tmpDir, PATCHES_DIR_NAME, '..', 'outside-managed-file.txt')), false); + assert.equal(fs.existsSync(path.join(tmpDir, PATCHES_DIR_NAME, '..', path.basename(outside))), false); }); }); diff --git a/tests/bug-3033-sdk-flag-wired.test.cjs b/tests/bug-3033-sdk-flag-wired.test.cjs index ee867735a..276a25699 100644 --- a/tests/bug-3033-sdk-flag-wired.test.cjs +++ b/tests/bug-3033-sdk-flag-wired.test.cjs @@ -150,6 +150,21 @@ describe('bug #3033: --sdk flag (opts.forceSdk) must be wired into installSdkIfN ); }); + test('throwOnFailure=true converts missing SDK dist into catchable error', () => { + fs.mkdirSync(sdkDir, { recursive: true }); + + assert.throws( + () => captureConsole(() => { + installSdkIfNeeded({ sdkDir, isLocal: true, forceSdk: true, throwOnFailure: true }); + }), + (error) => { + assert.equal(error.code, 'GSD_SDK_MISSING_DIST'); + assert.equal(error.exitCode, 1); + return true; + } + ); + }); + test('forceSdk=false (default) + isLocal=true + dist missing: retains #2678 soft-skip', () => { // Verify the #2678 contract is not broken for the default (no --sdk) path. fs.mkdirSync(sdkDir, { recursive: true }); diff --git a/tests/installer-migration-install-integration.test.cjs b/tests/installer-migration-install-integration.test.cjs index 6e7b496ed..9f831fe81 100644 --- a/tests/installer-migration-install-integration.test.cjs +++ b/tests/installer-migration-install-integration.test.cjs @@ -86,6 +86,34 @@ function withWriteFailure(matchPath, fn) { } } +function withSdkDistPresent(fn) { + const sdkCliPath = path.join(__dirname, '..', 'sdk', 'dist', 'cli.js'); + const originalExistsSync = fs.existsSync; + const originalStatSync = fs.statSync; + const originalChmodSync = fs.chmodSync; + fs.existsSync = (filePath) => { + if (path.resolve(String(filePath)) === path.resolve(sdkCliPath)) return true; + return originalExistsSync.call(fs, filePath); + }; + fs.statSync = (filePath, ...args) => { + if (path.resolve(String(filePath)) === path.resolve(sdkCliPath)) { + return { mode: 0o755 }; + } + return originalStatSync.call(fs, filePath, ...args); + }; + fs.chmodSync = (filePath, ...args) => { + if (path.resolve(String(filePath)) === path.resolve(sdkCliPath)) return; + return originalChmodSync.call(fs, filePath, ...args); + }; + try { + return fn(); + } finally { + fs.existsSync = originalExistsSync; + fs.statSync = originalStatSync; + fs.chmodSync = originalChmodSync; + } +} + function stripAnsi(value) { return value.replace(/\x1b\[[0-9;]*m/g, ''); } @@ -188,8 +216,10 @@ describe('installer migration install integration', { concurrency: false }, () = assert.throws( () => captureConsole(() => withEnv('CLAUDE_CONFIG_DIR', claudeHome, () => - withWriteFailure(path.join(claudeHome, 'settings.json'), () => - installModule.installAllRuntimes(['claude'], true, false) + withSdkDistPresent(() => + withWriteFailure(path.join(claudeHome, 'settings.json'), () => + installModule.installAllRuntimes(['claude'], true, false) + ) ) ) ), @@ -203,6 +233,44 @@ describe('installer migration install integration', { concurrency: false }, () = assert.equal(fs.existsSync(path.join(claudeHome, 'gsd-install-state.json')), false); }); + test('rolls back completed runtime migrations when a later runtime install fails', () => { + const claudeHome = path.join(tmpRoot, '.claude'); + fs.mkdirSync(claudeHome, { recursive: true }); + writeFile(claudeHome, 'hooks/statusline.js', 'legacy managed hook\n'); + writeManifest(claudeHome, { + 'hooks/statusline.js': sha256('legacy managed hook\n'), + }); + + writeFile(codexHome, 'hooks/statusline.js', 'legacy managed hook\n'); + writeManifest(codexHome, { + 'hooks/statusline.js': sha256('legacy managed hook\n'), + }); + + assert.throws( + () => captureConsole(() => + withEnv('CLAUDE_CONFIG_DIR', claudeHome, () => + withEnv('CODEX_HOME', codexHome, () => + withWriteFailure(path.join(codexHome, 'get-shit-done', 'VERSION'), () => + installModule.installAllRuntimes(['claude', 'codex'], true, false) + ) + ) + ) + ), + /injected write failure for VERSION/ + ); + + assert.equal( + fs.readFileSync(path.join(claudeHome, 'hooks/statusline.js'), 'utf8'), + 'legacy managed hook\n' + ); + assert.equal(fs.existsSync(path.join(claudeHome, 'gsd-install-state.json')), false); + assert.equal( + fs.readFileSync(path.join(codexHome, 'hooks/statusline.js'), 'utf8'), + 'legacy managed hook\n' + ); + assert.equal(fs.existsSync(path.join(codexHome, 'gsd-install-state.json')), false); + }); + for (const runtime of SUPPORTED_RUNTIMES) { test(`runs managed cleanup migrations for ${runtime}`, () => { const targetDir = path.join(tmpRoot, `.${runtime}-managed-cleanup`); diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index 5a112fd97..02bd9685c 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -934,6 +934,33 @@ test('skips migration records already present in install state', () => { } }); +test('marks zero-action pending migrations as applied', () => { + const configDir = createTempInstall(); + try { + writeManifest(configDir, {}); + + const result = runInstallerMigrations({ + configDir, + migrations: [ + { + id: '2026-05-11-noop-cleanup', + description: 'No-op cleanup', + plan: () => [], + }, + ], + now: () => '2026-05-11T00:00:06.000Z', + }); + + assert.deepEqual(result.appliedMigrationIds, ['2026-05-11-noop-cleanup']); + assert.equal(result.journalRelPath, null); + assert.deepEqual(readInstallState(configDir).appliedMigrations.map((entry) => entry.id), [ + '2026-05-11-noop-cleanup', + ]); + } finally { + cleanup(configDir); + } +}); + test('refuses to plan an already-applied migration whose checksum changed', () => { const configDir = createTempInstall(); try { @@ -1064,6 +1091,37 @@ test('rejects migration actions that escape the install root', () => { } }); +test('rejects migration actions that normalize to the install root', () => { + const configDir = createTempInstall(); + try { + writeManifest(configDir, {}); + + for (const relPath of ['.', 'hooks/..']) { + assert.throws( + () => planInstallerMigrations({ + configDir, + migrations: [ + { + id: `2026-05-11-bad-path-${relPath.replace(/[^a-z0-9]/gi, '-')}`, + description: 'Bad path', + plan: () => [ + { + type: 'remove-managed', + relPath, + reason: 'bad path', + }, + ], + }, + ], + }), + /relPath must stay inside configDir/ + ); + } + } finally { + cleanup(configDir); + } +}); + test('runs discovered installer migrations against manifest-managed legacy orphan files', () => { const configDir = createTempInstall(); try {