Skip to content

fix(installer): dedupe user shim log#3421

Merged
jyaunches merged 2 commits into
NVIDIA:mainfrom
yimoj:fix/3400-dedupe-cli-shim-log
May 14, 2026
Merged

fix(installer): dedupe user shim log#3421
jyaunches merged 2 commits into
NVIDIA:mainfrom
yimoj:fix/3400-dedupe-cli-shim-log

Conversation

@yimoj

@yimoj yimoj commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Make installer shim creation idempotent so the install-plus-verify path no longer rewrites an unchanged user-local shim or logs duplicate creation messages.

Related Issue

Fixes #3400

Changes

  • Build the expected user-local CLI shim content once in ensure_cli_shim.
  • Return early without logging when the existing executable shim already matches the expected content, while still refreshing PATH/profile state.
  • Update the installer shim regression test to exercise install plus verify and assert one creation log line.

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)

Verification notes:

  • bash -n scripts/install.sh passes.
  • PATH="$HOME/.local/node-v22.16.0-linux-x64/bin:$PATH" ./node_modules/.bin/vitest run test/install-preflight.test.ts -t "creates a user-local shim when npm installs outside the current PATH" passes.
  • PATH="$HOME/.local/node-v22.16.0-linux-x64/bin:$PATH" npm run typecheck:cli passes.
  • cd nemoclaw && PATH="$HOME/.local/node-v22.16.0-linux-x64/bin:$PATH" npm test passes.
  • npm test and npx prek run --all-files were attempted; both are blocked in this local non-root worktree by the pre-existing test/fetch-guard-patch-regression.test.ts case that executes rm -rf /usr/local/lib/node_modules/openclaw and fails with Permission denied before its mocked npm assertion.

Signed-off-by: Yimo Jiang yimoj@nvidia.com

Summary by CodeRabbit

  • Improvements

    • Installer avoids rewriting an identical user-local shim and still refreshes PATH and shell profile, reducing unnecessary changes.
    • Improved reliability of command-line tool initialization during installation.
  • Tests

    • Strengthened installation tests: expanded checkout mocking and tightened assertions to ensure the shim creation message appears exactly once.

Review Change Stack

Make ensure_cli_shim return early when the existing executable shim already matches the expected wrapper content, so the install-plus-verify path refreshes PATH/profile without rewriting or logging a second creation line.

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
@yimoj yimoj self-assigned this May 12, 2026
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f63dda94-25d1-44f7-9f54-2c9b39c26322

📥 Commits

Reviewing files that changed from the base of the PR and between c26cfe0 and 058f2f3.

📒 Files selected for processing (2)
  • scripts/install.sh
  • test/install-preflight.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • scripts/install.sh
  • test/install-preflight.test.ts

📝 Walkthrough

Walkthrough

The PR fixes duplicate log output during shim creation by making ensure_cli_shim idempotent. The function now builds expected content, compares it with the existing shim, and skips rewriting when they match. The test is updated to verify the log message appears exactly once.

Changes

Shim Idempotency and Log Suppression

Layer / File(s) Summary
Shim idempotency implementation
scripts/install.sh
ensure_cli_shim adds expected_shim variable to build shim content upfront, then uses cmp to compare against the existing shim. If content matches, rewriting is skipped; otherwise, the shim directory is created (if needed) and new content written via printf.
Test validation of shim idempotency
test/install-preflight.test.ts
The "user-local shim" test expands the fake git stub to handle -C flag shifts and implement git init directory creation behavior. The shim output assertion is strengthened to count occurrences of "Created user-local shim" and assert it appears exactly once.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A shim that writes but once, not twice,
Makes logs both clean and oh-so-nice!
With cmp we check what came before,
And skip the rewrite—what's the chore?
One message printed, not a pair—
The install flow's no longer spare! 🎯

🚥 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(installer): dedupe user shim log' accurately summarizes the main change: making shim creation idempotent to eliminate duplicate log messages.
Linked Issues check ✅ Passed The PR fully addresses issue #3400 objectives: ensure_cli_shim() now generates expected content, compares against existing shim using cmp, and skips rewriting/logging when already present.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing issue #3400: the bash script update makes shim idempotent, and the test update validates the fix by asserting exactly one creation log.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

@yimoj yimoj added the v0.0.41 label May 13, 2026

@jyaunches jyaunches left a comment

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.

Looks good — scoped installer idempotency fix with regression coverage. Approving, but leaving merge to the author/maintainers.

@wscurran wscurran added fix and removed v0.0.41 labels May 13, 2026
@wscurran

Copy link
Copy Markdown
Contributor

✨ Thanks for submitting this detailed PR to make the installer shim creation idempotent. This change aims to improve the robustness of the installation process by preventing duplicate creation messages.


Related open issues:

@jyaunches jyaunches merged commit 4e07d59 into NVIDIA:main May 14, 2026
28 checks passed
@miyoungc miyoungc mentioned this pull request May 14, 2026
12 tasks
miyoungc added a commit that referenced this pull request May 14, 2026
## Summary
Refreshes the NemoClaw documentation for the local `main` changes
included in the 0.0.42 release. The update adds release notes, updates
the affected user-facing setup and troubleshooting pages, bumps docs
metadata to 0.0.42, and regenerates the matching user skills.

## Changes
- #3537 -> `docs/reference/commands.md`,
`docs/reference/troubleshooting.md`: Documented host-level status
fields, cloudflared state-specific recovery hints, and Local Ollama auth
proxy status diagnostics.
- #3454 -> `docs/get-started/prerequisites.md`,
`docs/get-started/quickstart.md`: Documented macOS Docker-driver
onboarding and removed the expectation that standard macOS setup needs a
VM driver helper.
- #3514 -> `docs/inference/use-local-inference.md`: Documented
compatible-endpoint retry behavior for reasoning-only smoke responses.
- #3448 -> `docs/reference/commands.md`,
`docs/manage-sandboxes/messaging-channels.md`: Documented canonical
channel names and policy preset hints after `channels add`.
- #3520 -> `docs/about/release-notes.md`: Captured clearer GPU recovery
and uninstall wording in the 0.0.42 release notes.
- #3313 -> `docs/get-started/quickstart.md`,
`docs/reference/troubleshooting.md`: Documented stronger dashboard port
detection and rollback when a forward cannot start.
- #3502 -> `docs/about/release-notes.md`: Captured batched onboarding
policy preset application in the 0.0.42 release notes.
- #3505 -> `docs/reference/troubleshooting.md`: Documented the top-level
Colima socket path.
- #3421 -> `docs/about/release-notes.md`: Captured idempotent installer
shim logging in the 0.0.42 release notes.
- Updated `docs/project.json`, `docs/versions1.json`, and regenerated
`.agents/skills/nemoclaw-user-*` outputs.

## Type of Change
- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] 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
- [x] No secrets, API keys, or credentials committed
- [x] Docs updated for user-facing behavior changes
- [x] `make docs` builds without warnings (doc changes only)
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Release Notes - v0.0.42

* **Documentation**
  * Enhanced macOS onboarding guidance for Docker gateway setup
  * Improved dashboard port conflict handling with automatic rollback
* Better local Ollama inference diagnostics and authentication proxy
checks
  * Clarified status command output and recovery procedures
  * Refined messaging channel setup documentation

* **Chores**
  * Version bump to 0.0.42

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3540)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-authored-by: Carlos Villela <cvillela@nvidia.com>
@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression and removed NemoClaw CLI labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Ubuntu 24.04][Install] duplicate user-local shim creation log line during curl|bash install

3 participants