feat(onboard): add Windows MXC host qualification - #8236
Conversation
📝 WalkthroughWalkthroughThe change adds Windows/MXC host qualification contracts, Windows build parsing, ordered host checks, structured results, and Vitest coverage for supported and rejected host conditions. ChangesWindows MXC qualification
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 `@src/lib/onboard/windows-mxc/host-qualification.test.ts`:
- Around line 14-28: Update the boundary tests in the Windows MXC host
qualification suite, including the exact-floor and below-floor cases, to use
literal build values: 26100 must qualify and 26099 must be rejected. Keep
implementation-derived constants only where they do not define the tested
boundary, and ensure expected windowsBuild values are also literal so changes to
WINDOWS_MXC_PROCESS_CONTAINER_MINIMUM_BUILD cannot make the tests pass
spuriously.
In `@src/lib/onboard/windows-mxc/host-qualification.ts`:
- Around line 9-12: Update WindowsMxcHostFacts and the onboarding qualification
flow so facts is required and architecture is populated from the native Windows
host architecture rather than process.arch. Ensure qualification rejects an x64
Node.js process running on an ARM64 host, and add a regression test covering
this emulation scenario.
🪄 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: e380899d-729e-4313-9366-4d18d7782732
📒 Files selected for processing (2)
src/lib/onboard/windows-mxc/host-qualification.test.tssrc/lib/onboard/windows-mxc/host-qualification.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
7 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 warning · 0 suggestionsWarningsWarnings do not block.
|
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 08d8115 in the TypeScript / code-coverage/cliThe overall coverage in commit 08d8115 in the Show a code coverage summary of the most impacted files.
Updated |
apurvvkumaria
left a comment
There was a problem hiding this comment.
Reviewed current head 21bb036. The new host-qualification contract is inactive and has no production caller or provider registration. It fails closed for non-Windows hosts, non-x64 native architecture, malformed release data, and builds below 26100; the focused 13-test suite passes, and current build/typecheck, CLI, static-analysis, CodeQL, and dependency-audit checks are green. No blocking correctness, security, compatibility, or regression issue found.
<!-- markdownlint-disable MD041 --> ## Summary Adds an inactive OpenShell MXC runtime-provider candidate for native Windows/OpenClaw work. The candidate consumes the merged host-qualification and native-artifact contracts, but remains absent from production selection and fails closed for every unqualified lifecycle or mutation surface. ## Related Issue Related to #8178. ## Changes - Extend the provider workload profile and persisted workload receipt to recognize the strict Windows/OpenClaw native-artifact contract from #8243 while making Docker reject that receipt. - Add one identity-consistent `mxc` provider bundle that reports candidate host facts through #8236 and accepts only validated native-artifact receipts. - Keep lifecycle, mutation authority, bootstrap, snapshot, recovery, cleanup, and container-engine operations typed as unsupported until the corresponding OpenShell contracts and protected live E2E pass. - Preserve native-artifact receipts through replacement bookkeeping and add registry, source-boundary, and fail-closed tests. This uses #7744's provider bundle rather than adding MXC branches to central orchestration. ## 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: MXC remains absent from `CURRENT_RUNTIME_PROVIDER_BUNDLES`, has no production import or selection path, and exposes no CLI, configuration, workflow, or supported behavior. - [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: Exact-head nine-category security review PASS at `8e8a32b73bbc9938018a0980d2a99f0f74720f4e` against base `962f1c3bf3a1354a8cfdc2056e04a27828be0dc0`. The effective binary diff SHA-256 remains `157885993ef0731db6b77653e778b296d1f15b786431b64832e9da4e3aff42ca`, identical to the previously reviewed patch. MXC remains unregistered and unselectable; the strict native-receipt parser and Docker rejection are unchanged; lifecycle and mutation operations fail closed. The merge adds only current-main history outside the effective diff. No secrets, dependencies, network calls, privilege paths, credential handling, authentication, or cryptography changed. - [ ] 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: Reviewed exact base `0385c0423125abbb29aa6028877c3e5c34ef8b46` through head `8239b1ad312d3e8278f4945537b6264dba405dc2`. The immutable compare contains 14 source and test files with 469 insertions and 11 deletions; it contains no documentation or Fern files. `CURRENT_RUNTIME_PROVIDER_BUNDLES` still registers only Docker and Kubernetes, so MXC remains unregistered and unselectable. Issue #8178 explicitly sequences #8271 as an inactive provider slice and keeps activation and support documentation gated on later package contracts and protected Windows/MXC/OpenClaw live E2E. Changed comments, diagnostics, and behavior-oriented test titles have no writing findings. No user-facing command, configuration, workflow, default, error, or supported behavior changed. A docs build is not applicable. - Agent: Codex Desktop <!-- docs-review-head-sha: 8239b1a --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; `scripts/prepare-dgx-station-host.sh` is unchanged. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every published 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 — exact head `8e8a32b73`: 4 focused files and 64 tests passed. `npm run build:cli`, `npm run typecheck:cli`, `npm run validate:pr`, and `git diff --check` passed. The effective patch is byte-identical to the previously reviewed head. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not applicable because the provider is inactive and unregistered; protected CI is authoritative for the complete repository matrix. - [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: Senthil Ravichandran <senthilr@nvidia.com> --------- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Summary
Adds an inactive host-facts assessment for the native Windows/OpenShell MXC
process_containercandidate. The assessment fails closed for non-Windows hosts, unqualified architectures, malformed release data, and Windows builds below the candidate floor.Related Issue
Part of #8178
Changes
process_containercandidate required by [Epic] Support native Windows through OpenShell MXC #8178.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededDGX Station Hardware Evidence
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 --project cli src/lib/onboard/windows-mxc/host-qualification.test.tspassed 13 tests.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable to the isolated two-file source change.npm run checks:repositorypassed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Senthil Ravichandran senthilr@nvidia.com
Summary by CodeRabbit
New Features
Tests