feat(install): add express install prompt for DGX Spark and Station#3317
Conversation
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds DGX platform detection and an interactive express-install offer for DGX Spark/Station after the third‑party notice; accepting sets non-interactive/yes/policy env and platform-specific provider/model defaults. Tests opt out of the express prompt during preflight runs. ChangesExpress Install Platform Detection
Sequence Diagram(s)sequenceDiagram
participant main
participant preflight_usage_notice_prompt
participant maybe_offer_express_install
main->>preflight_usage_notice_prompt: show third-party notice
preflight_usage_notice_prompt-->>main: return after acceptance
main->>maybe_offer_express_install: attempt express offer (if platform & TTY & no overrides)
maybe_offer_express_install-->>main: return (may export env and set provider/model)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/install.sh`:
- Around line 1698-1704: maybe_offer_express_install currently never respects a
hard opt-out env var; update the function to honor NEMOCLAW_NO_EXPRESS by
checking it at the start of maybe_offer_express_install and returning
immediately when set (e.g., when NEMOCLAW_NO_EXPRESS equals "1" or a truthy
value) before calling detect_express_platform; this ensures automation/tests can
skip the express prompt path by setting NEMOCLAW_NO_EXPRESS and leaves the rest
of the function (including the call to detect_express_platform) unchanged.
- Around line 1720-1725: The current read call `read -r reply </dev/tty || true`
silences TTY read failures and leaves `reply` empty, which matches the
default-accept branch; change this so a failed read sets a negative/decline
value instead of empty. Replace the `read -r reply </dev/tty || true` with logic
that checks the read exit status (e.g., `if ! read -r reply </dev/tty; then
reply="n"; fi`) so that on read failure `reply` is explicitly "n" (or "no") and
will not fall through to the `"" | y | yes)` accept case; keep the subsequent
normalization `reply="$(printf "%s" "$reply" | tr '[:upper:]' '[:lower:]')"` and
existing case handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f48d0ab5-471a-4447-88d4-36e8389d05c2
📒 Files selected for processing (1)
scripts/install.sh
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
ericksoa
left a comment
There was a problem hiding this comment.
Thanks for splitting the license notice from the express prompt and addressing the CodeRabbit items. I think this still needs one installer-flow fix before approval.
The express prompt is gated on if [ ! -t 0 ]; then ... skip, but the canonical installer path is curl -fsSL https://www.nvidia.com/nemoclaw.sh | bash, where stdin is the script pipe even in a real interactive terminal. The existing preflight_usage_notice_prompt handles this correctly by opening /dev/tty; after the user accepts that notice, maybe_offer_express_install immediately sees non-TTY stdin and skips express mode with Skipping express prompt (no TTY). That means the main Spark/Station install path never offers the new express install.
I reproduced the final helper behavior by sourcing the PR head, stubbing detect_express_platform to return DGX Spark, and running under a controlling TTY with stdin redirected away, matching the curl-pipe shape:
[INFO] Detected DGX Spark. Skipping express prompt (no TTY).
NON_INTERACTIVE= PROVIDER= MODEL= POLICY=
Please make maybe_offer_express_install use the same /dev/tty prompt path as the license/onboard prompts instead of treating non-TTY stdin as no prompt capability. This should also get a focused regression test for the piped-stdin-with-controlling-TTY case, plus the no-TTY skip case, so the express path is covered directly rather than only disabled via NEMOCLAW_NO_EXPRESS in existing license tests.
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/install-preflight.test.ts (1)
2844-2854: ⚡ Quick winCover the DGX Station express branch too.
This new suite only locks down the Spark defaults, but the feature also has a Station-specific path with different env output. Adding a sibling case here for
"DGX Station"would protect theinstall-vllmbranch from silently drifting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/install-preflight.test.ts` around lines 2844 - 2854, Add a sibling test to cover the DGX Station express branch by copying the existing "offers express install when curl-piped stdin still has a controlling TTY" case and modifying it to simulate the Station variant (e.g., call runExpressPromptWithTty with the input/environment that triggers "DGX Station"); assert exit status is 0, that output matches /Detected DGX Station/, that it suggests running express install, and that the environment/result line matches the Station-specific expected string (e.g., includes PROVIDER=install-vllm and the corresponding MODEL/POLICY/YES values) so the install-vllm branch is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/install-preflight.test.ts`:
- Around line 2844-2854: Add a sibling test to cover the DGX Station express
branch by copying the existing "offers express install when curl-piped stdin
still has a controlling TTY" case and modifying it to simulate the Station
variant (e.g., call runExpressPromptWithTty with the input/environment that
triggers "DGX Station"); assert exit status is 0, that output matches /Detected
DGX Station/, that it suggests running express install, and that the
environment/result line matches the Station-specific expected string (e.g.,
includes PROVIDER=install-vllm and the corresponding MODEL/POLICY/YES values) so
the install-vllm branch is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3cba91be-7cf2-443a-abf2-633d2ab9c12d
📒 Files selected for processing (2)
scripts/install.shtest/install-preflight.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/install.sh
ericksoa
left a comment
There was a problem hiding this comment.
Approved after pushing the follow-up fix in b42c817. The express install prompt now uses the controlling TTY for curl-piped installs, while true no-TTY runs still skip. Local validation passed: bash syntax/diff check, installer hash check, focused express prompt Vitest tests, license acceptance Vitest tests, and build:cli. GitHub checks are green on the updated head.
## Summary Refreshes the release-prep docs for v0.0.39 based on changes merged since the Friday 4pm doc refresh. Updates the source docs, bumps the docs version metadata, and regenerates the NemoClaw user skills from the refreshed docs. ## Changes - #3314 -> `docs/get-started/prerequisites.md`, `docs/get-started/quickstart.md`, `docs/reference/troubleshooting.md`: Documents installer Docker setup, Docker group activation, and retry guidance. - #3317 -> `docs/get-started/quickstart.md`, `docs/reference/commands.md`: Documents the DGX Spark and DGX Station express install prompt and `NEMOCLAW_NO_EXPRESS`. - #3328 and #3329 -> `docs/security/best-practices.md`, `docs/deployment/sandbox-hardening.md`: Updates sandbox capability hardening docs for the stricter bounding-set and `setpriv` step-down behavior. - #3330, #3335, and #3346 -> `docs/inference/use-local-inference.md`: Documents Windows-host Ollama relaunch behavior, NIM key passthrough, early health-fail diagnostics, and mixed-GPU preflight detail. - #2406, #2883, #3001, #3244, #3267, #3318, #3320, and #3354 -> `docs/about/release-notes.md`: Adds the v0.0.39 release-prep section while keeping the v0.0.38 release notes intact. - Advances the release-prep docs metadata from v0.0.38 to v0.0.39. - Regenerates `.agents/skills/nemoclaw-user-*` from the updated source docs. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [x] `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.39 * **New Features** * Host alias management commands for easier configuration * Sandbox GPU control options during onboarding * Update command with check and confirmation modes * **Documentation** * Enhanced Linux installer guidance with Docker and group membership handling * Expanded troubleshooting for permission and connectivity issues * Improved capability-dropping security documentation * Updated inference model switching commands * Brev environment-specific troubleshooting * **Improvements** * DGX Spark/Station express install flow * Windows Ollama relay and health-check enhancements * NVIDIA NIM preflight GPU reporting [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3375) <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
On DGX Spark and DGX Station, install.sh now offers an "express install" that auto-configures local inference and applies the suggested security policy after the user has accepted the third-party software notice. The license prompt is unbundled from express — the user is shown the third-party notice first, types
yesto accept it, and only then sees the express prompt. Skipped silently on other platforms; users who already set--non-interactiveorNEMOCLAW_PROVIDERare unaffected.Changes
scripts/install.sh:detect_express_platform()reads/sys/class/dmi/id/product_name(DMI) with a/sys/firmware/devicetree/base/modelfallback. Echoes "DGX Spark", "DGX Station", or empty.maybe_offer_express_install()skips silently when not on a supported platform. On Spark/Station, prints the skip reason if--non-interactive,NEMOCLAW_PROVIDER, or no TTY would block the prompt; otherwise asks:Run express install (auto-configures inference and applies suggested security policy)? [Y/n].NEMOCLAW_NON_INTERACTIVE=1,NEMOCLAW_YES=1,NEMOCLAW_POLICY_MODE=suggested, plusNEMOCLAW_PROVIDER=install-ollama+NEMOCLAW_MODEL=qwen3.6:35bfor Spark, orNEMOCLAW_PROVIDER=install-vllmfor Station. Does not auto-accept the third-party notice; that's handled by the existing preflight prompt.main()sopreflight_usage_notice_promptruns beforemaybe_offer_express_install. The user always sees and explicitly accepts the third-party notice (or pre-accepts via env var) before the express question.test/install-preflight.test.ts:runInstallerWithTty(and its piped/interactive wrappers) now setsNEMOCLAW_NO_EXPRESS=1in the test env so the existing license-flow tests stay focused on what they're verifying when run on real Spark/Station hardware.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: zyang-dev 267119621+zyang-dev@users.noreply.github.com
Summary by CodeRabbit
New Features
Tests