feat(cli): add N1x Express preview - #8903
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds deferred N1x FASTOS support across host detection, readiness qualification, managed vLLM selection, Express installation, onboarding behavior, tests, support matrices, and setup documentation. ChangesN1x FASTOS deferred preview
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Host as N1x FASTOS host
participant Installer as Express installer
participant Readiness as Readiness qualification
participant Onboarding as Managed vLLM onboarding
participant vLLM as N1X_PROFILE
Host->>Installer: Provide FastOS metadata and PCI identity
Installer->>Readiness: Evaluate N1x readiness
Readiness-->>Installer: Return deferred validation status
Installer->>Onboarding: Request explicit N1x preview
Onboarding->>vLLM: Select managed vLLM profile
vLLM-->>Onboarding: Use Qwen3.6-35B-A3B-NVFP4
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
🌿 Preview your docs: https://nvidia-preview-pr-8903.docs.buildwithfern.com/nemoclaw |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/lib/readiness/platform-qualification.ts (1)
367-406: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the N1x qualification block into a helper.
projectPlatformQualificationnow derives WSL, Station, Spark, and N1x state in one function body. The N1x block adds three derived values, a mutable status ladder, and two extra clauses toplatformSupported. A small pure helper that returns{ identity, qualified, status }would keep the projection readable and match the existingisQualifiedStationProfileandisQualifiedStationRuntimepattern in this file.Also note that
n1xIdentity &&on Line 370 is implied byinput.nvidiaPlatform === "n1x"on Line 371, becausen1xIdentityis true whenevernvidiaPlatform === "n1x". The conjunct is harmless but redundant.The behavior is correct as written. Treat this as deferrable.
🤖 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 `@src/lib/readiness/platform-qualification.ts` around lines 367 - 406, Extract the N1x derivation from projectPlatformQualification into a small pure helper returning identity, qualified, and status, following the existing isQualifiedStationProfile and isQualifiedStationRuntime pattern; use that result in platformSupported and evidence-related logic. While extracting, remove the redundant n1xIdentity conjunct from the n1xQualified expression, preserving the current behavior and treating this as a deferrable cleanup.src/lib/onboard/fatal-runtime-preflight.ts (1)
153-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a shared provider identifier for
"install-vllm".No shared constant or predicate currently defines this identifier. Add one and use it in the preflight and vLLM menu checks. Preserve the current matching semantics: the preflight uses an exact comparison, while the menu trims and lowercases the value.
🤖 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 `@src/lib/onboard/fatal-runtime-preflight.ts` at line 153, Define a shared provider identifier or predicate for the "install-vllm" value, then update the allowDeferredN1xManagedVllm check and the vLLM menu checks to use it. Preserve exact matching in the preflight path and trimming/lowercasing before matching in the menu path.Source: Coding guidelines
🤖 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 `@docs/inference/set-up-vllm.mdx`:
- Around line 48-49: Update the N1x documentation entry in the existing-server
section so the “Deferred preview” label applies only to Local vLLM managed
install/start flows, not to already-running local vLLM servers; retain the
experimental label for generic hosts.
In `@scripts/install.sh`:
- Line 3882: Replace the chained &&/|| length-check expression in the
surrounding vendor-device iteration with an explicit if condition that continues
when vendor, device, or pci_class exceeds 64 characters. Preserve the existing
filtering behavior while eliminating ShellCheck SC2015.
In `@src/lib/inference/nim.ts`:
- Line 47: Add "n1x" to ARM64_DGX_NIM_PLATFORMS and update the platform-label
selection used by the ARM64 NIM compatibility warning to provide a matching N1X
label. Extend the relevant tests to cover both the warning path and the
non-warning path for N1X hosts.
In `@src/lib/inference/platform-identity/n1x.test.ts`:
- Around line 36-41: Replace the conditional fixture chains with path-keyed
lookup tables: in src/lib/inference/platform-identity/n1x.test.ts lines 36-41,
update the readFile fixture to map vendor, device, and class values and retain
the unexpected-path throw fallback; in src/lib/inference/nim-n1x.test.ts lines
8-20, update withFirmwareModel to map absolute firmware paths and fall back to
the original fs.readFileSync for all other paths.
- Around line 92-101: Update the openFile assertion in the n1xFixture test to
verify that the flags argument includes the fs.constants.O_NOFOLLOW bit,
importing node:fs as needed, while preserving the existing path and descriptor
assertions.
In `@src/lib/readiness/platform-qualification.test.ts`:
- Around line 326-346: Update the `it.each` table in the N1x FastOS marker test
to include the expected `host.platform.n1x` capability state as explicit data
for each scenario. Replace the conditional mapping from `expectedStatus` in the
capability assertion with that table value, while keeping the qualification and
finding assertions unchanged.
---
Nitpick comments:
In `@src/lib/onboard/fatal-runtime-preflight.ts`:
- Line 153: Define a shared provider identifier or predicate for the
"install-vllm" value, then update the allowDeferredN1xManagedVllm check and the
vLLM menu checks to use it. Preserve exact matching in the preflight path and
trimming/lowercasing before matching in the menu path.
In `@src/lib/readiness/platform-qualification.ts`:
- Around line 367-406: Extract the N1x derivation from
projectPlatformQualification into a small pure helper returning identity,
qualified, and status, following the existing isQualifiedStationProfile and
isQualifiedStationRuntime pattern; use that result in platformSupported and
evidence-related logic. While extracting, remove the redundant n1xIdentity
conjunct from the n1xQualified expression, preserving the current behavior and
treating this as a deferrable cleanup.
🪄 Autofix
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: a04b6fbb-cfec-431f-98d7-b5a82237df95
📒 Files selected for processing (30)
ci/platform-matrix.jsondocs/get-started/prerequisites.mdxdocs/get-started/quickstart-hermes.mdxdocs/get-started/quickstart-langchain-deepagents-code.mdxdocs/get-started/quickstart.mdxdocs/inference/choose-inference-provider.mdxdocs/inference/set-up-vllm.mdxdocs/reference/commands.mdxdocs/reference/platform-support.mdxdocs/reference/system-readiness.mdxscripts/install.shsrc/lib/inference/nim-n1x.test.tssrc/lib/inference/nim.tssrc/lib/inference/platform-identity/n1x.test.tssrc/lib/inference/platform-identity/n1x.tssrc/lib/inference/vllm-models.test.tssrc/lib/inference/vllm-models.tssrc/lib/inference/vllm.tssrc/lib/onboard/fatal-runtime-preflight.test.tssrc/lib/onboard/fatal-runtime-preflight.tssrc/lib/onboard/vllm-menu.test.tssrc/lib/onboard/vllm-menu.tssrc/lib/readiness/host.tssrc/lib/readiness/onboard-admission.test.tssrc/lib/readiness/onboard-admission.tssrc/lib/readiness/platform-qualification.test.tssrc/lib/readiness/platform-qualification.tstest/detect-vllm-profile.test.tstest/install-express-n1x.test.tstest/onboard-selection-vllm.test.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
4 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: Manual-only E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/security/best-practices.mdx`:
- Around line 330-338: Update the N1x documentation in the surrounding
managed-vLLM availability section to explicitly identify N1x as preview-only and
outside the supported-platform set, note that physical Express E2E validation is
unavailable, and state that managed-vLLM requires explicit user intent after
identity qualification. Add these constraints to the existing table or
recommendation without changing availability behavior for other hosts.
🪄 Autofix
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: 36e55418-0b68-4baa-8901-d3a7dea232b3
📒 Files selected for processing (20)
ci/platform-matrix.jsondocs/inference/choose-inference-provider.mdxdocs/inference/choose-local-inference-server.mdxdocs/inference/set-up-nvidia-nim.mdxdocs/inference/set-up-vllm.mdxdocs/reference/platform-support.mdxdocs/security/best-practices.mdxscripts/install.shsrc/lib/inference/nim-n1x.test.tssrc/lib/inference/platform-identity/n1x.test.tssrc/lib/onboard/fatal-runtime-preflight.tssrc/lib/onboard/nim-image-compat-warning.test.tssrc/lib/onboard/provider-menu.test.tssrc/lib/onboard/provider-menu.tssrc/lib/onboard/setup-nim-flow.tssrc/lib/onboard/vllm-menu.test.tssrc/lib/onboard/vllm-menu.tssrc/lib/readiness/platform-qualification.test.tssrc/lib/readiness/platform-qualification.tstest/onboard-selection-vllm.test.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- src/lib/onboard/vllm-menu.test.ts
- src/lib/inference/platform-identity/n1x.test.ts
- ci/platform-matrix.json
- src/lib/onboard/vllm-menu.ts
- src/lib/onboard/fatal-runtime-preflight.ts
- docs/reference/platform-support.mdx
- docs/inference/set-up-vllm.mdx
- src/lib/inference/nim-n1x.test.ts
- test/onboard-selection-vllm.test.ts
- src/lib/readiness/platform-qualification.test.ts
- scripts/install.sh
Signed-off-by: San Dang <sdang@nvidia.com>
sandl99
added
platform: n1x
wscurran
added
area: cli
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
-
scripts/install.sh:3824: N1x detection compares GNUstat %Foutput with the English textregular file.%Fis locale-sensitive: the same regular file reportsNormale Dateiunderde_DE.utf8,fichierunderfr_FR.utf8, and通常ファイルunderja_JP.utf8. A valid N1x marker therefore fails qualification under non-English locales, and the installer omits the preview prompt. Make all marker metadata reads locale-invariant or use a numeric file-type check, including the descriptor and post-open checks at lines 3827, 3831, 3848, and 3852. Add a non-C-locale regression test that exercises metadata collection;test/install-express-n1x.test.ts:60currently supplies the English type directly and cannot detect this failure. -
docs/inference/set-up-vllm.mdx:355anddocs/get-started/quickstart.mdx:60state that an N1x user can decline or disable Express and continue to interactive provider selection. The Hermes and Deep Agents quickstarts make the same claim at lines 177 and 110. In the implemented flow,src/lib/onboard/fatal-runtime-preflight.ts:154evaluates readiness before provider selection and waiveshost.platform.n1x_validation_pendingonly whenNEMOCLAW_PROVIDERis already exactlyinstall-vllm. An N1x user who declines the prompt or sets onlyNEMOCLAW_NO_EXPRESS=1is blocked before selecting an agent or provider. Align the installer message and documentation with the accepted managed-vLLM-only preview boundary. Do not broaden the waiver to arbitrary providers without a product decision. Add regression evidence for the N1x decline andNEMOCLAW_NO_EXPRESS=1paths.
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Addressed both requested blockers on current head d757108: FastOS metadata collection now uses locale-invariant numeric stat file-mode data for the path, opened descriptor, and post-open comparisons; the installer now stops N1x decline and NEMOCLAW_NO_EXPRESS-only paths with exact managed-vLLM remediation instead of promising interactive provider selection. Added non-C-locale collection plus decline, disable, and explicit install-vllm bypass regressions. Installer suite 10/10, docs build (0 errors; 2 existing warnings), repository checks, ShellCheck, shfmt, Biome, CLI build/typecheck, and normal commit/pre-push hooks pass. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/get-started/quickstart-langchain-deepagents-code.mdx`:
- Around line 58-59: Restrict the interactive Express instructions to supported
non-N1x Express platforms rather than all non-N1x hosts. Update
docs/get-started/quickstart-langchain-deepagents-code.mdx lines 58-59 and
111-113 with this condition, and apply the same condition to NEMOCLAW_NO_EXPRESS
in docs/get-started/quickstart.mdx line 178.
In `@test/install-express-n1x.test.ts`:
- Around line 121-129: Update the test’s fake stat() validation to accept any
/proc/self/fd/* path for the -Lc argument, while preserving the existing trusted
marker alternative, locale check, and format-string check; do not require file
descriptor 9 specifically.
- Around line 32-41: Update the test case around runExpressPromptWithTty in
“stops before onboarding when the Deferred preview is declined” to use stdinMode
"tty" instead of "pipe", so the supplied “n\n” reaches the preview prompt and
exercises the user-decline path while preserving the existing assertions.
🪄 Autofix
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: 23493515-dddc-454e-b471-543afcec028d
📒 Files selected for processing (7)
docs/get-started/quickstart-hermes.mdxdocs/get-started/quickstart-langchain-deepagents-code.mdxdocs/get-started/quickstart.mdxdocs/inference/set-up-vllm.mdxdocs/reference/commands.mdxscripts/install.shtest/install-express-n1x.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/get-started/quickstart-hermes.mdx
- docs/inference/set-up-vllm.mdx
- scripts/install.sh
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Follow-up review fixes are now published at exact head
Validation: focused installer regression suite 10/10 passed after hooks; normal pre-commit and pre-push hooks passed (including repository checks, secret scan, CLI typecheck, and version sync); docs build remains 0 errors / 2 existing warnings. Independent docs review: no further documentation changes needed. |
|
Exact-head CI classification for This is the same OpenClaw cold-start runner signature independently reproduced minutes earlier on approved PR #8901 after all three of its direct startup producers passed. It is therefore shared infrastructure evidence rather than a branch-specific regression. The other red lane is the informational Nemotron advisor ( |
Manual N1x Express end-to-end test evidenceI ran Test scope
Physical host
Verified journey
Evidence boundaryThe transcript verifies the install, onboarding, managed-vLLM startup, sandbox creation, GPU access, policy application, and deployment health for the tested commit and host. It does not include a user inference request from the dashboard or TUI. The PR branch advanced after this run. Later N1x installer changes are not covered by this evidence. This manual test does not by itself change the Deferred status or establish product support. |
<!-- markdownlint-disable MD041 --> ## Summary Refresh the Deep Agents Code staging QA source from `af2a73f…` to PR #8941's merge commit, `ce96811…`. The rebuilt staging base now contains the `libssh2` `nemoclaw2` inventory required by current candidate images, so unrelated PRs such as #8903 no longer inherit this mismatch from `main`. ## Changes - Pin `STAGING_QA_SOURCE_SHA` to `ce96811ddb418ad01c040521a1fe912b5bcb405e`. - Update the workflow comment and exact integration assertion for the `nemoclaw2` contract. - Root cause: PR #8941 advanced the managed-image `libssh2` contract, while the staging QA source remained on the earlier `nemoclaw1` inventory. - Detection gap: the source test enforced the recorded SHA but could not establish image compatibility. The staging QA job detected the package mismatch and remains the integration evidence for this repair. ## Type of Change - [x] 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) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: This change only refreshes an internal staging QA source pin and its exact workflow assertion. It changes no CLI, public configuration, API, default, or supported user workflow. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Codex Desktop reviewed commit `7445324bf` across all nine security categories. The full commit SHA remains immutable, checkout credentials remain disabled, permissions remain `contents: read`, and identity failures still stop the job. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Independent review: PASS — the internal staging comment identifies the exact libssh2 inventory mismatch and the workflow assertion pins the same reviewed source; no supported user-facing behavior changes. - Agent: Codex Desktop <!-- docs-review-head-sha: 7445324 --> <!-- docs-review-agents-blob-sha: c4923a3 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable; `scripts/prepare-dgx-station-host.sh` is unchanged. - Result: Not applicable - Supporting evidence: Not applicable ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run --project integration test/managed-image-publication-workflow.test.ts` passed 19 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not applicable to this focused workflow pin update. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] 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: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated the staging quality-assurance workflow to use the latest approved source revision and description. * Improved consistency between staging validation settings and the intended candidate configuration. * **Tests** * Updated workflow validation expectations to reflect the revised staging source revision. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
The locale-sensitive N1x marker handling and preview-boundary documentation findings were addressed in later commits.
5 participants
Summary
Add a Deferred N1x Express preview that recognizes the qualified FastOS and PCI identity and offers one-host managed vLLM. N1x remains outside the supported-platform set until physical NemoClaw Express E2E validation is complete.
Related Issue
Fixes #8574
Changes
/etc/fastos-releasemarker and exact NVIDIA display PCI identity; focused identity and installer tests protect the boundary.nvidia/Qwen3.6-35B-A3B-NVFP4. A separate profile prevents N1x from inheriting DGX Spark cluster, fixed-catalog, llama.cpp, or NIM behavior; vLLM profile and menu tests protect the selection.nim-localis rejected before NIM setup while managed vLLM remains available.host.platform.supportedabsent and add a blocking pending-validation finding. Only explicit managed-vLLM preview intent can waive that finding after N1x identity qualifies; readiness and onboarding-admission tests protect the exception.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/get-started/prerequisites.mdx,docs/get-started/quickstart-hermes.mdx,docs/get-started/quickstart-langchain-deepagents-code.mdx,docs/get-started/quickstart.mdx,docs/inference/choose-inference-provider.mdx,docs/inference/choose-local-inference-server.mdx,docs/inference/set-up-nvidia-nim.mdx,docs/inference/set-up-vllm.mdx,docs/reference/commands.mdx,docs/reference/platform-support.mdx,docs/reference/system-readiness.mdx,docs/security/best-practices.mdx; synchronized source:ci/platform-matrix.jsonDGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run test/install-express-n1x.test.ts(10 passed), including non-C locale metadata collection plus N1x decline,NEMOCLAW_NO_EXPRESS=1, and explicitinstall-vllmbypass coverage.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable; this change does not modify the runtime-wide test harness or repository-wide validation configuration.npm run docsbuilds without warnings (doc changes only) — build passed with 0 errors and 2 pre-existing Fern warnings.Signed-off-by: San Dang sdang@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation