Walkthrough

Adds a new Vitest live E2E test for sandbox operations (repo-CLI→Docker→OpenShell) with helper utilities and ordered contract assertions, plus a GitHub Actions job that runs the test (optional Docker Hub auth with retries and anonymous fallback), installs OpenShell, resolves OPENSHELL_BIN, uploads artifacts, and updates validators/tests to enforce the workflow shape.

Changes

Sandbox Operations E2E Test Coverage

Sequence Diagram(s)

sequenceDiagram
  participant CLI as NemoClaw CLI
  participant Docker as Docker Engine
  participant Gateway as OpenClaw Gateway Container
  participant OpenShell as OpenShell inside sandbox
  CLI->>Docker: create/start gateway & sandbox containers
  CLI->>Gateway: onboard agent / start OpenClaw agent
  Gateway->>OpenShell: expose shell & agent endpoints
  CLI->>OpenShell: exec commands, list/status/logs
  OpenShell->>Gateway: agent inference requests
  Gateway->>CLI: status/logs responses

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• NVIDIA/NemoClaw#5243: Related job-selector plumbing and selector gating that this new job builds upon.
• NVIDIA/NemoClaw#5152: Modifies workflow boundary validation/tests for added free-standing Vitest jobs.

Suggested labels

area: e2e, area: sandbox, area: ci

Suggested reviewers

• cv
• prekshivyas

Poem

🐰 In a sandbox where containers play,
CLI and agents hop throughout the day,
Logs stream, tmux twirls, recovery sings,
Registry rebuilt and the gateway springs,
A rabbit cheers — Vitest saves the day!

❌ Failed checks (1 warning)

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• None. No merge-blocking E2E is required because the PR is tests-only and cannot affect runtime/user flows unless the new test is wired into CI. The existing legacy sandbox-operations-e2e can be run optionally for migration confidence.

Optional E2E

• sandbox-operations-e2e (high; live Docker/OpenShell/NVIDIA_API_KEY flow, creates two sandboxes, job timeout 60 minutes): Useful non-blocking baseline because the new Vitest file states it is an anchor for the existing legacy sandbox operations script and covers the same live sandbox lifecycle/recovery contracts.

New E2E recommendations

• sandbox-lifecycle-e2e-coverage (high): The new free-standing live Vitest test is not currently present in the e2e-vitest-scenarios workflow allow-list or as a workflow job, so selective dispatch cannot run it by name. Add a dedicated workflow job/selector if this Vitest migration is intended to be executable in CI.
  • Suggested test: Add a sandbox-operations Vitest job in .github/workflows/e2e-vitest-scenarios.yaml that runs test/e2e-scenario/live/sandbox-operations.test.ts with the required OpenShell install, Docker, NVIDIA_API_KEY, OPENSHELL_GATEWAY, and artifact upload setup.

Dispatch hint

• Workflow: .github/workflows/nightly-e2e.yaml
• jobs input: sandbox-operations-e2e

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

• ubuntu-repo-cloud-openclaw: The PR adds a live Vitest sandbox-operations anchor for Ubuntu repo Docker cloud OpenClaw behavior. The new free-standing test ID is not a trusted-main live-supported typed registry scenario, so target the closest live-supported typed scenario that exercises the same OpenClaw onboarding, Docker sandbox, CLI, inference, and credential surface.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Optional Vitest E2E scenarios

• ubuntu-repo-docker-post-reboot-recovery: Optional adjacent coverage for the recovery/status side of the new sandbox-operations live test, using the existing live-supported post-reboot recovery typed scenario.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-docker-post-reboot-recovery

Relevant changed files

• test/e2e-scenario/live/sandbox-operations.test.ts

🛠️ Needs attention

• None.

🔎 Worth checking

• Source-of-truth review needed: parseOpenClawAgentText tolerant JSON parsing: The advisor marked localized patch analysis as needs_followup.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: `parseOpenClawAgentText` catches parse failures, scans later JSON objects, and collects many envelope keys.
• Source-of-truth review needed: TC-SBX-08 in-sandbox OpenClaw gateway process recovery: The advisor marked localized patch analysis as missing.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: `assertProcessRecovery` ignores the kill probe result and then accepts broad status text plus an echo sentinel.
• Source-of-truth review needed: TC-SBX-06 gateway container recovery soft-skips: The advisor marked localized patch analysis as needs_followup.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: `assertGatewayRecovery` returns early on several tolerance paths while the outer test writes `status: "passed"`.
• Source-of-truth review needed: Legacy import-stream-reset onboard resume retry: The advisor marked localized patch analysis as missing.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: The legacy script defines `resume_onboard_after_import_stream_reset`; the new helper performs one `nemoclaw onboard` call and expects exit zero.
• TC-SBX-06 gateway recovery soft-skips are recorded as full scenario success (test/e2e-scenario/live/sandbox-operations.test.ts:498): The gateway-container recovery helper returns early for several skip-like runner states, but the outer scenario always writes `scenario-result.json` with `status: "passed"`. That can make the migration claim the sandbox recovery contract was validated even when the destructive gateway recovery signal was absent or unobservable.
  • Recommendation: Return structured TC-SBX-06 evidence such as `asserted`, `skipped_gateway_absent`, `skipped_restarted_too_quickly`, or `skipped_no_restart_signal`, and include that state in the scenario result instead of reporting all tolerated paths as a full pass.
  • Evidence: `assertGatewayRecovery` returns when the gateway is absent, when `afterKill.stdout.trim() === "true"`, and when `afterStatus.stdout.trim() !== "true"`; later the test writes `{ id: "sandbox-operations", status: "passed" }`.
• TC-SBX-08 recovery check does not prove the OpenClaw gateway process was killed (test/e2e-scenario/live/sandbox-operations.test.ts:363): The process-recovery path sends a broad kill pipeline but ignores whether any OpenClaw gateway process was found or killed. A run with no matching target can still pass if status emits generic healthy/running text and a later sandbox exec echoes the sentinel, weakening security-sensitive recovery coverage.
  • Recommendation: Capture the gateway PID or process list before killing, assert that a target existed, verify that the PID disappeared or changed, and only then accept status plus post-recovery sandbox exec as recovery evidence. If no target exists, record a structured skip or partial result instead of full success.
  • Evidence: `assertProcessRecovery` awaits `execInSandbox(..., "tc-sbx-08-kill-openclaw-gateway")` without inspecting the result, then matches status against `/recover|running|healthy|OpenClaw/i` and checks only `echo process-recovery-ok`.
• Tolerant OpenClaw JSON parser can create false-positive TC-SBX-02 evidence (test/e2e-scenario/live/sandbox-operations.test.ts:164): `parseOpenClawAgentText` locally duplicates tolerant parsing for wrapper output, envelope drift, choices/message/delta fields, and later JSON objects after parse failures. Because it is only exercised indirectly by the live model path, parser drift can match unrelated metadata or wrapper text instead of the actual model reply.
  • Recommendation: Move the parser into a focused tested helper or add deterministic support tests for wrapper text before JSON, malformed earlier JSON followed by valid JSON, supported envelopes, choices.message and choices.delta fields, and a metadata-only `42` case that must not satisfy TC-SBX-02. Document the output-shape source boundary and removal condition.
  • Evidence: `parseOpenClawAgentText` catches `JSON.parse` failures, scans later JSON objects, and collects broad keys such as `text`, `content`, `reasoning_content`, `result`, `payload`, `messages`, `choices`, `data`, and `delta`.
• Migrated onboarding path omits the legacy import-stream-reset resume retry (test/e2e-scenario/live/sandbox-operations.test.ts:82): The legacy sandbox-operations script handled import stream resets and transient gateway/resume errors by retrying `nemoclaw onboard --resume`. The new Vitest helper performs one onboard call and immediately expects exit zero, reducing migration parity for the same gateway/import transport failure mode.
  • Recommendation: Either preserve the legacy resume behavior in the Vitest helper or add code-backed documentation and focused regression evidence proving that import stream reset/resume handling is no longer required for this scenario.
  • Evidence: The legacy script defines `resume_onboard_after_import_stream_reset`; `onboardSandbox` calls `host.nemoclaw([...onboard...])` once and then `expectExitZero(result, `nemoclaw onboard ${sandboxName}`)`.
• TC-SBX-05 destroy verification can pass if post-destroy list commands fail (test/e2e-scenario/live/sandbox-operations.test.ts:451): After destroying sandbox B, the test checks that NemoClaw and OpenShell list outputs do not contain the sandbox name, but it does not assert that either list command succeeded. A failed list command that omits the sandbox name could satisfy the absence check without proving cleanup.
  • Recommendation: Assert exit zero for the post-destroy `nemoclaw list` and `openshell sandbox list` commands before checking that the sandbox name is absent.
  • Evidence: `assertDestroyRemovesSandbox` runs `host.nemoclaw(["list"])` and `sandbox.list()`, then only checks `outputContainsSandbox(...).toBe(false)` for each result.

🌱 Nice ideas

• None.

• **Runtime validation** — parseOpenClawAgentText parses wrapper text before JSON and malformed earlier JSON followed by valid JSON. The change is test-only and uses real Docker/OpenShell/sandbox boundaries, but several recovery and tolerant parsing paths need deterministic or structured evidence to avoid false positives.
• **Runtime validation** — parseOpenClawAgentText extracts reply text from result/payload/messages and choices.message/choices.delta envelopes. The change is test-only and uses real Docker/OpenShell/sandbox boundaries, but several recovery and tolerant parsing paths need deterministic or structured evidence to avoid false positives.
• **Runtime validation** — parseOpenClawAgentText does not satisfy TC-SBX-02 from metadata-only 42 outside the model reply. The change is test-only and uses real Docker/OpenShell/sandbox boundaries, but several recovery and tolerant parsing paths need deterministic or structured evidence to avoid false positives.
• **Runtime validation** — assertGatewayRecovery records asserted vs skipped_gateway_absent vs skipped_restarted_too_quickly vs skipped_no_restart_signal in scenario-result.json. The change is test-only and uses real Docker/OpenShell/sandbox boundaries, but several recovery and tolerant parsing paths need deterministic or structured evidence to avoid false positives.
• **Runtime validation** — assertProcessRecovery fails or records a structured skip when no OpenClaw gateway PID is found before kill. The change is test-only and uses real Docker/OpenShell/sandbox boundaries, but several recovery and tolerant parsing paths need deterministic or structured evidence to avoid false positives.
• **Acceptance clause:** Refs Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 — add test evidence or identify existing coverage. The deterministic context did not include linked issue Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 or any issue comments, so no trusted linked-issue clauses were available to verify.
• **Acceptance clause:** Migrate `test/e2e/test-sandbox-operations.sh` with equivalent live Vitest coverage. — add test evidence or identify existing coverage. The new file maps TC-SBX-01 through TC-SBX-11, but parity gaps remain for import-stream-reset resume, TC-SBX-06 structured skip/pass evidence, TC-SBX-08 killed-process proof, and TC-SBX-05 list-command exit checks.
• **Acceptance clause:** Legacy assertion: TC-SBX-02 connect/chat computes 6×7 through OpenClaw — add test evidence or identify existing coverage. `assertAgentCanAnswer` runs `openclaw agent --json` and expects `42`, but the broad tolerant parser lacks focused negative tests to prove it is matching the model reply rather than wrapper or metadata content.

Current findings:

• Source-of-truth review needed: parseOpenClawAgentText tolerant JSON parsing: The advisor marked localized patch analysis as needs_followup.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: `parseOpenClawAgentText` catches parse failures, scans later JSON objects, and collects many envelope keys.
• Source-of-truth review needed: TC-SBX-08 in-sandbox OpenClaw gateway process recovery: The advisor marked localized patch analysis as missing.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: `assertProcessRecovery` ignores the kill probe result and then accepts broad status text plus an echo sentinel.
• Source-of-truth review needed: TC-SBX-06 gateway container recovery soft-skips: The advisor marked localized patch analysis as needs_followup.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: `assertGatewayRecovery` returns early on several tolerance paths while the outer test writes `status: "passed"`.
• Source-of-truth review needed: Legacy import-stream-reset onboard resume retry: The advisor marked localized patch analysis as missing.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: The legacy script defines `resume_onboard_after_import_stream_reset`; the new helper performs one `nemoclaw onboard` call and expects exit zero.
• TC-SBX-06 gateway recovery soft-skips are recorded as full scenario success (test/e2e-scenario/live/sandbox-operations.test.ts:498): The gateway-container recovery helper returns early for several skip-like runner states, but the outer scenario always writes `scenario-result.json` with `status: "passed"`. That can make the migration claim the sandbox recovery contract was validated even when the destructive gateway recovery signal was absent or unobservable.
  • Recommendation: Return structured TC-SBX-06 evidence such as `asserted`, `skipped_gateway_absent`, `skipped_restarted_too_quickly`, or `skipped_no_restart_signal`, and include that state in the scenario result instead of reporting all tolerated paths as a full pass.
  • Evidence: `assertGatewayRecovery` returns when the gateway is absent, when `afterKill.stdout.trim() === "true"`, and when `afterStatus.stdout.trim() !== "true"`; later the test writes `{ id: "sandbox-operations", status: "passed" }`.
• TC-SBX-08 recovery check does not prove the OpenClaw gateway process was killed (test/e2e-scenario/live/sandbox-operations.test.ts:363): The process-recovery path sends a broad kill pipeline but ignores whether any OpenClaw gateway process was found or killed. A run with no matching target can still pass if status emits generic healthy/running text and a later sandbox exec echoes the sentinel, weakening security-sensitive recovery coverage.
  • Recommendation: Capture the gateway PID or process list before killing, assert that a target existed, verify that the PID disappeared or changed, and only then accept status plus post-recovery sandbox exec as recovery evidence. If no target exists, record a structured skip or partial result instead of full success.
  • Evidence: `assertProcessRecovery` awaits `execInSandbox(..., "tc-sbx-08-kill-openclaw-gateway")` without inspecting the result, then matches status against `/recover|running|healthy|OpenClaw/i` and checks only `echo process-recovery-ok`.
• Tolerant OpenClaw JSON parser can create false-positive TC-SBX-02 evidence (test/e2e-scenario/live/sandbox-operations.test.ts:164): `parseOpenClawAgentText` locally duplicates tolerant parsing for wrapper output, envelope drift, choices/message/delta fields, and later JSON objects after parse failures. Because it is only exercised indirectly by the live model path, parser drift can match unrelated metadata or wrapper text instead of the actual model reply.
  • Recommendation: Move the parser into a focused tested helper or add deterministic support tests for wrapper text before JSON, malformed earlier JSON followed by valid JSON, supported envelopes, choices.message and choices.delta fields, and a metadata-only `42` case that must not satisfy TC-SBX-02. Document the output-shape source boundary and removal condition.
  • Evidence: `parseOpenClawAgentText` catches `JSON.parse` failures, scans later JSON objects, and collects broad keys such as `text`, `content`, `reasoning_content`, `result`, `payload`, `messages`, `choices`, `data`, and `delta`.
• Migrated onboarding path omits the legacy import-stream-reset resume retry (test/e2e-scenario/live/sandbox-operations.test.ts:82): The legacy sandbox-operations script handled import stream resets and transient gateway/resume errors by retrying `nemoclaw onboard --resume`. The new Vitest helper performs one onboard call and immediately expects exit zero, reducing migration parity for the same gateway/import transport failure mode.
  • Recommendation: Either preserve the legacy resume behavior in the Vitest helper or add code-backed documentation and focused regression evidence proving that import stream reset/resume handling is no longer required for this scenario.
  • Evidence: The legacy script defines `resume_onboard_after_import_stream_reset`; `onboardSandbox` calls `host.nemoclaw([...onboard...])` once and then `expectExitZero(result, `nemoclaw onboard ${sandboxName}`)`.
• TC-SBX-05 destroy verification can pass if post-destroy list commands fail (test/e2e-scenario/live/sandbox-operations.test.ts:451): After destroying sandbox B, the test checks that NemoClaw and OpenShell list outputs do not contain the sandbox name, but it does not assert that either list command succeeded. A failed list command that omits the sandbox name could satisfy the absence check without proving cleanup.
  • Recommendation: Assert exit zero for the post-destroy `nemoclaw list` and `openshell sandbox list` commands before checking that the sandbox name is absent.
  • Evidence: `assertDestroyRemovesSandbox` runs `host.nemoclaw(["list"])` and `sandbox.list()`, then only checks `outputContainsSandbox(...).toBe(false)` for each result.