From 48687521db087877911f4b514da29e4a1079757f Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 21 Jun 2026 15:09:20 -0400 Subject: [PATCH] fix(#1545): make no-phantom-issue-refs walk() robust to broken symlinks (#1546) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit walk() collected dirents by extension only, then readFileSync'd each — so a broken symlink named *.md (e.g. the gitignored CLAUDE.md worktree symlink whose target is absent in a gsd-test Docker copy) crashed the #1073 guard with ENOENT. It passed on a developer Mac (the symlink resolves) and in GitHub CI (CLAUDE.md is gitignored/absent), failing only on the gsd-test-docker-from-worktree path — a real error hiding behind the local environment, not a flake. Guard the collection with entry.isFile() so symlinks (and other non-regular dirents) are skipped deterministically on every platform; gitignored files are not shipped repo text, so excluding CLAUDE.md is correct. Add a deterministic regression test (dangling *.md symlink fixture, Windows-symlink-guarded with a genuine t.skip) asserting walk() excludes the broken symlink and the read loop never throws ENOENT. Closes #1545. Co-authored-by: Claude Opus 4.8 --- tests/no-phantom-issue-refs.test.cjs | 44 +++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/tests/no-phantom-issue-refs.test.cjs b/tests/no-phantom-issue-refs.test.cjs index cc204a9a1..bf862e5b0 100644 --- a/tests/no-phantom-issue-refs.test.cjs +++ b/tests/no-phantom-issue-refs.test.cjs @@ -11,6 +11,7 @@ const { test } = require('node:test'); const assert = require('node:assert'); const fs = require('node:fs'); const path = require('node:path'); +const os = require('node:os'); const ROOT = path.resolve(__dirname, '..'); @@ -32,7 +33,10 @@ function walk(dir, acc) { for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { if (entry.isDirectory()) { if (!SKIP_DIRS.has(entry.name)) walk(path.join(dir, entry.name), acc); - } else if (SCAN_EXT.has(path.extname(entry.name))) { + // entry.isFile() excludes symlinks (and other non-regular dirents) so a broken symlink like + // a gitignored CLAUDE.md worktree symlink is skipped deterministically on every platform — + // it can't be read and isn't shipped repo text (#1545). + } else if (entry.isFile() && SCAN_EXT.has(path.extname(entry.name))) { acc.push(path.join(dir, entry.name)); } } @@ -56,3 +60,41 @@ test('no phantom pre-migration issue references remain in repo text (#1073)', () `successor (#717/#720) or rewrite as prose (see #1073):\n` + offenders.join('\n'), ); }); + +test('walk() skips broken symlinks and does not throw ENOENT (#1545)', (t) => { + const fixture = fs.mkdtempSync(path.join(os.tmpdir(), 'nophantom-symlink-')); + let symlinkCreated = false; + try { + fs.writeFileSync(path.join(fixture, 'real.md'), '# real, no phantom refs\n'); + try { + fs.symlinkSync( + path.join(fixture, 'does-not-exist-target'), + path.join(fixture, 'broken.md'), + ); + // Verify the symlink actually exists (lstat succeeds even for dangling symlinks) + fs.lstatSync(path.join(fixture, 'broken.md')); + symlinkCreated = true; + } catch (e) { + // Windows without symlink privilege — genuine skip + } + + if (!symlinkCreated) { + t.skip('platform cannot create symlinks unprivileged'); + return; + } + + const found = walk(fixture, []).map((f) => path.basename(f)); + + assert.ok(found.includes('real.md'), 'walk() must include real.md'); + assert.ok(!found.includes('broken.md'), 'walk() must NOT include broken.md (broken symlink)'); + + // Mirror the production read loop — must not throw ENOENT + assert.doesNotThrow( + () => found.length && walk(fixture, []).forEach((fp) => fs.readFileSync(fp, 'utf8')), + 'readFileSync on every walk() result must not throw (no broken symlinks returned)', + ); + } finally { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- local cleanup in standalone guard test; no helpers import available (would introduce a test-dep cycle) + fs.rmSync(fixture, { recursive: true, force: true }); + } +});