fix(e2e): align Windows MXC process container harness - #8388
Conversation
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
📝 WalkthroughWalkthroughThe change updates Windows MXC process-container helpers and semantic-phase validation. It removes gateway selection from copied environments, normalizes parenthesized revisions, changes sandbox creation arguments, and rejects sandbox creation commands that request in-sandbox execution. ChangesWindows MXC validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 07995f6 in the TypeScript / code-coverage/cliThe overall coverage in commit 07995f6 in the Show a code coverage summary of the most impacted files.
Updated |
senthilr-nv
added
chore
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
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: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/e2e/support/windows-mxc-openclaw-process-container.test.ts (1)
140-142: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd boundary cases for the parenthesized hexadecimal suffix.
The test covers only the 7-character form. It does not verify the 40-character limit or reject 6-character, 41-character, and non-hex suffixes. Add these cases.
As per path instructions, review tests for behavioral confidence rather than implementation lock-in. Based on the PR objective, the accepted suffix length is 7–40 hexadecimal characters.
Proposed test additions
+ expect( + normalizeReportedVersion( + "OpenClaw 2026.7.1 (0123456789abcdef0123456789abcdef01234567)\n", + ), + ).toBe("2026.7.1"); + expect(normalizeReportedVersion("OpenClaw 2026.7.1 (2d2ddc)\n")).toBeNull(); + expect( + normalizeReportedVersion( + "OpenClaw 2026.7.1 (0123456789abcdef0123456789abcdef012345678)\n", + ), + ).toBeNull(); + expect(normalizeReportedVersion("OpenClaw 2026.7.1 (2d2ddcZ)\n")).toBeNull();🤖 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/e2e/support/windows-mxc-openclaw-process-container.test.ts` around lines 140 - 142, Extend the normalizeReportedVersion tests to cover parenthesized hexadecimal suffix boundaries: accept valid 7- and 40-character suffixes, and return null for 6-character, 41-character, and non-hex suffixes. Keep the assertions focused on the documented 7–40 hexadecimal character behavior.Source: Path instructions
test/e2e/support/e2e-semantic-phase-check.test.ts (1)
77-102: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the
runOpenShellCommandcall shape.Both fixtures call
runCommand.validateWindowsMxcControlBoundarySourceuses a different argument position forrunOpenShellCommand. The production helper usesrunOpenShellCommandfor sandbox creation at Line 1164. A regression in that branch can pass these tests. UserunOpenShellCommandin one fixture and keeprunCommandin the other.As per path instructions, review tests for behavioral confidence rather than implementation lock-in. Based on the supplied production path,
runOpenShellCommandis the call form used by sandbox creation.Proposed fixture change
- await runCommand(cli, ["sandbox", "create", "--", "exit"], env, progress, "create"); + await runOpenShellCommand(["sandbox", "create", "--", "exit"], "create");🤖 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/e2e/support/e2e-semantic-phase-check.test.ts` around lines 77 - 102, Update the two tests in validateWindowsMxcControlBoundarySource coverage so one fixture invokes runOpenShellCommand with the sandbox-create arguments and the other continues using runCommand. Preserve both expected validation errors and the existing scenarios, ensuring the alternate helper’s argument position is exercised without changing production behavior.Source: Path instructions
🤖 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 `@test/e2e/live/windows-mxc-openclaw-process-container-helpers.ts`:
- Around line 431-432: Update the environment isolation logic around the
isolated copy to remove every key whose lowercase form is openshell_gateway,
regardless of casing, while preserving OPENSHELL_GATEWAY_CONFIG. In
test/e2e/live/windows-mxc-openclaw-process-container-helpers.ts:431-432, apply
the case-insensitive key filtering; in
test/e2e/support/windows-mxc-openclaw-process-container.test.ts:299-306, add
coverage using a non-empty mixed-case selector and verify the config key
remains.
---
Nitpick comments:
In `@test/e2e/support/e2e-semantic-phase-check.test.ts`:
- Around line 77-102: Update the two tests in
validateWindowsMxcControlBoundarySource coverage so one fixture invokes
runOpenShellCommand with the sandbox-create arguments and the other continues
using runCommand. Preserve both expected validation errors and the existing
scenarios, ensuring the alternate helper’s argument position is exercised
without changing production behavior.
In `@test/e2e/support/windows-mxc-openclaw-process-container.test.ts`:
- Around line 140-142: Extend the normalizeReportedVersion tests to cover
parenthesized hexadecimal suffix boundaries: accept valid 7- and 40-character
suffixes, and return null for 6-character, 41-character, and non-hex suffixes.
Keep the assertions focused on the documented 7–40 hexadecimal character
behavior.
🪄 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: a078bef0-37d9-4c07-a484-ee21323993b1
📒 Files selected for processing (4)
test/e2e/live/windows-mxc-openclaw-process-container-helpers.tstest/e2e/support/e2e-semantic-phase-check.test.tstest/e2e/support/windows-mxc-openclaw-process-container.test.tstools/e2e/check-semantic-phases.mts
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security review — exact head 06c607f38d46bb5b0b584c2032c944b138d4d3f0 against base c6713c9dd17a9682654713ccb6829b8714f1b307: PASS with no findings, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. No credential source or value is added. The harness retains the existing host-process allowlist, keeps the readiness token out of command arguments and source literals, and removes only the gateway selector from the isolated child environment.
-
Input Validation and Data Sanitization — PASS. Reported OpenClaw versions accept only an exact version with an optional parenthesized 7–40 character hexadecimal suffix. Boundary tests accept 7 and 40 characters and reject 6, 41, non-hex, and moving local aliases.
-
Authentication and Authorization — PASS. No identity, permission, or authorization behavior changes. The selected OpenShell gateway remains authoritative through isolated CLI state.
-
Dependencies and Third-Party Libraries — PASS. No dependency, package, image, or artifact identity changes. Exact artifact digests and source revisions remain mandatory.
-
Error Handling and Logging — PASS. The semantic guard fails closed when any
process_containercreate requests an in-sandbox command, including later create calls. No sensitive value is added to errors or logs. -
Cryptography and Data Protection — PASS. No cryptographic primitive, key handling, protected-data flow, or retention boundary changes.
-
Configuration and Security Headers — PASS. Gateway selection removal is case-insensitive for Windows while preserving
OPENSHELL_GATEWAY_CONFIG. No network policy, port, privilege, capability, or security-header setting changes. -
Security Testing — PASS. Focused E2E-support tests pass 38/38 on the exact head, including gateway-key casing, version boundaries, both OpenShell command helpers, and multiple-create bypass coverage. The semantic inventory passes 118 tests across 75 files. Fresh GitHub CI and protected E2E remain mandatory after the signed base refresh.
-
System Security — PASS. NemoClaw continues to use OpenShell as the control boundary and does not invoke
wxc-exec.exedirectly. The change removes unsupported in-sandbox exec from the inactive qualification flow and adds a repository guard against regression.
Files Reviewed
test/e2e/live/windows-mxc-openclaw-process-container-helpers.tstest/e2e/support/e2e-semantic-phase-check.test.tstest/e2e/support/windows-mxc-openclaw-process-container.test.tstools/e2e/check-semantic-phases.mts
Provenance and Required Gates
Issue #8178 explicitly permits the dormant, non-activating qualification harness while keeping native Windows support unapproved. The signed merge refresh preserves all three contributor commits and their Verified status. All four PR commits are Verified. The effective patch has raw SHA-256 b9904e26477bc3f23ac09265180a6e87429a0931746b28d5df141b06f91c824d and stable patch ID 89553648cbccf2cd9f45e4e39c99e670e5e6c369. Documentation writer review passed with no docs needed at this exact head and .agents tree 7320c9e331ddbcd9ff83e0fce9387a004e3bd1aa.
Fresh required CI, protected E2E, unresolved-review resolution, and independent exact-head approval remain mandatory. This review waives none of those gates.
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security revalidation — exact head 3b0222942507f1996198cbf04c6a431861d34000 against current base 1608281462923ff8282151c7be1a109ee262b4e9: PASS with no findings, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. The effective PR patch is unchanged from the prior exact review. The base refresh adds no overlapping secret, credential, environment, or sensitive-logging path.
-
Input Validation and Data Sanitization — PASS. The reviewed validation and command/path construction are unchanged. The new base delta does not alter an effective PR file.
-
Authentication and Authorization — PASS. No identity, ownership, permission, or authorization boundary changed in the PR patch or its integration with the refreshed base.
-
Dependencies and Third-Party Libraries — PASS. The PR's reviewed dependency and artifact identities are unchanged. Base PR #8387 adds verified llama.cpp GGUF acquisition in eight disjoint files and does not change this PR's dependency surface.
-
Error Handling and Logging — PASS. The reviewed fail-closed behavior and diagnostics are unchanged; the base refresh introduces no overlapping error or logging path.
-
Cryptography and Data Protection — PASS. The PR's reviewed cryptographic and protected-data behavior is unchanged. No new cross-patch data flow exists.
-
Configuration and Security Headers — PASS. The effective PR patch preserves its reviewed configuration, network, privilege, capability, port, and header boundaries.
-
Security Testing — PASS. Raw patch SHA-256 remains
b9904e26477bc3f23ac09265180a6e87429a0931746b28d5df141b06f91c824dand stable patch ID remains89553648cbccf2cd9f45e4e39c99e670e5e6c369. Documentation writer revalidation passed at this exact head with no docs change needed. Prior targeted test evidence remains applicable; fresh exact-head GitHub Actions is authoritative. -
System Security — PASS. The signed refresh is conflict-free, preserves the reviewed patch byte-for-byte, and introduces no new runtime trust-boundary interaction with the disjoint base delta.
Provenance and Required Gates
This exact-head revalidation supplements the full prior nine-category review at #8388 (review). It applies the current rubric from .agents tree 7320c9e331ddbcd9ff83e0fce9387a004e3bd1aa.
Fresh required CI, protected E2E, the exact documentation receipt, unresolved-review resolution, GitHub commit verification, and an independent exact-head approval remain mandatory. This review waives none of those gates.
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Verdict
Security review — exact head 07995f6254f4fac438d418e75f749bc99fe7367b against current base b148bc13ee53b7a1f612d07e42275ec732e71877: PASS with no findings, subject to every repository merge gate.
Findings Table
No findings.
Detailed Analysis
-
Secrets and Credentials — PASS. The effective PR patch is unchanged and adds no secret, credential source, environment exposure, or sensitive logging. The refreshed base introduces no overlapping credential path.
-
Input Validation and Data Sanitization — PASS. Reviewed validation, command construction, and path handling remain unchanged. The portable-profile base delta has no exact file overlap with this PR.
-
Authentication and Authorization — PASS. No identity, ownership, permission, or authorization boundary is widened by the PR patch or its integration with the refreshed base.
-
Dependencies and Third-Party Libraries — PASS. The PR's reviewed dependency and artifact identities are unchanged. Base PRs #8333 and #8376 add disjoint E2E artifact restoration and portable experimental onboarding behavior.
-
Error Handling and Logging — PASS. Reviewed fail-closed behavior and diagnostics remain unchanged; the signed integrations add no overlapping error or logging path.
-
Cryptography and Data Protection — PASS. Reviewed cryptographic and protected-data behavior remains unchanged, with no new cross-patch data flow.
-
Configuration and Security Headers — PASS. The effective PR patch preserves its reviewed configuration, network, privilege, capability, port, and header boundaries. The hidden portable profile does not bypass this PR's authority checks.
-
Security Testing — PASS. Raw patch SHA-256 remains
b9904e26477bc3f23ac09265180a6e87429a0931746b28d5df141b06f91c824dand stable patch ID remains89553648cbccf2cd9f45e4e39c99e670e5e6c369. After the intervening E2E artifact-boundary base change, the exact patch and affected base contracts pass 3 files and 72 tests. Exact-head documentation writer revalidation passed; fresh GitHub Actions is authoritative. -
System Security — PASS. Both signed refreshes are conflict-free, preserve the effective patch byte-for-byte, and introduce no new runtime trust-boundary interaction. For #7853 specifically, the portable profile supplies onboarding defaults but neither stages messaging credentials nor bypasses registry-driven credential-drift validation.
Provenance and Required Gates
The exact head preserves contributor history through signed merge commits and applies the current rubric from .agents tree 7320c9e331ddbcd9ff83e0fce9387a004e3bd1aa. This supplements the full prior nine-category review at #8388 (review).
Fresh required CI, protected E2E, the exact documentation receipt, unresolved-review resolution, GitHub commit verification, and an independent exact-head approval remain mandatory. This review waives none of those gates.
3 participants
Summary
Correct the inactive Windows MXC OpenClaw qualification harness for the current
process_containercontract. The harness now accepts OpenClaw's exact-version banner with a hexadecimal build suffix, relies on isolated CLI state instead of an empty gateway override, and creates the configured one-shot workload without requesting unsupported in-sandbox exec.Related Issue
Refs #8178
Changes
OPENSHELL_GATEWAYfrom the isolated process environment so the explicitly selected test gateway remains authoritative.process_containerworkload through OpenShell without a trailing exec command.Type of Change
Quality Gates
07995f6254f4fac438d418e75f749bc99fe7367bagainst baseb148bc13ee53b7a1f612d07e42275ec732e71877: fix(e2e): align Windows MXC process container harness #8388 (review)Documentation Writer Review
no-docs-neededDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHub — all 7 commits appear asVerified.pre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run --project e2e-support test/e2e/support/windows-mxc-openclaw-process-container.test.ts test/e2e/support/e2e-semantic-phase-check.test.tspassed 38 tests;npm run test:e2e-phases:checkcovered 118 tests across 75 files.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Senthil Ravichandran senthilr@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests