fix(coding-agent): allow the self-update tarball install on npm 12 - #1272
Open
smwbev wants to merge 2 commits into
Open
fix(coding-agent): allow the self-update tarball install on npm 12#1272smwbev wants to merge 2 commits into
smwbev wants to merge 2 commits into
Conversation
npm 12 defaults allow-remote to none, so `prime-agent update` fails with EALLOWREMOTE when the release is installed from its tarball URL: the download itself is a remote fetch, and the released package depends on further tarball URLs, which npm treats as non-root, so "root" is not enough either. Carry an optional env on each self-update step and set npm_config_allow_remote=all on the npm install step, but only when the update spec is a direct package artifact (URL, file:, .tgz/.tar.gz). Registry specs keep the stricter default, the user's npmrc is untouched, and the grant lasts for the single install that needs it. pnpm, yarn and bun have no equivalent gate, so those branches are unchanged. Refs PrimeIntellect-ai#741
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
prime-agent updatecannot update an npm-managed install on npm 12:Reproduced on macOS (Apple Silicon), Node v26.5.0, npm 12.0.1, global npm install with prefix
/opt/homebrew, updating 0.7.1 -> 0.7.2.npm 12 defaults
allow-remotetonone. The self-update installs the release artifact by URL, which is a remote fetch, and the released package declares further tarball URLs (@earendil-works/pi-agent-core,pi-ai,pi-tui), which npm classifies as non-root — sorootis not enough either, matching what @felipecsl measured in #741.Change
SelfUpdateCommandStepcarries an optionalenv, merged over the current environment when that step runs.getSelfUpdateCommandForMethodsetsnpm_config_allow_remote=allon the install step, and only whenisDirectPackageArtifactSpec(updateSpec)holds. Registry specs keep the stricter npm 12 default, and the follow-up uninstall step never gets the grant.runSelfUpdatepasses the step environment tospawn; steps withoutenvkeep inheritingprocess.envunchanged.The grant is scoped to the one process that needs it. Nothing is written to the user's
~/.npmrc, which is the same approach #773 took for the installer.isDirectPackageArtifactSpecrather than an npm version gate: the predicate already distinguishes the case that needs the config (a release artifact fetched by URL or path) from the case that must not get it (a registry spec), and it is the same condition that already decides the install/uninstall ordering. A version gate would grantallow-remoteto registry updates as well on npm 12, which is strictly weaker; on npm < 12 the extra environment variable is ignored, so gating on the version buys nothing.pnpm, yarn and bun install remote tarballs without an equivalent opt-in — pnpm's nearest setting,
blockExoticSubdeps, is off by default and is not an env-scoped grant — so their branches are unchanged.The step environment is rendered into
display, so the "Updating ..." line and the copy-paste fallback describe the command that actually runs instead of one that fails without the assignment. The assignment is rendered in POSIX form; on Windows it reads as a description rather than a paste-ready line.Verification
npm run checkclean.packages/coding-agent/test/suite/regressions/741-npm-self-update-allow-remote.test.ts; it fails onmainand passes with this change. Existingtest/config.test.tsexpectations updated for the new field.The
spawnwiring itself is covered only by that manual run; the test asserts the command thatrunSelfUpdatereceives.Scope
Complementary to #773, which fixes the same npm 12 default on the
install.shside; there is no file overlap with it. Its author noted in #741 that the self-update path was deliberately left out, which is what this PR closes.Two things stay out of scope:
postinstallthat prepares uv and the IPython runtime unlessallow-scriptscovers it (reported by @felipecsl in install fails on npm 12+ due to allow-remote=none #741). That is a separate default with a different trade-off, so it is not silently granted here.Refs #741 — deliberately not
Fixes, since the issue also covers the installer surface handled in #773.Note
Fix npm self-update to set
npm_config_allow_remote=allfor artifact installsenvfield to theSelfUpdateCommandStepinterface in config.ts, allowing per-step environment variable overrides merged overprocess.envat execution time.getSelfUpdateCommandForMethod, the npm install step now setsnpm_config_allow_remote=allwhen the update spec is a direct artifact (e.g.file:or tarball URL); registry-based specs and the uninstall step are unaffected.runSelfUpdatein package-manager-cli.ts appliesstep.envwhen spawning each step if present.NAME=valueassignments.npm_config_allow_remote=all; all other update flows are unchanged.Macroscope summarized 8c86051.