Skip to content

fix(npm-globals): install transitive native binaries dropped by bun #2027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shunkakinoki merged 1 commit into from
Jul 8, 2026
Merged

fix(npm-globals): install transitive native binaries dropped by bun #2027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 80 additions & 17 deletions home-manager/modules/npm-globals/install-npm-globals.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,32 +87,83 @@ x86_64 | amd64) PLATFORM_CPU="x64" ;;
*) PLATFORM_CPU="" ;;
esac

# Returns 0 when a package declares a platform-native optionalDependency for the
# current OS/CPU but that dependency is not installed. Many CLIs ship their real
# binary this way; a version-only skip would otherwise leave such a package
# "installed" yet non-functional (e.g. after an `omit=optional` install).
missing_native_optional_dep() {
local dep="$1"
local pj="${GLOBAL_MODULES}/${dep}/package.json"
[ -f "$pj" ] || return 1
[ -n "$PLATFORM_OS" ] && [ -n "$PLATFORM_CPU" ] || return 1

local opt_deps name matched=0 present=0
# Prints the name of a platform-native optionalDependency declared in the given
# package.json that is NOT installed, or nothing if all present / none declared.
# Emits "<native-dep-name> <declaring-version>" so callers can install the exact
# binary that bun dropped, at the version that matches its declaring wrapper.
missing_native_from_pkg() {
local pj="$1"
[ -f "$pj" ] || return 0
local opt_deps name decl_ver
Comment on lines +95 to +97

@gemini-code-assist gemini-code-assist Bot Jul 8, 2026

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.

medium

To prevent unexpected behavior or incorrect pattern matching when PLATFORM_OS or PLATFORM_CPU are empty (e.g., on unsupported platforms), we should add a defensive guard at the beginning of missing_native_from_pkg to return early.

Suggested change
local pj="$1"
[ -f "$pj" ] || return 0
local opt_deps name decl_ver
local pj="$1"
[ -f "$pj" ] || return 0
[ -n "$PLATFORM_OS" ] && [ -n "$PLATFORM_CPU" ] || return 0
local opt_deps name decl_ver

opt_deps=$(jq -r '.optionalDependencies // {} | keys[]' "$pj" 2>/dev/null || true)
[ -n "$opt_deps" ] || return 1

[ -n "$opt_deps" ] || return 0
decl_ver=$(jq -r '.version // empty' "$pj" 2>/dev/null || true)

@indent-zero indent-zero Bot Jul 8, 2026

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.

decl_ver reads the wrapper's own .version, not the pinned native version. The emitted "<name> <decl_ver>" is later concatenated into bun add --global <name>@<decl_ver>. That's only correct under the convention that a wrapper pins its native optionalDeps at its own version.

For wrappers that pin at a different version (e.g. republished natives, or a shared native versioned independently), or that pin at a semver range like ^1.0.0, we'll install a mismatched or nonexistent version. The new spec at spec/npm_globals_spec.sh:381-387 sets wrapper and native both to 1.0.0 so this case is not exercised.

Consider reading the actual pin with jq -r --arg n "$name" '.optionalDependencies[$n] // empty' "$pj" and falling back to .version only when that yields empty or a non-concrete range.

while IFS= read -r name; do
[ -z "$name" ] && continue
# Only weigh native deps targeting this platform.
case "$name" in
*"$PLATFORM_OS"*"$PLATFORM_CPU"* | *"$PLATFORM_CPU"*"$PLATFORM_OS"*) ;;
*) continue ;;
esac
matched=1
[ -d "${GLOBAL_MODULES}/${name}" ] && present=1
[ -d "${GLOBAL_MODULES}/${name}" ] && continue
printf '%s %s\n' "$name" "$decl_ver"
return 0
done <<<"$opt_deps"
}

[ "$matched" -eq 1 ] && [ "$present" -eq 0 ]
# Candidate package.json paths that may declare a package's real native binary:
# the package itself, plus its direct dependencies (thin wrappers hide the binary
# one level down, e.g. tokscale -> @tokscale/cli -> @tokscale/cli-darwin-arm64).
native_candidate_pkgs() {
local dep="$1"
local pj="${GLOBAL_MODULES}/${dep}/package.json"
printf '%s\n' "$pj"
Comment on lines +118 to +120

@gemini-code-assist gemini-code-assist Bot Jul 8, 2026

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.

medium

The variable GLOBAL_MODULES is used in several helper functions (such as native_candidate_pkgs, missing_native_optional_dep, and repair_native_optional_dep), but it is defined globally much later in the script (on line 255) and redundantly redefined on line 468.

This is fragile and can lead to bugs if any of these functions are called earlier in the script. It is highly recommended to define GLOBAL_MODULES at the top of the script (e.g., right after PACKAGE_JSON on line 20) and remove the redundant definitions.

local child
while IFS= read -r child; do
[ -z "$child" ] && continue
printf '%s\n' "${GLOBAL_MODULES}/${child}/package.json"
done < <(jq -r '.dependencies // {} | keys[]' "$pj" 2>/dev/null || true)
}

# Returns 0 when a package (or its immediate wrapper dependency) declares a
# platform-native optionalDependency for the current OS/CPU that is not
# installed. Many CLIs ship their real binary this way; a version-only skip would
# otherwise leave such a package "installed" yet non-functional (bun silently
# drops these transitive optional deps during global installs).
missing_native_optional_dep() {
local dep="$1"
[ -f "${GLOBAL_MODULES}/${dep}/package.json" ] || return 1
[ -n "$PLATFORM_OS" ] && [ -n "$PLATFORM_CPU" ] || return 1

local pj
while IFS= read -r pj; do
[ -n "$(missing_native_from_pkg "$pj")" ] && return 0
done < <(native_candidate_pkgs "$dep")
return 1
}

# Directly install the platform-native binary package that bun dropped for a
# wrapper dep, at the version its declaring package pins, so wrapper and binary
# always match. Returns 0 when the binary is present afterwards. Preferred over
# reinstalling the wrapper, which just re-triggers the same bun drop.
repair_native_optional_dep() {
local dep="$1"
local pj found native decl_ver spec
while IFS= read -r pj; do
found=$(missing_native_from_pkg "$pj")
[ -n "$found" ] && break
done < <(native_candidate_pkgs "$dep")
[ -n "$found" ] || return 1

native="${found%% *}"

@mesa-dot-dev mesa-dot-dev Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

The word-splitting logic here assumes missing_native_from_pkg output format is stable. If decl_ver contains spaces or the native package name contains spaces, this parsing could break. Consider using a more robust delimiter (e.g., tab \t or null byte) or explicitly validate that neither field is empty before splitting to avoid silent failures.

Fix in Cursor • Fix in Claude

Prompt for Agent 
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#2027
File: home-manager/modules/npm-globals/install-npm-globals.sh#L158
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
The word-splitting logic here assumes `missing_native_from_pkg` output format is stable. If `decl_ver` contains spaces or the native package name contains spaces, this parsing could break. Consider using a more robust delimiter (e.g., tab `\t` or null byte) or explicitly validate that neither field is empty before splitting to avoid silent failures.

decl_ver="${found##* }"
spec="$native"
[ -n "$decl_ver" ] && spec="${native}@${decl_ver}"
echo "Installing missing native binary: $spec"
timeout 600 bun add --global "$spec" --minimum-release-age 0 2>/dev/null ||

@mesa-dot-dev mesa-dot-dev Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

The bun add --global command output is redirected to /dev/null, which may hide useful diagnostic information when the install fails. Consider capturing stderr to a variable or logging it for debugging purposes, especially since this is a repair path that may fail due to network issues, version conflicts, or package availability.

Fix in Cursor • Fix in Claude

Prompt for Agent 
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#2027
File: home-manager/modules/npm-globals/install-npm-globals.sh#L163
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
The `bun add --global` command output is redirected to `/dev/null`, which may hide useful diagnostic information when the install fails. Consider capturing stderr to a variable or logging it for debugging purposes, especially since this is a repair path that may fail due to network issues, version conflicts, or package availability.

@indent-zero indent-zero Bot Jul 8, 2026

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.

Missing the documented bun add -g no-op workaround. The aliased-native block at lines 425-431 explicitly strips the target from $GLOBAL_PKG's optionalDependencies before bun add --global, with the comment:

Bun's bun add -g <pkg> is a no-op if is already in the global package.json's optionalDependencies (it never materializes the dir). Strip from optionalDependencies first so the add becomes a real install.

This repair path skips that mitigation. Transitive natives aren't normally promoted into the global optionalDependencies map, so the common case is safe — but if the entry ever ends up there (e.g. a prior aliased-native run for the same package, or a stale entry left from user-declared optionalDependencies), bun add --global <native>@<ver> will silently no-op. The presence check at line 166 will then fail, the caller logs "native binary repair failed, reinstalling wrapper", rm -rfs the wrapper and reinstalls it — which just re-triggers the same bun transitive drop. That's a permanent loop across activations for the exact scenario this function is meant to fix.

Suggest applying the same strip-before-add pattern here, or hoisting it into a shared helper so both call sites stay consistent.

echo "Install failed: $spec" >&2
purge_bun_npm_shim
[ -d "${GLOBAL_MODULES}/${native}" ]

@indent-zero indent-zero Bot Jul 8, 2026

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.

Presence check accepts phantom-empty native dir. This function returns success purely on directory existence, but the rest of the script already recognizes that as insufficient for natives:

  • Line 421-423 explicitly treats a directory without a package.json as a phantom and reinstalls it.
  • Line 435-438 warns "$dep still missing after install ($spec)" when package.json is absent after a bun add.

Bun has been observed to create empty native dirs during transitive drops — exactly the scenario this function is invoked for. When that happens, this returns 0, the caller logs "$dep native binary repaired in place", and the destructive wrapper-reinstall fallback is skipped, leaving the user with a broken CLI and a misleading success message.

Suggest matching the aliased-native check at line 414: verify ${GLOBAL_MODULES}/${native}/package.json exists (and optionally that .version matches $decl_ver).

}

# Remove the npm "bun" wrapper package from global node_modules.
Expand Down Expand Up @@ -221,7 +272,14 @@ if [ -n "$DEPS" ]; then
# Version matches, but only skip if the native binary is actually
# present. Drop a broken install so the reinstall below refetches it.
if missing_native_optional_dep "$dep"; then
echo "$dep@$installed_ver installed but native binary missing, reinstalling"
echo "$dep@$installed_ver installed but native binary missing"
# Install the dropped binary directly; reinstalling the wrapper just
# re-triggers the same bun transitive-optional drop.
if repair_native_optional_dep "$dep"; then
echo "$dep native binary repaired in place"
continue
fi
echo "$dep native binary repair failed, reinstalling wrapper"
rm -rf "${GLOBAL_MODULES:?}/${dep}"
MISSING+=("$dep")
continue
Expand All @@ -246,6 +304,11 @@ if [ "${#MISSING[@]}" -gt 0 ]; then
timeout 600 bun add --global "$dep" 2>/dev/null || echo "Install failed: $dep"
purge_bun_npm_shim
run_postinstall_if_needed "$dep"
# Heal in the same run: bun drops transitive platform binaries on fresh
# global installs, so repair immediately instead of waiting for next activation.
if missing_native_optional_dep "$dep"; then
repair_native_optional_dep "$dep" || echo "Native binary repair failed: $dep" >&2

@mesa-dot-dev mesa-dot-dev Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

The repair happens after run_postinstall_if_needed completes. If the postinstall script depends on the native binary being present, it would fail before the repair runs. Consider moving the repair check before the postinstall step, or add error handling in postinstall to gracefully handle missing binaries during this window.

Fix in Cursor • Fix in Claude

Prompt for Agent 
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#2027
File: home-manager/modules/npm-globals/install-npm-globals.sh#L310
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
The repair happens after `run_postinstall_if_needed` completes. If the postinstall script depends on the native binary being present, it would fail before the repair runs. Consider moving the repair check before the postinstall step, or add error handling in postinstall to gracefully handle missing binaries during this window.

fi
done
else
echo "All npm global packages already installed"
Expand Down
85 changes: 83 additions & 2 deletions spec/npm_globals_spec.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,19 @@ When run bash -c "grep 'optionalDependencies' '$SCRIPT'"
The output should include 'optionalDependencies'
End

It 'reinstalls a version-matched package whose native binary is missing'
It 'detects a version-matched package whose native binary is missing'
When run bash -c "grep 'installed but native binary missing' '$SCRIPT'"
The output should include 'reinstalling'
The output should include 'installed but native binary missing'
End

It 'repairs the dropped native binary in place before reinstalling the wrapper'
When run bash -c "grep 'repair_native_optional_dep' '$SCRIPT'"
The output should include 'repair_native_optional_dep'
End

It 'follows one level of wrapper indirection to find the native binary'
When run bash -c "grep 'native_candidate_pkgs' '$SCRIPT'"
The output should include 'native_candidate_pkgs'
End
End

Expand Down Expand Up @@ -337,6 +347,77 @@ The output should include 'bun add --global nativecli'
End
End

Describe 'wrapper-indirection native binary self-heal (tokscale pattern)'
setup() {
TEMP_HOME=$(mktemp -d)
MOCK_BIN=$(mktemp -d)
MOCK_LOG="$TEMP_HOME/mock.log"
REAL_BIN_DIR="$(dirname "$(command -v jq)")"
REAL_SYSTEM_BIN_DIR="$(dirname "$(command -v mv)")"
: >"$MOCK_LOG"

GM="$TEMP_HOME/.bun/install/global/node_modules"
mkdir -p "$TEMP_HOME/dotfiles" "$TEMP_HOME/.bun/install/global" "$TEMP_HOME/.bun/bin"

os_tok=$(uname -s | tr '[:upper:]' '[:lower:]')
[ "$os_tok" = "darwin" ] || os_tok="linux"
cpu_tok=$(uname -m)
case "$cpu_tok" in arm64 | aarch64) cpu_tok=arm64 ;; *) cpu_tok=x64 ;; esac
NATIVE_DEP="@wrap/cli-${os_tok}-${cpu_tok}"
EXPECT_INSTALL="bun add --global ${NATIVE_DEP}@1.0.0"

cat >"$TEMP_HOME/dotfiles/package.json" <<'EOF'
{
"dependencies": { "wrapcli": "^1.0.0" }
}
EOF

# Thin wrapper: no optionalDependencies of its own; the native binary is
# declared one level down on @wrap/cli, which bun dropped.
mkdir -p "$GM/wrapcli" "$GM/@wrap/cli"
cat >"$GM/wrapcli/package.json" <<'EOF'
{ "name": "wrapcli", "version": "1.0.0", "dependencies": { "@wrap/cli": "1.0.0" } }
EOF
cat >"$GM/@wrap/cli/package.json" <<EOF
{
"name": "@wrap/cli",
"version": "1.0.0",
"optionalDependencies": { "@wrap/cli-${os_tok}-${cpu_tok}": "1.0.0" }
}
EOF

cat >"$MOCK_BIN/timeout" <<'EOF'
#!/usr/bin/env bash
set -euo pipefail
shift
if [ "${1:-}" = "bash" ] && [ "${2:-}" = "-c" ] && [ "${3:-}" = "exec 3<>/dev/tcp/1.1.1.1/53" ]; then
exit 0
fi
exec "$@"
EOF
chmod +x "$MOCK_BIN/timeout"

cat >"$MOCK_BIN/bun" <<'EOF'
#!/usr/bin/env bash
set -euo pipefail
printf 'bun %s\n' "$*" >>"$MOCK_LOG"
EOF
chmod +x "$MOCK_BIN/bun"

@indent-zero indent-zero Bot Jul 8, 2026

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.

Mock bun only logs argv — doesn't exercise the presence check. The bun mock at lines 400-405 just appends to MOCK_LOG and creates no files, and the assertion at line 417 only checks that the correct argv was invoked.

That means repair_native_optional_dep's trailing [ -d "${GLOBAL_MODULES}/${native}" ] always returns false, so the spec doesn't distinguish "repair succeeded" from "bun ran but no dir was materialized" (the phantom-empty case and the documented bun add -g no-op case). Extending the mock to mkdir -p "$GM/@wrap/cli-${os_tok}-${cpu_tok}" (and optionally writing a stub package.json inside) when the invoked argv matches would meaningfully validate the end-to-end heal.

}

cleanup() {
rm -rf "$TEMP_HOME" "$MOCK_BIN"
}

Before 'setup'
After 'cleanup'

It 'installs the nested native binary directly instead of reinstalling the wrapper'
When run bash -c "HOME='$TEMP_HOME' MOCK_LOG='$MOCK_LOG' PATH='$MOCK_BIN:$REAL_BIN_DIR:$REAL_SYSTEM_BIN_DIR:/usr/bin:/bin' bash '$SCRIPT' >/dev/null 2>&1; cat '$MOCK_LOG'"
The output should include "$EXPECT_INSTALL"
End
End

Describe 'stale global package pruning'
setup() {
TEMP_HOME=$(mktemp -d)
Expand Down
Loading