fix(install): stop the postinstall symlink race that fails pnpm install - #1300
Conversation
`pnpm install` intermittently died with EEXIST while linking the in-repo
plugin-sdk:
plugin-workspace-diff postinstall: Error: EEXIST: file already exists,
symlink '../../../../sdk' -> '.../plugin-orchestration-smoke-example/
node_modules/@paperclipai/plugin-sdk'
at linkSdkInto (scripts/link-plugin-dev-sdk.mjs:96:3)
plugin-workspace-diff is a pnpm workspace member, so pnpm ran its
postinstall concurrently with the root one. Both invoke the same script,
which walks the full excluded-plugin list, so both raced to create the
same symlink and the loser failed the whole install. The member's hook
also did no work for itself: workspace members get their SDK link from
pnpm, and plugin-workspace-diff is not in excludedPluginDirs().
Removed as pure duplication. The header comment claiming no plugin
references this script was already stale, so it now describes the actual
rule and points at the prepack whitelist that keeps `scripts` out of
published tarballs.
Also made the symlink itself race-tolerant. The lstat probe and the
symlink call are not atomic, and packages excluded from the workspace
legitimately keep their own postinstall for standalone installs. An
identical link now counts as success; a divergent one still throws.
Regression coverage asserts no workspace member reruns the shared linker
from postinstall -- it fails with the offending package name if the hook
comes back.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Hey @kkroo! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 60c765e
Critical Issues (0)
Important Issues (0)
Suggestions (0)
Strengths
- The root postinstall now has one authoritative linker for excluded plugins, while workspace packages retain pnpm-managed SDK resolution.
- The
EEXISTfallback accepts only an identical symlink and retains failures for divergent or non-symlink targets. - Coverage protects both race outcomes and prevents a workspace plugin from reintroducing the duplicate postinstall hook.
Recommended Action
- Merge when the remaining repository checks are green.
allyblockcast
left a comment
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 60c765e
Critical Issues (0)
Important Issues (0)
Suggestions (0)
Strengths
- The root postinstall now has one authoritative linker for excluded plugins, while workspace packages retain pnpm-managed SDK resolution.
- The
EEXISTfallback accepts only an identical symlink and retains failures for divergent or non-symlink targets. - Coverage protects both race outcomes and prevents a workspace plugin from reintroducing the duplicate postinstall hook.
Recommended Action
- Merge when the remaining repository checks are green.
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: b2094e9
Critical Issues (0)
Important Issues (0)
Suggestions (0)
Strengths
- The root lifecycle hook is now the single linker for excluded plugins, eliminating the concurrent workspace-hook path that produced
EEXIST. - The race fallback accepts only an identical symlink and still fails closed for a divergent or non-symlink target.
- Regression coverage exercises both race outcomes and prevents workspace plugins from reintroducing the duplicate hook.
Recommended Action
- Merge when the remaining repository checks are green.
allyblockcast
left a comment
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: b2094e9
Critical Issues (0)
Important Issues (0)
Suggestions (0)
Strengths
- The root lifecycle hook is now the single linker for excluded plugins, eliminating the concurrent workspace-hook path that produced
EEXIST. - The race fallback accepts only an identical symlink and still fails closed for a divergent or non-symlink target.
- Regression coverage exercises both race outcomes and prevents workspace plugins from reintroducing the duplicate hook.
Recommended Action
- Merge when the remaining repository checks are green.
Thinking Path
Linked Issues or Issue Description
No GitHub issue exists; filed internally as BLO-25567. Describing it here per the bug-report template.
What happened?
pnpm installintermittently dies while linking the in-repo plugin-sdk, failing whatever job it lands in. Most recently it took out Typecheck + Release Registry on #1294.The install log shows the two racers interleaved within the same second:
Root cause
plugin-workspace-diffis a pnpm workspace member (packages/plugins/*), so pnpm runs itspostinstallconcurrently with the root one. Both invoke the same script, and the script walks the entire excluded-plugin list rather than just the calling package — so both raced on the same symlink.The member's hook did no work for itself either: workspace members get their SDK link from pnpm, and
plugin-workspace-diffis not inexcludedPluginDirs(). The root hook's own log line confirms it did all the work: 1 linked, 7 skipped.Expected behavior
pnpm installcompletes deterministically regardless of hook scheduling order.Side note on diagnosis
The rollup reported this as a Typecheck failure while nothing was ever typechecked — the install step died first. Worth knowing when reading a red Typecheck job.
What Changed
postinstallfrompackages/plugins/plugin-workspace-diff/package.json— pure duplication of the root hook, and the only racer in a root install.scripts/link-plugin-dev-sdk.mjs. Thelstatprobe and thesymlinkSynccall are not atomic, and packages excluded from the workspace legitimately keep their ownpostinstallfor standalone installs (they never see the root hook). An identical link now counts as success; a divergent one still throws.package.jsonreferences the script, which was already false, and justified that with a published-tarball concern thatprepackalready handles —generate-plugin-package-json.mjsrebuildspackage.jsonfrom a field whitelist that omitsscriptsentirely.postinstall, plus both branches of the race-tolerant symlink.Verification
node --test scripts/link-plugin-dev-sdk.test.js— 10 pass locally.The structural guard is a real regression test, not a tautology. With the
postinstallrestored it fails and names the offender:Worktree install (NODE_ENV=production)passed on this PR, which exercises the install path.Risks
Low. The removed hook was doing no work for its own package — workspace members are linked by pnpm. The only behavioral change for excluded packages is accepting a byte-identical concurrent link instead of crashing; a link pointing anywhere else still throws as before.
Unrelated known-red on this branch:
General tests (workspaces-b)failed on@paperclipai/db > heartbeat-runs-agent-dispatch-index-migrationwithTest timed out in 60000ms. That is an embedded-Postgres migration test with no connection to the plugin linker.Model Used
Claude Opus 5 (
claude-opus-5[1m], 1M context) via Claude Code, with extended thinking and tool use (shell, GitHub CLI, file edits).Checklist
EEXIST|postinstall|link-pluginand by changed-file forscripts/link-plugin-dev-sdk.*; no overlap foundFixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template