Skip to content

Commit 111e0a4

Browse files
fix(onboard): extend dashboard readiness wait (#9089)
<!-- markdownlint-disable MD041 --> ## Summary Dashboard-forward startup now gives OpenShell's exact `sandbox is not ready` handoff up to 1 minute to settle. [Actions run 31767577321](https://github.com/NVIDIA/NemoClaw/actions/runs/31767577321) showed multiple onboarding lanes exhausting the ordinary three-retry allowance while the new sandbox was still transitioning to forward-ready state. Authentication, ownership, port-conflict, and ordinary listener failures keep their existing retry and cleanup behavior. ## Changes - Give the exact OpenShell readiness response a separate default of 12 retries with 5-second delays. - Preserve `maxRetries` as an explicit override for every retryable outcome. - Cover recovery after four consecutive readiness responses and the persistent 12-retry bound. - Update the OpenClaw and Hermes quickstarts with the readiness-specific wait. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: The repository security checklist covered the complete four-file diff. The longer wait remains limited to one exact OpenShell diagnostic and one minute; authentication, ownership, cleanup, and ordinary listener retry boundaries are unchanged. No security finding was identified. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Updated both quickstarts to describe the exact readiness response, 12 retries, 5-second intervals, and the 1-minute maximum readiness-specific delay. `npm run docs`, file-scoped hooks, and diff validation passed. - Agent: Codex documentation writer subagent <!-- docs-review-head-sha: 6cb3217 --> <!-- docs-review-agents-blob-sha: e30afb2 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; DGX Station preparation is unchanged. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — focused forward-start suite: 47 passed; affected test selection: 370 passed across 28 files - [x] Applicable broad gate passed — `npm run typecheck:cli`, `npm run docs`, source-shape and test-conditional validation, targeted repository hooks, and diff validation passed - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [x] `npm run docs` builds without warnings (doc changes only) — result: zero errors; two existing Fern notices - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved sandbox startup handoff reliability by retrying “sandbox is not ready” responses up to 12 times. - Added five-second delays between readiness retries, allowing up to one minute for the sandbox to become available. - Preserved existing sandbox and selected host-port settings throughout retries. - Kept standard retry behavior separate and unchanged. - **Documentation** - Updated quickstart guides to explain the extended sandbox-readiness retry behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
1 parent c1ed614 commit 111e0a4

4 files changed

Lines changed: 98 additions & 15 deletions

File tree

docs/get-started/quickstart-hermes.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,8 @@ Use these details when your first-run path needs more control.
286286
The onboard flow starts both port forwards automatically.
287287
For a new sandbox, NemoClaw reserves the selected dashboard loopback port through sandbox preparation and the image build.
288288
If another listener claims the port before NemoClaw binds the reservation, NemoClaw selects another port before changing sandbox resources.
289-
If OpenShell reports `sandbox is not ready`, NemoClaw waits 5 seconds and retries the affected forward up to three times.
290-
These retries preserve the existing sandbox and selected host port.
289+
If OpenShell returns the exact `sandbox is not ready` response, NemoClaw waits 5 seconds and retries the affected forward up to 12 times.
290+
The readiness-specific delays total at most 1 minute and preserve the existing sandbox and selected host port.
291291
NemoClaw releases the reservation immediately before OpenShell starts the dashboard forward.
292292
If forwarding then fails, onboarding removes the new sandbox and tells you to resolve the reported error before retrying.
293293
The Hermes dashboard URL does not include an OpenClaw `#token=` fragment.

docs/get-started/quickstart.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,8 +419,8 @@ Use these details when your first-run path needs more control.
419419
When that port is occupied, NemoClaw uses the next free dashboard port, such as `18790`, and includes the port in the URL.
420420
For a new sandbox, NemoClaw reserves the selected loopback port through sandbox preparation and the image build.
421421
If another listener claims the port before NemoClaw binds the reservation, NemoClaw selects another port before changing sandbox resources.
422-
If OpenShell reports `sandbox is not ready`, NemoClaw waits 5 seconds and retries the dashboard forward up to three times.
423-
These retries preserve the existing sandbox and selected port.
422+
If OpenShell returns the exact `sandbox is not ready` response, NemoClaw waits 5 seconds and retries the dashboard forward up to 12 times.
423+
The readiness-specific delays total at most 1 minute and preserve the existing sandbox and selected port.
424424
NemoClaw releases the reservation immediately before OpenShell starts the dashboard forward.
425425
If forwarding then fails, onboarding removes the new sandbox and tells you to resolve the reported error before retrying.
426426
The installation transcript does not print the gateway token.

src/lib/onboard/forward-start.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,21 @@ function forwardListWith(
2727
return [header, ...rows].join("\n");
2828
}
2929

30+
const SANDBOX_NOT_READY_FORWARD_DIAGNOSTIC = `Error: × code: 'The system is not in a state required for the operation's
31+
│ execution', message: "sandbox is not ready"
32+
`;
33+
34+
function readinessHandoffSpawn(rejections: number) {
35+
const diagnostics = [
36+
...Array<string>(rejections).fill(SANDBOX_NOT_READY_FORWARD_DIAGNOSTIC),
37+
"",
38+
];
39+
return vi.fn(({ stderr }: { stderr: number }) => {
40+
fs.writeSync(stderr, diagnostics.shift() ?? "");
41+
return {};
42+
});
43+
}
44+
3045
describe("runDetachedForwardStartWithDiagnostics", () => {
3146
it("returns ok as soon as the forward appears in the list", () => {
3247
const fetchList = vi
@@ -983,6 +998,59 @@ describe("runDetachedForwardStartWithRetries", () => {
983998
expect(events).toEqual(["spawn-1", "sleep-5000", "spawn-2"]);
984999
});
9851000

1001+
it("keeps retrying when four consecutive readiness handoffs are still settling", () => {
1002+
const spawn = readinessHandoffSpawn(4);
1003+
const fetchList = vi.fn(() =>
1004+
spawn.mock.calls.length >= 5
1005+
? forwardListWith([{ sandbox: "my-sandbox", port: 18789 }])
1006+
: forwardListWith([]),
1007+
);
1008+
const beforeRetry = vi.fn();
1009+
const sleep = vi.fn();
1010+
1011+
const result = runDetachedForwardStartWithRetries(
1012+
spawn,
1013+
fetchList,
1014+
{ port: 18789, sandboxName: "my-sandbox" },
1015+
beforeRetry,
1016+
{
1017+
sleepMs: sleep,
1018+
isPortListening: vi.fn().mockReturnValue(false),
1019+
},
1020+
);
1021+
1022+
expect(result.ok).toBe(true);
1023+
expect(result.reason).toBe("ok");
1024+
expect(beforeRetry).not.toHaveBeenCalled();
1025+
expect(spawn).toHaveBeenCalledTimes(5);
1026+
expect(sleep).toHaveBeenCalledTimes(4);
1027+
expect(sleep).toHaveBeenCalledWith(5_000);
1028+
});
1029+
1030+
it("stops after the independent sandbox readiness retry bound", () => {
1031+
const spawn = readinessHandoffSpawn(13);
1032+
const beforeRetry = vi.fn();
1033+
const sleep = vi.fn();
1034+
1035+
const result = runDetachedForwardStartWithRetries(
1036+
spawn,
1037+
vi.fn().mockReturnValue(forwardListWith([])),
1038+
{ port: 18789, sandboxName: "my-sandbox" },
1039+
beforeRetry,
1040+
{
1041+
sleepMs: sleep,
1042+
isPortListening: vi.fn().mockReturnValue(false),
1043+
},
1044+
);
1045+
1046+
expect(result.ok).toBe(false);
1047+
expect(result.reason).toBe("listener-start-failure");
1048+
expect(beforeRetry).not.toHaveBeenCalled();
1049+
expect(spawn).toHaveBeenCalledTimes(13);
1050+
expect(sleep).toHaveBeenCalledTimes(12);
1051+
expect(sleep).toHaveBeenCalledWith(5_000);
1052+
});
1053+
9861054
it("does not retry a composite authentication diagnostic that mentions readiness", () => {
9871055
let now = 0;
9881056
vi.spyOn(Date, "now").mockImplementation(() => now);

src/lib/onboard/forward-start.ts

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ export interface DetachedForwardStartOptions {
6161
onProgress?: (info: { elapsedMs: number; listSnapshot: string }) => void;
6262
progressIntervalMs?: number;
6363
// Number of retryable startup attempts after the initial attempt. Honoured
64-
// only by `runDetachedForwardStartWithRetries`. Defaults to 3.
64+
// only by `runDetachedForwardStartWithRetries`. An explicit value applies
65+
// to every retryable outcome. Ordinary failures default to 3 retries; exact
66+
// sandbox readiness handoffs use their own longer default below.
6567
maxRetries?: number;
6668
// Loopback port-liveness probe. Defaults to `probeLocalPortListening` (a
6769
// synchronous Node TCP connect to 127.0.0.1:port). The retry wrapper uses it
@@ -191,6 +193,11 @@ function blockingSleepMs(ms: number): void {
191193
// exposes an atomic recovery operation.
192194
const DEAD_FORWARD_GRACE_MS = 2_000;
193195
const SANDBOX_READY_RETRY_SETTLE_MS = 5_000;
196+
// A newly created sandbox can remain between OpenShell's create-ready and
197+
// forward-ready states longer than the ordinary listener retry budget. Keep
198+
// this exact-diagnostic path bounded to one additional minute without
199+
// widening retries for authentication, ownership, or listener failures.
200+
const SANDBOX_READY_MAX_RETRIES = 12;
194201

195202
/**
196203
* Build a `DetachedForwardSpawnRunner` that spawns the given argv as a
@@ -515,8 +522,10 @@ export function runDetachedForwardStartWithRetries(
515522
options: DetachedForwardStartOptions = {},
516523
): DetachedForwardStartOutcome {
517524
const maxRetries = options.maxRetries ?? 3;
525+
const maxSandboxReadyRetries = options.maxRetries ?? SANDBOX_READY_MAX_RETRIES;
518526
const sleepImpl = options.sleepMs ?? blockingSleepMs;
519527
let deadForwardRecoveryAvailable = true;
528+
let sandboxReadyRetries = 0;
520529
const isPortListening = options.isPortListening ?? probeLocalPortListening;
521530
const runAttempt = (): DetachedForwardStartOutcome =>
522531
isPortListening(expect.port)
@@ -534,20 +543,26 @@ export function runDetachedForwardStartWithRetries(
534543
deadForwardRecoveryAvailable = false;
535544
beforeRetryCleanup();
536545
} else {
537-
const isRetryableStandardFailure =
538-
(attempt.reason !== "listener-ownership-conflict" &&
539-
looksLikeForwardPortConflict(attempt.diagnostic)) ||
540-
attempt.reason === "listener-start-failure";
541-
if (!isRetryableStandardFailure || standardRetries >= maxRetries) break;
542-
if (looksLikeForwardPortConflict(attempt.diagnostic)) {
543-
beforeRetryCleanup();
544-
}
545-
if (looksLikeSandboxNotReadyForwardStart(attempt.diagnostic)) {
546+
const isSandboxReadinessHandoff =
547+
attempt.reason === "listener-start-failure" &&
548+
looksLikeSandboxNotReadyForwardStart(attempt.diagnostic);
549+
if (isSandboxReadinessHandoff) {
550+
if (sandboxReadyRetries >= maxSandboxReadyRetries) break;
546551
// Keep the existing sandbox and port ownership intact while the
547552
// OpenShell gateway finishes the readiness handoff.
548553
sleepImpl(SANDBOX_READY_RETRY_SETTLE_MS);
554+
sandboxReadyRetries++;
555+
} else {
556+
const isRetryableStandardFailure =
557+
(attempt.reason !== "listener-ownership-conflict" &&
558+
looksLikeForwardPortConflict(attempt.diagnostic)) ||
559+
attempt.reason === "listener-start-failure";
560+
if (!isRetryableStandardFailure || standardRetries >= maxRetries) break;
561+
if (looksLikeForwardPortConflict(attempt.diagnostic)) {
562+
beforeRetryCleanup();
563+
}
564+
standardRetries++;
549565
}
550-
standardRetries++;
551566
}
552567
attempt = runAttempt();
553568
}

0 commit comments

Comments
 (0)