fix(sandbox): align recreate readiness-wait budget with connect (120s)

[yanyunl1991]

Problem (#7227)

start's post-recreate recovery waits for OpenShell to re-register a just-recreated sandbox as Ready, then gives up after 30s and surfaces the manual-recover hint. connect's readiness wait (waitForSandboxReadyOrExit) proves the same post-recreate sandbox readiness but allows 120s (NEMOCLAW_CONNECT_TIMEOUT).

On a cold start the sandbox can sit in a transient phase: Error settling window that exceeds 30s but is comfortably within 120s. When it does, the recovery path abandons the sandbox and prints the manual-recover hint, while connect --probe-only — with the larger budget — recovers the exact same sandbox and starts the primary dashboard/API forward. That 4x budget asymmetry is the intermittent start/recovery failure in #7227.

Change

• Introduce a single GATEWAY_RECOVERY_WAIT_DEFAULT_SECONDS = 120 and use it for both readiness-wait default sites (waitForRecreatedSandboxOpenShellReadyResult and waitForRecoveredSandboxGateway), aligning them with connect.
• NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS (existing env override) is unchanged.
• Deliberately untouched because they are different concepts: the agent health-probe timeout (30s) and the post-ready settle pause (NEMOCLAW_GATEWAY_RECOVERY_SETTLE_SECONDS, 25s).

Verification / honesty note

This is a low-probability cold-start race. On latest main it reproduced 1 in 12 start cycles on a DGX Spark and could not be re-triggered deterministically afterward (0 in a further 20 warm + cold retries). Because the timing is non-deterministic, this PR is a principled budget alignment (match the two paths that prove the same readiness) rather than an empirically-timed value — I could not produce a green/red toggle that flips solely on the 30→120 change.

Given that, keeping #7227 open to confirm over repeated cold starts is reasonable; hence Refs rather than Fixes.

Tests

• New unit test locks the default: with no timeoutSeconds option and no env override, a persistent transient phase: Error is retried past the old 11-attempt (30s) cap — the 12th probe must still be reached. Would fail under the old 30s default.
• Existing process-recovery suite unchanged and green (42 tests). typecheck:cli and prek clean.

Refs #7227

Signed-off-by: Yanyun Liao yanyunl@nvidia.com

Summary by CodeRabbit

• Bug Fixes
  • Improved gateway recovery reliability by extending the default readiness window from 30 to 120 seconds.
  • Recovery checks now continue through longer periods of temporary OpenShell errors before reporting failure.

[coderabbitai]

📝 Walkthrough

Walkthrough

Recovery readiness now uses a shared 120-second default instead of 30 seconds for recreated-sandbox OpenShell checks and recovered gateway waits. A test verifies readiness succeeds after eleven transient failures and a successful twelfth probe.

Changes

Gateway recovery budget

Layer / File(s)	Summary
Shared recovery timeout default src/lib/actions/sandbox/process-recovery.ts	Introduces a 120-second recovery default and applies it to recreated-sandbox readiness and recovered gateway timeout calculations while retaining environment overrides.
Extended transient retry coverage src/lib/actions/sandbox/process-recovery.test.ts	Adds coverage for eleven transient OpenShell failures followed by a successful twelfth probe using the default recovery budget.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• NVIDIA/NemoClaw#7650: Updates related recreated-sandbox OpenShell transient-error retry behavior and test coverage.

Suggested labels: area: sandbox, bug-fix

Suggested reviewers: cv, sandl99

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	It clearly summarizes the main change: increasing sandbox recreate readiness-wait defaults to match connect's 120s budget.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ 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 fix/7227-recreate-readiness-wait-budget

---

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

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 7d06310 in the fix/7227-recreate-re... branch remains at 96%, unchanged from commit 125ca30 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 7d06310 in the fix/7227-recreate-re... branch remains at 81%, unchanged from commit cf34b72 in the main branch.

Show a code coverage summary of the most impacted files.

File	main cf34b72	fix/7227-recreate-re... 7d06310	+/-
src/lib/onboard...box-prebuild.ts	92%	74%	-18%
src/lib/actions...ocker-health.ts	82%	65%	-17%
src/lib/actions...confirmation.ts	79%	69%	-10%
src/lib/onboard...box-gpu-mode.ts	97%	92%	-5%
src/lib/actions...light-guards.ts	90%	86%	-4%
src/lib/onboard/preflight.ts	80%	80%	0%
src/lib/onboard...host-anchors.ts	90%	94%	+4%
src/lib/policy/...ne-exclusion.ts	91%	97%	+6%
src/lib/domain/.../connect-env.ts	89%	97%	+8%
src/lib/onboard/docker-cdi.ts	70%	80%	+10%

---

_{Updated July 29, 2026 05:22 UTC}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/lib/actions/sandbox/process-recovery.test.ts (1)

112-147: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover the separately changed recovered-gateway default.

This regression test only exercises waitForRecreatedSandboxOpenShellReady. Add a behavioral test where waitForRecoveredSandboxGateway also survives the old 30-second/11-attempt budget; its existing immediate-success test would pass with either default.

🤖 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 `@src/lib/actions/sandbox/process-recovery.test.ts` around lines 112 - 147,
Extend the recovery tests with a behavioral case for
waitForRecoveredSandboxGateway that omits timeoutSeconds and the environment
override, returns transient failures for at least 11 probes, then succeeds, and
verifies the 12th probe is reached and recovery succeeds. Keep the existing
immediate-success test unchanged and mirror the default-budget setup used by
waitForRecreatedSandboxOpenShellReady.

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 `@src/lib/actions/sandbox/process-recovery.test.ts`:
- Line 118: Update the test setup around the
NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS environment variable to save its prior
value, then restore that exact value in a finally block after the test
completes, including when assertions or setup fail. Preserve the existing test
behavior while preventing process.env state from leaking into later tests.

---

Nitpick comments:
In `@src/lib/actions/sandbox/process-recovery.test.ts`:
- Around line 112-147: Extend the recovery tests with a behavioral case for
waitForRecoveredSandboxGateway that omits timeoutSeconds and the environment
override, returns transient failures for at least 11 probes, then succeeds, and
verifies the 12th probe is reached and recovery succeeds. Keep the existing
immediate-success test unchanged and mirror the default-budget setup used by
waitForRecreatedSandboxOpenShellReady.

🪄 Autofix (Beta)

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: 472a454e-2d75-4af5-bd40-3b88b9b6d210

📥 Commits

Reviewing files that changed from the base of the PR and between eeab81c and 7d06310.

📒 Files selected for processing (2)

• src/lib/actions/sandbox/process-recovery.test.ts
• src/lib/actions/sandbox/process-recovery.ts

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Restore the previous environment value.

This test mutates process.env directly and never restores the prior value, so later tests can become order-dependent. Save the value and restore it in a finally block.

Proposed fix

+    const previousRecoveryWait = process.env.NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS;
     delete process.env.NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS;
+    try {
       // existing assertions
+    } finally {
+      if (previousRecoveryWait === undefined) {
+        delete process.env.NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS;
+      } else {
+        process.env.NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS = previousRecoveryWait;
+      }
+    }

🤖 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 `@src/lib/actions/sandbox/process-recovery.test.ts` at line 118, Update the
test setup around the NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS environment
variable to save its prior value, then restore that exact value in a finally
block after the test completes, including when assertions or setup fail.
Preserve the existing test behavior while preventing process.env state from
leaking into later tests.

Source: Path instructions

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 1 warning · 0 suggestions
• Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 more warning, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: onboard-repair, onboard-resume

1 optional E2E recommendation

• sandbox-operations

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[jyaunches]

@yanyunl1991 if you continue working on this, consider a wider refactoring for timeout handling