fix(e2e): re-add ~/.local/bin to PATH after .bashrc sourcing by jyaunches · Pull Request #3200 · NVIDIA/NemoClaw

jyaunches · 2026-05-07T19:36:53Z

Summary

The e2e test harness sources ~/.bashrc to pick up the freshly-installed nemoclaw CLI, but on images whose .bashrc resets PATH (or unconditionally prepends system paths) this wipes the ~/.local/bin entry that install.sh just added, breaking every downstream nemoclaw ... invocation. This PR re-adds ~/.local/bin to PATH after .bashrc sourcing and extracts the pattern into a shared helper so it stays consistent as more scripts adopt it.

Related Issue

Supersedes #3156 (cherry-picked with author credit preserved to Hung Le).

Changes

Fix (cherry-picked from fix(e2e): re-add ~/.local/bin to PATH after .bashrc sourcing #3156, attribution preserved): re-add ~/.local/bin to PATH after sourcing ~/.bashrc in the cloud e2e test scripts, so the installed nemoclaw CLI remains resolvable on images whose .bashrc rewrites PATH.
Refactor: extract the PATH-refresh pattern into test/e2e/lib/install-path-refresh.sh with two helpers:
- nemoclaw_refresh_install_env — source .bashrc then guarantee ~/.local/bin is on PATH
- nemoclaw_ensure_local_bin_on_path — idempotent prepend of ~/.local/bin
Migrate 6 scripts (4 PR-touched + 2 adjacent references) to the helper: test-cloud-inference-e2e.sh, test-cloud-onboard-e2e.sh, test-credential-migration.sh, test-deployment-services.sh, test-diagnostics.sh, test-network-policy.sh.
Net diff: +67 / −33 lines across 7 files (including the new 41-line helper).

Follow-up (not in this PR): ~23 additional e2e scripts also carry a copy of the inline .local/bin on PATH guard. Intentionally left out to keep this PR scoped to the scripts flagged in the original review. Tracked in #3201.

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 syntax check passes on the new helper and all 6 migrated scripts.
Helper is sourceable via all three patterns the scripts use: dirname "${BASH_SOURCE[0]}"/lib/, ${E2E_DIR}/lib/, and ${SCRIPT_DIR_TIMEOUT}/lib/.
Functional test of nemoclaw_ensure_local_bin_on_path confirmed: prepends ~/.local/bin when missing, is idempotent when already present.
Zero inline leftovers of the old pattern in the 6 migrated scripts.
npm test has 18 unrelated timeout failures in test/install-preflight.test.ts (installer integration suite) on this machine; this PR does not touch that file (0 diff lines against it) and does not modify any code path it exercises. The actual e2e scripts changed here run in the Brev cloud suite, which is out of the npm test scope.

Signed-off-by: Julie Yaunches jyaunches@nvidia.com

install.sh places openshell/nemoclaw under ~/.local/bin, but sourcing .bashrc triggers nvm.sh which rebuilds $PATH from scratch, dropping the entry. Four E2E test helpers (deployment-services, diagnostics, network-policy, credential-migration) checked for the binaries after .bashrc but before re-adding ~/.local/bin, causing them to fail with "nemoclaw not found" / "openshell still missing after install". Apply the same PATH-refresh guard already used by the passing test-cloud-onboard-e2e.sh: re-add ~/.local/bin immediately after sourcing .bashrc so the check succeeds regardless of nvm ordering. Signed-off-by: Hung Le <hple@nvidia.com> (cherry picked from commit 372e514)

Collapse six copies of the "source ~/.bashrc + re-add ~/.local/bin to PATH" recipe into a single test/e2e/lib/install-path-refresh.sh helper with two small functions: - nemoclaw_ensure_local_bin_on_path — defensive PATH guard - nemoclaw_refresh_install_env — post-install .bashrc + PATH refresh Updated callers (the four scripts patched in the preceding commit plus the two existing reference scripts that already had the inline pattern): - test/e2e/test-credential-migration.sh - test/e2e/test-deployment-services.sh - test/e2e/test-diagnostics.sh - test/e2e/test-network-policy.sh - test/e2e/test-cloud-onboard-e2e.sh - test/e2e/test-cloud-inference-e2e.sh Net −27 lines in the callers; logic is byte-equivalent to the previous inline recipe. Follow-up: the remaining ~23 e2e scripts also carry a copy of the ".local/bin on PATH" guard and can be migrated in a separate pass. Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>

coderabbitai · 2026-05-07T19:37:05Z

📝 Walkthrough

Walkthrough

This pull request extracts common E2E test environment and PATH setup logic into a shared Bash helper script, then refactors six E2E test scripts to source and invoke these helper functions instead of duplicating inline shell profile sourcing and $HOME/.local/bin PATH management.

Changes

E2E PATH Management Refactoring

Layer / File(s)	Summary
Helper Script Definition `test/e2e/lib/install-path-refresh.sh`	New shared helper script with two functions: `nemoclaw_ensure_local_bin_on_path()` conditionally prepends `$HOME/.local/bin` to `PATH`, and `nemoclaw_refresh_install_env()` sources `~/.bashrc` then re-applies the local-bin guard to recover from profile rebuilds.
Helper Integration Across Tests `test/e2e/test-cloud-inference-e2e.sh`, `test/e2e/test-cloud-onboard-e2e.sh`, `test/e2e/test-credential-migration.sh`, `test/e2e/test-deployment-services.sh`, `test/e2e/test-diagnostics.sh`, `test/e2e/test-network-policy.sh`	Six E2E test scripts now source the shared helper and call its functions during NemoClaw installation instead of inlining bashrc sourcing and PATH manipulation. Each test replaces prior manual environment setup with calls to `nemoclaw_refresh_install_env()` and/or `nemoclaw_ensure_local_bin_on_path()`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A PATH that's perplexed, a helper set free,
Six tests now sing in harmony!
No more bashrc lines, scattered about,
One shared function makes PATH dreams come true! 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically identifies the main change: extracting and applying a PATH fix pattern to ensure ~/.local/bin persists after .bashrc sourcing in e2e tests.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
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/e2e-install-path-refresh

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms

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

## Summary - Bump the docs release metadata to `0.0.37`. - Document release-prep updates for messaging policy presets, sandbox runtime utilities, and the GPU CDI troubleshooting path. - Refresh generated `nemoclaw-user-*` skills from the updated docs. ## Source summary - #3159 -> `docs/reference/troubleshooting.md`: Documents the GPU CDI preflight warning and remediation for `nvidia.com/gpu=all` gateway start failures. - #2415 -> `docs/reference/network-policies.md`, `docs/manage-sandboxes/messaging-channels.md`, `docs/network-policy/customize-network-policy.md`: Clarifies that Telegram, Discord, and Slack egress comes from opt-in messaging presets, not the baseline policy. - #3091 -> `docs/deployment/sandbox-hardening.md`, `docs/network-policy/customize-network-policy.md`: Documents the retained sandbox utilities `vi`, `jq`, and `dos2unix` while keeping host-side policy files as the durable source of truth. ## Test plan - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user` - `make docs` - `npm run build:cli` - `npm run typecheck:cli` - Commit and pre-push hooks: markdownlint, docs-to-skills verification, gitleaks, commitlint, CLI typecheck ## Skipped - #3193 and #3191 matched `docs/.docs-skip` entries for experimental shields/config paths. - #3200 and #3183 were test-only fixes. - #3189 and #3163 were internal documentation/refactor changes with no public docs impact. Made with [Cursor](https://cursor.com)  ## Summary by CodeRabbit * **Documentation** * Clarified which utilities remain in the sandbox runtime for lightweight inspection and cleanup * Noted that messaging endpoints (Discord, Slack, Telegram) are not in the baseline policy and that channel presets are applied during onboarding * Added GPU passthrough troubleshooting for gateway startup * Updated release/version bump and release-prep workflow guidance, including Discord preset description updates  --------- Co-authored-by: Cursor <cursoragent@cursor.com>

hunglp6d and others added 2 commits May 7, 2026 15:23

jyaunches self-assigned this May 7, 2026

jyaunches mentioned this pull request May 7, 2026

refactor(e2e): migrate remaining 23 scripts to install-path-refresh helper #3201

Open

5 tasks

hunglp6d mentioned this pull request May 7, 2026

fix(e2e): re-add ~/.local/bin to PATH after .bashrc sourcing #3156

Closed

8 tasks

Merge branch 'main' into fix/e2e-install-path-refresh

8f3e8f5

cv approved these changes May 7, 2026

View reviewed changes

jyaunches merged commit ab3ab6d into main May 7, 2026
16 checks passed

miyoungc mentioned this pull request May 7, 2026

docs: prepare 0.0.37 release docs #3215

Merged

cjagwani mentioned this pull request May 14, 2026

feat(skills): add nemoclaw-maintainer-scope-issues skill #3524

Closed

wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026

jyaunches deleted the fix/e2e-install-path-refresh branch June 12, 2026 13:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(e2e): re-add ~/.local/bin to PATH after .bashrc sourcing#3200

fix(e2e): re-add ~/.local/bin to PATH after .bashrc sourcing#3200
jyaunches merged 3 commits into
mainfrom
fix/e2e-install-path-refresh

jyaunches commented May 7, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 7, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Review ran into problems

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jyaunches commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Type of Change

Verification

Uh oh!

coderabbitai Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Review ran into problems

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jyaunches commented May 7, 2026 •

edited

Loading

coderabbitai Bot commented May 7, 2026 •

edited

Loading