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.
This commit is contained in:
Santhi Prakash
2026-08-24 20:09:12 -04:00
committed by haelyra
parent 56dafcc5e3
commit 0b04c1bfa1
2 changed files with 57 additions and 2 deletions
+9 -2
View File
@@ -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;
}
+48
View File
@@ -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);
}