Ben Younes eae20a410a fix(setup): auto-install plugin dependencies at Setup phase (2649) (#2650) ...

* fix(setup): auto-install plugin dependencies at Setup phase (2649)

The plugin marketplace extracts files into ~/.claude/plugins/cache/...
but does not run `bun install`. On fresh installs the worker crashes
with `Cannot find module 'zod/v3'` on the first hook invocation
(gh #2640, #2637).

PR #2644 fixed this by auto-installing dependencies inside
`bun-runner.js`, but that runs on the SessionStart / UserPromptSubmit
critical path. Review on #2644 (YOMXXX, #2649) flagged this as the
wrong architectural home: corporate proxies, offline machines,
permissions, registry timeouts — every install failure lands on the
user's first prompt instead of at install time.

Move the auto-install to `version-check.js` (Setup phase), the only
standalone hook script and the natural place to materialise plugin
runtime state. Setup has a 300s timeout (vs 60s for SessionStart),
runs once per Claude Code launch, and the node_modules guard short-
circuits subsequent invocations.

Failure modes surfaced explicitly (spawn exception, non-zero exit,
signal-killed including OOM SIGKILL — where spawnSync leaves
status=null and error=undefined).

RED → GREEN proof:
- /tmp/issue-2649-proof/RED.log  (unpatched: 1 pass, 1 fail)
- /tmp/issue-2649-proof/GREEN.log (patched: 2 pass, 0 fail)
- Full-suite baseline: 1803 pass / 55 fail (unchanged after fix:
  1805 pass / 55 fail — +2 = new tests, 0 regressions).

Existing `plugin-version-check.test.ts` updated to pre-create
node_modules in beforeEach so its marker-compat assertions
(`stderr === ''`) are unaffected by the new Setup-phase install path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(setup): cleanup partial node_modules + success log (#2650 review)

Address Greptile review findings on PR #2650:

P1 — failed install permanently blocks retry. `bun install` often
creates the node_modules directory before it terminates under failure
(network timeout mid-fetch, registry 5xx, OOM kill). The
existsSync(node_modules) guard would then permanently skip every
subsequent Setup run, leaving the plugin broken with no recovery short
of manual `rm -rf node_modules`. Remove the partial dir in the failure
branch so the next Setup invocation can retry automatically.

P2 — no completion confirmation. A Setup hook that can block for up to
120s needs an explicit success line so users can distinguish a hung
install from one that finished silently. Emit a success diagnostic in
the no-error else branch.

New test `cleans up partial node_modules after a failed install` uses
a `partial-then-fail` fake-bun behavior that creates node_modules then
exits non-zero (mirrors real bun under mid-fetch failure). Asserts the
failure diagnostic surfaces AND node_modules is gone after, proving
the retry path is unblocked.

RED -> GREEN proof:
- /tmp/issue-2650-proof/RED-review-fixes.log
  (without fix: 1 pass / 2 fail — success diagnostic missing AND
   partial node_modules NOT cleaned up)
- /tmp/issue-2650-proof/GREEN-review-fixes.log
  (with fix: 3 pass / 0 fail)

Full-suite baseline preserved:
- /tmp/issue-2650-proof/full-suite-GREEN.log
  (1806 pass / 19 skip / 55 fail — same 55 pre-existing failures,
   +1 new test vs prior commit, 0 regressions)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

2026-06-05 23:04:54 -07:00

7.3 KiB

Raw Permalink Blame History

View Raw