From 90fff40f5a61d3fdfe7e8c74e50b248cfed83c55 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 29 Aug 2026 17:24:40 -0400 Subject: [PATCH] =?UTF-8?q?fix(#3900):=20overlay=20walker=20skips=20non-re?= =?UTF-8?q?gular=20files=20=E2=80=94=20a=20repo-root=20socket=20no=20longe?= =?UTF-8?q?r=20fails=2032=20tests=20(#4062)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#3900): a unix socket under the repo root must not break the overlay (failing first) * fix(#3900): the overlay walker skips non-regular files (sockets, FIFOs, device nodes) The walker classified every not-a-directory entry as a file, so a unix socket anywhere under the repo root (an MCP server, a language-server daemon, a watcher) made copyFileSync throw ENXIO — the reporter measured 32 local test failures across 6 files, all in before() hooks, none about the code under test; a FIFO would block until a writer appeared. The hard-link path does not save it: the overlay sits under tmpfs, so cross-device EXDEV always falls through to the copy. One guard at the classification site (srcStat.isFile() before the copy/link branches) — sockets, FIFOs, and device nodes are not repository content; CI never saw this because a fresh clone has no working-tree daemons, which is exactly why it ate local debugging time. Also adds opts.root (test-only root override, default REPO_ROOT) so the regression tests build the overlay from a tiny synthetic root in milliseconds instead of paying a full repo walk. * chore(#3900): changeset fragment (pr number backfilled after PR creation) * fix(#3900): review fold-ins — sibling cold-fixture guard, changeset format, root doc buildColdInstallTree enumerated the live repo's hooks/ with a NAME-only filter — the same class #3900 documents: a daemon socket or FIFO inside hooks/ would reach cpSync and throw ENXIO (or block). Dirent type guard added. The changeset gains the mandated bold user-visible lead, and opts.root is documented at its definition (mirroring cold-runtime-lib- fixture's repoRoot convention). * chore(#3900): backfill changeset PR number (4062) --------- Co-authored-by: sim --- .changeset/vivid-lynx-zip.md | 5 ++ tests/helpers/cold-runtime-lib-fixture.cjs | 4 ++ tests/helpers/overlay-repo.cjs | 14 +++++- tests/overlay-repo-helpers.test.cjs | 56 ++++++++++++++++++++++ 4 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 .changeset/vivid-lynx-zip.md diff --git a/.changeset/vivid-lynx-zip.md b/.changeset/vivid-lynx-zip.md new file mode 100644 index 000000000..14e3fd89b --- /dev/null +++ b/.changeset/vivid-lynx-zip.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4062 +--- +local test runs no longer fail when a daemon keeps a unix socket under the repo root — the overlay builder classified every non-directory entry as a file, so copyFileSync threw ENXIO and 32 tests failed in their before() hooks with no connection to the code under test (#3900) diff --git a/tests/helpers/cold-runtime-lib-fixture.cjs b/tests/helpers/cold-runtime-lib-fixture.cjs index 5c20cbccd..b26ac2688 100644 --- a/tests/helpers/cold-runtime-lib-fixture.cjs +++ b/tests/helpers/cold-runtime-lib-fixture.cjs @@ -77,6 +77,10 @@ function buildColdInstallTree(opts = {}) { fs.mkdirSync(hooksDestDir, { recursive: true }); for (const entry of fs.readdirSync(path.join(repoRoot, 'hooks'), { withFileTypes: true })) { if (!shouldCopyHookEntry(entry.name)) continue; + // #3900 (same-class guard, found in review): the live repo tree is walked + // here — a daemon's socket/FIFO inside hooks/ would reach cpSync and + // throw ENXIO (a FIFO would block). Dirent type check, not name-only. + if (!entry.isFile() && !entry.isDirectory()) continue; fs.cpSync(path.join(repoRoot, 'hooks', entry.name), path.join(hooksDestDir, entry.name), { recursive: true, }); diff --git a/tests/helpers/overlay-repo.cjs b/tests/helpers/overlay-repo.cjs index 9fccec835..4a437e953 100644 --- a/tests/helpers/overlay-repo.cjs +++ b/tests/helpers/overlay-repo.cjs @@ -211,6 +211,11 @@ function linkOrCopyFile(src, dest) { function buildOverlayRepo(fileOverrides, opts = {}) { const mode = opts.mode || 'link'; const warn = opts.warn || console.warn; + // #3900: test-only root override (mirrors cold-runtime-lib-fixture's + // repoRoot) — the #3900 regression tests build the overlay from a tiny + // synthetic root instead of walking the whole repo; every existing caller + // uses the default REPO_ROOT and is unaffected. + const root = opts.root || REPO_ROOT; const tmpRepo = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2930-overlay-')); const entries = Object.entries(fileOverrides).map(([relPath, content]) => ({ parts: relPath.split('/'), @@ -255,6 +260,13 @@ function buildOverlayRepo(fileOverrides, opts = {}) { } if (srcStat.isDirectory()) { place(srcPath, destPath, overridden || [], false); + } else if (!srcStat.isFile()) { + // #3900: sockets, FIFOs, device nodes are not repository content. + // Classifying them as files made copyFileSync throw ENXIO on a + // socket (a FIFO would block until a writer appears) — 32 local + // test failures from a daemon's working-tree socket, none about the + // code under test. Skip: the overlay mirrors files, not devices. + continue; } else if (mode === 'copy') { // Real independent inode — a write through this path in the overlay // can never alias back to REPO_ROOT's own tracked file (see @@ -272,7 +284,7 @@ function buildOverlayRepo(fileOverrides, opts = {}) { } } - place(REPO_ROOT, tmpRepo, entries, true); + place(root, tmpRepo, entries, true); if (skipped.length > 0) { // Not thrown: a source that left the tree mid-walk is genuinely not part of diff --git a/tests/overlay-repo-helpers.test.cjs b/tests/overlay-repo-helpers.test.cjs index c8eebc7bc..6917260e3 100644 --- a/tests/overlay-repo-helpers.test.cjs +++ b/tests/overlay-repo-helpers.test.cjs @@ -692,3 +692,59 @@ describe('placeVanishableLeaf: property coverage', () => { ); }); }); + +// ─── #3900: non-regular working-tree files are not repository content ──────── + +describe('buildOverlayRepo: non-regular files are skipped (#3900)', () => { + // A unix socket under the repo root (an MCP server, a language-server + // daemon, a watcher) made every suite built on the overlay fail in its + // before() hook: the walker classified anything not-a-directory as a file, + // and copyFileSync on a socket throws ENXIO (a FIFO would block). The + // reporter measured 32 failures across 6 files — none about the code under + // test. Unix-domain sockets bind only on POSIX; the Windows lane keeps the + // equivalence via a FIFO-shaped skip is impractical, so it verifies only + // the regular-file path. + const isPosix = process.platform !== 'win32'; + + test('a unix socket anywhere under the repo root does not break the overlay', { skip: !isPosix }, async () => { + // opts.root (also from this fix): a tiny synthetic root, so the test does + // not pay a full repo walk — the walker is the same either way. + const net = require('node:net'); + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3900-root-')); + const server = net.createServer(); + await new Promise((resolve) => server.listen(path.join(root, 'daemon.sock'), resolve)); + try { + fs.writeFileSync(path.join(root, 'real.txt'), 'x'); + const overlay = buildOverlayRepo({}, { root }); + try { + assert.equal( + fs.existsSync(path.join(overlay, 'daemon.sock')), + false, + 'the socket is not repository content — it must not be copied', + ); + assert.equal(fs.existsSync(path.join(overlay, 'real.txt')), true, + 'regular files in the same tree still overlay'); + } finally { + cleanup(overlay); + } + } finally { + server.close(); + cleanup(root); + } + }); + + test('regular files still copy (control, synthetic root)', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3900-ctrl-')); + try { + fs.writeFileSync(path.join(root, 'note.txt'), 'x'); + const overlay = buildOverlayRepo({}, { root }); + try { + assert.equal(fs.existsSync(path.join(overlay, 'note.txt')), true); + } finally { + cleanup(overlay); + } + } finally { + cleanup(root); + } + }); +});