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 <ilkmajans-cpu@users.noreply.github.com> Co-authored-by: wellkilo <wellkilo@foxmail.com>
2026-09-17 23:28:04 +02:00 · 2026-09-07 16:40:47 -04:00
parent 967a5fa4ba
commit 59b74901c4
6 changed files with 688 additions and 30 deletions
@@ -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) {
@@ -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,
+};
@@ -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 <command>` 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);
    }