Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions home-manager/modules/npm-globals/install-npm-globals.sh
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,20 @@ missing_native_optional_dep() {
[ "$matched" -eq 1 ] && [ "$present" -eq 0 ]
}

# Remove the npm "bun" wrapper package from global node_modules.
# Some transitive deps pull in the "bun" npm package whose postinstall
# downloads a bun binary. When that postinstall is skipped/fails, it leaves
# a stub .bin/bun shim that shadows the real system bun, breaking every
# subsequent postinstall that shells out to bun (e.g. @railway/cli).
purge_bun_npm_shim() {
local gm="${HOME}/.bun/install/global/node_modules"
if [ -d "${gm}/bun" ]; then

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.

high

If a previous run was interrupted, or if the ${gm}/bun directory was deleted/cleaned up but the shims in ${gm}/.bin were left behind, they will become dangling symlinks.

In Unix-like systems, a dangling symlink in a directory that is in the PATH (like node_modules/.bin during postinstall scripts) will still shadow the command and cause execution to fail with a No such file or directory error, rather than falling back to the real system bun.

Since the current check only looks for the directory [ -d "${gm}/bun" ], it will skip purging the shims if the directory is gone but the dangling symlinks remain.

To make this robust against dangling symlinks, we should also check for the existence of the symlinks/files themselves using [ -L ... ] and [ -e ... ].

Suggested change
if [ -d "${gm}/bun" ]; then
if [ -d "${gm}/bun" ] || [ -L "${gm}/.bin/bun" ] || [ -L "${gm}/.bin/bunx" ] || [ -e "${gm}/.bin/bun" ] || [ -e "${gm}/.bin/bunx" ]; then

rm -rf "${gm}/bun"
rm -f "${gm}/.bin/bun" "${gm}/.bin/bunx"
echo "Removed broken bun npm shim from global node_modules"
fi
}

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.

Purge doesn't cover ~/.bun/bin for later phases: home-manager/modules/npm-globals/default.nix puts $HOME/.bun/bin on home.sessionPath, and bun add -g on the wrapper package creates ~/.bun/bin/bun as a symlink into install/global/node_modules/bun/.... This function only deletes the target under install/global/node_modules, so ~/.bun/bin/bun becomes a dangling symlink. The dangling-shim sweep at lines 224–232 catches it, but it runs once — before the purges at lines 242 and 340. If either the overrides bun install or the optional-native bun add --global re-materializes the wrapper, ~/.bun/bin/bun is left dangling until the next script run.

Today this is not immediately harmful because bash's test -x returns false on dangling symlinks and PATH resolution falls through to the Nix bun. But it relies on bash-specific PATH semantics; interactive shells with a cached hash bun, or non-bash callers, may still resolve to the dead symlink and reproduce the original shadow. Consider extending the purge to also rm -f "${HOME}/.bun/bin/bun" "${HOME}/.bun/bin/bunx" (or moving the dangling sweep to run after every purge).


echo "Installing npm global packages from package.json using bun..."
cd "${HOME}/dotfiles"

Expand Down Expand Up @@ -154,6 +168,8 @@ if [ "${#STALE[@]}" -gt 0 ]; then
done
fi

purge_bun_npm_shim

# Build list of packages that need installing or updating
GLOBAL_MODULES="${HOME}/.bun/install/global/node_modules"
DEPS=$(jq -r '.dependencies | to_entries[] | "\(.key)=\(.value)"' "$PACKAGE_JSON" 2>/dev/null || true)
Expand Down Expand Up @@ -198,6 +214,7 @@ if [ "${#MISSING[@]}" -gt 0 ]; then
echo "Installing ${#MISSING[@]} missing packages..."
for dep in "${MISSING[@]}"; do
timeout 600 bun add --global "$dep" 2>/dev/null || echo "Install failed: $dep"
purge_bun_npm_shim
done
else
echo "All npm global packages already installed"
Expand All @@ -222,6 +239,7 @@ if [ -n "$OVERRIDES" ]; then
jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" &&
mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG"
(cd "${HOME}/.bun/install/global" && bun install 2>/dev/null || true)
purge_bun_npm_shim
echo "Applied dependency overrides to global install"
fi
fi
Expand Down Expand Up @@ -319,6 +337,7 @@ if [ -n "$OPTIONAL_DEPS" ]; then
fi
echo "Installing platform-native: $spec"
timeout 600 bun add --global "$spec" --minimum-release-age 0 2>/dev/null || echo "Install failed: $spec"
purge_bun_npm_shim
# Verify the payload actually materialized; bun can silently no-op.
if [ ! -f "${GLOBAL_MODULES}/${dep}/package.json" ]; then
echo "Warning: $dep still missing after install ($spec)" >&2
Expand Down
22 changes: 22 additions & 0 deletions spec/npm_globals_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,28 @@ The output should include 'find'
End
End

Describe 'bun npm shim purge'
It 'defines a purge_bun_npm_shim function'

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.

P3: These tests only verify that certain string patterns exist in the source file (via grep). They'll pass as long as the tokens are present, regardless of whether the purge logic actually removes the wrapper correctly. The spec file already has good integration-style precedents (e.g. native-binary reinstall integration at line 251) — a test that seeds a fake bun/package.json under a temp global modules dir and asserts it's gone after sourcing/running the function would provide real regression protection.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spec/npm_globals_spec.sh, line 191:

<comment>These tests only verify that certain string patterns exist in the source file (via `grep`). They'll pass as long as the tokens are present, regardless of whether the purge logic actually removes the wrapper correctly. The spec file already has good integration-style precedents (e.g. `native-binary reinstall integration` at line 251) — a test that seeds a fake `bun/package.json` under a temp global modules dir and asserts it's gone after sourcing/running the function would provide real regression protection.</comment>

<file context>
@@ -187,6 +187,28 @@ The output should include 'find'
 End
 
+Describe 'bun npm shim purge'
+  It 'defines a purge_bun_npm_shim function'
+    When run bash -c "grep 'purge_bun_npm_shim()' '$SCRIPT'"
+    The output should include 'purge_bun_npm_shim'
</file context>

When run bash -c "grep 'purge_bun_npm_shim()' '$SCRIPT'"
The output should include 'purge_bun_npm_shim'
End

It 'removes the bun package from global node_modules'
When run bash -c "grep 'rm -rf.*gm.*bun' '$SCRIPT'"
The output should include 'bun'
End

It 'removes bun shims from .bin'
When run bash -c "grep 'rm -f.*\.bin/bun' '$SCRIPT'"
The output should include '.bin/bun'
End

It 'calls purge after each bun add --global'
When run bash -c "grep -A 1 'bun add --global.*dep.*2>/dev/null' '$SCRIPT' | grep 'purge_bun_npm_shim'"
The output should include 'purge_bun_npm_shim'
End
End

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.

Behavior not exercised by tests: These four cases only grep the source file for literal strings (purge_bun_npm_shim(), rm -rf.*gm.*bun, rm -f.*\.bin/bun, bun add --global.*dep.*purge_bun_npm_shim). They pass whenever those tokens appear in the file, regardless of whether the purge actually fires or removes the wrapper.

The file already has good precedent for real integration tests — see native-binary reinstall integration (line 251) and stale global package pruning (line 318). A similar block that seeds $TEMP_HOME/.bun/install/global/node_modules/bun/package.json and asserts it's gone after running the script would actually guard the fix from regressing.


Describe 'native addon repair'
It 'has a sqlite3 native binding repair step'
When run bash -c "grep 'repair_sqlite3_native_binding' '$SCRIPT'"
Expand Down
Loading