Skip to content

fix(core): honor ignoreScripts for git dependency preparation - #60201

Closed
kodurd wants to merge 1 commit into
anomalyco:devfrom
kodurd:pacote-ignore-scripts
Closed

kodurd wants to merge 1 commit into
anomalyco:devfrom
kodurd:pacote-ignore-scripts

Conversation

@kodurd

@kodurd kodurd commented Oct 29, 2026 •

Copy link
Copy Markdown

Issue for this PR

Closes #49704

Type of change

  • Bug fix

What does this PR do?

OpenCode installs Git plugins with ignoreScripts: true, but pacote 21.5.1 still runs its Git preparation step for packages that declare build/prepare/install/postinstall scripts. That step spawns npm install inside the clone.

In a compiled OpenCode binary the npm path resolves into Bun's bundled filesystem, so the spawn re-executes opencode with $bunfs/bin/npm-cli.js install .... OpenCode prints help and exits, and the install fails with git dep preparation failed.

This backports pacote 22's guard into the existing pacote patch: skip Git preparation when ignoreScripts is set.

How did you verify your code works?

Added skips Git package preparation when scripts are ignored to packages/core/test/npm.test.ts. The test fails before the patch with git dep preparation failed and passes after it.

I also ran bun dev plugin add github:psh4607-works/opencode-agent-sidebar#v0.2.5; installation succeeds, and the TUI renders the plugin's Agents section. Targeted test and @opencode/core / @opencode/util typecheck pass.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Added by the opencode bot (AI-generated).

Fixes #59504

Backport pacote 22's ignoreScripts guard so installing a Git dependency does not spawn npm to run lifecycle scripts. OpenCode already passes ignoreScripts, and in bundled builds the spawned npm path points into Bun's virtual filesystem.

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. I ran the #49704 reproduction from source. On the base, a git+file:// plugin with a build script fails with git dep preparation failed. With this PR it installs. The new test fails on the base and passes here, and the rest of npm.test.ts and tsc -b for packages/core pass.

The new guard is the same as the one in upstream pacote's main lib/git.js, in the same place. Skipping preparation matches how OpenCode already installs packages: both Arborist calls in packages/util/src/npm.ts set ignoreScripts: true. So Git plugins now install like registry ones, without running their scripts. Before, they could not install at all. Because the nested npm spawn no longer happens, this also fixes the Windows .cmd spawn failure reported separately.

Public API: unchanged; only the vendored pacote patch and a test change.

No blocking concerns.

Fixes #59504

@JoshRob297

Copy link
Copy Markdown

Measured what honouring ignoreScripts means here, because it changes what "installed" is for plugins that build themselves.

With @npmcli/arborist directly, same options as Npm.reify, on a Git dependency whose prepare compiles the entry point:

options result node_modules/<pkg>/dist
ignoreScripts: true (any npmBin) install succeeds absent
ignoreScripts: false, npmBin: "npm" install succeeds present

So the guard removes the error but ships the dependency unbuilt. For a plugin with main: ./dist/index.js, that file does not exist afterwards and OpenCode loads nothing: a silent failure where today there is a loud one, for every plugin that compiles during install.

The preparation does not fail because building is unwanted — it fails because the child process is wrong. pacote spawns process.execPath with /$bunfs/bin/npm-cli.js when npmBin ends in .js, and the binary runs its CLI instead of that script (the strace is in #49704).

Suggestion: keep this guard as the fallback for machines with no npm, but try the preparation first with a resolved npm, and when it is skipped for lack of one, say so instead of reporting success. #62817 does the first half.

@kodurd

kodurd commented Nov 9, 2026

Copy link
Copy Markdown
Author

@JoshRob297 is right, and the measurement settles it. Skipping preparation makes the install succeed while leaving the package unbuilt, so a plugin with main: ./dist/index.js resolves to a file that does not exist — a silent failure in place of a loud one. My test here asserts exactly that skipped-script behaviour, which means it was pinning down the wrong contract.

The preparation step is not the problem; the child process is. #62817 fixes that by resolving a real npm instead of a path into the bundle, which is the cause rather than the symptom. Closing this in favour of it.

@kodurd kodurd closed this Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants