fix: installer slice from PR #3120 review (Haelyra)

Honor ECC_DRY_RUN=1 in install and repair arg parsing (previously only --dry-run flag worked, so ecc --dry-run install/repair silently performed real writes). Make writeInstallState atomic via writeFileAtomic (O_EXCL staging, fsync, cleanup on failure, atomic rename preserving previous state). Add writeInstallState failure/concurrency/symlink regression tests and disposable-home CLI dry-run regression tests for both global and subcommand forms.
This commit is contained in:
Geronimo committed 2026-10-10 13:56:00 +05:30
1 parent 4eb71d92a3
commit e1546ed45d
5 files changed
+259 -2

No files matched your search

+2 -2
View File
@@ -341,8 +341,8 @@ function readInstallState(filePath) {
function writeInstallState(filePath, state) {
assertValidInstallState(state, filePath);
fs.mkdirSync(path.dirname(filePath), { recursive: true });
fs.writeFileSync(filePath, `${JSON.stringify(state, null, 2)}\n`);
const { writeFileAtomic } = require('./atomic-write');
writeFileAtomic(filePath, `${JSON.stringify(state, null, 2)}\n`);
return state;
}
+6
View File
@@ -88,6 +88,12 @@ function parseInstallArgs(argv) {
}
}
const dryRunEnv = process.env.ECC_DRY_RUN;
if (dryRunEnv !== undefined && dryRunEnv !== '0' && dryRunEnv !== '1') {
throw new Error('ECC_DRY_RUN must be "1" or "0" when set');
}
parsed.dryRun = parsed.dryRun || dryRunEnv === '1';
return parsed;
}
+6
View File
@@ -40,6 +40,12 @@ function parseArgs(argv) {
}
}
const dryRunEnv = process.env.ECC_DRY_RUN;
if (dryRunEnv !== undefined && dryRunEnv !== '0' && dryRunEnv !== '1') {
throw new Error('ECC_DRY_RUN must be "1" or "0" when set');
}
parsed.dryRun = parsed.dryRun || dryRunEnv === '1';
return parsed;
}
+125
View File
@@ -328,6 +328,131 @@ function runTests() {
}
})) passed++; else failed++;
function makeState(targetRoot, statePath, marker) {
return createInstallState({
adapter: { id: 'claude-home' },
targetRoot,
installStatePath: statePath,
request: {
profile: 'core',
modules: [],
legacyLanguages: [],
legacyMode: false,
},
resolution: {
selectedModules: ['rules-core'],
skippedModules: [],
},
operations: [],
source: {
repoVersion: CURRENT_PACKAGE_VERSION,
repoCommit: 'abc123',
manifestVersion: 1,
},
lastValidatedAt: marker,
});
}
function listStagingFiles(dirPath) {
return fs.readdirSync(dirPath).filter(name => name.includes('.tmp'));
}
if (test('writeInstallState preserves the previous valid state when rename fails', () => {
const testDir = createTestDir();
const statePath = path.join(testDir, 'ecc-install-state.json');
try {
writeInstallState(statePath, makeState(testDir, statePath, '2026-01-01T00:00:00Z'));
const originalRenameSync = fs.renameSync;
fs.renameSync = () => { throw new Error('injected rename failure'); };
try {
assert.throws(
() => writeInstallState(statePath, makeState(testDir, statePath, '2026-02-01T00:00:00Z')),
/injected rename failure/
);
} finally {
fs.renameSync = originalRenameSync;
}
assert.strictEqual(readInstallState(statePath).lastValidatedAt, '2026-01-01T00:00:00Z');
assert.deepStrictEqual(listStagingFiles(testDir), []);
} finally {
cleanupTestDir(testDir);
}
})) passed++; else failed++;
if (test('writeInstallState cleans up staging files when the write fails', () => {
const testDir = createTestDir();
const statePath = path.join(testDir, 'ecc-install-state.json');
try {
const originalWriteFileSync = fs.writeFileSync;
fs.writeFileSync = () => { throw new Error('injected write failure'); };
try {
assert.throws(
() => writeInstallState(statePath, makeState(testDir, statePath, '2026-01-01T00:00:00Z')),
/injected write failure/
);
} finally {
fs.writeFileSync = originalWriteFileSync;
}
assert.ok(!fs.existsSync(statePath));
assert.deepStrictEqual(listStagingFiles(testDir), []);
} finally {
cleanupTestDir(testDir);
}
})) passed++; else failed++;
if (test('writeInstallState replaces symlinks instead of following them', () => {
if (process.platform === 'win32') return;
const testDir = createTestDir();
const statePath = path.join(testDir, 'ecc-install-state.json');
const targetPath = path.join(testDir, 'link-target.json');
fs.writeFileSync(targetPath, '{"untouched":true}\n');
try {
fs.symlinkSync(targetPath, statePath);
writeInstallState(statePath, makeState(testDir, statePath, '2026-01-01T00:00:00Z'));
assert.ok(fs.lstatSync(statePath).isFile());
assert.strictEqual(readInstallState(statePath).lastValidatedAt, '2026-01-01T00:00:00Z');
assert.strictEqual(fs.readFileSync(targetPath, 'utf8'), '{"untouched":true}\n');
} finally {
cleanupTestDir(testDir);
}
})) passed++; else failed++;
if (test('concurrent writeInstallState calls always leave a complete valid state', () => {
const testDir = createTestDir();
const statePath = path.join(testDir, 'ecc-install-state.json');
const markers = ['2026-03-01T00:00:00Z', '2026-03-02T00:00:00Z', '2026-03-03T00:00:00Z', '2026-03-04T00:00:00Z'];
try {
writeInstallState(statePath, makeState(testDir, statePath, markers[0]));
const { spawnSync } = require('child_process');
const script = 'const {readInstallState, writeInstallState} = require(process.env.REPO_ROOT + "/scripts/lib/install-state");' +
'const state = readInstallState(process.env.STATE_PATH);' +
'state.lastValidatedAt = process.env.MARKER;' +
'writeInstallState(process.env.STATE_PATH, state);';
for (const marker of markers.slice(1)) {
const child = spawnSync(process.execPath, ['-e', script], {
encoding: 'utf8',
timeout: 30000,
env: {
...process.env,
REPO_ROOT: path.join(__dirname, '..', '..'),
STATE_PATH: statePath,
MARKER: marker,
},
});
assert.strictEqual(child.status, 0, child.stderr);
}
const final = readInstallState(statePath);
assert.ok(markers.includes(final.lastValidatedAt));
assert.deepStrictEqual(listStagingFiles(testDir), []);
} finally {
cleanupTestDir(testDir);
}
})) passed++; else failed++;
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
process.exit(failed > 0 ? 1 : 0);
}
+120
View File
@@ -0,0 +1,120 @@
'use strict';
const assert = require('assert');
const fs = require('fs');
const os = require('os');
const path = require('path');
const { spawnSync } = require('child_process');
const repoRoot = path.join(__dirname, '..', '..');
const eccCli = path.join(repoRoot, 'scripts', 'ecc.js');
let passed = 0;
let failed = 0;
function test(name, fn) {
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-install-dry-run-'));
try {
const projectRoot = path.join(root, 'project');
const homeDir = path.join(root, 'home');
fs.mkdirSync(projectRoot, { recursive: true });
fs.mkdirSync(homeDir, { recursive: true });
fn({ projectRoot, homeDir });
passed++;
console.log(` PASS ${name}`);
} catch (error) {
failed++;
console.error(` FAIL ${name}: ${error.stack}`);
} finally {
fs.rmSync(root, { recursive: true, force: true });
}
}
function makeEnv(homeDir) {
const env = {
...process.env,
HOME: homeDir,
USERPROFILE: homeDir,
CODEX_HOME: path.join(homeDir, '.codex'),
XDG_CONFIG_HOME: path.join(homeDir, '.config'),
};
delete env.ECC_DRY_RUN;
return env;
}
function runEcc(args, context) {
return spawnSync(process.execPath, [eccCli, ...args], {
cwd: context.projectRoot,
encoding: 'utf8',
timeout: 120000,
env: makeEnv(context.homeDir),
});
}
function walkFiles(dirPath) {
const found = [];
const visit = current => {
for (const entry of fs.readdirSync(current, { withFileTypes: true })) {
const full = path.join(current, entry.name);
if (entry.isDirectory()) visit(full);
else if (entry.isFile()) found.push(full);
}
};
if (fs.existsSync(dirPath)) visit(dirPath);
return found.sort();
}
function findStateFiles(homeDir) {
return walkFiles(homeDir).filter(file => file.endsWith('ecc-install-state.json'));
}
function testInstallForms() {
for (const args of [
['--dry-run', 'install', '--target', 'cursor', '--modules', 'rules-core'],
['install', '--target', 'cursor', '--modules', 'rules-core', '--dry-run'],
]) {
test(`dry-run install writes nothing (${args.slice(0, 2).join(' ')})`, context => {
const result = runEcc(args, context);
assert.strictEqual(result.status, 0, result.stderr);
assert.deepStrictEqual(walkFiles(context.homeDir), []);
assert.deepStrictEqual(findStateFiles(context.homeDir), []);
});
}
}
function testRepairForms() {
for (const args of [
['--dry-run', 'repair', '--target', 'cursor'],
['repair', '--target', 'cursor', '--dry-run'],
]) {
test(`dry-run repair preserves drift and state (${args.slice(0, 2).join(' ')})`, context => {
const installed = runEcc(['install', '--target', 'cursor', '--modules', 'rules-core'], context);
assert.strictEqual(installed.status, 0, installed.stderr);
const roots = [context.homeDir, context.projectRoot];
const stateFiles = roots.flatMap(findStateFiles);
assert.ok(stateFiles.length > 0, 'expected a real install to write state first');
const managedFile = roots.flatMap(walkFiles).find(
file => !file.endsWith('ecc-install-state.json') && !file.endsWith('.db')
);
assert.ok(managedFile, 'expected a real install to write managed files first');
fs.appendFileSync(managedFile, '\n# local drift marker\n');
const drifted = fs.readFileSync(managedFile, 'utf8');
const stateBefore = stateFiles.map(file => fs.readFileSync(file));
const result = runEcc(args, context);
assert.strictEqual(result.status, 0, result.stderr);
assert.strictEqual(fs.readFileSync(managedFile, 'utf8'), drifted);
assert.deepStrictEqual(
stateFiles.map(file => fs.readFileSync(file)),
stateBefore
);
});
}
}
console.log('\nInstall dry-run CLI regression tests:\n');
testInstallForms();
testRepairForms();
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
process.exitCode = failed ? 1 : 0;