Skip to content

Commit 4c6b094

Browse files
vivek7405vivek7405
committed
fix: judge a command, not a token, in the install gate
The false-positive class fixed for bare `yarn` last round was left standing for the other three managers, and the remove verbs widened it. The gate matched a manager+verb pair ANYWHERE in the line, so it blocked this PR's own commit subject, `grep -rn "npm ci" AGENTS.md` and `git log --grep "npm install"`. A linked worktree is the mandated working state here, so that fires on ordinary commands constantly, and a gate that cries wolf gets turned off. The command is now split on `&&`, `||`, `;`, `|`, `(` and `)`, and each segment is judged only by what it STARTS with, after leading env assignments and wrappers like `sudo` are stripped. That kills the class structurally rather than by adding another anchored special case per manager. A GLOBAL install is never blocked either. `-g` writes to the npm prefix, never through the link, and `npm update -g webjsdev` is this repo's own documented post-release step, so the gate was refusing a workflow the repo requires over a corruption that cannot occur. Docs updated for both, plus the remove verbs the last commit added silently.
1 parent f5cfde9 commit 4c6b094

4 files changed

Lines changed: 141 additions & 89 deletions

File tree

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

Lines changed: 93 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,16 @@
1515
# manager starts. `scripts/warn-worktree-install.mjs` covers every other tool by
1616
# reporting rather than blocking.
1717
#
18+
# ## It matches a COMMAND, never a token
19+
#
20+
# The command is split on `&&`, `||`, `;`, `|`, `(` and `)`, and each segment is
21+
# judged only by what it STARTS with. Matching the manager token anywhere in the
22+
# line instead is the obvious shortcut and it is badly wrong: it blocks
23+
# `git commit -m "fix: npm install ..."`, `grep -rn "npm ci" AGENTS.md` and
24+
# `git log --grep "npm install"`. Every worktree here is a linked worktree, so
25+
# that fires on ordinary commands constantly, and a gate that cries wolf is a
26+
# gate someone turns off.
27+
#
1828
# The predicate runs against the COMMAND's target directory, not just the session
1929
# cwd: the harness resets cwd to the primary checkout between commands, so a real
2030
# install in a worktree arrives as `cd /path/to/worktree && npm ci`.
@@ -29,109 +39,105 @@ input=$(cat)
2939
cmd=$(printf '%s' "$input" | jq -r '.tool_input.command // empty' 2>/dev/null || true)
3040
if [ -z "$cmd" ]; then exit 0; fi
3141

32-
# Install verbs only. `npm run test`, `npm test`, `npm exec`, `npm ls`, and every
33-
# `npx ...` must pass, so the verb is matched at a word boundary on both sides.
3442
# Every manager's documented install ALIASES, not just its canonical spelling.
3543
# The gate is worthless if `bun i` walks past it, and Bun is the manager that
36-
# writes THROUGH the symlink into the primary rather than replacing it, so its
37-
# aliases are the consequential ones. npm's list is long because npm ships a
38-
# large alias table of its own (`npm help install`), typo aliases included.
39-
#
40-
# HYPHENATED verbs must be spelled out. The trailing word boundary excludes `-`,
41-
# so `install-test` is NOT reached by listing `install`; each hyphenated command
42-
# needs its own entry, and the short aliases (`it`, `cit`, `sit`) do not cover
43-
# the long spellings.
44+
# writes THROUGH the symlink into the primary rather than replacing it.
4445
#
45-
# The REMOVE verbs are here too. `npm rm <pkg>` in a linked worktree deletes
46-
# from the checkout that owns the tree, the same corruption in the other
47-
# direction.
46+
# HYPHENATED verbs are spelled out: the trailing word boundary excludes `-`, so
47+
# `install-test` is NOT reached by listing `install`, and the short aliases
48+
# (`it`, `cit`, `sit`) do not cover the long spellings.
4849
#
49-
# Word boundaries on BOTH sides keep this narrow: `npm init` does not match `in`
50-
# or `i`, because the next character is alphanumeric, and `npm run install-deps`
51-
# does not match because `run` is not a verb here.
50+
# The REMOVE verbs are here too. `npm rm <pkg>` in a linked worktree deletes from
51+
# the checkout that owns the tree, the same corruption in the other direction.
5252
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'
5353
BUN_VERBS='install|i|add|a|update|up|remove|rm'
5454
PNPM_VERBS='install|i|add|update|upgrade|up|dedupe|remove|rm|uninstall|un'
5555
YARN_VERBS='install|add|upgrade|up|dedupe|remove'
56-
# `npm --prefix <dir> install`, flags BEFORE the verb. Only the two flags that
56+
# `npm --prefix <dir> install` puts flags BEFORE the verb. Only the two flags that
5757
# themselves name a target directory are admitted, so this stays targeted rather
5858
# than swallowing a token run and matching `npm run install`.
59-
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}))"
59+
SEG_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}))([^[:alnum:]_-]|\$)"
60+
# Bare `yarn` IS an install in yarn classic, but only when it is the whole
61+
# command: `yarn test` is not one.
62+
SEG_BARE_YARN='^yarn([[:space:]]+-[^[:space:]]*)*[[:space:]]*$'
63+
# A GLOBAL install writes to the npm prefix, never through the local link, and
64+
# `npm update -g webjsdev` is this repo's documented post-release step.
65+
GLOBAL='(^|[[:space:]])(-g|--global)([[:space:]]|$)'
6066

61-
# Bare `yarn` IS an install in yarn classic, and it needs its own anchored
62-
# pattern rather than a branch of VERBS. VERBS is wrapped in a generic
63-
# non-word-character prefix, which any space satisfies, so a bare-yarn branch
64-
# inside it matched the token ANYWHERE in the command: `which yarn`,
65-
# `rm -rf /tmp/yarn` and `git switch -c feat/yarn` all blocked. Here `yarn` must
66-
# sit in COMMAND position, at the start or straight after a `&&`, `||`, `;` or
67-
# `|`, and be followed only by flags.
68-
BARE_YARN='(^|[&|;])[[:space:]]*yarn([[:space:]]+-[^[:space:]]*)*[[:space:]]*($|[&|;])'
69-
70-
if ! printf '%s' "$cmd" | grep -Eq "(^|[^[:alnum:]_-])${VERBS}([^[:alnum:]_-]|\$)" \
71-
&& ! printf '%s' "$cmd" | grep -Eq "$BARE_YARN"; then
72-
exit 0
73-
fi
67+
# Walk the segments in order so a `cd` earlier in the line moves the target the
68+
# way the shell would.
69+
eff="$PWD"
70+
target=""
71+
while IFS= read -r seg; do
72+
seg="${seg#"${seg%%[![:space:]]*}"}"
73+
[ -z "$seg" ] && continue
7474

75-
# Candidate target directories. An install acts on ONE directory, so the session
76-
# cwd counts only until the command changes out of it: `cd /tmp && npm install`
77-
# targets /tmp, not the worktree this shell happens to sit in. So walk the `cd`
78-
# tokens that appear BEFORE the install verb and let them supersede the cwd, then
79-
# add any directory a package manager is pointed at explicitly.
80-
prefix=$(printf '%s' "$cmd" | sed -E "s/(^|[^[:alnum:]_-])${VERBS}([^[:alnum:]_-]|\$).*//")
81-
prefix=$(printf '%s' "$prefix" | sed -E "s/${BARE_YARN}.*//")
75+
# Strip leading env assignments and benign wrappers, so `FOO=1 npm ci` and
76+
# `sudo npm ci` are still judged on the manager that follows them.
77+
while :; do
78+
case "$seg" in
79+
[A-Za-z_]*=*)
80+
rest="${seg#* }"; [ "$rest" = "$seg" ] && break
81+
seg="${rest#"${rest%%[![:space:]]*}"}" ;;
82+
sudo\ *|env\ *|time\ *|nice\ *)
83+
rest="${seg#* }"
84+
seg="${rest#"${rest%%[![:space:]]*}"}" ;;
85+
*) break ;;
86+
esac
87+
done
8288

83-
eff="$PWD"
84-
resolve_against_eff() {
85-
local d="$1"
86-
case "$d" in
87-
/*) printf '%s' "$d" ;;
88-
~*) printf '%s' '' ;;
89-
*) printf '%s' "$eff/$d" ;;
89+
case "$seg" in
90+
cd|cd\ *)
91+
d="${seg#cd}"; d="${d#"${d%%[![:space:]]*}"}"; d="${d%% *}"
92+
d=$(printf '%s' "$d" | tr -d "\"'")
93+
case "$d" in
94+
'') ;;
95+
/*) eff="$d" ;;
96+
'~'*) ;;
97+
*) eff="$eff/$d" ;;
98+
esac
99+
continue ;;
90100
esac
91-
}
92-
while IFS= read -r d; do
93-
[ -n "$d" ] || continue
94-
d=$(printf '%s' "$d" | tr -d "\"'")
95-
r=$(resolve_against_eff "$d")
96-
[ -n "$r" ] && eff="$r"
97-
done < <(printf '%s\n' "$prefix" \
98-
| grep -oE '(^|[^[:alnum:]_./-])cd[[:space:]]+[^[:space:]&|;]+' \
99-
| sed -E 's/.*cd[[:space:]]+//')
100101

101-
cands=("$eff")
102-
while IFS= read -r d; do
103-
[ -n "$d" ] || continue
104-
d=$(printf '%s' "$d" | tr -d "\"'")
105-
r=$(resolve_against_eff "$d")
106-
[ -n "$r" ] && cands+=("$r")
107-
done < <(printf '%s\n' "$cmd" \
108-
| grep -oE '(^|[[:space:]])(-C|--prefix)[[:space:]=]+[^[:space:]&|;]+' \
109-
| sed -E 's/.*(-C|--prefix)[[:space:]=]+//')
102+
if printf '%s' "$seg" | grep -Eq "$SEG_VERBS" || printf '%s' "$seg" | grep -Eq "$SEG_BARE_YARN"; then
103+
printf '%s' "$seg" | grep -Eq "$GLOBAL" && continue
104+
target="$eff"
105+
# An explicit --prefix / -C on the install itself wins over the cwd.
106+
p=$(printf '%s' "$seg" | grep -oE '(^|[[:space:]])(-C|--prefix)[[:space:]=]+[^[:space:]]+' | sed -E 's/.*(-C|--prefix)[[:space:]=]+//' | tr -d "\"'" | head -1)
107+
if [ -n "$p" ]; then
108+
case "$p" in /*) target="$p" ;; '~'*) ;; *) target="$eff/$p" ;; esac
109+
fi
110+
break
111+
fi
112+
done <<EOF
113+
$(printf '%s' "$cmd" | tr '&|;()' '\n\n\n\n\n')
114+
EOF
110115

111-
for cand in "${cands[@]}"; do
112-
[ -d "$cand" ] || continue
113-
# The install lands at the package root, which for a subdirectory is the
114-
# enclosing checkout, so judge the git toplevel too.
115-
top=$(git -C "$cand" rev-parse --show-toplevel 2>/dev/null || true)
116-
for dir in "$cand" "$top"; do
117-
[ -n "$dir" ] || continue
118-
[ -L "$dir/node_modules" ] || continue
119-
target=$(cd "$(dirname "$dir/node_modules")" 2>/dev/null && readlink "node_modules" || true)
120-
owner=$(cd "$dir" 2>/dev/null && cd "$(readlink node_modules)" 2>/dev/null && pwd -P || printf '%s' "${target:-the primary checkout}")
121-
{
122-
echo "BLOCKED: this command installs into $dir, whose node_modules is a SYMLINK at $owner."
123-
echo "An install through that link damages the checkout that OWNS the tree, not this one:"
124-
echo " npm ci DELETES $owner outright, before any lifecycle script can run"
125-
echo " bun install writes packages and .bin entries straight into $owner"
126-
echo " npm install silently REPLACES the link with a real tree, detaching this worktree"
127-
echo "Safe alternatives:"
128-
echo " npm run worktree:link links a fresh worktree; it never installs"
129-
echo " a real install with NO symlink in the way. Run \`rm node_modules\` first (it is"
130-
echo " only a link, nothing else is lost), or install in the PRIMARY checkout."
131-
echo "Escape hatch for a deliberate exception: WEBJS_NO_WORKTREE_INSTALL_GATE=1."
132-
} >&2
133-
exit 2
134-
done
116+
[ -n "$target" ] || exit 0
117+
[ -d "$target" ] || exit 0
118+
119+
# The install lands at the package root, which for a subdirectory is the
120+
# enclosing checkout, so judge the git toplevel too.
121+
top=$(git -C "$target" rev-parse --show-toplevel 2>/dev/null || true)
122+
for dir in "$target" "$top"; do
123+
[ -n "$dir" ] || continue
124+
[ -L "$dir/node_modules" ] || continue
125+
owner=$(cd "$dir" 2>/dev/null && cd "$(readlink node_modules)" 2>/dev/null && pwd -P) || owner="the checkout it points at"
126+
{
127+
echo "BLOCKED: this command installs into $dir, whose node_modules is a SYMLINK at $owner."
128+
echo "An install through that link damages the checkout that OWNS the tree, not this one:"
129+
echo " npm ci DELETES $owner outright, before any lifecycle script can run"
130+
echo " bun install writes packages and .bin entries straight into $owner"
131+
echo " npm install silently REPLACES the link with a real tree, detaching this worktree"
132+
echo "A remove verb (npm rm, bun remove) deletes from that same owning checkout."
133+
echo "Safe alternatives:"
134+
echo " npm run worktree:link links a fresh worktree; it never installs"
135+
echo " a real install with NO symlink in the way. Run \`rm node_modules\` first (it is"
136+
echo " only a link, nothing else is lost), or install in the PRIMARY checkout."
137+
echo "A GLOBAL install (-g) is not affected by this and is never blocked."
138+
echo "Escape hatch for a deliberate exception: WEBJS_NO_WORKTREE_INSTALL_GATE=1."
139+
} >&2
140+
exit 2
135141
done
136142

137143
exit 0

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ The script discovers the `node_modules` set from the primary checkout rather tha
6666

6767
**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.
6868

69-
**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`.
69+
**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

7171
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.
7272

‎framework-dev.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ Measured on npm 11.19.0 and bun 1.3.14:
162162

163163
So prevention lives one layer up, and the rest is repair:
164164

165-
- **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`.
165+
- **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`, covering every manager's documented aliases (`bun i` matters most, since Bun writes THROUGH the link) and the REMOVE verbs too, because `npm rm` in a linked worktree deletes from the checkout that owns the tree. Escape hatch `WEBJS_NO_WORKTREE_INSTALL_GATE=1`.
166+
167+
It judges a COMMAND, never a token. The command is split on `&&`, `||`, `;`, `|`, `(` and `)`, and each segment is judged only by what it STARTS with, after leading env assignments and wrappers like `sudo` are stripped. Matching the manager token anywhere in the line is the obvious shortcut and it is badly wrong: it blocks `git commit -m "fix: npm install ..."`, `grep -rn "npm ci" AGENTS.md` and `git log --grep "npm install"`. A linked worktree is the mandated working state here, so that fires on ordinary commands constantly, and a gate that cries wolf is a gate someone turns off. `npm test`, `npm run <script>`, `npx ...`, `npm init` and `yarn test` all pass, and so does a GLOBAL install (`-g` / `--global`), which writes to the npm prefix rather than through the link and is this repo's documented post-release step.
166168
- **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.
167169
- **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.
168170
- **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`.

0 commit comments

Comments
 (0)