Skip to content

Commit 5982f55

Browse files
committed
fix: judge nested node_modules symlinks, and correct the escape advice
`worktree:link` plants a symlink per workspace that carries its own tree, eight in today's layout, and every escape-path remedy in this PR said `rm node_modules`, singular. Following that advice removes the root link, leaves seven live links into the primary, and the next install writes through the nested ones, the same corruption one level down. The gate had the same blind spot: it judged only the target's root node_modules, so with a real root tree and a nested symlink still standing, `npm install` was allowed. The hook now finds a nested node_modules symlink under the target and the git toplevel (find without -L neither follows nor descends symlinks, and real node_modules trees are pruned, so it is a handful of directory reads), and every advice site names the plural remedy: find . -maxdepth 4 -type l -name node_modules -delete Sites corrected: the hook message, the link script docblock, the preinstall reporter, the doctor reinstall remedy, and AGENTS.md.
1 parent 38024bd commit 5982f55

6 files changed

Lines changed: 63 additions & 16 deletions

File tree

‎.claude/hooks/block-install-in-linked-worktree.sh‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -254,23 +254,40 @@ EOF
254254
[ -d "$target" ] || exit 0
255255

256256
# The install lands at the package root, which for a subdirectory is the
257-
# enclosing checkout, so judge the git toplevel too.
257+
# enclosing checkout, so judge the git toplevel too. And judge NESTED
258+
# node_modules symlinks, not only the root: `npm run worktree:link` plants one
259+
# per workspace that carries its own tree (packages/server, website, ...), so a
260+
# worktree whose root link was removed for a real install still holds seven live
261+
# links into the primary, and an install writes through the nested ones the same
262+
# way. This was the gate's blind spot in exactly the escape path its own message
263+
# recommends.
264+
#
265+
# find without -L neither follows nor descends symlinks, and the prune keeps it
266+
# out of real node_modules trees, so this is a handful of directory reads.
258267
top=$(git -C "$target" rev-parse --show-toplevel 2>/dev/null || true)
259268
for dir in "$target" "$top"; do
260269
[ -n "$dir" ] || continue
261-
[ -L "$dir/node_modules" ] || continue
262-
owner=$(cd "$dir" 2>/dev/null && cd "$(readlink node_modules)" 2>/dev/null && pwd -P) || owner="the checkout it points at"
270+
hit=""
271+
if [ -L "$dir/node_modules" ]; then
272+
hit="$dir/node_modules"
273+
else
274+
hit=$(find "$dir" -maxdepth 4 \( -type d \( -name .git -o -name node_modules \) \) -prune -o -type l -name node_modules -print 2>/dev/null | head -1)
275+
fi
276+
[ -n "$hit" ] || continue
277+
owner=$(cd "$(dirname "$hit")" 2>/dev/null && cd "$(readlink "$(basename "$hit")")" 2>/dev/null && pwd -P) || owner="the checkout it points at"
263278
{
264-
echo "BLOCKED: this command installs into $dir, whose node_modules is a SYMLINK at $owner."
279+
echo "BLOCKED: this command installs into $dir, where $hit is a SYMLINK at $owner."
265280
echo "An install through that link damages the checkout that OWNS the tree, not this one:"
266-
echo " npm ci DELETES $owner outright, before any lifecycle script can run"
267-
echo " bun install writes packages and .bin entries straight into $owner"
268-
echo " npm install silently REPLACES the link with a real tree, detaching this worktree"
281+
echo " npm ci DELETES the linked tree outright, before any lifecycle script can run"
282+
echo " bun install writes packages and .bin entries straight through the link"
283+
echo " npm install silently REPLACES a root link with a real tree, detaching this worktree"
269284
echo "A remove verb (npm rm, bun remove) deletes from that same owning checkout."
270285
echo "Safe alternatives:"
271286
echo " npm run worktree:link links a fresh worktree; it never installs"
272-
echo " a real install with NO symlink in the way. Run \`rm node_modules\` first (it is"
273-
echo " only a link, nothing else is lost), or install in the PRIMARY checkout."
287+
echo " a real install with NO symlink in the way. The link script plants NESTED"
288+
echo " node_modules links too (packages/server, website, ...), so remove them ALL first:"
289+
echo " find . -maxdepth 4 -type l -name node_modules -delete"
290+
echo " (they are only links, nothing else is lost), or install in the PRIMARY checkout."
274291
echo "A GLOBAL install (-g) is not affected by this and is never blocked."
275292
echo "Escape hatch for a deliberate exception: WEBJS_NO_WORKTREE_INSTALL_GATE=1."
276293
} >&2

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ Fix it with **`npm run worktree:link`** from inside the worktree. A full `npm in
6464

6565
The script discovers the `node_modules` set from the primary checkout rather than hardcoding a list (it changes whenever a package gains a nested tree), never overwrites an existing path, and never creates a dangling link, so it is safe to re-run and safe in a worktree where you already ran a real `npm install`. The seed step keeps the same contract: it only ever applies pending migrations and inserts demo rows that are not there, so a database with rows in it is left untouched.
6666

67-
**Know what this does NOT give you.** The worktree then runs the PRIMARY checkout's framework source through every bare `@webjsdev/*` specifier, because `<primary>/node_modules/@webjsdev/core` is a relative symlink into `<primary>/packages/core` and resolving through the linked root lands there. Relative imports (`../../../src/x.js`) and the browser suite, which web-test-runner serves from the worktree, do use the worktree's own files. So linking makes the suite RUNNABLE, not self-testing: if you are editing `packages/core/src` or `packages/server/src` and need a bare-specifier consumer to exercise YOUR copy, delete the `node_modules` SYMLINK first (`rm node_modules`, it is only a link and nothing else is lost) and then install, or repoint the individual `@webjsdev/<pkg>` entries at it. CI always builds from the branch, so it is unaffected either way.
67+
**Know what this does NOT give you.** The worktree then runs the PRIMARY checkout's framework source through every bare `@webjsdev/*` specifier, because `<primary>/node_modules/@webjsdev/core` is a relative symlink into `<primary>/packages/core` and resolving through the linked root lands there. Relative imports (`../../../src/x.js`) and the browser suite, which web-test-runner serves from the worktree, do use the worktree's own files. So linking makes the suite RUNNABLE, not self-testing: if you are editing `packages/core/src` or `packages/server/src` and need a bare-specifier consumer to exercise YOUR copy, delete EVERY `node_modules` SYMLINK first, not only the root one, because the link script plants one per workspace that carries its own tree (`find . -maxdepth 4 -type l -name node_modules -delete`, they are only links and nothing else is lost) and then install, or repoint the individual `@webjsdev/<pkg>` entries at it. CI always builds from the branch, so it is unaffected either way.
6868

6969
**NEVER install while the `node_modules` symlink is standing (#1442).** This is the trap the two paragraphs above used to walk you into, and the damage lands on a checkout you are not working in, so the failure surfaces in someone else's session with nothing naming the cause. Measured on npm 11.19.0 and bun 1.3.14: `npm ci` DELETES the primary's whole `node_modules` through the link before any lifecycle script runs, `bun install` writes packages and `.bin` entries straight into the primary through it, and `npm install` silently replaces the link with a real tree, detaching the worktree from the shared source. No `preinstall` script can prevent any of it, because npm removes the symlink before `preinstall` runs, `npm ci` has already emptied the primary by then, and Bun runs it in time but ignores a non-zero exit. So the layers are: Claude Code BLOCKS the command through `.claude/hooks/block-install-in-linked-worktree.sh`, which covers every manager's install aliases plus the REMOVE verbs (`npm rm` in a linked worktree deletes from the owning checkout), judges a COMMAND rather than a token so `git commit -m "fix: npm install ..."` and `grep -rn "npm ci"` are unaffected, and never blocks a GLOBAL `-g` install such as the post-release `npm update -g webjsdev` (escape hatch `WEBJS_NO_WORKTREE_INSTALL_GATE=1`), the root `preinstall` REPORTS it for every other tool without ever blocking, `npm run worktree:link` REPAIRS an already-damaged primary, and `npm run check:worktree-links` reports what it would repair without changing anything, exiting non-zero when there is work. `WEBJS_NO_WORKTREE_REPAIR=1` suppresses the repair WRITE, so it has no effect on `--check`, which never writes and always inspects. Tests: `test/hooks/block-install-in-linked-worktree.test.mjs`, `test/repo-health/warn-worktree-install.test.mjs`, `test/repo-health/link-worktree-deps.test.mjs`.
7070

‎packages/cli/lib/doctor/probes/framework-resolves.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ function linkAwareReinstallFix(appDir) {
150150
return (
151151
'node_modules here is a SYMLINK at another checkout, so do NOT run `npm install`: it would act ' +
152152
'on that checkout, not this one (#1442). Either reinstall in the checkout that owns the tree, ' +
153-
'or `rm node_modules` first (it is only a link) and install here.' + linkScriptHint(appDir)
153+
'or remove every node_modules symlink first, nested ones included ' +
154+
'(`find . -maxdepth 4 -type l -name node_modules -delete`), and install here.' + linkScriptHint(appDir)
154155
);
155156
}
156157
return 'Reinstall dependencies (`npm install`, or remove node_modules and reinstall).';

‎scripts/link-worktree-deps.mjs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,12 @@
3434
*
3535
* So this makes the suite RUNNABLE, not self-testing. If you are editing
3636
* `packages/core/src` or `packages/server/src` and need a bare-specifier
37-
* consumer to exercise YOUR copy, delete the `node_modules` SYMLINK first
38-
* (`rm node_modules`, it is only a link and nothing else is lost) and then
39-
* install, or point the individual `@webjsdev/<pkg>` entries at this worktree
40-
* instead. CI always builds from the branch, so it is unaffected either way.
37+
* consumer to exercise YOUR copy, delete EVERY `node_modules` SYMLINK first,
38+
* not only the root one, because this script plants one per workspace that
39+
* carries its own tree (`find . -maxdepth 4 -type l -name node_modules -delete`,
40+
* they are only links and nothing else is lost) and then install, or point the
41+
* individual `@webjsdev/<pkg>` entries at this worktree instead. CI always
42+
* builds from the branch, so it is unaffected either way.
4143
*
4244
* NEVER install while the link is standing (#1442). Measured on npm 11.19.0 and
4345
* bun 1.3.14: `npm ci` DELETES the primary's whole `node_modules` through the

‎scripts/warn-worktree-install.mjs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ try {
7474
say(` ${primaryModules}`);
7575
say('An install now writes THROUGH that link, into a checkout you are not working in.');
7676
say('Stop it if you can. Use `npm run worktree:link` to set this worktree up, or');
77-
say('`rm node_modules` first if a real, self-contained install is what you want.');
77+
say('remove EVERY node_modules symlink first (the link script plants nested ones too:');
78+
say('`find . -maxdepth 4 -type l -name node_modules -delete`) if a real install is what you want.');
7879
} else if (haveOwnModules && !hasEntries(primaryModules)) {
7980
// The `npm ci` aftermath: npm deleted the link's target before we ran.
8081
say('the PRIMARY checkout\'s node_modules is now EMPTY or missing:');

‎test/hooks/block-install-in-linked-worktree.test.mjs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,3 +447,29 @@ test('mines --cwd and --dir as target directories, not just --prefix', () => {
447447
assert.equal(runHook(`yarn --cwd ${worktree} install`, primary).status, 2);
448448
} finally { rmSync(root, { recursive: true, force: true }); }
449449
});
450+
451+
test('a NESTED node_modules symlink blocks, not only the root one', () => {
452+
// `worktree:link` plants a symlink per workspace that carries its own tree
453+
// (packages/server is the live example). Removing just the root link for a
454+
// real install leaves those live, and an install writes through them into the
455+
// primary the same way. This was the gate's blind spot in exactly the escape
456+
// path its own message used to recommend.
457+
const root = mkdtempSync(join(tmpdir(), 'install-gate-nested-'));
458+
const primary = join(root, 'primary');
459+
const worktree = join(root, 'worktree');
460+
mkdirSync(join(primary, 'packages', 'server', 'node_modules'), { recursive: true });
461+
mkdirSync(join(worktree, 'packages', 'server'), { recursive: true });
462+
mkdirSync(join(worktree, 'node_modules'), { recursive: true }); // REAL root tree
463+
writeFileSync(join(worktree, 'package.json'), '{"name":"w"}\n');
464+
symlinkSync(join(primary, 'packages', 'server', 'node_modules'), join(worktree, 'packages', 'server', 'node_modules'));
465+
try {
466+
const r = runHook('npm install', worktree);
467+
assert.equal(r.status, 2, `expected block, got ${r.status}: ${r.stderr}`);
468+
assert.match(r.stderr, /packages\/server\/node_modules is a SYMLINK/);
469+
assert.match(r.stderr, /find \. -maxdepth 4 -type l -name node_modules -delete/, 'the remedy removes them ALL');
470+
// COUNTERFACTUAL: the same layout with a real nested directory passes.
471+
rmSync(join(worktree, 'packages', 'server', 'node_modules'));
472+
mkdirSync(join(worktree, 'packages', 'server', 'node_modules'));
473+
assert.equal(runHook('npm install', worktree).status, 0);
474+
} finally { rmSync(root, { recursive: true, force: true }); }
475+
});

0 commit comments

Comments
 (0)