fix: harden installer migration integration
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -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`);
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user