fix: close five more defects the delta review found

The bare-`yarn` branch was the worst of these. It lived inside the generic
VERBS pattern, whose prefix any space satisfies, so it matched the token
ANYWHERE and blocked `which yarn`, `rm -rf /tmp/yarn` and `git switch -c
feat/yarn`. That fires on ordinary commands in a linked worktree, which is the
mandated working state here, and a gate that cries wolf gets turned off. It is
now its own anchored pattern requiring command position.

The hyphenated npm verbs needed spelling out. The trailing word boundary
excludes `-`, so listing `install` never reached `install-test`,
`install-ci-test` or `clean-install-test`, and the short aliases do not cover
them. The remove verbs are in the table too: `npm rm` in a linked worktree
deletes from the checkout that owns the tree.

`--check` under WEBJS_NO_WORKTREE_REPAIR=1 exited 0 while inspecting nothing,
because `touched` kept its initializer when the repair block was skipped. The
hatch suppresses the repair WRITE, and `--check` never writes, so it now always
inspects. My previous test asserted the exit 0 and locked the defect in.

The `entry_name` rename gets the source guard the last commit claimed for it.

Author: vivek7405

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

@@ -37,21 +37,38 @@ if [ -z "$cmd" ]; then exit 0; fi
 # aliases are the consequential ones. npm's list is long because npm ships a
 # large alias table of its own (`npm help install`), typo aliases included.
 #
+# HYPHENATED verbs must be spelled out. The trailing word boundary excludes `-`,
+# so `install-test` is NOT reached by listing `install`; each hyphenated command
+# needs its own entry, and the short aliases (`it`, `cit`, `sit`) do not cover
+# the long spellings.
+#
+# The REMOVE verbs are here too. `npm rm <pkg>` in a linked worktree deletes
+# from the checkout that owns the tree, the same corruption in the other
+# direction.
+#
 # Word boundaries on BOTH sides keep this narrow: `npm init` does not match `in`
 # or `i`, because the next character is alphanumeric, and `npm run install-deps`
 # does not match because `run` is not a verb here.
-NPM_VERBS='install|i|in|ins|inst|insta|instal|isnt|isnta|isntal|isntall|add|ci|clean-install|ic|install-clean|sit|it|cit|update|up|upgrade|udpate|dedupe|ddp'
-BUN_VERBS='install|i|add|a|update|up'
-PNPM_VERBS='install|i|add|update|up'
-YARN_VERBS='install|add|up|upgrade'
-# The last two branches are the two shapes a plain verb regex cannot see:
-#   * `npm --prefix <dir> install`, flags BEFORE the verb. Only the two flags
-#     that themselves name a target directory are admitted, so this stays
-#     targeted rather than swallowing a token run and matching `npm run install`.
-#   * bare `yarn`, which IS an install in yarn classic. Matched only when it ends
-#     the command or is followed by a flag, so `yarn test` stays allowed.
-VERBS="(npm[[:space:]]+(${NPM_VERBS})|bun[[:space:]]+(${BUN_VERBS})|pnpm[[:space:]]+(${PNPM_VERBS})|yarn[[:space:]]+(${YARN_VERBS})|(npm|pnpm|yarn)[[:space:]]+(--prefix|-C)[[:space:]=]+[^[:space:]&|;]+[[:space:]]+(${NPM_VERBS})|yarn([[:space:]]+-[^[:space:]]*)*[[:space:]]*(\$|[&|;]))"
-if ! printf '%s' "$cmd" | grep -Eq "(^|[^[:alnum:]_-])${VERBS}([^[:alnum:]_-]|\$)"; then
+NPM_VERBS='install-ci-test|clean-install-test|install-clean|clean-install|install-test|install|isntall|isntal|isnta|isnt|instal|insta|inst|ins|in|i|add|ci|cit|sit|it|ic|update|upgrade|udpate|up|dedupe|ddp|uninstall|unlink|un|remove|rm|r'
+BUN_VERBS='install|i|add|a|update|up|remove|rm'
+PNPM_VERBS='install|i|add|update|upgrade|up|dedupe|remove|rm|uninstall|un'
+YARN_VERBS='install|add|upgrade|up|dedupe|remove'
+# `npm --prefix <dir> install`, flags BEFORE the verb. Only the two flags that
+# themselves name a target directory are admitted, so this stays targeted rather
+# than swallowing a token run and matching `npm run install`.
+VERBS="(npm[[:space:]]+(${NPM_VERBS})|bun[[:space:]]+(${BUN_VERBS})|pnpm[[:space:]]+(${PNPM_VERBS})|yarn[[:space:]]+(${YARN_VERBS})|(npm|pnpm|yarn)[[:space:]]+(--prefix|-C)[[:space:]=]+[^[:space:]&|;]+[[:space:]]+(${NPM_VERBS}))"
+
+# Bare `yarn` IS an install in yarn classic, and it needs its own anchored
+# pattern rather than a branch of VERBS. VERBS is wrapped in a generic
+# non-word-character prefix, which any space satisfies, so a bare-yarn branch
+# inside it matched the token ANYWHERE in the command: `which yarn`,
+# `rm -rf /tmp/yarn` and `git switch -c feat/yarn` all blocked. Here `yarn` must
+# sit in COMMAND position, at the start or straight after a `&&`, `||`, `;` or
+# `|`, and be followed only by flags.
+BARE_YARN='(^|[&|;])[[:space:]]*yarn([[:space:]]+-[^[:space:]]*)*[[:space:]]*($|[&|;])'
+
+if ! printf '%s' "$cmd" | grep -Eq "(^|[^[:alnum:]_-])${VERBS}([^[:alnum:]_-]|\$)" \
+  && ! printf '%s' "$cmd" | grep -Eq "$BARE_YARN"; then
   exit 0
 fi
 
@@ -61,6 +78,7 @@ fi
 # tokens that appear BEFORE the install verb and let them supersede the cwd, then
 # add any directory a package manager is pointed at explicitly.
 prefix=$(printf '%s' "$cmd" | sed -E "s/(^|[^[:alnum:]_-])${VERBS}([^[:alnum:]_-]|\$).*//")
+prefix=$(printf '%s' "$prefix" | sed -E "s/${BARE_YARN}.*//")
 
 eff="$PWD"
 resolve_against_eff() {

AGENTS.md

@@ -66,7 +66,7 @@ The script discovers the `node_modules` set from the primary checkout rather tha
 
 **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.
 
-**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` (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 (escape hatch `WEBJS_NO_WORKTREE_REPAIR=1`). 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`.
+**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` (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`.
 
 Note the `webjs doctor` / `webjs dev` remedy message suggests the root-only symlink. That advice is correct for a scaffolded APP worktree, which has no nested trees and no built `dist/`, and wrong only for this monorepo. It stays app-generic on purpose, because it ships in the published CLI and `webjs dev` prints it verbatim to someone whose app has none of this repo's scripts; it names `npm run worktree:link` only when it finds a package.json actually declaring that script, so in this repo you get the monorepo path and in a scaffolded app you do not.
 

framework-dev.md

@@ -165,7 +165,7 @@ So prevention lives one layer up, and the rest is repair:
 - **Block.** `.claude/hooks/block-install-in-linked-worktree.sh` is a `PreToolUse` (Bash) hook, the only layer that sees the state before the package manager starts. It refuses an install verb whose target directory has a symlinked `node_modules`, and stays narrow: `npm test`, `npm run <script>`, and `npx ...` all pass. Escape hatch `WEBJS_NO_WORKTREE_INSTALL_GATE=1`.
 - **Report.** The root `preinstall` runs `scripts/warn-worktree-install.mjs`, which ALWAYS exits 0 and returns immediately unless `.git` is a FILE, so a normal clone and CI never see it. It names whichever of the three states it landed in and prints the repair.
 - **Repoint on teardown.** `.claude/hooks/cleanup-merged-worktree.sh` repoints any `<primary>/node_modules/@webjsdev/*` link targeting a worktree it is about to remove, before removing it.
-- **Repair on demand.** `npm run worktree:link` repairs the primary's `@webjsdev/*` scope: a dangling link, a link into a live foreign checkout, and an absolute in-primary link all become the relative form, and a DANGLING `.name-HASH` npm staging entry is dropped. A LIVE staging entry is left strictly alone. `npm run check:worktree-links` reports without writing and exits non-zero when there is work. Escape hatch `WEBJS_NO_WORKTREE_REPAIR=1`, which the `defaultPrimary()` test needs for the same reason it needs `WEBJS_NO_WORKTREE_SEED=1`: the repair pass sits ABOVE the primary-checkout guard by design, so it runs in both positions and would otherwise rewrite the real checkout during `npm test`.
+- **Repair on demand.** `npm run worktree:link` repairs the primary's `@webjsdev/*` scope: a dangling link, a link into a live foreign checkout, and an absolute in-primary link all become the relative form, and a DANGLING `.name-HASH` npm staging entry is dropped. A LIVE staging entry is left strictly alone. `npm run check:worktree-links` reports without writing and exits non-zero when there is work. Escape hatch `WEBJS_NO_WORKTREE_REPAIR=1` suppresses the repair WRITE only, so `--check` ignores it and still inspects; the `defaultPrimary()` test needs the hatch for the same reason it needs `WEBJS_NO_WORKTREE_SEED=1`: the repair pass sits ABOVE the primary-checkout guard by design, so it runs in both positions and would otherwise rewrite the real checkout during `npm test`.
 - **Detect.** `webjs doctor`'s `framework-links` check warns on a dangling or foreign `@webjsdev/core` link. `FRAMEWORK_RESOLVE` cannot see the foreign case, because a link into a live checkout resolves perfectly.
 
 Regression 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`, `test/hooks/cleanup-merged-worktree.test.mjs`, `test/cli/doctor.test.mjs`.

scripts/link-worktree-deps.mjs

@@ -409,7 +409,10 @@ if (!existsSync(join(primary, 'package.json'))) {
 // is why the `defaultPrimary()` test can run this against the real checkout
 // without mutating it, exactly as `WEBJS_NO_WORKTREE_SEED=1` does for seeding.
 let touched = 0;
-if (process.env.WEBJS_NO_WORKTREE_REPAIR === '1') {
+// The hatch suppresses the repair WRITE. `--check` never writes, so there is
+// nothing for it to suppress there, and skipping the inspection too would make
+// `check:worktree-links` exit 0 reporting a clean tree it never looked at.
+if (process.env.WEBJS_NO_WORKTREE_REPAIR === '1' && !CHECK) {
   console.log('[link-worktree-deps] framework-link repair skipped (WEBJS_NO_WORKTREE_REPAIR=1).');
 } else {
   const repair = repairPrimaryFrameworkLinks(primary, { check: CHECK });

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

@@ -96,12 +96,53 @@ test('blocks every manager\'s install ALIAS, not just the canonical spelling', (
   } finally { rmSync(root, { recursive: true, force: true }); }
 });
 
+test('blocks the HYPHENATED npm verbs, which the short aliases do not cover', () => {
+  // The trailing word boundary excludes `-`, so listing `install` does not reach
+  // `install-test`. Each hyphenated command needs its own entry.
+  const { root, worktree } = makeLinkedPair();
+  try {
+    for (const cmd of ['npm install-test', 'npm install-ci-test', 'npm clean-install-test', 'npm clean-install', 'npm install-clean']) {
+      assert.equal(runHook(cmd, worktree).status, 2, `expected block for \`${cmd}\``);
+    }
+  } finally { rmSync(root, { recursive: true, force: true }); }
+});
+
+test('blocks the REMOVE verbs, which delete from the owning checkout', () => {
+  const { root, worktree } = makeLinkedPair();
+  try {
+    for (const cmd of ['npm uninstall x', 'npm rm x', 'npm r x', 'bun rm x', 'pnpm rm x', 'yarn remove x', 'pnpm upgrade', 'pnpm dedupe', 'yarn dedupe']) {
+      assert.equal(runHook(cmd, worktree).status, 2, `expected block for \`${cmd}\``);
+    }
+  } finally { rmSync(root, { recursive: true, force: true }); }
+});
+
+test('bare `yarn` blocks only in COMMAND position, never as a trailing word', () => {
+  // The bare-yarn branch first lived inside the generic VERBS pattern, whose
+  // prefix any space satisfies, so it matched the token ANYWHERE and blocked
+  // `which yarn`, `rm -rf /tmp/yarn` and `git switch -c feat/yarn`. That fires
+  // on ordinary commands in a linked worktree, which is the mandated state here,
+  // and a gate that cries wolf gets turned off.
+  const { root, worktree } = makeLinkedPair();
+  try {
+    for (const cmd of ['yarn', 'yarn --frozen-lockfile', 'cd . && yarn']) {
+      assert.equal(runHook(cmd, worktree).status, 2, `expected block for \`${cmd}\``);
+    }
+    for (const cmd of [
+      'which yarn', 'command -v yarn', 'ls -la ~/.yarn', 'rm -rf /tmp/yarn',
+      'git switch -c feat/yarn', 'cat README | grep yarn', 'echo yarn',
+      'npm run build --workspace yarn',
+    ]) {
+      assert.equal(runHook(cmd, worktree).status, 0, `expected allow for \`${cmd}\``);
+    }
+  } finally { rmSync(root, { recursive: true, force: true }); }
+});
+
 test('the broadened alias table does not swallow non-install commands', () => {
   const { root, worktree } = makeLinkedPair();
   try {
     // `npm init` must not match `in` or `i`, and a bare `yarn test` must not
     // match the bare-yarn install branch.
-    for (const cmd of ['npm init', 'npm init -y', 'yarn test', 'yarn run build', 'bun run dev', 'bun test', 'pnpm run build']) {
+    for (const cmd of ['npm init', 'npm init -y', 'yarn test', 'yarn run build', 'bun run dev', 'bun test', 'pnpm run build', 'git rm x', 'rm -rf node_modules', 'npm run rm']) {
       assert.equal(runHook(cmd, worktree).status, 0, `expected allow for \`${cmd}\``);
     }
   } finally { rmSync(root, { recursive: true, force: true }); }

test/hooks/cleanup-merged-worktree.test.mjs

@@ -13,7 +13,7 @@
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
 import { execFileSync, spawnSync } from 'node:child_process';
-import { mkdtempSync, mkdirSync, writeFileSync, existsSync, chmodSync, symlinkSync, readlinkSync, lstatSync } from 'node:fs';
+import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, chmodSync, symlinkSync, readlinkSync, lstatSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join, dirname, resolve, delimiter } from 'node:path';
 import { fileURLToPath } from 'node:url';
@@ -282,3 +282,19 @@ test('leaves the primary links untouched when the worktree is KEPT', () => {
   assert.ok(existsSync(wt), 'a dirty worktree is kept');
   assert.equal(readlinkSync(entry), target, 'so its links are left pointing at it');
 });
+
+test('repoint_primary_links never shadows the script-global `base` merge ref', () => {
+  // `base` holds the merge base ref resolved once at the top and read by
+  // `is_merged()`. bash `local` is dynamically scoped, so declaring `base` local
+  // inside this function blanks the ref for anything the function calls. Nothing
+  // calls a helper from there TODAY, which is why this is a source assertion
+  // rather than a behavioural one: the defect is unreachable until someone adds
+  // that call, and then it silently leaks the worktree this hook exists to remove.
+  const src = readFileSync(HOOK, 'utf8');
+  const fn = src.slice(src.indexOf('repoint_primary_links() {'));
+  const locals = fn.slice(0, fn.indexOf('\n}')).match(/^\s*local .*$/gm) || [];
+  assert.ok(locals.length > 0, 'the function still declares locals');
+  for (const line of locals) {
+    assert.doesNotMatch(line, /\blocal\b[^\n]*\bbase\b/, `shadows the global merge ref: ${line.trim()}`);
+  }
+});

test/repo-health/link-worktree-deps.test.mjs

@@ -562,22 +562,32 @@ describe('framework-link repair (#1442)', () => {
     } finally { rmSync(primary, { recursive: true, force: true }); }
   });
 
-  test('--check stays read-only even with WEBJS_NO_WORKTREE_REPAIR=1 set', () => {
-    // The exit for `--check` used to live INSIDE the repair `else`, so this one
-    // combination fell through to the linking loop and the seed step. The flag
-    // pair AGENTS.md documents as changing nothing was the pair that wrote.
+  test('--check with WEBJS_NO_WORKTREE_REPAIR=1 still INSPECTS, and still writes nothing', () => {
+    // Two separate bugs met here. The exit for `--check` used to live INSIDE the
+    // repair `else`, so this pair fell through to the linking loop and the seed
+    // step, and the flag pair documented as changing nothing was the pair that
+    // wrote. Moving the exit out fixed the write but left `touched` at its
+    // initializer, so the run then exited 0 reporting a clean tree it had never
+    // looked at. The hatch suppresses the repair WRITE; `--check` never writes,
+    // so it must still inspect.
     const primary = makeRepairPrimary();
     const wt = makeWorktree();
     try {
+      plant(primary, 'core', '/nonexistent/gone-worktree/packages/core');
       const r = spawnSync(process.execPath, [SCRIPT, primary, '--check'], {
         cwd: wt,
         encoding: 'utf8',
         env: cleanEnv({ WEBJS_NO_WORKTREE_REPAIR: '1' }),
       });
-      assert.equal(r.status, 0);
-      assert.match(r.stdout, /repair skipped/);
+      assert.equal(r.status, 1, 'a broken link is still reported as work to do');
+      assert.match(r.stdout, /would repoint/, 'it actually inspected');
       assert.doesNotMatch(r.stdout, /linked node_modules/, '--check must never link');
       assert.ok(!existsSync(join(wt, 'node_modules')), '--check must not create the symlink');
+      assert.equal(
+        readlinkSync(join(scopeOf(primary), 'core')),
+        '/nonexistent/gone-worktree/packages/core',
+        '--check must not repair either',
+      );
     } finally { rmSync(primary, { recursive: true, force: true }); rmSync(wt, { recursive: true, force: true }); }
   });