From 0b04c1bfa14aaa13ef295dc2b4ae1cc958fc6278 Mon Sep 17 00:00:00 2001
From: Santhi Prakash <b.santhiprakash@gmail.com>
Date: Sat, 22 Aug 2026 22:02:41 +0000
Subject: [PATCH] fix(scripts): surface unreadable AGENTS.md in legacy codex
 sync detection

hasMarkerBlock previously swallowed every read/open error and returned
false, so an unreadable AGENTS.md (EACCES, EMFILE, EISDIR, ...) made
detectLegacyCodexSync report a clean Codex home instead of an
indeterminate inspection result. The fallback path could then skip
legacy cleanup and exit 0 with legacy artifacts still in place.

Restrict the catch to ENOENT (a missing file legitimately means no
marker block) and rethrow everything else. detectLegacyCodexSync
already propagates from hasMarkerBlock, so callers now see the actual
inspection error instead of a misleading 'no marker'.

Regression test in tests/lib/codex-legacy-sync.test.js makes a
detectLegacyCodexSync call against an unreadable AGENTS.md and asserts
that it throws something other than ENOENT, plus a sanity check that
a missing AGENTS.md still reads as no-marker.
---
 scripts/lib/codex-legacy-sync.js    | 11 +++++--
 tests/lib/codex-legacy-sync.test.js | 48 +++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/codex-legacy-sync.js b/scripts/lib/codex-legacy-sync.js
index 2afa26b00..5eb92d180 100644
--- a/scripts/lib/codex-legacy-sync.js
+++ b/scripts/lib/codex-legacy-sync.js
@@ -483,8 +483,15 @@ function hasMarkerBlock(codexHome) {
       const stripped = stripMarkerBlock(snapshot.content);
       return stripped !== snapshot.content;
     }
-  } catch (_error) {
-    // Non-regular or unreadable AGENTS.md is not a clean marker signal.
+  } catch (error) {
+    // Only ENOENT means "no AGENTS.md" → no marker. Any other error
+    // (EACCES, EMFILE, EISDIR, symlink-ELOOP, ...) is an indeterminate
+    // inspection result and must propagate so callers do not read it as
+    // "clean home". Throwing here is intentional per the repo coding
+    // guideline: "Always handle errors explicitly at every level and never
+    // silently swallow errors."
+    if (error && error.code === 'ENOENT') return false;
+    throw error;
   }
   return false;
 }
diff --git a/tests/lib/codex-legacy-sync.test.js b/tests/lib/codex-legacy-sync.test.js
index ff98b06ca..a5cc5a4ce 100644
--- a/tests/lib/codex-legacy-sync.test.js
+++ b/tests/lib/codex-legacy-sync.test.js
@@ -7,6 +7,7 @@ const path = require('path');
 
 const {
   beginLegacySyncState,
+  detectLegacyCodexSync,
   finalizeLegacySyncState,
   recordLegacySyncPath,
   rollbackLegacyCodexSync,
@@ -521,6 +522,53 @@ function runTests() {
     fs.rmSync(homeDir, { recursive: true, force: true });
   })) passed += 1; else failed += 1;
 
+  if (test('detectLegacyCodexSync surfaces unreadable AGENTS.md instead of reporting clean', () => {
+    // hasMarkerBlock previously swallowed every read/open error and returned false,
+    // which made detectLegacyCodexSync claim a clean home even when AGENTS.md was
+    // unreadable (EACCES, EMFILE, ...). The fix is to rethrow every error except
+    // ENOENT (a missing file is a legitimate "no marker" signal).
+    const homeDir = tempDir('legacy-codex-home-');
+    const codexHome = path.join(homeDir, '.codex');
+    const agentsPath = path.join(codexHome, 'AGENTS.md');
+    fs.mkdirSync(codexHome, { recursive: true });
+    fs.writeFileSync(agentsPath, '# User instructions\n<!-- BEGIN ECC -->\n<!-- END ECC -->\n');
+
+    // chmod 000 to make AGENTS.md unreadable. Skip when running as root because
+    // root bypasses mode bits and the test would not exercise the error path.
+    if (typeof process.getuid === 'function' && process.getuid() !== 0) {
+      fs.chmodSync(agentsPath, 0o000);
+      let threw = null;
+      try {
+        detectLegacyCodexSync(codexHome);
+      } catch (error) {
+        threw = error;
+      }
+      assert.ok(threw, 'detectLegacyCodexSync must propagate the read error');
+      assert.notStrictEqual(threw && threw.code, 'ENOENT');
+      fs.chmodSync(agentsPath, 0o600);
+    } else {
+      // Root path: simulate the same failure by replacing AGENTS.md with a
+      // directory — openRegularFileNoFollow then throws EACCES-on-open on
+      // Linux when the path resolves to a non-regular file.
+      fs.rmSync(agentsPath);
+      fs.mkdirSync(agentsPath);
+      let threw = null;
+      try {
+        detectLegacyCodexSync(codexHome);
+      } catch (error) {
+        threw = error;
+      }
+      assert.ok(threw, 'detectLegacyCodexSync must propagate the inspection error');
+      fs.rmSync(agentsPath, { recursive: true });
+    }
+
+    // Sanity check: a missing AGENTS.md is still treated as no-marker (not an error).
+    fs.rmSync(agentsPath, { force: true });
+    assert.strictEqual(detectLegacyCodexSync(codexHome), false);
+
+    fs.rmSync(homeDir, { recursive: true, force: true });
+  })) passed += 1; else failed += 1;
+
   console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
   process.exit(failed > 0 ? 1 : 0);
 }