8000 Point npm's global prefix at ~/.local instead of the read-only /usr by agent-137 · Pull Request #657 · gominimal/pkgs · GitHub
[go: up one dir, main page]
Skip to content

Point npm's global prefix at ~/.local instead of the read-only /usr - #657

Merged
mitodrummer merged 1 commit into
mainfrom
node-npm-global-prefix
Aug 27, 2026
Merged

Point npm's global prefix at ~/.local instead of the read-only /usr#657
mitodrummer merged 1 commit into
mainfrom
node-npm-global-prefix

Conversation

@agent-137
@agent-137 agent-137 commented Aug 27, 2026 •
edited by coderabbitai Bot
Loading
Copy link
Copy Markdown
Member

Fixes gominimal/inbox#559.

What

npm i -g fails with ENOENT ... mkdir '/usr/lib/node_modules/...' in every session carrying node or node-lts: npm's compiled-in global prefix is /usr, which is the read-only package store at session time. This PR makes both packages ship a builtin npmrc (usr/lib/node_modules/npm/npmrc, npm's lowest-precedence config source) containing prefix=~/.local, and adds a npm_global_prefix regression test to each spec asserting the resolved prefix no longer points at the store.

Global installs then land in ~/.local/lib/node_modules with executables in ~/.local/bin, which is already on the session PATH (crates/sandbox2/src/config.rs in gominimal/minimal hardcodes it). Because builtin is the lowest-precedence source, any user --prefix, env var, or .npmrc still overrides it. The file is picked up by the existing node_modules output glob; no changes to outputs or to the CLI.

This matches the community workaround already circulating (varlock's sandbox guide hand-rolls --prefix ~/.local), so that documented quirk becomes deletable.

Why not env_state_wiring

Adding NPM_CONFIG_PREFIX via env_state_wiring (the router's approach A on the issue) would put globals under /state/npm-global, but nothing puts a /state/*/bin on the session PATH and no wiring facility exists to do so — that would require new machinery in gominimal/minimal. The builtin npmrc needs neither. If persistent-across-recreate npm globals become a want later, the state-dir approach can still be layered on top; this change doesn't foreclose it.

Verification

Verified empirically in a live session carrying node-lts (the packages weren't rebuilt locally — node is a 5x-cost build — so CI's package build + the new spec tests are the authoritative check):

  • Reproduced the bug: npm config get prefix → /usr; npm i -g fails with the issue's exact ENOENT.
  • Confirmed npm loads a builtin npmrc from its install dir using a writable copy of this package's npm: npm config ls -l reports prefix = "/usr" ; overridden by builtin and resolves prefix to /home/.local (~ expands via npm's path-type config handling).
  • With that prefix, npm i -g nanoid succeeds, the executable lands at /home/.local/bin/nanoid, resolves via PATH, and runs.

Notes for the reviewer

  • Task (non-session) sandboxes don't have ~/.local/bin on PATH; a global install there now succeeds but its bins don't resolve. That is status-quo minus the ENOENT — tasks should declare packages rather than global-install.
  • The issue also suggests auditing pip, gem, and cargo install for the same read-only-store failure mode. Left out deliberately: pip and gem have user-install fallbacks npm lacks, and cargo already defaults to ~/.cargo/bin. Follow-up material.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JGTXbdK7hgDupktNZEkALY

Summary by CodeRabbit

  • Bug Fixes

    • Updated npm’s global installation location to use the user-writable ~/.local directory.
    • Global npm package installations now avoid failures caused by the read-only system directory.

  • Tests

    • Added coverage verifying that npm resolves its global pa 8000 ckage prefix to the expected user-writable location.

@mitodrummer @claude
Point npm's global prefix at ~/.local instead of the read-only /usr
69abbc5 
npm's compiled-in prefix is /usr, the read-only package store at session
time, so `npm i -g` fails with ENOENT in every session carrying node or
node-lts. Ship a builtin npmrc (npm's lowest-precedence config source)
setting prefix=~/.local: global installs then land in a writable home
location whose bin/ is already on the session PATH, and any user-level
config still overrides it.

Fixes gominimal/inbox#559

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JGTXbdK7hgDupktNZEkALY
@coderabbitai
coderabbitai Bot commented Aug 27, 2026 •
edited
Loading
Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2cfe6935-7ef3-4248-81d6-f628595a318e

📥 Commits

Reviewing files that changed from the base of the PR and between 8efbcf7 and 69abbc5.

📒 Files selected for processing (4)
  • packages/node-lts/build.ncl
  • packages/node-lts/build.sh
  • packages/node/build.ncl
  • packages/node/build.sh

Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

📝 Walkthrough

Walkthrough

The Node and Node LTS packages now configure npm global installations to use ~/.local. New standalone tests verify that npm resolves the global prefix to a path ending in /.local.

Changes

npm global prefix

Layer / File(s) Summary
Configure npm global installation prefix
packages/node/build.sh, packages/node-lts/build.sh		 Each installation script writes an npmrc file with prefix=~/.local.
Validate configured npm prefix
packages/node/build.ncl, packages/node-lts/build.ncl		 Each package adds a standalone test that runs npm config get prefix and checks that the result ends in /.local.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 69abb

The change redirects npm global installs from the read-only system location to the user-writable local directory while preserving user overrides; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: twitchyliquid64

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: configuring npm's global prefix to use ~/.local instead of the read-only /usr store.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch node-npm-global-prefix

Comment @coderabbitai help to get the list of available commands.

@mitodrummer
mitodrummer marked this pull request as ready for review August 27, 2026 23:41
bryan-minimal
bryan-minimal approved these changes Aug 27, 2026
View reviewed changes
@bryan-minimal bryan-minimal left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@mitodrummer
mitodrummer added this pull request to the merge queue Aug 27, 2026
Merged via the queue into main with commit c89dd85 Aug 27, 2026
10 checks passed
@mitodrummer
mitodrummer deleted the node-npm-global-prefix branch August 27, 2026 23:59
@agent-137 agent-137 mentioned this pull request Aug 28, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bryan-minimal bryan-minimal bryan-minimal approved these changes
@norrietaylor norrietaylor Awaiting requested review from norrietaylor

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@agent-137 @bryan-minimal @mitodrummer
0