Merge pull request #3130 from shoyann/fix/preserve-codex-user-config

fix: preserve edited Codex user configuration during reinstall and repair

scripts/lib/install-lifecycle.js

@@ -9,6 +9,8 @@ const { loadInstallManifests } = require('./install-manifests');
 const { readInstallState, validateInstallState } = require('./install-state');
 const { assertWithinTrustedRoot } = require('./path-safety');
 const { createInstallPlanFromRequest } = require('./install/runtime');
+const { assertNoNewUserOwnedFile, prepareUserOwnedFileGuard } = require('./install/ownership-guard');
+const { isCodexUserConfig } = require('./install/codex-user-config');
 const { getRecordedHookConsent } = require('./install/hook-consent');
 const {
   prepareClaudeSkillMigration,
@@ -1871,7 +1873,7 @@ function assertValidInstallStateForWrite(state, label) {
   throw new Error(`Invalid install-state (${label}): ${details}`);
 }
 
-function writeRefreshedInstallState(record, statePreview) {
+function writeRefreshedInstallState(record, statePreview, writtenPaths = []) {
   const trustedStatePreview = buildAdapterDerivedStatePreview(statePreview, record);
   const stateWithCurrentDigests = {
     ...trustedStatePreview,
@@ -1879,6 +1881,19 @@ function writeRefreshedInstallState(record, statePreview) {
       if (!operation.destinationPath) {
         return { ...operation };
       }
+      // Refreshing a ledger is not a file write. Keep the last installed digest
+      // for untouched shared configs so a concurrent user edit is never claimed.
+      if (isCodexUserConfig(record, operation)
+        && !writtenPaths.some(writtenPath => path.relative(writtenPath, operation.destinationPath) === '')) {
+        const previousOperation = (record.state.operations || []).find(previous => (
+          previous.destinationPath
+          && path.relative(previous.destinationPath, operation.destinationPath) === ''
+        ));
+        const { contentSha256: _plannedDigest, ...operationWithoutDigest } = operation;
+        return previousOperation && previousOperation.contentSha256
+          ? { ...operationWithoutDigest, contentSha256: previousOperation.contentSha256 }
+          : operationWithoutDigest;
+      }
       try {
         const contentSha256 = crypto.createHash('sha256')
           .update(readFileNoFollow(operation.destinationPath))
@@ -1908,7 +1923,10 @@ function prepareRepairMigration(plan, record) {
     installStatePath: record.installStatePath,
     statePreview: buildAdapterDerivedStatePreview(plan.statePreview, record),
   };
-  const migration = prepareClaudeSkillMigration(trustedPlan);
+  const skillMigration = prepareClaudeSkillMigration(trustedPlan);
+  const migration = record.adapter.id === 'codex-home'
+    ? prepareUserOwnedFileGuard(trustedPlan, skillMigration)
+    : skillMigration;
   return {
     migration,
     plan: {
@@ -2157,6 +2175,9 @@ function repairInstalledStates(options = {}) {
       }
 
       for (const operation of repairOperations) {
+        if (record.adapter.id === 'codex-home') {
+          assertNoNewUserOwnedFile(migration, operation, desiredPlan);
+        }
         const repairedPath = executeRepairOperation(
           context.repoRoot,
           operation,
@@ -2192,7 +2213,7 @@ function repairInstalledStates(options = {}) {
             installedAt: record.state.installedAt,
             source: { ...record.state.source },
           };
-      writeRefreshedInstallState(record, statePreviewToWrite);
+      writeRefreshedInstallState(record, statePreviewToWrite, repairedPaths);
 
       return {
         adapter: record.adapter,

scripts/lib/install/apply.js

@@ -498,7 +498,7 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals
       if (typeof beforeOperationWrite === 'function') {
         beforeOperationWrite({ plan: appliedPlan, operation });
       }
-      assertNoNewUserOwnedFile(migration, operation);
+      assertNoNewUserOwnedFile(migration, operation, appliedPlan);
 
       if (
         operation.kind === 'update-claude-settings'

scripts/lib/install/codex-user-config.js

@@ -0,0 +1,54 @@
+'use strict';
+
+const crypto = require('crypto');
+const fs = require('fs');
+const path = require('path');
+const { assertWithinTrustedRoot } = require('../path-safety');
+
+function isCodexUserConfig(plan, operation) {
+  if (plan.adapter.id !== 'codex-home' || operation.kind !== 'copy-file') {
+    return false;
+  }
+  const relativePath = path.relative(plan.targetRoot, operation.destinationPath);
+  const name = process.platform === 'win32' ? relativePath.toLowerCase() : relativePath;
+  return name === 'config.toml' || name === (process.platform === 'win32' ? 'agents.md' : 'AGENTS.md');
+}
+
+function readConfigDigest(plan, destinationPath) {
+  assertWithinTrustedRoot(destinationPath, plan.targetRoot, 'inspect Codex user configuration');
+  const flags = fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0);
+  const descriptor = fs.openSync(destinationPath, flags);
+  try {
+    const opened = fs.fstatSync(descriptor, { bigint: true });
+    const current = fs.lstatSync(destinationPath, { bigint: true });
+    if (!opened.isFile() || !current.isFile() || current.isSymbolicLink()
+      || opened.ino !== current.ino || opened.dev !== current.dev) {
+      throw new Error(`Refusing to inspect changed Codex configuration: ${destinationPath}`);
+    }
+    assertWithinTrustedRoot(destinationPath, plan.targetRoot, 'inspect Codex user configuration');
+    return crypto.createHash('sha256').update(fs.readFileSync(descriptor)).digest('hex');
+  } finally {
+    fs.closeSync(descriptor);
+  }
+}
+
+function hasEditedCodexUserConfig(plan, operation, previousOperation) {
+  if (!isCodexUserConfig(plan, operation)) {
+    return false;
+  }
+  let digest;
+  try {
+    digest = readConfigDigest(plan, operation.destinationPath);
+  } catch (error) {
+    if (error.code === 'ENOENT') {
+      return false; // A missing scaffold can still be restored.
+    }
+    throw error;
+  }
+  // Compare with the bytes ECC actually installed, never the newest template.
+  // Old ledgers without a digest cannot prove that an existing file is unchanged.
+  const recorded = previousOperation && previousOperation.contentSha256;
+  return !/^[a-f0-9]{64}$/i.test(recorded || '') || digest !== recorded.toLowerCase();
+}
+
+module.exports = { hasEditedCodexUserConfig, isCodexUserConfig };

scripts/lib/install/ownership-guard.js

@@ -4,6 +4,7 @@ const fs = require('fs');
 const path = require('path');
 
 const { readInstallState } = require('../install-state');
+const { hasEditedCodexUserConfig } = require('./codex-user-config');
 
 function pathExists(filePath) {
   try {
@@ -60,24 +61,32 @@ function prepareUserOwnedFileGuard(plan, migration) {
   );
   const managedDestinations = new Set(previousManagedOperations.keys());
 
-  const appliedOperations = [];
+  const plannedOperations = (migration && migration.appliedOperations) || [];
+  const plannedDestinations = new Set(plannedOperations.map(operation => comparablePath(operation.destinationPath)));
+  // Selective reinstalls retain earlier modules in the ledger. Inspect those
+  // entries too, without turning them into additional writes in this install.
+  const retainedOperations = ((migration.finalState && migration.finalState.operations) || [])
+    .filter(operation => !plannedDestinations.has(comparablePath(operation.destinationPath))
+      && previousManagedOperations.has(comparablePath(operation.destinationPath)));
   const skippedOperations = [];
   const warnings = [];
-  for (const operation of (migration && migration.appliedOperations) || []) {
+  for (const operation of [...plannedOperations, ...retainedOperations]) {
+    const previousOperation = previousManagedOperations.get(comparablePath(operation.destinationPath));
+    const editedConfig = previousOperation
+      && hasEditedCodexUserConfig(plan, operation, previousOperation);
     if (
       operation
       && operation.kind === 'copy-file'
       && operation.destinationPath
       && pathExists(operation.destinationPath)
-      && !managedDestinations.has(comparablePath(operation.destinationPath))
+      && (!managedDestinations.has(comparablePath(operation.destinationPath)) || editedConfig)
     ) {
       skippedOperations.push(operation);
-      warnings.push(
-        `Skipped user-owned file ${operation.destinationPath}: the existing file is not recorded in ECC install-state.`
-      );
+      warnings.push(editedConfig
+        ? `Preserved user configuration ${operation.destinationPath}: changed or unverifiable since installation. ECC no longer manages this file; apply future configuration updates manually.`
+        : `Skipped user-owned file ${operation.destinationPath}: the existing file is not recorded in ECC install-state.`);
       continue;
     }
-    appliedOperations.push(operation);
   }
 
   if (skippedOperations.length === 0) {
@@ -87,6 +96,9 @@ function prepareUserOwnedFileGuard(plan, migration) {
   const skippedDestinations = new Set(
     skippedOperations.map(operation => comparablePath(operation.destinationPath))
   );
+  const appliedOperations = plannedOperations.filter(operation => (
+    !skippedDestinations.has(comparablePath(operation.destinationPath))
+  ));
   const filterStateOperations = operations => (operations || [])
     .filter(operation => !skippedDestinations.has(comparablePath(operation.destinationPath)));
 
@@ -125,7 +137,11 @@ function prepareUserOwnedFileGuard(plan, migration) {
   };
 }
 
-function assertNoNewUserOwnedFile(migration, operation) {
+function assertNoNewUserOwnedFile(migration, operation, plan) {
+  const previousOperation = migration.previousManagedOperations.get(comparablePath(operation.destinationPath));
+  if (plan && hasEditedCodexUserConfig(plan, operation, previousOperation)) {
+    throw new Error(`Refusing to overwrite user configuration changed after planning: ${operation.destinationPath}. Rerun to preserve it.`);
+  }
   if (operation.kind !== 'copy-file'
     || migration.managedDestinations.has(comparablePath(operation.destinationPath))
     || !pathExists(operation.destinationPath)) {