Skip to content

fix(install): stop the postinstall symlink race that fails pnpm install - #1300

Merged
kkroo merged 2 commits into
masterfrom
kkroo/fix-install-symlink-race
Aug 11, 2026
Merged

fix(install): stop the postinstall symlink race that fails pnpm install#1300
kkroo merged 2 commits into
masterfrom
kkroo/fix-install-symlink-race

Conversation

@kkroo

@kkroo kkroo commented Aug 11, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • Its monorepo ships first-party plugins, some inside the pnpm workspace and some deliberately excluded from it so their third-party deps stay out of the root lockfile
  • Excluded plugins do not get @paperclipai/plugin-sdk linked by pnpm, so a root postinstall walks the excluded list and symlinks the in-repo SDK into each one
  • A workspace member had also wired that same script into its own postinstall, and pnpm runs member hooks concurrently with the root hook
  • Both processes therefore walked the same excluded list and raced to create the same symlink, so the loser aborted the whole install with EEXIST
  • This pull request removes the duplicate hook and makes the symlink itself tolerant of losing that race
  • The benefit is that pnpm install stops failing intermittently, and the job it takes down stops reporting a misleading cause

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 install intermittently dies while linking the in-repo plugin-sdk, failing whatever job it lands in. Most recently it took out Typecheck + Release Registry on #1294.

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)
    at linkExcludedPlugins (scripts/link-plugin-dev-sdk.mjs:33:9)

The install log shows the two racers interleaved within the same second:

06:33:29.8  packages/plugins/plugin-workspace-diff postinstall$ node ../../../scripts/link-plugin-dev-sdk.mjs
06:33:30.2  . postinstall$ node scripts/link-plugin-dev-sdk.mjs
06:33:30.3  . postinstall:   ✓ Linked @paperclipai/plugin-sdk into 1 excluded plugin(s) (skipped 7)
06:33:30.3  packages/plugins/plugin-workspace-diff postinstall: Error: EEXIST ...
06:33:30.3  packages/plugins/plugin-workspace-diff postinstall: Failed

Root cause

plugin-workspace-diff is a pnpm workspace member (packages/plugins/*), so pnpm runs its postinstall concurrently 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-diff is not in excludedPluginDirs(). The root hook's own log line confirms it did all the work: 1 linked, 7 skipped.

Expected behavior

pnpm install completes 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

  • Removed the postinstall from packages/plugins/plugin-workspace-diff/package.json — pure duplication of the root hook, and the only racer in a root install.
  • Made the symlink race-tolerant in scripts/link-plugin-dev-sdk.mjs. The lstat probe and the symlinkSync call are not atomic, and packages excluded from the workspace legitimately keep their own postinstall for standalone installs (they never see the root hook). An identical link now counts as success; a divergent one still throws.
  • Corrected the script's header comment. It claimed no plugin's package.json references the script, which was already false, and justified that with a published-tarball concern that prepack already handles — generate-plugin-package-json.mjs rebuilds package.json from a field whitelist that omits scripts entirely.
  • Added regression coverage: a structural guard that no workspace member reruns the shared linker from 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 postinstall restored it fails and names the offender:

✖ no workspace-member plugin runs the shared linker from its own postinstall
    actual: [ '@paperclipai/plugin-workspace-diff' ],
    expected: [],

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-migration with Test 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

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above — scanned all open PRs by title for EEXIST|postinstall|link-plugin and by changed-file for scripts/link-plugin-dev-sdk.*; no overlap found
  • I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • If this change affects the UI, I have included before/after screenshots — n/a, no UI change
  • I have updated relevant documentation to reflect my changes — corrected the stale header comment in the script
  • I have considered and documented any risks above
  • All Paperclip CI gates are green
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups
  • I will address all Greptile and reviewer comments before requesting merge

`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.
@cursor

cursor Bot commented Aug 11, 2026

Copy link
Copy Markdown

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.

@allyblockcast

allyblockcast Bot commented Aug 11, 2026

Copy link
Copy Markdown

Hey @kkroo! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

  • Missing section: ## Thinking Path
  • Missing section: ## What Changed
  • Missing section: ## Model Used
  • Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 EEXIST fallback 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

  1. Merge when the remaining repository checks are green.

@allyblockcast allyblockcast left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 EEXIST fallback 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

  1. Merge when the remaining repository checks are green.

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Merge when the remaining repository checks are green.

@allyblockcast allyblockcast left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Merge when the remaining repository checks are green.

@kkroo
kkroo merged commit 114109d into master Aug 11, 2026
14 of 17 checks passed
@kkroo
kkroo deleted the kkroo/fix-install-symlink-race branch August 11, 2026 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants