Skip to content

fix(cli): stabilize platform vitest on macOS and WSL#5209

Merged
cv merged 4 commits into
mainfrom
codex/fix-platform-vitest-main
Jun 11, 2026
Merged

fix(cli): stabilize platform vitest on macOS and WSL#5209
cv merged 4 commits into
mainfrom
codex/fix-platform-vitest-main

Conversation

@cv

@cv cv commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR stabilizes the platform Vitest main-watch workflow across macOS and WSL. It fixes the Docker-driver gateway identity false positive, makes the gateway marker regression portable to macOS Bash, keeps direct oclif command execution aligned with translated argv, and prevents oclif from filling missing CLI positionals from stray stdin on WSL.

Changes

  • Match openshell-gateway process identity by command token or exact executable path instead of substring checks.
  • Preserve and restore process.argv around direct oclif command-id execution so parse/error helpers see translated native argv.
  • Mark the failing credentials reset <provider> and policy-add [preset] positionals as ignoreStdin so WSL probe output cannot become a provider or preset argument.
  • Keep the in-container gateway marker regression test portable on macOS by using a harmless non-empty step-down prefix.
  • Allow the E2E fixture timeout assertion to accept Darwin's reported SIGTERM when bounded cleanup still succeeds.
  • Add focused tests for gateway process identity, direct oclif argv restoration, and poisoned-stdin CLI regressions.

Type of Change

  • 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)

Verification

  • npx vitest run src/lib/onboard/docker-driver-gateway-runtime.test.ts test/nemoclaw-start-gateway-marker.test.ts src/lib/cli/oclif-runner.test.ts test/cli/credentials-command.test.ts test/cli/sandbox-mutations.test.ts test/cli-oclif-compatibility.test.ts test/e2e-scenario/support-tests/e2e-fixture-context.test.ts test/e2e-scenario/support-tests/e2e-shell-supervisor.test.ts --testTimeout 60000 passes
  • npm test -- --testTimeout 60000 passes
  • npx prek run --all-files passes
  • npm run typecheck:cli passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Ensure CLI process argv is restored after running commands to prevent state leakage.
  • New Features

    • Added shared gateway-process identification utilities for more reliable gateway detection across hosts.
  • Tests

    • Expanded and strengthened tests for CLI behavior and gateway detection; relaxed a flaky timeout assertion.
    • Added a test helper to run CLI commands with supplied stdin and broadened input-related coverage.
  • Behavior

    • CLI argument parsing updated to avoid consuming stdin during parsing.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Jun 11, 2026
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5d564862-ae45-480a-b0aa-1c1f133f13b3

📥 Commits

Reviewing files that changed from the base of the PR and between 887cec0 and 831e46b.

📒 Files selected for processing (3)
  • src/lib/onboard/docker-driver-gateway-runtime.ts
  • src/lib/onboard/gateway-process-identity.ts
  • src/lib/onboard/host-gateway-process.ts

📝 Walkthrough

Walkthrough

Overrides process.argv to a native layout while running oclif commands (restoring it after), centralizes gateway cmdline matching with token/path checks, adds ignoreStdin and a runWithInput test helper, and includes several test reliability fixes.

Changes

Process Argv Management, Gateway Detection, and Test Reliability

Layer / File(s) Summary
CLI Process Argv Override and Restoration
src/lib/cli/oclif-runner.ts, src/lib/cli/oclif-runner.test.ts
runOclifCommandById() saves and replaces process.argv with a native layout derived from commandId/args before invoking the oclif command and restores it in a finally block. Tests validate argv observed by the command and that argv is restored after completion.
Gateway Binary Matching with Token-Based Helpers
src/lib/onboard/gateway-process-identity.ts, src/lib/onboard/docker-driver-gateway-runtime.ts, src/lib/onboard/docker-driver-gateway-runtime.test.ts, src/lib/onboard/host-gateway-process.ts
Adds token-cleaning and cmdline-matching helpers (gatewayProcessCmdlineMatches, normalization, name/resolve/mount-path rules). Runtime uses these helpers instead of substring checks; tests add negative and docker-compat positive cases.
ignoreStdin and stdin-driven Test Helpers
src/commands/credentials/reset.ts, src/lib/sandbox/policy-command-support.ts, test/cli/helpers.ts, test/cli/credentials-command.test.ts, test/cli/sandbox-mutations.test.ts
Add ignoreStdin: true to relevant oclif Arg definitions, introduce runWithInput test helper to pipe stdin into CLI runs, and add tests ensuring required-arg validation still fails (and does not consume stdin) when stdin contains extraneous data.
Bash Array Fix for Deterministic Test Execution
test/nemoclaw-start-gateway-marker.test.ts
Set STEP_DOWN_PREFIX_GATEWAY to (env) instead of an empty array to avoid empty-array expansion interaction with nounset on macOS Bash 3.2.
E2E Probe Timeout Signal Relaxation
test/e2e-scenario/support-tests/e2e-fixture-context.test.ts
Relax final timeout escalation assertion to accept SIGKILL or SIGTERM or any non-zero exitCode; add comments about Darwin signal reporting differences.

Sequence Diagram(s):

sequenceDiagram
  participant Runner as runOclifCommandById
  participant Process as process
  participant OclifConfig as OclifConfig
  Runner->>Process: save original argv
  Runner->>Process: set argv [execPath, CLI_NAME, ...nativeArgs]
  Runner->>OclifConfig: config.runCommand(commandId, args)
  OclifConfig-->>Runner: result or throw
  Runner->>Process: restore original argv (finally)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5116: Related gateway marker/launch test behavior and marker-placement assertions.

Suggested labels

area: cli, bug-fix, area: e2e

Suggested reviewers

  • laitingsheng
  • sandl99
  • yimoj

Poem

🐇 I hopped through argv, neat and spry,
I set it native, watched it fly.
Tokens scrubbed, the gateway true,
Stdin ignored — tests pass through.
Bash arrays fixed, the garden's bright. 🌷

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main objective: stabilizing Vitest on macOS and WSL through CLI and process handling fixes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-platform-vitest-main

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas
Since last review: 1 prior item resolved, 0 still apply, 0 new items found

Consider writing more tests for
  • **Runtime validation** — Verify `sandbox policy add` with no sandbox name ignores poisoned stdin for the required sandboxName positional and reports oclif's missing-argument error.. The added unit and subprocess tests are focused and cover the main regressions. Because the touched surfaces include CLI dispatch, process identity heuristics, and gateway/runtime cleanup paths, a few behavior-specific runtime checks would further increase confidence, but no missing test is a blocker based on the current diff.
  • **Runtime validation** — Verify `sandbox policy remove` with a missing optional preset ignores poisoned stdin and takes the same non-interactive missing-preset path as policy add.. The added unit and subprocess tests are focused and cover the main regressions. Because the touched surfaces include CLI dispatch, process identity heuristics, and gateway/runtime cleanup paths, a few behavior-specific runtime checks would further increase confidence, but no missing test is a blocker based on the current diff.
  • **Runtime validation** — Verify `runOclifCommandById` restores `process.argv` when an oclif parse error takes the injected `exit()` branch.. The added unit and subprocess tests are focused and cover the main regressions. Because the touched surfaces include CLI dispatch, process identity heuristics, and gateway/runtime cleanup paths, a few behavior-specific runtime checks would further increase confidence, but no missing test is a blocker based on the current diff.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard-e2e, diagnostics-e2e, network-policy-e2e, gateway-drift-preflight-e2e
Optional E2E: sandbox-operations-e2e, sandbox-survival-e2e, macos-e2e, credential-migration-e2e

Dispatch hint: cloud-onboard-e2e,diagnostics-e2e,network-policy-e2e

Auto-dispatched E2E: cloud-onboard-e2e, diagnostics-e2e, network-policy-e2e via nightly-e2e.yaml at 831e46be03a6490106d8806498a40fdfd3b21955nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard-e2e (medium): Exercises a real install/onboard path with Docker-driver OpenShell gateway startup and sandbox creation, covering the changed gateway process identity/reuse path in src/lib/onboard.
  • diagnostics-e2e (medium): Includes live credentials list/reset coverage; required because credentials reset mutates OpenShell gateway providers and the PR changes provider arg parsing behavior.
  • network-policy-e2e (medium): Validates policy mutation plus sandbox network enforcement. The touched policy-command-support arg parsing is part of the network security boundary.
  • gateway-drift-preflight-e2e (low-medium): Focused regression coverage for host-process/Docker-driver gateway drift and identity checks; directly relevant to the new shared gateway process argv0 matcher and false-positive process matching fixes.

Optional E2E

  • sandbox-operations-e2e (medium-high): Useful broad confidence for public sandbox CLI routes after the oclif process.argv change, including lifecycle operations that may depend on command dispatch correctness.
  • sandbox-survival-e2e (medium): Additional confidence for gateway stop/start and recovery behavior after changing gateway process identity and host gateway process matching.
  • macos-e2e (high): Optional platform confidence because the PR touches Docker-driver gateway runtime behavior with Darwin-specific marker/VM-driver logic and adjusts macOS-sensitive test expectations.
  • credential-migration-e2e (medium): Adjacent credential gateway coverage if maintainers want more assurance that credential provider storage/migration still interoperates with provider reset/list flows.

New E2E recommendations

  • CLI stdin poisoning for oclif positional args (medium): The PR fixes stdin-derived positional argument poisoning for credentials reset and policy-add at unit/CLI-test level, but there is no existing live E2E that pipes probe output into installed nemoclaw commands and verifies gateway providers/policies are not mutated.
    • Suggested test: Add a focused installed-CLI E2E that pipes hostile stdin into nemoclaw credentials reset --yes and nemoclaw <sandbox> policy-add and asserts required-arg/non-interactive failures occur before provider or policy mutation.
  • Docker compatibility gateway process identity (high): Unit tests now cover accepting a docker run ... /opt/nemoclaw/openshell-gateway parent and rejecting suffix/later-argument false positives, but existing live E2E does not force the compatibility-container gateway parent path.
    • Suggested test: Add a gateway identity E2E that forces Docker-driver compatibility gateway launch, records the host-side parent PID, verifies reuse/drift detection recognizes only the container runtime argv0 plus distinct compat mount token, and verifies unrelated processes containing the same path are ignored.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: cloud-onboard-e2e,diagnostics-e2e,network-policy-e2e

@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: ubuntu-repo-cloud-openclaw
Optional Vitest E2E scenarios: ubuntu-repo-docker-post-reboot-recovery

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • ubuntu-repo-cloud-openclaw: Primary live-supported Ubuntu Docker/OpenClaw scenario. It exercises repo-current CLI onboarding through the oclif runner and Docker-driver gateway startup/health paths affected by the process-identity and host-gateway changes.
    • 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: Adjacent live-supported Docker/OpenClaw lifecycle scenario that runs status/recovery checks after a simulated container stop. Useful extra coverage for gateway/sandbox preservation behavior, but not the smallest primary target.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-docker-post-reboot-recovery

Relevant changed files

  • src/lib/cli/oclif-runner.ts
  • src/lib/onboard/docker-driver-gateway-runtime.ts
  • src/lib/onboard/gateway-process-identity.ts
  • src/lib/onboard/host-gateway-process.ts
  • src/lib/sandbox/policy-command-support.ts
  • src/commands/credentials/reset.ts
  • test/e2e-scenario/support-tests/e2e-fixture-context.test.ts

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27333702326
Target ref: 9237c268ec0457ce79d1a305efd9371dfac25674
Workflow ref: main
Requested jobs: diagnostics-e2e,network-policy-e2e,cloud-onboard-e2e,sandbox-survival-e2e
Summary: 4 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success
diagnostics-e2e ✅ success
network-policy-e2e ✅ success
sandbox-survival-e2e ✅ success

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv added the v0.0.64 Release target label Jun 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27334726578
Target ref: 887cec0118c808cb27fcc8170b5dfd0ba3e0de18
Workflow ref: main
Requested jobs: diagnostics-e2e,network-policy-e2e,cloud-onboard-e2e,sandbox-survival-e2e
Summary: 0 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ⚠️ cancelled
diagnostics-e2e ⚠️ cancelled
network-policy-e2e ⚠️ cancelled
sandbox-survival-e2e ⚠️ cancelled

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27334899286
Target ref: 887cec0118c808cb27fcc8170b5dfd0ba3e0de18
Workflow ref: main
Requested jobs: diagnostics-e2e,network-policy-e2e,cloud-onboard-e2e,sandbox-survival-e2e
Summary: 4 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success
diagnostics-e2e ✅ success
network-policy-e2e ✅ success
sandbox-survival-e2e ✅ success

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27336922398
Target ref: 831e46be03a6490106d8806498a40fdfd3b21955
Workflow ref: main
Requested jobs: cloud-onboard-e2e,diagnostics-e2e,network-policy-e2e
Summary: 3 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success
diagnostics-e2e ✅ success
network-policy-e2e ✅ success

@cv cv merged commit 4f86c46 into main Jun 11, 2026
63 checks passed
@cv cv deleted the codex/fix-platform-vitest-main branch June 11, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.64 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant