test(cli): trim remaining subprocess waits

[cv]

Summary

This PR trims the next slow subprocess-heavy CLI target after #4913 by removing avoidable test-time waits from sandbox connect route repair and the stuck-sandbox timeout fixture. In the profiled six-file slow bucket, wall-clock runtime dropped from 28.07s to 12.14s locally.

Related Issue

Part of #4892

Changes

• Skip sleepSync route-repair backoff waits when CLI subprocesses run under Vitest or NEMOCLAW_TEST_NO_SLEEP=1, matching the existing inference probe test-wait convention.
• Keep the route-repair production retry contract intact; the unit tests still assert the real 3-attempt, 2s-delay probe options.
• Reduce the stuck Provisioning subprocess fixture from a 3s connect timeout to the shortest accepted 1s timeout.

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 prek run --all-files passes
• npm test 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

• Tests
  • Improved test execution performance by skipping sleep operations during test runs.
  • Reduced test execution time for sandbox connectivity validation tests.

[coderabbitai]

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: 5a034b01-8295-4583-a436-3c59a8253062

📥 Commits

Reviewing files that changed from the base of the PR and between da08dde and d903610.

📒 Files selected for processing (2)

• src/lib/actions/sandbox/connect.ts
• test/sandbox-stuck-recovery.test.ts

---

📝 Walkthrough

Walkthrough

This PR optimizes test execution time for the sandbox connect path by skipping artificial sleep delays in test environments and reducing the timeout override in a specific readiness test, while preserving the polling and error-guidance behavior being tested.

Changes

Sandbox connect test performance

Layer / File(s)	Summary
Skip sleep in test environments src/lib/actions/sandbox/connect.ts	sleepSync helper now early-returns without spawning a timer when VITEST is true or NEMOCLAW_TEST_NO_SLEEP is set, preventing artificial delays during test execution.
Reduce sandbox readiness timeout in test test/sandbox-stuck-recovery.test.ts	The stuck Provisioning test reduces NEMOCLAW_CONNECT_TIMEOUT from "3" to "1" second, shortening wall time while exercising the same non-terminal readiness polling logic and timeout guidance.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Suggested labels

area: cli, area: performance, chore

Suggested reviewers

• prekshivyas
• cjagwani

Poem

🐰 Tests race faster now, no sleep required!
Timeouts trim down, same checks—all wired.
Sandbox readiness dances quicker still,
Vitest and a flag, performance thrill. 🚀

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 'test(cli): trim remaining subprocess waits' accurately reflects the main change: optimizing test execution by removing avoidable waits in CLI subprocess tests.
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/cli-runtime-targets

---

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

[github-actions]

E2E Advisor Recommendation

Required E2E: issue-2478-crash-loop-recovery-e2e, sandbox-operations-e2e
Optional E2E: sandbox-survival-e2e, inference-routing-e2e

Dispatch hint: issue-2478-crash-loop-recovery-e2e,sandbox-operations-e2e

Auto-dispatched E2E: issue-2478-crash-loop-recovery-e2e, sandbox-operations-e2e via nightly-e2e.yaml at d90361061500bf0e1d1f43edf7822607eb337530 — nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• issue-2478-crash-loop-recovery-e2e (medium): Best targeted existing E2E for nemoclaw <sandbox> connect --probe-only recovery behavior. It kills the in-sandbox gateway and verifies probe-only connect recovers the gateway and preserves inference readiness, covering the recovery/probe path adjacent to the changed connect timing helper.
• sandbox-operations-e2e (high): Broad sandbox lifecycle smoke for real connect/user-flow behavior. It exercises live sandbox operations including connect/chat, status, logs, and lifecycle edge cases, providing merge-blocking confidence for changes in the connect action implementation.

Optional E2E

• sandbox-survival-e2e (medium): Useful adjacent confidence for gateway restart/survival behavior after onboard, inference, and gateway stop/start. This overlaps with connect recovery surfaces but is less directly targeted than the crash-loop recovery test.
• inference-routing-e2e (medium): Optional because the changed helper is used by inference route probing/repair during connect. Run if maintainers want extra confidence that connect-time route validation still preserves inference.local behavior.

New E2E recommendations

• stuck-sandbox-readiness-guidance (medium): The touched unit test covers a sandbox stuck in Provisioning and terminal Failed-state guidance using fakes, but there does not appear to be a dedicated existing E2E that creates or simulates a real non-Ready OpenShell sandbox phase and verifies nemoclaw <name> connect timeout/recovery guidance end-to-end.
  • Suggested test: Add a focused sandbox readiness/stuck-phase E2E that forces or stubs a real OpenShell sandbox into Provisioning/Failed-like states, then verifies connect timeout, terminal-state fast-fail, and guidance text without waiting for the default 120s timeout.

Dispatch hint

• Workflow: nightly-e2e.yaml
• jobs input: issue-2478-crash-loop-recovery-e2e,sandbox-operations-e2e

[github-actions]

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

• None. No scenario E2E jobs recommended: the change is limited to the sandbox connect command and a non-scenario unit/integration test, and the scenario suite/catalog under test/e2e-scenario does not exercise the connect command surface.

Optional scenario E2E

• None.

Relevant changed files

• None.

[github-actions]

PR Review Advisor

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

Review findings

🛠️ Needs attention

• None.

🔎 Worth checking

• Source-of-truth review needed: src/lib/actions/sandbox/connect.ts sleepSync test-runtime bypass for sandbox connect route repair: 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: `connect.ts:233` adds the no-sleep env bypass; `ci/env-var-doc-allowlist.json:35` still documents `NEMOCLAW_TEST_NO_SLEEP` only for onboard inference probes.
• Document broadened no-sleep sentinel (src/lib/actions/sandbox/connect.ts:233): The change broadens the existing `NEMOCLAW_TEST_NO_SLEEP`/`VITEST` sleep bypass into sandbox connect route-repair backoff, but the source-of-truth env-var allowlist still describes `NEMOCLAW_TEST_NO_SLEEP` as applying only to onboard inference probes. This leaves future maintainers without a clear explanation of why route repair uses the same production-readable test sentinel, what invalid state it handles, and when the workaround can be removed.
  • Recommendation: Update `ci/env-var-doc-allowlist.json` to mention sandbox connect route repair, and/or add a nearby `connect.ts` comment explaining the test-runtime invalid state, why an injected sleeper/source fix is not used here, the regression coverage, and the removal condition.
  • Evidence: `connect.ts:233` now returns early from route-repair `sleepSync` for `process.env.VITEST === "true" || process.env.NEMOCLAW_TEST_NO_SLEEP === "1"`; `ci/env-var-doc-allowlist.json:35` still says `NEMOCLAW_TEST_NO_SLEEP` bypasses sleep calls in onboard inference probes only. This was also raised in the previous advisor review and remains unaddressed.

🌱 Nice ideas

• None.

Consider writing more tests for

• **Runtime validation** — Connect route repair performs all three post-repair route probes when `VITEST=true` skips the 2s backoff.. The production change is in a sandbox lifecycle route-repair path. Existing tests cover route-repair behavior and the shortened stuck-sandbox timeout fixture, but direct timing-boundary coverage would improve confidence that the test sentinel does not alter retry contracts outside Vitest.
• **Runtime validation** — Connect route repair still passes `{ attempts: 3, delayMs: 2000 }` for VM monkeypatch, legacy DNS repair, and managed route reset paths.. The production change is in a sandbox lifecycle route-repair path. Existing tests cover route-repair behavior and the shortened stuck-sandbox timeout fixture, but direct timing-boundary coverage would improve confidence that the test sentinel does not alter retry contracts outside Vitest.
• **Runtime validation** — The connect route-repair sleep bypass is limited to `VITEST=true` or `NEMOCLAW_TEST_NO_SLEEP=1`, with non-test mode preserving the configured delay through an injectable sleeper or focused unit seam.. The production change is in a sandbox lifecycle route-repair path. Existing tests cover route-repair behavior and the shortened stuck-sandbox timeout fixture, but direct timing-boundary coverage would improve confidence that the test sentinel does not alter retry contracts outside Vitest.
• **Runtime validation** — Stuck Provisioning connect with `NEMOCLAW_CONNECT_TIMEOUT=1` still enters the readiness wait and prints timeout guidance.. The production change is in a sandbox lifecycle route-repair path. Existing tests cover route-repair behavior and the shortened stuck-sandbox timeout fixture, but direct timing-boundary coverage would improve confidence that the test sentinel does not alter retry contracts outside Vitest.
• **Acceptance clause:** Part of ci: safely split slow CLI coverage suites #4892 — add test evidence or identify existing coverage. No linked issue body or issue comments for ci: safely split slow CLI coverage suites #4892 were provided in the deterministic context, so issue-level acceptance clauses could not be mapped literally.
• **src/lib/actions/sandbox/connect.ts sleepSync test-runtime bypass for sandbox connect route repair** — Existing route-repair tests indirectly run under `VITEST=true`, and unit coverage asserts at least one `{ attempts: 3, delayMs: 2000 }` path. There is no direct assertion that the bypass is limited to `VITEST=true` or `NEMOCLAW_TEST_NO_SLEEP=1`, nor that non-test mode preserves real delay behavior.. `connect.ts:233` adds the no-sleep env bypass; `ci/env-var-doc-allowlist.json:35` still documents `NEMOCLAW_TEST_NO_SLEEP` only for onboard inference probes.

Since last review details

Current findings:

• Source-of-truth review needed: src/lib/actions/sandbox/connect.ts sleepSync test-runtime bypass for sandbox connect route repair: 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: `connect.ts:233` adds the no-sleep env bypass; `ci/env-var-doc-allowlist.json:35` still documents `NEMOCLAW_TEST_NO_SLEEP` only for onboard inference probes.
• Document broadened no-sleep sentinel (src/lib/actions/sandbox/connect.ts:233): The change broadens the existing `NEMOCLAW_TEST_NO_SLEEP`/`VITEST` sleep bypass into sandbox connect route-repair backoff, but the source-of-truth env-var allowlist still describes `NEMOCLAW_TEST_NO_SLEEP` as applying only to onboard inference probes. This leaves future maintainers without a clear explanation of why route repair uses the same production-readable test sentinel, what invalid state it handles, and when the workaround can be removed.
  • Recommendation: Update `ci/env-var-doc-allowlist.json` to mention sandbox connect route repair, and/or add a nearby `connect.ts` comment explaining the test-runtime invalid state, why an injected sleeper/source fix is not used here, the regression coverage, and the removal condition.
  • Evidence: `connect.ts:233` now returns early from route-repair `sleepSync` for `process.env.VITEST === "true" || process.env.NEMOCLAW_TEST_NO_SLEEP === "1"`; `ci/env-var-doc-allowlist.json:35` still says `NEMOCLAW_TEST_NO_SLEEP` bypasses sleep calls in onboard inference probes only. This was also raised in the previous advisor review and remains unaddressed.

Workflow run details

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

[github-actions]

Selective E2E Results — ✅ All requested jobs passed

Run: 27097648858
Target ref: eaed713221de1bed0d7a763a4e24b096aae1db4f
Workflow ref: main
Requested jobs: sandbox-operations-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job	Result
sandbox-operations-e2e	✅ success

[github-actions]

Selective E2E Results — ✅ All requested jobs passed

Run: 27104357446
Target ref: d90361061500bf0e1d1f43edf7822607eb337530
Workflow ref: main
Requested jobs: issue-2478-crash-loop-recovery-e2e,sandbox-operations-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job	Result
issue-2478-crash-loop-recovery-e2e	✅ success
sandbox-operations-e2e	✅ success