From 59b74901c451b899fa873b8a4a2ced1ab945db4d Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:32:58 -0400 Subject: [PATCH] fix(install): preserve user files and honor uninstall previews Generalize PR #2981 ownership protection to every managed target. Reject mismatched target state and preserve files that appear during writes or failed-install checkpoints. Keep prior hashes for managed files a failed attempt never writes. Integrate PR #2980 preview wording and global dry-run propagation, with PR #2956 fail-closed environment validation and CLI/legacy regression coverage. Fixes #2964. Fixes #2952. Co-authored-by: ilkmajans-cpu Co-authored-by: wellkilo --- scripts/lib/install/apply.js | 33 +++- scripts/lib/install/ownership-guard.js | 159 +++++++++++++++++++ scripts/uninstall.js | 39 +++-- tests/scripts/ecc.test.js | 97 ++++++++++-- tests/scripts/ownership-guard.test.js | 180 +++++++++++++++++++++ tests/scripts/uninstall.test.js | 210 ++++++++++++++++++++++++- 6 files changed, 688 insertions(+), 30 deletions(-) create mode 100644 scripts/lib/install/ownership-guard.js create mode 100644 tests/scripts/ownership-guard.test.js diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index 8a726883e..fbab1293b 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -28,6 +28,11 @@ const { removeLegacyClaudeSkillFiles, } = require('./claude-skill-migration'); const { cleanupLegacyAntigravityInstall } = require('./antigravity-legacy-migration'); +const { + assertNoNewUserOwnedFile, + prepareUserOwnedFileGuard, + preserveUnwrittenFiles, +} = require('./ownership-guard'); const { cleanupLegacyOpencodeInstall } = require('./opencode-legacy-migration'); const { buildInstallIndex, rewriteRelativeLinks } = require('./link-rewrite'); const { adaptAntigravityAgent } = require('./antigravity-agent'); @@ -393,7 +398,7 @@ function prepareHookConsentMigration(plan, migration) { function previewInstallPlan(plan) { const migration = prepareHookConsentMigration( plan, - prepareClaudeSkillMigration(plan) + prepareUserOwnedFileGuard(plan, prepareClaudeSkillMigration(plan)) ); const appliedPlan = { ...plan, @@ -446,7 +451,7 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals } const migration = prepareHookConsentMigration( plan, - prepareClaudeSkillMigration(plan) + prepareUserOwnedFileGuard(plan, prepareClaudeSkillMigration(plan)) ); const appliedPlan = { ...plan, @@ -460,6 +465,7 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals operation.kind === 'remove-claude-settings-hooks' )).length; let completedHookRemovalCount = 0; + const writtenDestinations = new Set(); if (migration.requiresBridgeState) { // Own every operation that may be written during a flat-skill migration // before the first copy. A later failure is retryable and uninstall can @@ -485,6 +491,7 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals if (typeof beforeOperationWrite === 'function') { beforeOperationWrite({ plan: appliedPlan, operation }); } + assertNoNewUserOwnedFile(migration, operation); if ( operation.kind === 'update-claude-settings' @@ -516,6 +523,7 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals assertSafeInstallOperation(appliedPlan, operation); }, }); + writtenDestinations.add(operation.destinationPath); if (operation.kind === 'remove-claude-settings-hooks') { completedHookRemovalCount += 1; } @@ -540,6 +548,7 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals ); const mergedValue = deepMergeJson(currentValue, filteredPayload); fs.writeFileSync(operation.destinationPath, formatJson(mergedValue), 'utf8'); + writtenDestinations.add(operation.destinationPath); continue; } @@ -547,6 +556,7 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals const sourceConfig = readJsonObject(operation.sourcePath, 'MCP config'); const filteredConfig = filterMcpConfig(sourceConfig, disabledServers).config; fs.writeFileSync(operation.destinationPath, formatJson(filteredConfig), 'utf8'); + writtenDestinations.add(operation.destinationPath); continue; } @@ -569,10 +579,12 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals }) : transformed; fs.writeFileSync(operation.destinationPath, installedContent, 'utf8'); + writtenDestinations.add(operation.destinationPath); continue; } fs.copyFileSync(operation.sourcePath, operation.destinationPath); + writtenDestinations.add(operation.destinationPath); } if (hasLegacyMigration) { @@ -600,10 +612,19 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals persistInstallState( plan.installStatePath, stateWithContentDigests( - hookRemovalCount > 0 && completedHookRemovalCount === hookRemovalCount - ? migration.finalState - : migration.bridgeState, - appliedPlan + preserveUnwrittenFiles( + hookRemovalCount > 0 && completedHookRemovalCount === hookRemovalCount + ? migration.finalState + : migration.bridgeState, + migration, + writtenDestinations + ), + { + ...appliedPlan, + operations: appliedPlan.operations.filter(operation => ( + writtenDestinations.has(operation.destinationPath) + )), + } ) ); } catch (checkpointError) { diff --git a/scripts/lib/install/ownership-guard.js b/scripts/lib/install/ownership-guard.js new file mode 100644 index 000000000..962c62e88 --- /dev/null +++ b/scripts/lib/install/ownership-guard.js @@ -0,0 +1,159 @@ +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const { readInstallState } = require('../install-state'); + +function pathExists(filePath) { + try { + fs.lstatSync(filePath); + return true; + } catch (error) { + if (error && error.code === 'ENOENT') { + return false; + } + throw error; + } +} + +function comparablePath(filePath) { + const resolvedPath = path.resolve(filePath); + return process.platform === 'win32' ? resolvedPath.toLowerCase() : resolvedPath; +} + +/** + * #2964: the shared copy path used to write every copy-file operation + * unconditionally and record the destination as `ownership: 'managed'` even + * when the file already existed and was authored by the user. The visible + * symptom is a lost edit; the dangerous one is the install-state record, + * which makes a later uninstall delete the user's file. + * + * This guard generalises the Claude flat-skill migration conflict pattern to + * every adapter copy operation: when a destination exists and is NOT recorded + * as an ECC-managed operation in the previous install-state, the operation is + * skipped with a warning instead of overwriting and claiming ownership. + * + * All managed targets share this ownership boundary (#2964). + */ +function prepareUserOwnedFileGuard(plan, migration) { + const previousState = pathExists(plan.installStatePath) + ? readInstallState(plan.installStatePath) + : null; + if (previousState && ( + previousState.target.id !== plan.adapter.id + || comparablePath(previousState.target.root) !== comparablePath(plan.targetRoot) + || comparablePath(previousState.target.installStatePath) !== comparablePath(plan.installStatePath) + )) { + throw new Error(`Refusing install: install-state target does not match the current plan at ${plan.installStatePath}.`); + } + // Recorded files remain updateable by reinstall/repair. Preserve their prior + // digests if an attempt fails before writing them so uninstall detects drift. + const previousManagedOperations = new Map( + ((previousState && previousState.operations) || []) + .filter(operation => ( + operation + && operation.ownership === 'managed' + && operation.destinationPath + )) + .map(operation => [comparablePath(operation.destinationPath), operation]) + ); + const managedDestinations = new Set(previousManagedOperations.keys()); + + const appliedOperations = []; + const skippedOperations = []; + const warnings = []; + for (const operation of (migration && migration.appliedOperations) || []) { + if ( + operation + && operation.kind === 'copy-file' + && operation.destinationPath + && pathExists(operation.destinationPath) + && !managedDestinations.has(comparablePath(operation.destinationPath)) + ) { + skippedOperations.push(operation); + warnings.push( + `Skipped user-owned file ${operation.destinationPath}: the existing file is not recorded in ECC install-state.` + ); + continue; + } + appliedOperations.push(operation); + } + + if (skippedOperations.length === 0) { + return { ...migration, managedDestinations, previousManagedOperations }; + } + + const skippedDestinations = new Set( + skippedOperations.map(operation => comparablePath(operation.destinationPath)) + ); + const filterStateOperations = operations => (operations || []) + .filter(operation => !skippedDestinations.has(comparablePath(operation.destinationPath))); + + // Never leave a skipped destination inside the install-state: recording it + // would claim ownership of a file ECC did not create and make uninstall + // delete it (#2964). + const bridgeState = migration.bridgeState + ? { + ...migration.bridgeState, + operations: filterStateOperations(migration.bridgeState.operations), + } + : migration.bridgeState; + const finalState = migration.finalState + ? { + ...migration.finalState, + operations: filterStateOperations(migration.finalState.operations), + } + : migration.finalState; + + return { + ...migration, + managedDestinations, + previousManagedOperations, + appliedOperations, + skippedOperations: [ + ...((migration && migration.skippedOperations) || []), + ...skippedOperations, + ], + warnings: [...((migration && migration.warnings) || []), ...warnings], + bridgeState, + finalState, + // Only keep bridge persistence when operations actually remain; a fully + // skipped plan installs nothing and must not claim anything. + requiresBridgeState: Boolean(migration.requiresBridgeState) + && appliedOperations.length > 0, + }; +} + +function assertNoNewUserOwnedFile(migration, operation) { + if (operation.kind !== 'copy-file' + || migration.managedDestinations.has(comparablePath(operation.destinationPath)) + || !pathExists(operation.destinationPath)) { + return; + } + throw new Error(`Refusing install: a user-owned file appeared at ${operation.destinationPath} after planning. Rerun the installer to preserve it.`); +} + +function preserveUnwrittenFiles(state, migration, writtenDestinations) { + const writtenPaths = new Set([...writtenDestinations].map(comparablePath)); + return { + ...state, + operations: state.operations.filter(operation => ( + operation.kind !== 'copy-file' + || migration.managedDestinations.has(comparablePath(operation.destinationPath)) + || writtenPaths.has(comparablePath(operation.destinationPath)) + || !pathExists(operation.destinationPath) + )).map(operation => { + const destination = comparablePath(operation.destinationPath); + return operation.kind === 'copy-file' && !writtenPaths.has(destination) + ? migration.previousManagedOperations.get(destination) || operation + : operation; + }), + }; +} + +module.exports = { + assertNoNewUserOwnedFile, + prepareUserOwnedFileGuard, + preserveUnwrittenFiles, +}; diff --git a/scripts/uninstall.js b/scripts/uninstall.js index abeb2efa8..0a7f41231 100644 --- a/scripts/uninstall.js +++ b/scripts/uninstall.js @@ -61,10 +61,15 @@ function printHuman(result) { return; } - console.log('Uninstall summary:\n'); + // Dry-run output must be phrased as a preview so it can never be mistaken + // for a completed uninstall (#2952). + console.log(`Uninstall summary${result.dryRun ? ' (dry run; nothing was removed)' : ''}:\n`); for (const entry of result.results) { console.log(`- ${entry.adapter.id}`); - console.log(` Status: ${entry.status.toUpperCase()}`); + const statusLabel = result.dryRun && entry.status === 'planned' + ? 'WOULD UNINSTALL (dry run)' + : entry.status.toUpperCase(); + console.log(` Status: ${statusLabel}`); console.log(` Install-state: ${entry.installStatePath}`); if (entry.error) { @@ -84,10 +89,10 @@ function printHuman(result) { const candidatePaths = result.dryRun ? entry.plannedRemovals : entry.removedPaths; const paths = Array.isArray(candidatePaths) ? candidatePaths : []; - console.log(` ${result.dryRun ? 'Planned removals' : 'Removed paths'}: ${paths.length}`); + console.log(` ${result.dryRun ? 'Would remove' : 'Removed paths'}: ${paths.length}`); } - console.log(`\nSummary: checked=${result.summary.checkedCount}, ${result.dryRun ? 'planned' : 'uninstalled'}=${result.dryRun ? result.summary.plannedRemovalCount : result.summary.uninstalledCount}, partial=${result.summary.partialCount}, errors=${result.summary.errorCount}`); + console.log(`\nSummary${result.dryRun ? ' (dry run)' : ''}: checked=${result.summary.checkedCount}, ${result.dryRun ? 'planned' : 'uninstalled'}=${result.dryRun ? result.summary.plannedRemovalCount : result.summary.uninstalledCount}, partial=${result.summary.partialCount}, errors=${result.summary.errorCount}`); if (!result.dryRun) { console.log(`\n${exitFeedbackLines().join('\n')}`); @@ -114,6 +119,20 @@ function codexHomePath() { return process.env.CODEX_HOME || path.join(process.env.HOME || os.homedir(), '.codex'); } +/** + * Dry-run is enabled either by the subcommand-level `--dry-run` flag or by the + * global `ecc --dry-run ` prefix, which sets ECC_DRY_RUN=1 (#2952). + * Destructive subcommands must honor both forms rather than silently ignoring + * the global flag. + */ +function isDryRun(options) { + 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'); + } + return options.dryRun || dryRunEnv === '1'; +} + function includesCodexTarget(targets) { return targets.length === 0 || targets.includes('codex'); } @@ -125,6 +144,8 @@ async function main() { showHelp(0); } + const dryRun = isDryRun(options); + if (options.legacyCodexSync && options.targets.length > 0) { throw new Error('--legacy-codex-sync cannot be combined with --target'); } @@ -135,7 +156,7 @@ async function main() { if (options.legacyCodexSync) { result = uninstallLegacyCodexSync({ codexHome: codexHomePath(), - dryRun: options.dryRun, + dryRun, }); mode = 'legacy-codex-sync'; } else { @@ -144,7 +165,7 @@ async function main() { env: process.env, projectRoot: process.cwd(), targets: options.targets, - dryRun: options.dryRun, + dryRun, }); if ( @@ -154,12 +175,12 @@ async function main() { ) { result = uninstallLegacyCodexSync({ codexHome: codexHomePath(), - dryRun: options.dryRun, + dryRun, }); mode = 'legacy-codex-sync'; } - if (mode === 'install-state' && !options.dryRun) { + if (mode === 'install-state' && !dryRun) { const { reconcileCanonicalInstallStates } = require('./lib/install-state-store-sync'); result.installStateProjection = await reconcileCanonicalInstallStates({ homeDir: process.env.HOME || os.homedir(), @@ -177,7 +198,7 @@ async function main() { if (options.json) { console.log(JSON.stringify(result, null, 2)); } else if (mode === 'legacy-codex-sync') { - printLegacy(result, options.dryRun); + printLegacy(result, dryRun); } else { printHuman(result); } diff --git a/tests/scripts/ecc.test.js b/tests/scripts/ecc.test.js index 82f58e306..ea6a0fbcd 100644 --- a/tests/scripts/ecc.test.js +++ b/tests/scripts/ecc.test.js @@ -3,10 +3,12 @@ */ const assert = require('assert'); +const crypto = require('crypto'); const fs = require('fs'); const os = require('os'); const path = require('path'); const { spawnSync } = require('child_process'); +const { createInstallState, writeInstallState } = require('../../scripts/lib/install-state'); const SCRIPT = path.join(__dirname, '..', '..', 'scripts', 'ecc.js'); @@ -14,23 +16,21 @@ function runCli(args, options = {}) { const envOverrides = { ...(options.env || {}), }; - - if (typeof envOverrides.HOME === 'string' && !('USERPROFILE' in envOverrides)) { - envOverrides.USERPROFILE = envOverrides.HOME; - } - - if (typeof envOverrides.USERPROFILE === 'string' && !('HOME' in envOverrides)) { - envOverrides.HOME = envOverrides.USERPROFILE; - } + const inheritedEnv = Object.fromEntries( + Object.entries(process.env).filter(([key]) => key !== 'ECC_DRY_RUN') + ); + const homeAlias = typeof envOverrides.HOME === 'string' && !('USERPROFILE' in envOverrides) + ? { USERPROFILE: envOverrides.HOME } + : typeof envOverrides.USERPROFILE === 'string' && !('HOME' in envOverrides) + ? { HOME: envOverrides.USERPROFILE } + : {}; + const env = { ...inheritedEnv, ...envOverrides, ...homeAlias }; return spawnSync('node', [SCRIPT, ...args], { encoding: 'utf8', cwd: options.cwd || process.cwd(), maxBuffer: 10 * 1024 * 1024, - env: { - ...process.env, - ...envOverrides, - }, + env, }); } @@ -153,6 +153,79 @@ function main() { const payload = parseJson(result.stdout); assert.deepStrictEqual(payload.records, []); }], + ['keeps uninstall read-only when global --dry-run precedes the command', () => { + const homeDir = createTempDir('ecc-cli-uninstall-home-'); + const projectRoot = createTempDir('ecc-cli-uninstall-project-'); + + try { + const targetRoot = path.join(projectRoot, '.cursor'); + const statePath = path.join(targetRoot, 'ecc-install-state.json'); + const managedPath = path.join(targetRoot, 'managed-rule.md'); + const managedContent = 'managed\n'; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(managedPath, managedContent); + writeInstallState(statePath, createInstallState({ + adapter: { id: 'cursor-project', target: 'cursor', kind: 'project' }, + targetRoot, + installStatePath: statePath, + request: { + profile: null, + modules: [], + includeComponents: [], + excludeComponents: [], + legacyLanguages: ['typescript'], + legacyMode: true, + }, + resolution: { + selectedModules: ['legacy-cursor-install'], + skippedModules: [], + }, + source: { + repoVersion: null, + repoCommit: null, + manifestVersion: 1, + }, + operations: [{ + kind: 'copy-file', + moduleId: 'rules-core', + sourceRelativePath: 'rules/common/coding-style.md', + destinationPath: managedPath, + strategy: 'preserve-relative-path', + ownership: 'managed', + scaffoldOnly: false, + contentSha256: crypto.createHash('sha256').update(managedContent).digest('hex'), + }], + })); + + const jsonResult = runCli(['--dry-run', 'uninstall', '--target', 'cursor', '--json'], { + cwd: projectRoot, + env: { HOME: homeDir }, + }); + + assert.strictEqual(jsonResult.status, 0, jsonResult.stderr); + const preview = parseJson(jsonResult.stdout); + assert.strictEqual(preview.dryRun, true); + assert.strictEqual(preview.results[0].status, 'planned'); + assert.deepStrictEqual( + preview.results[0].plannedRemovals.map(candidate => fs.realpathSync(candidate)).sort(), + [managedPath, statePath].map(candidate => fs.realpathSync(candidate)).sort() + ); + + const humanResult = runCli(['--dry-run', 'uninstall', '--target', 'cursor'], { + cwd: projectRoot, + env: { HOME: homeDir }, + }); + assert.strictEqual(humanResult.status, 0, humanResult.stderr); + assert.match(humanResult.stdout, /Status: WOULD UNINSTALL/); + assert.match(humanResult.stdout, /Would remove: 2/); + assert.doesNotMatch(humanResult.stdout, /Status: UNINSTALLED|Removed paths:/); + assert.ok(fs.existsSync(managedPath), 'global dry-run must preserve managed files'); + assert.ok(fs.existsSync(statePath), 'global dry-run must preserve install-state'); + } finally { + fs.rmSync(homeDir, { force: true, recursive: true }); + fs.rmSync(projectRoot, { force: true, recursive: true }); + } + }], ['delegates auto-update command', () => { const homeDir = createTempDir('ecc-cli-home-'); const projectRoot = createTempDir('ecc-cli-project-'); diff --git a/tests/scripts/ownership-guard.test.js b/tests/scripts/ownership-guard.test.js new file mode 100644 index 000000000..1856fc0fc --- /dev/null +++ b/tests/scripts/ownership-guard.test.js @@ -0,0 +1,180 @@ +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { execFileSync } = require('child_process'); +const { createManifestInstallPlan } = require('../../scripts/lib/install/plan'); +const { applyInstallPlan, previewInstallPlan } = require('../../scripts/lib/install/apply'); +const { listInstallTargetAdapters } = require('../../scripts/lib/install-targets/registry'); +const { uninstallInstalledStates } = require('../../scripts/lib/install-lifecycle'); + +let passed = 0; +let failed = 0; +function test(name, fn) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-ownership-')); + try { + const projectRoot = path.join(root, 'project'); + const homeDir = path.join(root, 'home'); + fs.mkdirSync(projectRoot); + fs.mkdirSync(homeDir); + fn({ projectRoot, homeDir, env: {} }); + passed++; + console.log(` PASS ${name}`); + } catch (error) { + failed++; + console.error(` FAIL ${name}: ${error.stack}`); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +} + +function readState(plan) { + return JSON.parse(fs.readFileSync(plan.installStatePath, 'utf8')); +} + +for (const adapter of listInstallTargetAdapters()) { + test(`${adapter.target}: preserve user files through preview, install, reinstall and uninstall`, context => { + const nativeTarget = ['codex', 'gemini', 'opencode'].includes(adapter.target); + const resolved = createManifestInstallPlan({ + ...context, target: adapter.target, + moduleIds: [nativeTarget ? 'platform-configs' : 'rules-core'], + // This test exercises ownership of source files, not plugin compilation. + exemptValidationCodes: ['opencode-plugin-not-built'], + }); + const operation = resolved.operations.find(item => item.kind === 'copy-file'); + assert.ok(operation, 'target must produce a real copy operation'); + const plan = { + ...resolved, operations: [operation], + statePreview: { ...resolved.statePreview, operations: [operation] }, + }; + const destination = operation.destinationPath; + fs.mkdirSync(path.dirname(destination), { recursive: true }); + fs.writeFileSync(destination, 'User-authored content\n'); + const preview = previewInstallPlan(plan); + assert.ok(preview.skippedOperations.some(item => item.destinationPath === destination)); + assert.ok(!preview.statePreview.operations.some(item => item.destinationPath === destination)); + assert.ok(!fs.existsSync(plan.installStatePath), 'preview must not create state'); + for (let attempt = 0; attempt < 2; attempt++) { + const installed = applyInstallPlan(plan); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'User-authored content\n'); + assert.ok(installed.warnings.some(warning => warning.includes('Skipped user-owned file'))); + assert.ok(!readState(plan).operations.some(item => item.destinationPath === destination)); + } + const result = uninstallInstalledStates({ ...context, targets: [adapter.target] }); + assert.strictEqual(result.summary.errorCount, 0); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'User-authored content\n'); + assert.ok(!fs.existsSync(plan.installStatePath)); + }); +} + +test('Antigravity transforms preserve a conflicting agent and still update managed files', context => { + const plan = createManifestInstallPlan({ ...context, target: 'antigravity', moduleIds: ['agents-core'] }); + const userOperation = plan.operations.find(item => item.sourceRelativePath === 'agents/architect.md'); + fs.mkdirSync(path.dirname(userOperation.destinationPath), { recursive: true }); + fs.writeFileSync(userOperation.destinationPath, 'My architect\n'); + applyInstallPlan(plan); + const managed = plan.operations.find(item => item.destinationPath !== userOperation.destinationPath); + const original = fs.readFileSync(managed.destinationPath, 'utf8'); + fs.writeFileSync(managed.destinationPath, 'old managed version\n'); + applyInstallPlan(plan); + assert.strictEqual(fs.readFileSync(managed.destinationPath, 'utf8'), original); + assert.strictEqual(fs.readFileSync(userOperation.destinationPath, 'utf8'), 'My architect\n'); + assert.ok(!readState(plan).operations.some(item => item.destinationPath === userOperation.destinationPath)); +}); + +for (const field of ['id', 'root', 'installStatePath']) { + test(`rejects previous state with mismatched target ${field}`, context => { + const plan = createManifestInstallPlan({ ...context, target: 'antigravity', moduleIds: ['rules-core'] }); + applyInstallPlan(plan); + const operation = plan.operations[0]; + const state = readState(plan); + const mismatched = { ...state, target: { ...state.target, [field]: `${state.target[field]}-other` } }; + fs.writeFileSync(plan.installStatePath, JSON.stringify(mismatched)); + fs.writeFileSync(operation.destinationPath, 'User file\n'); + assert.throws(() => applyInstallPlan(plan), /install-state target does not match/); + assert.strictEqual(fs.readFileSync(operation.destinationPath, 'utf8'), 'User file\n'); + assert.deepStrictEqual(readState(plan), mismatched); + }); +} + +test('preserves multiple user files created at the write boundary without checkpoint ownership', context => { + const plan = createManifestInstallPlan({ ...context, target: 'antigravity', moduleIds: ['rules-core'] }); + const collisions = plan.operations.slice(0, 2).map(operation => operation.destinationPath); + assert.throws(() => applyInstallPlan(plan, { + beforeOperationWrite({ operation }) { + if (operation.destinationPath === collisions[0]) { + for (const collision of collisions) { + fs.mkdirSync(path.dirname(collision), { recursive: true }); + fs.writeFileSync(collision, 'Concurrent user file\n'); + } + } + }, + }), /user-owned file appeared/); + for (const collision of collisions) { + assert.strictEqual(fs.readFileSync(collision, 'utf8'), 'Concurrent user file\n'); + assert.ok(!readState(plan).operations.some(item => item.destinationPath === collision)); + } + uninstallInstalledStates({ ...context, targets: ['antigravity'] }); + for (const collision of collisions) { + assert.strictEqual(fs.readFileSync(collision, 'utf8'), 'Concurrent user file\n'); + } +}); + +test('failed reinstall preserves prior hashes for modified managed files it never wrote', context => { + const plan = createManifestInstallPlan({ ...context, target: 'antigravity', moduleIds: ['rules-core'] }); + applyInstallPlan(plan); + const modified = plan.operations[1].destinationPath; + const prior = readState(plan).operations.find(operation => operation.destinationPath === modified); + fs.writeFileSync(modified, 'My modified managed file\n'); + assert.throws(() => applyInstallPlan(plan, { + beforeOperationWrite() { throw new Error('injected early failure'); }, + }), /injected early failure/); + assert.strictEqual(readState(plan).operations.find(operation => operation.destinationPath === modified).contentSha256, + prior.contentSha256, 'unattempted managed files must keep their prior digest'); + uninstallInstalledStates({ ...context, targets: ['antigravity'] }); + assert.strictEqual(fs.readFileSync(modified, 'utf8'), 'My modified managed file\n'); +}); + +test('partial install checkpoints never claim skipped user files', context => { + const plan = createManifestInstallPlan({ ...context, target: 'antigravity', moduleIds: ['rules-core'] }); + const userOperation = plan.operations[0]; + fs.mkdirSync(path.dirname(userOperation.destinationPath), { recursive: true }); + fs.writeFileSync(userOperation.destinationPath, 'Keep this file\n'); + assert.throws(() => applyInstallPlan(plan, { + beforeOperationWrite() { throw new Error('injected write failure'); }, + }), /injected write failure/); + assert.ok(!readState(plan).operations.some(item => item.destinationPath === userOperation.destinationPath)); + uninstallInstalledStates({ ...context, targets: ['antigravity'] }); + assert.strictEqual(fs.readFileSync(userOperation.destinationPath, 'utf8'), 'Keep this file\n'); +}); + +test('global CLI dry-run preserves installed files, state and canonical database', context => { + const plan = createManifestInstallPlan({ ...context, target: 'cursor', moduleIds: ['rules-core'] }); + applyInstallPlan(plan); + const stateBefore = fs.readFileSync(plan.installStatePath); + const operation = plan.operations[0]; + const fileBefore = fs.readFileSync(operation.destinationPath); + const env = { + ...process.env, HOME: context.homeDir, USERPROFILE: context.homeDir, + CODEX_HOME: path.join(context.homeDir, '.codex'), + XDG_CONFIG_HOME: path.join(context.homeDir, '.config'), + ECC_DRY_RUN: '0', + }; + const cli = path.join(__dirname, '../../scripts/ecc.js'); + for (const args of [['--dry-run', 'uninstall'], ['uninstall', '--dry-run']]) { + const stdout = execFileSync(process.execPath, [cli, ...args, '--target', 'cursor'], { + cwd: context.projectRoot, env, encoding: 'utf8', timeout: 30000, + }); + assert.match(stdout, /WOULD UNINSTALL/); + assert.match(stdout, /Would remove:/); + assert.doesNotMatch(stdout, /Status: UNINSTALLED|Removed paths:/); + assert.deepStrictEqual(fs.readFileSync(plan.installStatePath), stateBefore); + assert.deepStrictEqual(fs.readFileSync(operation.destinationPath), fileBefore); + assert.deepStrictEqual(fs.readdirSync(context.homeDir), [], 'dry-run must not initialize canonical state'); + } +}); + +console.log(`Results: Passed: ${passed}, Failed: ${failed}`); +process.exitCode = failed ? 1 : 0; diff --git a/tests/scripts/uninstall.test.js b/tests/scripts/uninstall.test.js index 7369b2d5b..75759969d 100644 --- a/tests/scripts/uninstall.test.js +++ b/tests/scripts/uninstall.test.js @@ -47,9 +47,13 @@ function writeState(filePath, options) { } function run(args = [], options = {}) { - const env = options.homeDir - ? { ...process.env, HOME: options.homeDir, CODEX_HOME: path.join(options.homeDir, '.codex') } - : Object.fromEntries(Object.entries(process.env).filter(([key]) => key !== 'CODEX_HOME')) + const inheritedEnv = Object.fromEntries( + Object.entries(process.env).filter(([key]) => key !== 'ECC_DRY_RUN' && key !== 'CODEX_HOME') + ); + const homeEnv = options.homeDir + ? { HOME: options.homeDir, USERPROFILE: options.homeDir, CODEX_HOME: path.join(options.homeDir, '.codex') } + : {}; + const env = { ...inheritedEnv, ...(options.env || {}), ...homeEnv }; try { const stdout = execFileSync('node', [SCRIPT, ...args], { @@ -291,6 +295,138 @@ function runTests() { } })) passed++; else failed++; + // #2952: the global `ecc --dry-run uninstall` prefix sets ECC_DRY_RUN=1. + // The uninstaller must honor it exactly like the subcommand-level flag. + if (test('honors global ECC_DRY_RUN=1 without mutating managed files (#2952)', () => { + const homeDir = createTempDir('uninstall-home-'); + const projectRoot = createTempDir('uninstall-project-'); + + try { + const targetRoot = path.join(projectRoot, '.cursor'); + fs.mkdirSync(targetRoot, { recursive: true }); + const normalizedTargetRoot = fs.realpathSync(targetRoot); + const statePath = path.join(normalizedTargetRoot, 'ecc-install-state.json'); + const renderedPath = path.join(normalizedTargetRoot, 'generated.md'); + fs.writeFileSync(renderedPath, '# generated\n'); + + writeState(statePath, { + adapter: { id: 'cursor-project', target: 'cursor', kind: 'project' }, + targetRoot: normalizedTargetRoot, + installStatePath: statePath, + request: { + profile: null, + modules: ['platform-configs'], + includeComponents: [], + excludeComponents: [], + legacyLanguages: [], + legacyMode: false, + }, + resolution: { + selectedModules: ['platform-configs'], + skippedModules: [], + }, + operations: [ + { + kind: 'render-template', + moduleId: 'platform-configs', + sourceRelativePath: '.cursor/generated.md.template', + destinationPath: renderedPath, + strategy: 'render-template', + ownership: 'managed', + scaffoldOnly: false, + renderedContent: '# generated\n', + }, + ], + source: { + repoVersion: CURRENT_PACKAGE_VERSION, + repoCommit: 'abc123', + manifestVersion: CURRENT_MANIFEST_VERSION, + }, + }); + + // No --dry-run flag: the global flag form must still be a no-op. + const uninstallResult = run(['--target', 'cursor', '--json'], { + cwd: projectRoot, + homeDir, + env: { ECC_DRY_RUN: '1' }, + }); + assert.strictEqual(uninstallResult.code, 0, uninstallResult.stderr); + + const parsed = JSON.parse(uninstallResult.stdout); + assert.strictEqual(parsed.dryRun, true, 'ECC_DRY_RUN=1 must enable dry-run mode'); + assert.ok(parsed.results[0].plannedRemovals.includes(renderedPath)); + assert.ok(fs.existsSync(renderedPath), 'managed file must survive the dry run'); + assert.ok(fs.existsSync(statePath), 'install-state must survive the dry run'); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('phrases dry-run human output as an unmistakable preview (#2952)', () => { + const homeDir = createTempDir('uninstall-home-'); + const projectRoot = createTempDir('uninstall-project-'); + + try { + const targetRoot = path.join(projectRoot, '.cursor'); + fs.mkdirSync(targetRoot, { recursive: true }); + const normalizedTargetRoot = fs.realpathSync(targetRoot); + const statePath = path.join(normalizedTargetRoot, 'ecc-install-state.json'); + const renderedPath = path.join(normalizedTargetRoot, 'generated.md'); + fs.writeFileSync(renderedPath, '# generated\n'); + + writeState(statePath, { + adapter: { id: 'cursor-project', target: 'cursor', kind: 'project' }, + targetRoot: normalizedTargetRoot, + installStatePath: statePath, + request: { + profile: null, + modules: ['platform-configs'], + includeComponents: [], + excludeComponents: [], + legacyLanguages: [], + legacyMode: false, + }, + resolution: { + selectedModules: ['platform-configs'], + skippedModules: [], + }, + operations: [ + { + kind: 'render-template', + moduleId: 'platform-configs', + sourceRelativePath: '.cursor/generated.md.template', + destinationPath: renderedPath, + strategy: 'render-template', + ownership: 'managed', + scaffoldOnly: false, + renderedContent: '# generated\n', + }, + ], + source: { + repoVersion: CURRENT_PACKAGE_VERSION, + repoCommit: 'abc123', + manifestVersion: CURRENT_MANIFEST_VERSION, + }, + }); + + const uninstallResult = run(['--target', 'cursor', '--dry-run'], { + cwd: projectRoot, + homeDir, + }); + assert.strictEqual(uninstallResult.code, 0, uninstallResult.stderr); + + assert.ok(uninstallResult.stdout.includes('dry run'), 'summary header must carry the dry-run marker'); + assert.ok(uninstallResult.stdout.includes('WOULD UNINSTALL (dry run)'), 'status must use conditional wording'); + assert.ok(uninstallResult.stdout.includes('Would remove:'), 'path count must use conditional wording'); + assert.ok(!uninstallResult.stdout.includes('Status: UNINSTALLED'), 'dry run must not claim UNINSTALLED'); + assert.ok(!uninstallResult.stdout.includes('Removed paths:'), 'dry run must not claim removal'); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + if (test('reports preserved legacy Antigravity files as an incomplete uninstall', () => { const homeDir = createTempDir('uninstall-home-'); const projectRoot = createTempDir('uninstall-project-'); @@ -415,6 +551,74 @@ function runTests() { } })) passed++; else failed++; + if (test('global dry-run environment previews legacy Codex cleanup without removing artifacts', () => { + const homeDir = createTempDir('uninstall-legacy-codex-dry-run-home-'); + const projectRoot = createTempDir('uninstall-legacy-codex-dry-run-project-'); + + try { + const codexHome = path.join(homeDir, '.codex'); + const promptPath = path.join(codexHome, 'prompts', 'ecc-plan.md'); + fs.mkdirSync(path.dirname(promptPath), { recursive: true }); + + const statePath = beginLegacySyncState({ + codexHome, + backupDir: path.join(codexHome, 'backups', 'ecc-test'), + }); + recordLegacySyncPath({ statePath, filePath: promptPath }); + fs.writeFileSync(promptPath, '# ECC generated prompt\n'); + finalizeLegacySyncState({ statePath }); + + const uninstallResult = run(['--legacy-codex-sync'], { + cwd: projectRoot, + homeDir, + env: { ECC_DRY_RUN: '1' }, + }); + + assert.strictEqual(uninstallResult.code, 0, uninstallResult.stderr); + assert.match(uninstallResult.stdout, /Status: PLANNED/); + assert.match(uninstallResult.stdout, /Planned changes:/); + assert.doesNotMatch(uninstallResult.stdout, /Status: UNINSTALLED|Removed paths:/); + assert.ok(fs.existsSync(promptPath), 'global dry-run must preserve legacy artifacts'); + assert.ok(fs.existsSync(statePath), 'global dry-run must preserve legacy state'); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('rejects an invalid global dry-run value before legacy cleanup', () => { + const homeDir = createTempDir('uninstall-legacy-codex-invalid-dry-run-home-'); + const projectRoot = createTempDir('uninstall-legacy-codex-invalid-dry-run-project-'); + + try { + const codexHome = path.join(homeDir, '.codex'); + const promptPath = path.join(codexHome, 'prompts', 'ecc-plan.md'); + fs.mkdirSync(path.dirname(promptPath), { recursive: true }); + + const statePath = beginLegacySyncState({ + codexHome, + backupDir: path.join(codexHome, 'backups', 'ecc-test'), + }); + recordLegacySyncPath({ statePath, filePath: promptPath }); + fs.writeFileSync(promptPath, '# ECC generated prompt\n'); + finalizeLegacySyncState({ statePath }); + + const uninstallResult = run(['--legacy-codex-sync'], { + cwd: projectRoot, + homeDir, + env: { ECC_DRY_RUN: 'true' }, + }); + + assert.strictEqual(uninstallResult.code, 1); + assert.match(uninstallResult.stderr, /ECC_DRY_RUN must be "1" or "0" when set/); + assert.ok(fs.existsSync(promptPath), 'invalid dry-run input must preserve legacy artifacts'); + assert.ok(fs.existsSync(statePath), 'invalid dry-run input must preserve legacy state'); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + if (test('does not misclassify a clean Codex home as a legacy install', () => { const homeDir = createTempDir('uninstall-clean-codex-home-'); const projectRoot = createTempDir('uninstall-clean-codex-project-');