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\n\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); }