fix(codex): install real native binary for aliased optional deps - #1982
Conversation
The @openai/codex-* optional pins used npm:@openai/codex@<ver> without the platform-triple version suffix, so bun installed the generic JS wrapper under the platform dir name instead of the tarball carrying vendor/.../bin/codex. The CLI then died with 'Missing optional dependency @openai/codex-darwin-arm64'. - package.json: pin the suffixed native packages (0.142.5-<triple>). - install-npm-globals.sh: reconstruct the -<triple> suffix and derive the version from the actually-installed parent so pin drift can't desync the binary from the wrapper; reject phantom/wrong-version dirs instead of trusting bare -d; warn if the payload fails to materialize. - spec: cover suffix reconstruction, parent-version derivation, phantom-dir reinstall, and correct-version skip.
|
|
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. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe install script now reconstructs npm: alias specs by deriving base package version and platform suffix, and validates actual installed package.json payloads instead of relying on directory existence before deciding to reinstall. Codex optionalDependencies versions are bumped, and new spec tests cover the behavior. ChangesAliased Native Binary Install Self-Healing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Script as install-npm-globals.sh
participant FS as GLOBAL_MODULES/package.json
participant Dotfiles as dotfiles package.json
participant Bun as bun add --global
Script->>FS: Read installed base package version
Script->>Script: Reconstruct spec and want_ver for npm: alias
Script->>FS: Check installed dep package.json version
alt version matches want_ver
Script->>Script: Skip install
else phantom or incorrect version
Script->>FS: Remove stale package directory
Script->>Dotfiles: Strip dep from optionalDependencies
Script->>Bun: Install dep with --minimum-release-age 0
Bun-->>FS: Materialize package.json
Script->>FS: Verify payload exists
Script->>Script: Warn if payload still missing
end
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request enhances the global npm installation script to robustly handle aliased native binary dependencies (such as @openai/codex) by reconstructing platform-specific suffixes, resolving versions from installed parent packages, and ensuring phantom directories are reinstalled. It also updates package.json with explicit platform-suffixed versions and adds comprehensive integration tests. Feedback was provided to address edge cases in the shell script's alias parsing and version reconstruction to prevent potential syntax errors with scoped packages.
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.
| if [[ $val == npm:* ]]; then | ||
| # Aliased native binary (codex pattern): npm:@openai/codex@<ver>. | ||
| # These packages are PUBLISHED as <base>@<ver>-<platform-suffix> (e.g. | ||
| # @openai/codex@0.142.5-darwin-arm64) and only that suffixed tarball | ||
| # carries the vendor binary. A bare <base>@<ver> pin (no suffix) silently | ||
| # reinstalls the generic JS wrapper under the platform dir name -- no | ||
| # binary -- and the CLI dies with "Missing optional dependency". So we | ||
| # reconstruct the suffixed spec here regardless of how package.json pins it. | ||
| alias_spec="${val#npm:}" # @openai/codex@0.142.2 | ||
| base_name="${alias_spec%@*}" # @openai/codex | ||
| base_ver="${alias_spec##*@}" # 0.142.2 (fallback if parent not installed) | ||
| # Prefer the ACTUALLY INSTALLED parent version so the binary always matches | ||
| # the wrapper even when the package.json pins have drifted behind it. | ||
| base_pj="${GLOBAL_MODULES}/${base_name}/package.json" | ||
| if [ -f "$base_pj" ]; then | ||
| installed_base=$(jq -r '.version // empty' "$base_pj" 2>/dev/null || true) | ||
| [ -n "$installed_base" ] && base_ver="$installed_base" | ||
| fi | ||
| suffix="${dep#"${base_name}"-}" # darwin-arm64 | ||
| if [ -n "$suffix" ] && [ "$suffix" != "$dep" ]; then | ||
| want_ver="${base_ver}-${suffix}" | ||
| else | ||
| want_ver="$base_ver" | ||
| fi | ||
| spec="${dep}@npm:${base_name}@${want_ver}" | ||
| fi |
There was a problem hiding this comment.
There are a few potential edge cases in the parsing of the alias spec and version reconstruction:
- Robust Alias Parsing: If
alias_specdoes not contain a version (e.g.,npm:@openai/codex),base_namewill resolve to an empty string andbase_verwill resolve to the package name itself. This is because${alias_spec%@*}strips the entire string when there is only one@(at the start of a scoped package). - Invalid Version Suffixing: If
base_veris empty, appending the suffix results in-${suffix}(e.g.,-darwin-arm64), which is an invalid version. - Trailing
@in Spec: Ifwant_veris empty, constructing the spec as${dep}@npm:${base_name}@${want_ver}results in a trailing@(e.g.,pkg@npm:base@), which is invalid npm/bun syntax.
We can make this more robust by checking if the stripped alias spec contains an @ before splitting, and only appending the suffix/version if they are non-empty.
if [[ $val == npm:* ]]; then
# Aliased native binary (codex pattern): npm:@openai/codex@<ver>.
# These packages are PUBLISHED as <base>@<ver>-<platform-suffix> (e.g.
# @openai/codex@0.142.5-darwin-arm64) and only that suffixed tarball
# carries the vendor binary. A bare <base>@<ver> pin (no suffix) silently
# reinstalls the generic JS wrapper under the platform dir name -- no
# binary -- and the CLI dies with "Missing optional dependency". So we
# reconstruct the suffixed spec here regardless of how package.json pins it.
alias_spec="${val#npm:}"
tmp="${alias_spec#@}"
if [[ $tmp == *@* ]]; then
base_name="${alias_spec%@*}"
base_ver="${tmp#*@}"
else
base_name="$alias_spec"
base_ver=""
fi
# Prefer the ACTUALLY INSTALLED parent version so the binary always matches
# the wrapper even when the package.json pins have drifted behind it.
base_pj="${GLOBAL_MODULES}/${base_name}/package.json"
if [ -f "$base_pj" ]; then
installed_base=$(jq -r '.version // empty' "$base_pj" 2>/dev/null || true)
[ -n "$installed_base" ] && base_ver="$installed_base"
fi
suffix="${dep#"${base_name}"-}" # darwin-arm64
if [ -n "$base_ver" ] && [ -n "$suffix" ] && [ "$suffix" != "$dep" ]; then
want_ver="${base_ver}-${suffix}"
else
want_ver="$base_ver"
fi
if [ -n "$want_ver" ]; then
spec="${dep}@npm:${base_name}@${want_ver}"
else
spec="${dep}@npm:${base_name}"
fi
fiReferences
- When 'set -e' is enabled in Bash scripts, ensure that individual command failures within a loop do not prematurely abort the entire script. Handle potential failures gracefully, for example by appending '|| true'.
| # reconstruct the suffixed spec here regardless of how package.json pins it. | ||
| alias_spec="${val#npm:}" # @openai/codex@0.142.2 | ||
| base_name="${alias_spec%@*}" # @openai/codex | ||
| base_ver="${alias_spec##*@}" # 0.142.2 (fallback if parent not installed) |
There was a problem hiding this comment.
Latent double-suffix when parent wrapper isn't installed.
With the new suffixed pins in package.json (npm:@openai/codex@0.142.5-darwin-arm64), the alias-only fallback path here produces the wrong want_ver.
Walking it through:
alias_spec=@openai/codex@0.142.5-darwin-arm64
base_name=${alias_spec%@*} # @openai/codex
base_ver=${alias_spec##*@} # 0.142.5-darwin-arm64 <-- already has the suffix
The installed-parent override on lines 284–288 is what normally rescues this — it replaces base_ver with the clean parent version from ${GLOBAL_MODULES}/${base_name}/package.json. But when that file is missing, base_ver stays 0.142.5-darwin-arm64 and the block below computes:
suffix=darwin-arm64
want_ver=0.142.5-darwin-arm64-darwin-arm64
spec=@openai/codex-darwin-arm64@npm:@openai/codex@0.142.5-darwin-arm64-darwin-arm64
bun add on that spec fails, only the Warning: <dep> still missing after install line fires, and the Codex CLI is back to Missing optional dependency @openai/codex-darwin-arm64.
Triggers this can happen under:
- The parent
@openai/codexinstall in the MISSING loop (lines 197–201) failed transiently — the|| echo "Install failed: $dep"swallows the error and the run continues to the optional loop with no parent on disk. - Someone drops
@openai/codexfromdependencieswhile keeping thesenpm:@openai/codex@<ver>-<triple>optional pins (the state this PR normalizes toward).
Strip the platform suffix from the fallback before appending it, e.g. after parsing:
# base_ver from the alias may already carry the -<triple> suffix under the new
# pinning scheme; drop it so the append below doesn't double it up.
base_ver=${base_ver%-"${PLATFORM_SUFFIX}"}
base_ver=${base_ver%-"${PLATFORM_MUSL_SUFFIX}"}or pull the version out with a regex that stops at the first -.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
home-manager/modules/npm-globals/install-npm-globals.sh (1)
320-325: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSwallowed stderr hides the reason for install failures.
2>/dev/nullon thebun addcall discards bun's actual error output, leaving only a generic "Install failed" message. This would also mask diagnosis of issues like the double-suffix defect above (bun's "version not found" error would be invisible).🤖 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 320 - 325, The `bun add` call in `install-npm-globals.sh` is swallowing stderr, which hides the real failure reason. Remove the `2>/dev/null` redirection from the `timeout 600 bun add --global "$spec" --minimum-release-age 0` command so bun’s own error output is preserved, while keeping the existing fallback and post-install verification around the `spec`/`dep` checks.spec/npm_globals_spec.sh (1)
369-390: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGrep-based assertions only check that literal strings exist in the source, not behavior.
These four tests (
want_ver,installed_base,phantom dir,still missing after install) merelygrepthe script for literal substrings rather than exercising behavior; the integration suite below does the real behavioral verification, so this is acceptable as a smoke-test layer. Consider adding one more integration case where the alias pin already includes the platform suffix (matching the newpackage.jsonformat) and the parent'spackage.jsonis absent, to cover the double-suffix fallback path flagged ininstall-npm-globals.sh(lines 271-296).🤖 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 `@spec/npm_globals_spec.sh` around lines 369 - 390, The grep-only checks in the aliased native binary spec are acceptable as smoke tests, but the missing coverage is the double-suffix fallback path in install-npm-globals.sh. Add an integration case that exercises the aliased native binary flow when the alias pin already includes the platform suffix and the parent package.json is absent, and assert the behavior through the existing npm globals install path rather than only literal grep matches.
🤖 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 271-296: The npm alias handling in install-npm-globals.sh is
appending the platform suffix twice when the parent package is missing and
base_ver already contains a suffix. Update the logic in the npm:* branch so the
spec construction around alias_spec, base_ver, suffix, and want_ver first
detects an already suffixed base_ver and avoids adding "-${suffix}" again before
setting spec for bun add.
---
Nitpick comments:
In `@home-manager/modules/npm-globals/install-npm-globals.sh`:
- Around line 320-325: The `bun add` call in `install-npm-globals.sh` is
swallowing stderr, which hides the real failure reason. Remove the `2>/dev/null`
redirection from the `timeout 600 bun add --global "$spec" --minimum-release-age
0` command so bun’s own error output is preserved, while keeping the existing
fallback and post-install verification around the `spec`/`dep` checks.
In `@spec/npm_globals_spec.sh`:
- Around line 369-390: The grep-only checks in the aliased native binary spec
are acceptable as smoke tests, but the missing coverage is the double-suffix
fallback path in install-npm-globals.sh. Add an integration case that exercises
the aliased native binary flow when the alias pin already includes the platform
suffix and the parent package.json is absent, and assert the behavior through
the existing npm globals install path rather than only literal grep matches.
🪄 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: 318bc9f9-d2e7-496c-872f-86e572bf3e02
📒 Files selected for processing (3)
home-manager/modules/npm-globals/install-npm-globals.shpackage.jsonspec/npm_globals_spec.sh
| if [[ $val == npm:* ]]; then | ||
| # Aliased native binary (codex pattern): npm:@openai/codex@<ver>. | ||
| # These packages are PUBLISHED as <base>@<ver>-<platform-suffix> (e.g. | ||
| # @openai/codex@0.142.5-darwin-arm64) and only that suffixed tarball | ||
| # carries the vendor binary. A bare <base>@<ver> pin (no suffix) silently | ||
| # reinstalls the generic JS wrapper under the platform dir name -- no | ||
| # binary -- and the CLI dies with "Missing optional dependency". So we | ||
| # reconstruct the suffixed spec here regardless of how package.json pins it. | ||
| alias_spec="${val#npm:}" # @openai/codex@0.142.2 | ||
| base_name="${alias_spec%@*}" # @openai/codex | ||
| base_ver="${alias_spec##*@}" # 0.142.2 (fallback if parent not installed) | ||
| # Prefer the ACTUALLY INSTALLED parent version so the binary always matches | ||
| # the wrapper even when the package.json pins have drifted behind it. | ||
| base_pj="${GLOBAL_MODULES}/${base_name}/package.json" | ||
| if [ -f "$base_pj" ]; then | ||
| installed_base=$(jq -r '.version // empty' "$base_pj" 2>/dev/null || true) | ||
| [ -n "$installed_base" ] && base_ver="$installed_base" | ||
| fi | ||
| suffix="${dep#"${base_name}"-}" # darwin-arm64 | ||
| if [ -n "$suffix" ] && [ "$suffix" != "$dep" ]; then | ||
| want_ver="${base_ver}-${suffix}" | ||
| else | ||
| want_ver="$base_ver" | ||
| fi | ||
| spec="${dep}@npm:${base_name}@${want_ver}" | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm bash parameter-expansion behavior for the suffixed-alias fallback path.
val="npm:`@openai/codex`@0.142.5-darwin-arm64"
dep="`@openai/codex-darwin-arm64`"
alias_spec="${val#npm:}"
base_name="${alias_spec%@*}"
base_ver="${alias_spec##*@}"
suffix="${dep#"${base_name}"-}"
if [ -n "$suffix" ] && [ "$suffix" != "$dep" ]; then
want_ver="${base_ver}-${suffix}"
else
want_ver="$base_ver"
fi
echo "base_name=$base_name base_ver=$base_ver suffix=$suffix want_ver=$want_ver"Repository: shunkakinoki/dotfiles
Length of output: 276
Avoid appending the platform suffix twice. If the alias pin already includes -darwin-arm64 and the parent package isn’t installed yet, base_ver stays suffixed and this code turns it into 0.142.5-darwin-arm64-darwin-arm64, which bun add can’t resolve. Guard against an already-suffixed base_ver before adding -${suffix}.
🤖 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 271 -
296, The npm alias handling in install-npm-globals.sh is appending the platform
suffix twice when the parent package is missing and base_ver already contains a
suffix. Update the logic in the npm:* branch so the spec construction around
alias_spec, base_ver, suffix, and want_ver first detects an already suffixed
base_ver and avoids adding "-${suffix}" again before setting spec for bun add.
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:280">
P2: For a scoped package alias without an explicit version (e.g., `npm:@openai/codex` with no trailing `@<ver>`), this parameter expansion splits incorrectly: `${alias_spec%@*}` strips at the scope-level `@`, yielding `@openai` instead of the full package name, and `${alias_spec##*@}` yields `openai/codex` instead of empty. Consider checking whether the portion after the first `@` contains another `@` before splitting, to handle versionless alias specs gracefully.</violation>
<violation number="2" location="home-manager/modules/npm-globals/install-npm-globals.sh:291">
P2: The fallback `base_ver` (derived from the package.json alias spec via `##*@`) now contains the platform suffix because the new pins embed it (e.g., `npm:@openai/codex@0.142.5-darwin-arm64`). When the parent `@openai/codex` package isn't yet installed, this suffixed fallback version gets the platform suffix appended again, producing a double-suffixed version like `0.142.5-darwin-arm64-darwin-arm64`. The install then tries to fetch a non-existent npm version and fails.
This only triggers when the parent package is missing (edge case), since the script correctly overrides `base_ver` from the parent's `.version` field in the common path. Consider stripping the known suffix from `base_ver` before appending it, or extracting a clean base version from the alias spec.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| fi | ||
| suffix="${dep#"${base_name}"-}" # darwin-arm64 | ||
| if [ -n "$suffix" ] && [ "$suffix" != "$dep" ]; then | ||
| want_ver="${base_ver}-${suffix}" |
There was a problem hiding this comment.
P2: The fallback base_ver (derived from the package.json alias spec via ##*@) now contains the platform suffix because the new pins embed it (e.g., npm:@openai/codex@0.142.5-darwin-arm64). When the parent @openai/codex package isn't yet installed, this suffixed fallback version gets the platform suffix appended again, producing a double-suffixed version like 0.142.5-darwin-arm64-darwin-arm64. The install then tries to fetch a non-existent npm version and fails.
This only triggers when the parent package is missing (edge case), since the script correctly overrides base_ver from the parent's .version field in the common path. Consider stripping the known suffix from base_ver before appending it, or extracting a clean base version from the alias spec.
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 291:
<comment>The fallback `base_ver` (derived from the package.json alias spec via `##*@`) now contains the platform suffix because the new pins embed it (e.g., `npm:@openai/codex@0.142.5-darwin-arm64`). When the parent `@openai/codex` package isn't yet installed, this suffixed fallback version gets the platform suffix appended again, producing a double-suffixed version like `0.142.5-darwin-arm64-darwin-arm64`. The install then tries to fetch a non-existent npm version and fails.
This only triggers when the parent package is missing (edge case), since the script correctly overrides `base_ver` from the parent's `.version` field in the common path. Consider stripping the known suffix from `base_ver` before appending it, or extracting a clean base version from the alias spec.</comment>
<file context>
@@ -262,27 +262,67 @@ if [ -n "$OPTIONAL_DEPS" ]; then
+ fi
+ suffix="${dep#"${base_name}"-}" # darwin-arm64
+ if [ -n "$suffix" ] && [ "$suffix" != "$dep" ]; then
+ want_ver="${base_ver}-${suffix}"
+ else
+ want_ver="$base_ver"
</file context>
| # binary -- and the CLI dies with "Missing optional dependency". So we | ||
| # reconstruct the suffixed spec here regardless of how package.json pins it. | ||
| alias_spec="${val#npm:}" # @openai/codex@0.142.2 | ||
| base_name="${alias_spec%@*}" # @openai/codex |
There was a problem hiding this comment.
P2: For a scoped package alias without an explicit version (e.g., npm:@openai/codex with no trailing @<ver>), this parameter expansion splits incorrectly: ${alias_spec%@*} strips at the scope-level @, yielding @openai instead of the full package name, and ${alias_spec##*@} yields openai/codex instead of empty. Consider checking whether the portion after the first @ contains another @ before splitting, to handle versionless alias specs gracefully.
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 280:
<comment>For a scoped package alias without an explicit version (e.g., `npm:@openai/codex` with no trailing `@<ver>`), this parameter expansion splits incorrectly: `${alias_spec%@*}` strips at the scope-level `@`, yielding `@openai` instead of the full package name, and `${alias_spec##*@}` yields `openai/codex` instead of empty. Consider checking whether the portion after the first `@` contains another `@` before splitting, to handle versionless alias specs gracefully.</comment>
<file context>
@@ -262,27 +262,67 @@ if [ -n "$OPTIONAL_DEPS" ]; then
+ # binary -- and the CLI dies with "Missing optional dependency". So we
+ # reconstruct the suffixed spec here regardless of how package.json pins it.
+ alias_spec="${val#npm:}" # @openai/codex@0.142.2
+ base_name="${alias_spec%@*}" # @openai/codex
+ base_ver="${alias_spec##*@}" # 0.142.2 (fallback if parent not installed)
+ # Prefer the ACTUALLY INSTALLED parent version so the binary always matches
</file context>
Problem
Running `codex` failed with:
```
Error: Missing optional dependency @openai/codex-darwin-arm64. Reinstall Codex: bun install -g @openai/codex@latest
```
The wrapper (`codex.js`) resolves its native binary purely by path -
`require.resolve('@openai/codex-darwin-arm64/package.json')` then
`vendor/aarch64-apple-darwin/bin/codex`. "Missing" means that binary file
wasn't on disk.
Root cause: the `@openai/codex-*` optional pins were malformed.
The `-` version suffix was missing, so `install-npm-globals.sh`
faithfully installed the wrapper package under the platform dir name. The
subsequent `-d` presence check then cached the broken dir across every run.
Fix
A caret can't float a suffixed version (semver treats `-darwin-arm64` as a
prerelease tag), so drift protection lives in the script instead.
suffix-less pin installs the real binary package;
can never desync the binary from the wrapper;
version matches - tear down phantom/wrong-version dirs instead of trusting
bare `-d`;
phantom-dir reinstall, and correct-version skip.
Testing
Follow-up (not in this PR)
`codex` is declared in three places - bun global (package.json), the Homebrew
cask (`homebrew.nix`), and the nix overlay (`overlays/default.nix`). The bun
global wins on PATH. Consolidating to a single source would remove the
redundancy but is out of scope here.
Summary by cubic
Fixes Codex CLI failures by installing the real platform-native
@openai/codex-*binary instead of the generic wrapper. Pins suffixed native packages and updates the installer to reconstruct the platform suffix, align with the installed parent version, and self-heal broken installs.Bug Fixes
-<triple>suffix for aliased optional deps.@openai/codexparent.package.jsonexists and the version matches; remove phantom/wrong-version dirs.Dependencies
@openai/codex-*optional deps to0.142.5-<triple>suffixed versions.Written for commit cbc9745. Summary will update on new commits.