fix(npm-globals): install platform-native binaries for claude-code and codex - #1890
Conversation
…d codex Both packages ship platform-specific binaries via optionalDependencies, but bun's transitive optional resolution silently drops them when the parent's postinstall is blocked by ignoreScripts. The result was claude-code stuck at 2.1.140 and codex at 0.130.0 despite the wrappers updating. - Add all platform variants to package.json optionalDependencies as a cross-platform manifest (darwin/linux/win32 x arm64/x64, plus musl). - Extend install-npm-globals.sh to detect current platform and install the matching variant as a direct global dep. Strips it from global optionalDependencies first because bun add -g no-ops when the package is already listed there. - Strip stale non-matching platform variants from global package.json to keep it clean across runs. - Handle both plain semver (claude-code) and npm: alias (codex) specs.
|
📝 WalkthroughWalkthroughThis PR adds platform-specific optional dependencies for npm globals. It declares OS/architecture-specific variants of ChangesPlatform-specific optional globals
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRFixes global installation of platform-native binaries for What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request adds platform-specific optional dependencies for @anthropic-ai/claude-code and @openai/codex to package.json and bun.lock, and updates the install-npm-globals.sh script to manually install the matching platform variant to bypass Bun's transitive-optional resolution limitations. The review feedback identifies three important issues in the script: a logic flaw on musl-based Linux systems that would install both glibc and musl variants, a portability issue on macOS due to the reliance on the GNU timeout command, and an inefficiency where jq is spawned repeatedly in a loop to clean up package.json instead of using a single atomic pass.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| PLATFORM_MUSL_SUFFIX="${PLATFORM_SUFFIX}-musl" | ||
|
|
||
| while IFS= read -r entry; do | ||
| dep="${entry%%=*}" | ||
| [ -z "$dep" ] && continue | ||
| # Match exact platform variant. Skip musl on darwin/win32. | ||
| if [[ "$dep" == *"-${PLATFORM_SUFFIX}" ]]; then | ||
| : | ||
| elif [ "$PLATFORM_OS" = "linux" ] && [[ "$dep" == *"-${PLATFORM_MUSL_SUFFIX}" ]]; then | ||
| # Only install musl variant on actual musl systems (NixOS uses glibc by default). | ||
| if ! ldd --version 2>&1 | grep -qi musl; then | ||
| continue | ||
| fi | ||
| else | ||
| continue | ||
| fi |
There was a problem hiding this comment.
On a musl-based Linux system, both the glibc variant (e.g., *-linux-x64) and the musl variant (e.g., *-linux-x64-musl) would match the current logic and be installed, because [[ "$dep" == *"-${PLATFORM_SUFFIX}" ]] matches any package ending with -linux-x64 (which includes the glibc one). This can lead to conflicts or broken binaries.
We can resolve this by pre-detecting if the system is musl-based, and then precisely matching only the musl variant on musl systems, and only the standard variant (while excluding musl) on non-musl systems.
| PLATFORM_MUSL_SUFFIX="${PLATFORM_SUFFIX}-musl" | |
| while IFS= read -r entry; do | |
| dep="${entry%%=*}" | |
| [ -z "$dep" ] && continue | |
| # Match exact platform variant. Skip musl on darwin/win32. | |
| if [[ "$dep" == *"-${PLATFORM_SUFFIX}" ]]; then | |
| : | |
| elif [ "$PLATFORM_OS" = "linux" ] && [[ "$dep" == *"-${PLATFORM_MUSL_SUFFIX}" ]]; then | |
| # Only install musl variant on actual musl systems (NixOS uses glibc by default). | |
| if ! ldd --version 2>&1 | grep -qi musl; then | |
| continue | |
| fi | |
| else | |
| continue | |
| fi | |
| PLATFORM_MUSL_SUFFIX="${PLATFORM_SUFFIX}-musl" | |
| IS_MUSL=false | |
| if [ "$PLATFORM_OS" = "linux" ] && ldd --version 2>&1 | grep -qi musl; then | |
| IS_MUSL=true | |
| fi | |
| while IFS= read -r entry; do | |
| dep="${entry%%=*}" | |
| [ -z "$dep" ] && continue | |
| # Match exact platform variant. | |
| if [ "$IS_MUSL" = "true" ]; then | |
| if [[ "$dep" != *"-${PLATFORM_MUSL_SUFFIX}" ]]; then | |
| continue | |
| fi | |
| else | |
| if [[ "$dep" != *"-${PLATFORM_SUFFIX}" ]] || [[ "$dep" == *"-musl" ]]; then | |
| continue | |
| fi | |
| fi |
| spec="$dep" | ||
| fi | ||
| echo "Installing platform-native: $spec" | ||
| timeout 600 bun add --global "$spec" --minimum-release-age 0 2>/dev/null || echo "Install failed: $spec" |
There was a problem hiding this comment.
The timeout command is a GNU utility and is not available by default on macOS (Darwin). Since macOS is a primary target platform, attempting to run timeout will fail with command not found, causing the installation of the platform-native binary to fail entirely.
Additionally, redirecting stderr to /dev/null (2>/dev/null) hides any actual installation errors (such as network issues or permission errors), making troubleshooting very difficult.
We should check if timeout is available before using it, and avoid silencing stderr.
| timeout 600 bun add --global "$spec" --minimum-release-age 0 2>/dev/null || echo "Install failed: $spec" | |
| if command -v timeout >/dev/null 2>&1; then | |
| timeout 600 bun add --global "$spec" --minimum-release-age 0 || echo "Install failed: $spec" | |
| else | |
| bun add --global "$spec" --minimum-release-age 0 || echo "Install failed: $spec" | |
| fi |
| # Strip non-matching platform-variant entries from global optionalDependencies. | ||
| # Bun lists them but never materializes them (os/cpu mismatch), so they just | ||
| # accumulate and clutter the global package.json across runs. | ||
| if [ -f "$GLOBAL_PKG" ]; then | ||
| STALE_OPTIONAL=$(jq -r '.optionalDependencies // {} | keys[]?' "$GLOBAL_PKG" 2>/dev/null || true) | ||
| if [ -n "$STALE_OPTIONAL" ]; then | ||
| while IFS= read -r dep; do | ||
| [ -z "$dep" ] && continue | ||
| if ! jq -e --arg dep "$dep" '.dependencies | has($dep)' "$GLOBAL_PKG" >/dev/null 2>&1; then | ||
| jq --arg dep "$dep" 'del(.optionalDependencies[$dep])' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | ||
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | ||
| fi | ||
| done <<<"$STALE_OPTIONAL" | ||
| fi | ||
| # Drop the optionalDependencies key entirely if now empty. | ||
| if [ "$(jq -r '.optionalDependencies // {} | length' "$GLOBAL_PKG" 2>/dev/null)" = "0" ]; then | ||
| jq 'del(.optionalDependencies)' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | ||
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Spawning jq and rewriting the global package.json file inside a loop for every single stale optional dependency is highly inefficient and can be slow.
We can perform this entire cleanup in a single, atomic jq pass. This filters the optionalDependencies to only keep keys that are also present in dependencies, and automatically deletes the optionalDependencies key if it becomes empty.
| # Strip non-matching platform-variant entries from global optionalDependencies. | |
| # Bun lists them but never materializes them (os/cpu mismatch), so they just | |
| # accumulate and clutter the global package.json across runs. | |
| if [ -f "$GLOBAL_PKG" ]; then | |
| STALE_OPTIONAL=$(jq -r '.optionalDependencies // {} | keys[]?' "$GLOBAL_PKG" 2>/dev/null || true) | |
| if [ -n "$STALE_OPTIONAL" ]; then | |
| while IFS= read -r dep; do | |
| [ -z "$dep" ] && continue | |
| if ! jq -e --arg dep "$dep" '.dependencies | has($dep)' "$GLOBAL_PKG" >/dev/null 2>&1; then | |
| jq --arg dep "$dep" 'del(.optionalDependencies[$dep])' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | |
| fi | |
| done <<<"$STALE_OPTIONAL" | |
| fi | |
| # Drop the optionalDependencies key entirely if now empty. | |
| if [ "$(jq -r '.optionalDependencies // {} | length' "$GLOBAL_PKG" 2>/dev/null)" = "0" ]; then | |
| jq 'del(.optionalDependencies)' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | |
| fi | |
| fi | |
| # Strip non-matching platform-variant entries from global optionalDependencies. | |
| # Bun lists them but never materializes them (os/cpu mismatch), so they just | |
| # accumulate and clutter the global package.json across runs. | |
| if [ -f "$GLOBAL_PKG" ]; then | |
| jq 'if .optionalDependencies then as $root | .optionalDependencies |= with_entries(select(.key as $k | ($root.dependencies // {}) | has($k))) | if (.optionalDependencies | length) == 0 then del(.optionalDependencies) else . end else . end' "$GLOBAL_PKG" > "${GLOBAL_PKG}.tmp" && | |
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | |
| fi |
There was a problem hiding this comment.
Performed full review of bd7cc99...cbc6041
Analysis
• Platform detection fails slowly: Invalid platform strings from uname fallback logic won't error until installation attempts fail cryptically. Implement fail-fast validation with explicit supported platform enumeration and immediate errors for unknown platforms.
• Cleanup logic lacks scope boundaries: The stale dependency removal targets ANY optional dependency not in dependencies, creating blast radius risk for unrelated packages in global node_modules. Restrict cleanup to known package name patterns with allowlist validation.
• Hardcoded platform variants create maintenance burden: 14+ platform variants in package.json require manual lockstep updates whenever upstream packages add/remove/rename variants. Implement CI validation comparing declared variants against npm registry metadata or generate the list programmatically.
• Missing error handling leaves inconsistent state: jq file operations, bun install, and cleanup lack comprehensive error handling, risking partial failures that leave package.json or node_modules in broken states requiring manual intervention. Add defensive error handling and make cleanup best-effort (non-fatal).
• Windows/cross-platform execution unclear: Script is Unix-centric (bash, uname) but includes win32-* variants suggesting cross-platform intent. Execution on Windows requires WSL/Git Bash/MSYS2, and no locking mechanism prevents concurrent runs from interfering with jq manipulations.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
0 files reviewed | 4 comments | Edit Agent Settings • Read Docs
| : | ||
| elif [ "$PLATFORM_OS" = "linux" ] && [[ "$dep" == *"-${PLATFORM_MUSL_SUFFIX}" ]]; then | ||
| # Only install musl variant on actual musl systems (NixOS uses glibc by default). | ||
| if ! ldd --version 2>&1 | grep -qi musl; then |
There was a problem hiding this comment.
The musl detection using ldd --version | grep -qi musl may fail on systems where ldd is not available or behaves differently. Consider using a more robust check like getconf GNU_LIBC_VERSION 2>/dev/null (which fails on musl) or checking for /lib/ld-musl-*.so.1 existence. This could cause the wrong binary variant to be installed on musl-based systems.
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#1890
File: home-manager/modules/npm-globals/install-npm-globals.sh#L210
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
The musl detection using `ldd --version | grep -qi musl` may fail on systems where ldd is not available or behaves differently. Consider using a more robust check like `getconf GNU_LIBC_VERSION 2>/dev/null` (which fails on musl) or checking for `/lib/ld-musl-*.so.1` existence. This could cause the wrong binary variant to be installed on musl-based systems.
| case "$(uname -s)" in | ||
| Darwin) PLATFORM_OS="darwin" ;; | ||
| Linux) PLATFORM_OS="linux" ;; | ||
| *) PLATFORM_OS="$(uname -s | tr '[:upper:]' '[:lower:]')" ;; |
There was a problem hiding this comment.
The fallback uname -s | tr '[:upper:]' '[:lower:]' for unsupported operating systems will silently produce package names that don't exist (e.g., 'freebsd-x64', 'openbsd-arm64'). This should fail fast with a clear error message instead of continuing with an invalid platform string that will cause cryptic installation failures later.
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#1890
File: home-manager/modules/npm-globals/install-npm-globals.sh#L191
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
The fallback `uname -s | tr '[:upper:]' '[:lower:]'` for unsupported operating systems will silently produce package names that don't exist (e.g., 'freebsd-x64', 'openbsd-arm64'). This should fail fast with a clear error message instead of continuing with an invalid platform string that will cause cryptic installation failures later.
| if [ -n "$STALE_OPTIONAL" ]; then | ||
| while IFS= read -r dep; do | ||
| [ -z "$dep" ] && continue | ||
| if ! jq -e --arg dep "$dep" '.dependencies | has($dep)' "$GLOBAL_PKG" >/dev/null 2>&1; then |
There was a problem hiding this comment.
The cleanup logic iterates through ALL optionalDependencies and removes any that aren't in .dependencies. This is unsafe if unrelated optional dependencies (not platform variants) are added to the global package.json by other tools or packages. Consider adding an allowlist check that only cleans up packages matching known patterns like @anthropic-ai/claude-code-* or @openai/codex-*.
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#1890
File: home-manager/modules/npm-globals/install-npm-globals.sh#L248
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
The cleanup logic iterates through ALL optionalDependencies and removes any that aren't in `.dependencies`. This is unsafe if unrelated optional dependencies (not platform variants) are added to the global package.json by other tools or packages. Consider adding an allowlist check that only cleans up packages matching known patterns like `@anthropic-ai/claude-code-*` or `@openai/codex-*`.
| arm64 | aarch64) PLATFORM_ARCH="arm64" ;; | ||
| x86_64) PLATFORM_ARCH="x64" ;; | ||
| *) PLATFORM_ARCH="$(uname -m)" ;; | ||
| esac |
There was a problem hiding this comment.
The architecture fallback uname -m for unrecognized architectures has the same issue as the OS detection - it will silently produce invalid package names. For example, 'i686' (32-bit x86) would create packages like 'claude-code-linux-i686' that don't exist. Add explicit validation or fail-fast behavior.
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#1890
File: home-manager/modules/npm-globals/install-npm-globals.sh#L198
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
The architecture fallback `uname -m` for unrecognized architectures has the same issue as the OS detection - it will silently produce invalid package names. For example, 'i686' (32-bit x86) would create packages like 'claude-code-linux-i686' that don't exist. Add explicit validation or fail-fast behavior.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@home-manager/modules/npm-globals/install-npm-globals.sh`:
- Around line 256-257: The mv call that replaces "${GLOBAL_PKG}" with the
temporary file should be made non-interactive; update the mv invocation used
after jq (the mv "${GLOBAL_PKG}.tmp" "${GLOBAL_PKG}" statement) to use the force
flag so it won't prompt (i.e., change it to use mv -f) to comply with the
non-interactive file-ops guideline; ensure only that mv call is modified and the
tmp->final move still happens after the jq command succeeds.
- Around line 249-250: The mv invocation that replaces "${GLOBAL_PKG}" should be
non-interactive; change the final command in the jq pipeline from mv
"${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" to mv -f "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" so
the move will not prompt; update the line handling the jq output (referencing
GLOBAL_PKG and the temporary file "${GLOBAL_PKG}.tmp") accordingly.
- Around line 224-226: The mv invocation that replaces the temp file with the
package file should be non-interactive; change the mv command that moves
"${GLOBAL_PKG}.tmp" to "$GLOBAL_PKG" to use the -f flag (i.e., mv -f
"${GLOBAL_PKG}.tmp" "$GLOBAL_PKG") so it won’t prompt if the destination exists;
update any identical mv usages handling GLOBAL_PKG temp replacement accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba9046da-fe4f-41ba-992b-43a45a4aa53a
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
home-manager/modules/npm-globals/install-npm-globals.shpackage.json
| jq --arg dep "$dep" 'del(.optionalDependencies[$dep])' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | ||
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | ||
| fi |
There was a problem hiding this comment.
Add -f flag to mv for non-interactive operation.
The mv command should use -f to avoid prompting when the destination file already exists (e.g., from a prior interrupted run).
As per coding guidelines: "Always use non-interactive flags with file operations (cp -f, mv -f, rm -f, rm -rf, cp -rf) to avoid hanging on confirmation prompts".
Proposed fix
if [ -f "$GLOBAL_PKG" ] && jq -e --arg dep "$dep" '.optionalDependencies | has($dep)' "$GLOBAL_PKG" >/dev/null 2>&1; then
jq --arg dep "$dep" 'del(.optionalDependencies[$dep])' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" &&
- mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG"
+ mv -f "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG"
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jq --arg dep "$dep" 'del(.optionalDependencies[$dep])' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | |
| fi | |
| jq --arg dep "$dep" 'del(.optionalDependencies[$dep])' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv -f "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@home-manager/modules/npm-globals/install-npm-globals.sh` around lines 224 -
226, The mv invocation that replaces the temp file with the package file should
be non-interactive; change the mv command that moves "${GLOBAL_PKG}.tmp" to
"$GLOBAL_PKG" to use the -f flag (i.e., mv -f "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG")
so it won’t prompt if the destination exists; update any identical mv usages
handling GLOBAL_PKG temp replacement accordingly.
| jq --arg dep "$dep" 'del(.optionalDependencies[$dep])' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | ||
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" |
There was a problem hiding this comment.
Add -f flag to mv for non-interactive operation.
Same issue as above—use mv -f to avoid potential prompts.
As per coding guidelines: "Always use non-interactive flags with file operations (cp -f, mv -f, rm -f, rm -rf, cp -rf) to avoid hanging on confirmation prompts".
Proposed fix
jq --arg dep "$dep" 'del(.optionalDependencies[$dep])' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" &&
- mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG"
+ mv -f "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jq --arg dep "$dep" 'del(.optionalDependencies[$dep])' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | |
| jq --arg dep "$dep" 'del(.optionalDependencies[$dep])' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv -f "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@home-manager/modules/npm-globals/install-npm-globals.sh` around lines 249 -
250, The mv invocation that replaces "${GLOBAL_PKG}" should be non-interactive;
change the final command in the jq pipeline from mv "${GLOBAL_PKG}.tmp"
"$GLOBAL_PKG" to mv -f "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" so the move will not
prompt; update the line handling the jq output (referencing GLOBAL_PKG and the
temporary file "${GLOBAL_PKG}.tmp") accordingly.
| jq 'del(.optionalDependencies)' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | ||
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" |
There was a problem hiding this comment.
Add -f flag to mv for non-interactive operation.
Same issue—use mv -f here as well.
As per coding guidelines: "Always use non-interactive flags with file operations (cp -f, mv -f, rm -f, rm -rf, cp -rf) to avoid hanging on confirmation prompts".
Proposed fix
jq 'del(.optionalDependencies)' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" &&
- mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG"
+ mv -f "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jq 'del(.optionalDependencies)' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | |
| jq 'del(.optionalDependencies)' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv -f "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@home-manager/modules/npm-globals/install-npm-globals.sh` around lines 256 -
257, The mv call that replaces "${GLOBAL_PKG}" with the temporary file should be
made non-interactive; update the mv invocation used after jq (the mv
"${GLOBAL_PKG}.tmp" "${GLOBAL_PKG}" statement) to use the force flag so it won't
prompt (i.e., change it to use mv -f) to comply with the non-interactive
file-ops guideline; ensure only that mv call is modified and the tmp->final move
still happens after the jq command succeeds.
| "@anthropic-ai/claude-code-linux-x64-musl": "^2.1.162", | ||
| "@anthropic-ai/claude-code-win32-arm64": "^2.1.162", | ||
| "@anthropic-ai/claude-code-win32-x64": "^2.1.162", | ||
| }, |
There was a problem hiding this comment.
Lock out of sync with package.json: package.json declares 14 entries under optionalDependencies (8 claude-code + 6 codex), but this workspace block only has the 8 claude-code variants. The 6 @openai/codex-* workspace entries are missing.
Running bun install --frozen-lockfile (CI, Nix builds, fresh clones) will fail with a lockfile-mismatch error; an unfrozen bun install will silently rewrite this block — that diff should be part of this PR. Regenerate with bun install and commit the updated lock.
| if [[ "$val" == npm:* ]]; then | ||
| spec="${dep}@${val}" | ||
| else | ||
| spec="$dep" |
There was a problem hiding this comment.
Platform binary install ignores the wrapper version: for non-npm: entries (the 8 claude-code variants), spec="$dep" installs the latest published binary, ignoring both the ^2.1.162 constraint in optionalDependencies and the actual installed wrapper version.
The wrapper itself only re-installs when installed_ver < wanted_ver (lines 131-137), so the wrapper can sit at 2.1.161 while this loop bumps the platform binary to e.g. 2.1.170. Once Anthropic ships a binary whose interface diverges from older wrappers, claude will break again — the same failure class this PR is trying to fix.
Suggestion: derive the wrapper name (strip the -${PLATFORM_SUFFIX} suffix), read its installed_ver from ${GLOBAL_MODULES}/<wrapper>/package.json, and pass spec="${dep}@${installed_ver}" so the binary is pinned to the wrapper it is paired with.
| "@openai/codex-linux-arm64": "npm:@openai/codex@0.136.0-linux-arm64", | ||
| "@openai/codex-linux-x64": "npm:@openai/codex@0.136.0-linux-x64", | ||
| "@openai/codex-win32-arm64": "npm:@openai/codex@0.136.0-win32-arm64", | ||
| "@openai/codex-win32-x64": "npm:@openai/codex@0.136.0-win32-x64" |
There was a problem hiding this comment.
Codex platform pins will drift behind the wrapper: each @openai/codex-* entry hardcodes 0.136.0, but @openai/codex in dependencies (line 34) is ^0.136.0 — it will auto-upgrade on the next install.
When the wrapper moves to e.g. 0.137.0, the install script will pull @openai/codex@0.137.0 but @openai/codex-darwin-arm64@0.136.0-darwin-arm64, reproducing the exact mismatch this PR is fixing.
Either: (a) bake the version into a single shell variable in install-npm-globals.sh derived from the installed wrapper, or (b) add a CI check / Renovate rule that bumps the six platform lines whenever the wrapper version changes, or (c) at minimum, add a comment near the wrapper in dependencies reminding maintainers to bump these six lines together.
| : | ||
| elif [ "$PLATFORM_OS" = "linux" ] && [[ "$dep" == *"-${PLATFORM_MUSL_SUFFIX}" ]]; then | ||
| # Only install musl variant on actual musl systems (NixOS uses glibc by default). | ||
| if ! ldd --version 2>&1 | grep -qi musl; then |
There was a problem hiding this comment.
ldd failure silently looks like glibc: ldd --version 2>&1 | grep -qi musl returns non-zero both when libc is glibc and when ldd is missing or returns an error. On a musl-only Linux box without ldd, the musl variant is skipped and no glibc variant matches either — the wrapper ends up with no platform binary installed.
This is unlikely on NixOS / Alpine (both ship ldd), so it's a latent issue rather than an immediate bug. A safer pattern is to capture the output once and case on musl vs GNU|GLIBC so unknown cases can fall back to the non-musl variant rather than installing nothing.
| if [ -n "$STALE_OPTIONAL" ]; then | ||
| while IFS= read -r dep; do | ||
| [ -z "$dep" ] && continue | ||
| if ! jq -e --arg dep "$dep" '.dependencies | has($dep)' "$GLOBAL_PKG" >/dev/null 2>&1; then |
There was a problem hiding this comment.
Nit: the comment on line 240-242 reads as 'strip non-matching variants' but the actual condition (if !.dependencies | has($dep)) leaves any matching variant alone if it ever appears in both blocks. That's the right behavior, just worth tightening the comment to 'remove optionalDependencies entries not present in dependencies' so future readers don't expect a stricter cleanup.
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="home-manager/modules/npm-globals/install-npm-globals.sh">
<violation number="1" location="home-manager/modules/npm-globals/install-npm-globals.sh:208">
P2: The musl detection via `ldd --version 2>&1 | grep -qi musl` is fragile: on musl systems `ldd --version` exits with code 1 (the grep still works on stderr output here, but the approach fails entirely when `ldd` is missing). A more robust approach is to check for `/lib/ld-musl-*.so.1` existence or use `ldd /bin/sh 2>&1 | grep -qi musl` which has a reliable exit code on both glibc and musl systems.</violation>
<violation number="2" location="home-manager/modules/npm-globals/install-npm-globals.sh:230">
P2: For non-`npm:` entries (claude-code platform variants), `spec="$dep"` installs the latest published binary without regard to the wrapper's installed version. If the wrapper is at 2.1.162 but a newer binary (e.g. 2.1.170) is published, the binary could diverge from the wrapper — the same version mismatch class this PR aims to fix. Consider deriving the installed wrapper version and passing `spec="${dep}@${installed_ver}"`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| val="${entry#*=}" | ||
| # For npm aliases (codex pattern), pass the full <name>@<spec>. For plain | ||
| # semver ranges, omitting the version lets bun resolve latest. | ||
| if [[ $val == npm:* ]]; then |
There was a problem hiding this comment.
P2: For non-npm: entries (claude-code platform variants), spec="$dep" installs the latest published binary without regard to the wrapper's installed version. If the wrapper is at 2.1.162 but a newer binary (e.g. 2.1.170) is published, the binary could diverge from the wrapper — the same version mismatch class this PR aims to fix. Consider deriving the installed wrapper version and passing spec="${dep}@${installed_ver}".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/modules/npm-globals/install-npm-globals.sh, line 230:
<comment>For non-`npm:` entries (claude-code platform variants), `spec="$dep"` installs the latest published binary without regard to the wrapper's installed version. If the wrapper is at 2.1.162 but a newer binary (e.g. 2.1.170) is published, the binary could diverge from the wrapper — the same version mismatch class this PR aims to fix. Consider deriving the installed wrapper version and passing `spec="${dep}@${installed_ver}"`.</comment>
<file context>
@@ -227,7 +227,7 @@ if [ -n "$OPTIONAL_DEPS" ]; then
# For npm aliases (codex pattern), pass the full <name>@<spec>. For plain
# semver ranges, omitting the version lets bun resolve latest.
- if [[ "$val" == npm:* ]]; then
+ if [[ $val == npm:* ]]; then
spec="${dep}@${val}"
else
</file context>
| # Match exact platform variant. Skip musl on darwin/win32. | ||
| if [[ $dep == *"-${PLATFORM_SUFFIX}" ]]; then | ||
| : | ||
| elif [ "$PLATFORM_OS" = "linux" ] && [[ $dep == *"-${PLATFORM_MUSL_SUFFIX}" ]]; then |
There was a problem hiding this comment.
P2: The musl detection via ldd --version 2>&1 | grep -qi musl is fragile: on musl systems ldd --version exits with code 1 (the grep still works on stderr output here, but the approach fails entirely when ldd is missing). A more robust approach is to check for /lib/ld-musl-*.so.1 existence or use ldd /bin/sh 2>&1 | grep -qi musl which has a reliable exit code on both glibc and musl systems.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/modules/npm-globals/install-npm-globals.sh, line 208:
<comment>The musl detection via `ldd --version 2>&1 | grep -qi musl` is fragile: on musl systems `ldd --version` exits with code 1 (the grep still works on stderr output here, but the approach fails entirely when `ldd` is missing). A more robust approach is to check for `/lib/ld-musl-*.so.1` existence or use `ldd /bin/sh 2>&1 | grep -qi musl` which has a reliable exit code on both glibc and musl systems.</comment>
<file context>
@@ -203,9 +203,9 @@ if [ -n "$OPTIONAL_DEPS" ]; then
+ if [[ $dep == *"-${PLATFORM_SUFFIX}" ]]; then
:
- elif [ "$PLATFORM_OS" = "linux" ] && [[ "$dep" == *"-${PLATFORM_MUSL_SUFFIX}" ]]; then
+ elif [ "$PLATFORM_OS" = "linux" ] && [[ $dep == *"-${PLATFORM_MUSL_SUFFIX}" ]]; then
# Only install musl variant on actual musl systems (NixOS uses glibc by default).
if ! ldd --version 2>&1 | grep -qi musl; then
</file context>
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="home-manager/modules/npm-globals/install-npm-globals.sh">
<violation number="1" location="home-manager/modules/npm-globals/install-npm-globals.sh:216">
P2: Platform-native optional package updates are skipped when an older version is already present.</violation>
<violation number="2" location="home-manager/modules/npm-globals/install-npm-globals.sh:233">
P1: Non-alias optionalDependencies are installed without their declared version, reintroducing version drift.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if [[ $val == npm:* ]]; then | ||
| spec="${dep}@${val}" | ||
| else | ||
| spec="$dep" |
There was a problem hiding this comment.
P1: Non-alias optionalDependencies are installed without their declared version, reintroducing version drift.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/modules/npm-globals/install-npm-globals.sh, line 233:
<comment>Non-alias optionalDependencies are installed without their declared version, reintroducing version drift.</comment>
<file context>
@@ -177,6 +177,87 @@ if [ -n "$OVERRIDES" ]; then
+ if [[ $val == npm:* ]]; then
+ spec="${dep}@${val}"
+ else
+ spec="$dep"
+ fi
+ echo "Installing platform-native: $spec"
</file context>
| spec="$dep" | |
| spec="${dep}@${val}" |
| else | ||
| continue | ||
| fi | ||
| if [ -d "${GLOBAL_MODULES}/${dep}" ]; then |
There was a problem hiding this comment.
P2: Platform-native optional package updates are skipped when an older version is already present.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/modules/npm-globals/install-npm-globals.sh, line 216:
<comment>Platform-native optional package updates are skipped when an older version is already present.</comment>
<file context>
@@ -177,6 +177,87 @@ if [ -n "$OVERRIDES" ]; then
+ else
+ continue
+ fi
+ if [ -d "${GLOBAL_MODULES}/${dep}" ]; then
+ echo "$dep already installed, skipping"
+ continue
</file context>
| else | ||
| continue | ||
| fi | ||
| if [ -d "${GLOBAL_MODULES}/${dep}" ]; then |
There was a problem hiding this comment.
Skip-if-dir-exists permanently freezes the platform binary version: this check only tests whether ${GLOBAL_MODULES}/${dep} exists — there is no version comparison. Once any version of the platform binary is installed, every future run prints already installed, skipping and the binary never updates, even when the wrapper bumps. Combined with spec="$dep" at line 233 (latest on first install) and the fact that wrappers DO update via the version check at lines 131-137, this means binary and wrapper will diverge silently — reintroducing the exact mismatch class this PR is fixing.
Mirror the wrapper update path at lines 124-144: read installed_ver from ${GLOBAL_MODULES}/${dep}/package.json, compare to the wanted version (derived from the wrapper's installed_ver for plain semver, or parsed from the npm: alias for codex), and only skip when installed_ver >= wanted_ver.
Summary
claude --versionwas stuck at 2.1.140 andcodex --versionat 0.130.0 even afterbun add -greported newer versions. Root cause: both packages ship native binaries viaoptionalDependencies, and bun's transitive optional resolution silently drops them when the parent'spostinstallis blocked byignoreScripts = truein~/.bunfig.toml.Changes
package.json: add all platform variants underoptionalDependenciesas a cross-platform manifest. Claude-code uses real platform packages (@anthropic-ai/claude-code-darwin-arm64etc.); codex uses npm aliases (@openai/codex-darwin-arm64→npm:@openai/codex@0.136.0-darwin-arm64).install-npm-globals.sh: detect current OS/arch, filter to the matching platform variant, and install it as a direct global dep. Strips from globaloptionalDependenciesfirst (bun bug:bun add -g <pkg>no-ops when<pkg>is already in optionalDeps). Cleans up stale non-matching variants across runs. Handles both plain semver and npm-alias specs.Why not transitive optionalDeps
Bun's
optional = trueshould fetch matching variants automatically, but empirically doesn't in our global install when the parent haspostinstalland global bunfig hasignoreScripts = true. The new variants would also be blocked byminimumReleaseAge = 604800because claude-code releases multiple times per day — the curated install passes--minimum-release-age 0to bypass the age gate for these vetted packages.Test plan
bash home-manager/modules/npm-globals/install-npm-globals.shon darwin-arm64claude --versionreturns 2.1.162codex --versionreturns 0.136.0package.jsonhas matching variants in.dependencies, no stale entries in.optionalDependencieslinux-x64/linux-arm64selection worksCloses the version-drift issue observed locally.
Summary by cubic
Fixes global installs of platform-native binaries for
@anthropic-ai/claude-codeand@openai/codexby directly installing the correct OS/arch variant. Prevents version drift when Bun skips transitive optional deps withignoreScripts = true; also formats the installer script withshfmt.Bug Fixes
optionalDependenciesas a cross-platform manifest.install-npm-globals.shto detect OS/arch and install the matching variant withbun add -g, removing it from globaloptionalDependenciesfirst.package.json.npm:alias specs, and pass--minimum-release-age 0for these vetted packages.shfmtfor consistency.Migration
home-manager/modules/npm-globals/install-npm-globals.shon each host to install the correct binary.Written for commit c816b9b. Summary will update on new commits.