Skip to content

fix(install): surface npm link failures and fall back to user-local shim#2520

Open
laitingsheng wants to merge 1 commit intomainfrom
fix/2515-npm-link-fallback-shim
Open

fix(install): surface npm link failures and fall back to user-local shim#2520
laitingsheng wants to merge 1 commit intomainfrom
fix/2515-npm-link-fallback-shim

Conversation

@laitingsheng
Copy link
Copy Markdown
Contributor

@laitingsheng laitingsheng commented Apr 27, 2026

Summary

A fresh git clone + npm install on a Linux host without a writable global npm prefix reported success but left no nemoclaw command on PATH — the prepare script's npm link 2>/dev/null || true swallowed both stderr and the exit code. This surfaces the real npm error and falls back to a user-local wrapper at ~/.local/bin/nemoclaw. uninstall.sh recognises the new shim shape so it cleans up later.

Related Issue

Closes #2515

Changes

  • Add scripts/npm-link-or-shim.sh: hardened set -eu helper that runs npm link and on failure writes a wrapper at ~/.local/bin/nemoclaw. Atomic write via tempfile + rename, marker-based refusal to overwrite foreign files, idempotent refresh of NemoClaw-managed shims.
  • package.json: prepare now invokes the helper instead of swallowing errors inline.
  • uninstall.sh: is_installer_managed_nemoclaw_shim recognises the dev-shim shape so remove_nemoclaw_cli cleans it up.
  • test/npm-link-or-shim.test.ts (new) + test/uninstall.test.ts: regression coverage for six failure/success paths plus the uninstaller cleanup.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

AI Disclosure

  • AI-assisted — tool: Claude Code, Codex

Signed-off-by: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • New Features

    • Enhanced CLI installation with fallback mechanism that automatically adapts when standard linking is unavailable, improving setup reliability
    • Improved uninstall process to properly handle new CLI installation variants
  • Tests

    • Added comprehensive test coverage for CLI installation and uninstall scenarios

## Summary

A fresh `git clone` + `npm install` on a Linux host without a writable
global npm prefix (and without sudo) reported success, but the
`nemoclaw` command never landed on PATH. Root cause: the package.json
`prepare` script invoked `NEMOCLAW_INSTALLING=1 npm link 2>/dev/null ||
true`, which suppressed both stderr and the exit code, so any link
failure was invisible to the user. They only discovered it when
`nemoclaw onboard` returned "No such file or directory".

Replace the silent npm-link step with a small helper that surfaces the
real npm error and falls back to a user-local wrapper at
`~/.local/bin/nemoclaw`. Wrapper preserves the Node directory that was
on PATH at install time so it works in shells where Node is provisioned
via nvm. uninstall.sh recognises the new shim shape so a later
uninstall cleans it up alongside the installer wrapper.

## Related Issue

Closes #2515

## Changes

- scripts/npm-link-or-shim.sh: new helper invoked by `npm install` via
  the package.json `prepare` script. `set -eu` so any unchecked failure
  halts. Runs `npm link` from the repo root; on failure, prints the
  captured npm error then writes a wrapper at `~/.local/bin/nemoclaw`
  that exports the captured Node directory and execs `bin/nemoclaw.js`.
  Refuses to overwrite a foreign file (only refreshes a NemoClaw-managed
  shim, identified by a marker line). Atomic write via tempfile + rename
  so a mid-write failure cannot leave a partial unrecognisable file.
  Honours an already-set `NEMOCLAW_INSTALLING` to short-circuit
  prepare-script recursion.
- package.json: replace the inline
  `NEMOCLAW_INSTALLING=1 npm link 2>/dev/null || true` with
  `bash scripts/npm-link-or-shim.sh`. Lenient chaining preserved (the
  helper warns loudly on stderr but does not fail `npm install`).
- uninstall.sh: extend `is_installer_managed_nemoclaw_shim` to also
  recognise the dev-shim shape (marker line + same wrapper structure)
  so a later `uninstall.sh` removes it cleanly instead of leaving a
  stale `nemoclaw` command behind after the source checkout is gone.
- test/npm-link-or-shim.test.ts: regression tests covering six cases —
  shim is created on link failure (wrapper structure, marker line,
  Node-directory PATH export), no shim on success, no-op when
  `NEMOCLAW_INSTALLING` is preset, refusal to overwrite a foreign file,
  idempotent refresh of an existing dev-shim, and the original
  high-severity reproduction (`~/.local` exists as a regular file).
- test/uninstall.test.ts: regression test that a dev-shim file in
  `~/.local/bin/nemoclaw` is removed by `remove_nemoclaw_cli`.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@laitingsheng laitingsheng self-assigned this Apr 27, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 27, 2026

📝 Walkthrough

Walkthrough

Updates the dev install process to handle npm link failures gracefully. Introduces a shell script that attempts npm link and falls back to creating a user-local shim at ~/.local/bin/nemoclaw if the link fails. Updates uninstall script to recognize and remove the managed shim.

Changes

Cohort / File(s) Summary
Package Configuration
package.json
Delegates npm link behavior from inline conditional logic to the new scripts/npm-link-or-shim.sh shell script in the prepare phase.
Fallback Shim Implementation
scripts/npm-link-or-shim.sh
New Bash script that attempts npm link from repo root; on failure, creates a managed user-local shim at ~/.local/bin/nemoclaw. Includes re-entrancy guards, atomic file operations, executable verification, and PATH instructions when needed.
Test Coverage
test/npm-link-or-shim.test.ts, test/uninstall.test.ts
Comprehensive test suite for the new script covering npm link success/failure, shim creation with management marker, re-entrancy prevention, foreign file protection, idempotent refresh, and missing ~/.local directory handling. Additional uninstall test verifies removal of managed shim.
Uninstall Logic
uninstall.sh
Updates is_installer_managed_nemoclaw_shim to recognize the new shim variant created by npm-link-or-shim.sh via its embedded marker comment and exec wrapper pattern.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 A rabbit's tale of clever shims—
When linking fails, we don't grow grim!
We fall back graceful, user-bound,
At ~/.local/bin, safe and sound! 🎯✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing npm link failure handling with a fallback to user-local shim installation.
Linked Issues check ✅ Passed All code changes directly address issue #2515: surfacing npm link failures and falling back to user-local shim via scripts/npm-link-or-shim.sh, updating package.json, uninstall.sh, and adding comprehensive tests.
Out of Scope Changes check ✅ Passed All changes are in-scope: npm-link-or-shim.sh implements the fallback shim logic, package.json integrates it, uninstall.sh extends cleanup, and tests cover the new functionality.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 fix/2515-npm-link-fallback-shim

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

@laitingsheng laitingsheng removed their assignment Apr 27, 2026
@wscurran wscurran added bug Something isn't working CI/CD Use this label to identify issues with NemoClaw CI/CD pipeline or GitHub Actions. NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). dependencies Pull requests that update a dependency file labels Apr 27, 2026
@wscurran
Copy link
Copy Markdown
Contributor

✨ Thanks for submitting this pull request that proposes a way to fix a bug where npm link failures were not surfaced and fell back to a user-local shim. This identifies a bug and proposes a change to surface the real npm error and fall back to a user-local wrapper at ~/.local/bin/nemoclaw, with matching recognition in uninstall.sh to clean up the shim later. The addition of a hardened set -eu helper script and updates to package.json and uninstall.sh will help ensure the issue does not recur.


Related open issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CI/CD Use this label to identify issues with NemoClaw CI/CD pipeline or GitHub Actions. dependencies Pull requests that update a dependency file NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Linux][Install] Dev install: npm install silently succeeds when npm link fails — nemoclaw command not available afterwards

2 participants