From d9d5aef787346c8282000da53771b7d5b49a0d16 Mon Sep 17 00:00:00 2001 From: Yimo Jiang Date: Thu, 2 Jul 2026 02:54:34 +0000 Subject: [PATCH 1/6] fix(onboard): debounce transient sandbox Error during readiness wait On a fresh onboard the OpenShell gateway can (re)start its supervisor session and re-register the just-created sandbox. During that window `openshell sandbox list` briefly reports the sandbox in Error phase before it flips to Ready (observed on DGX Spark, where the dashboard port fallback + supervisor restart race the sandbox bootstrap). The create/readiness waiter fast-failed on the first Error poll, turning that transient into a terminal onboard failure: Sandbox '' entered Error phase before it became ready (waited up to 1500s). Apply a bounded consecutive-Error debounce in waitForCreatedSandboxReadyWithTrace, mirroring the Docker GPU supervisor-reconnect path: tolerate a transient Error (default 30 polls / ~60s, env NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE) and recover if the sandbox flips to Ready, while still fast-failing (with full failure diagnostics) on sustained Error well before the readiness timeout. Callers can pass errorPhaseDebouncePolls: 1 to restore the original fast-fail behavior. Fixes #6043 Signed-off-by: Yimo Jiang --- src/lib/onboard/docker-gpu-patch.test.ts | 58 +++++++++++++++- src/lib/onboard/sandbox-readiness-tracing.ts | 69 +++++++++++++++++++- 2 files changed, 122 insertions(+), 5 deletions(-) diff --git a/src/lib/onboard/docker-gpu-patch.test.ts b/src/lib/onboard/docker-gpu-patch.test.ts index 2c67062fbbe..5937c7fcf4d 100644 --- a/src/lib/onboard/docker-gpu-patch.test.ts +++ b/src/lib/onboard/docker-gpu-patch.test.ts @@ -931,7 +931,7 @@ describe("docker-gpu-patch Error-phase diagnostics (#4316)", () => { expect(getSandboxFailurePhase("", "my-sandbox")).toBeNull(); }); - it("short-circuits the readiness wait when the sandbox enters Error phase", () => { + it("short-circuits the readiness wait when the sandbox enters Error phase (K=1 opt-out)", () => { const outputs = ["my-sandbox Provisioning 1s ago", "my-sandbox Error 3s ago"]; let i = 0; const runCaptureOpenshell = vi.fn(() => outputs[Math.min(i++, outputs.length - 1)]); @@ -940,11 +940,13 @@ describe("docker-gpu-patch Error-phase diagnostics (#4316)", () => { const ready = waitForCreatedSandboxReadyWithTrace({ sandboxName: "my-sandbox", // 600 / 2 = 300 readyAttempts. Without short-circuit we'd loop 300 - // times. With short-circuit we should bail out after the 2nd poll. + // times. With the K=1 (no-debounce) opt-out we bail out after the 2nd + // poll, preserving the original fast-fail intent. timeoutSecs: 600, runCaptureOpenshell, isSandboxReady, getSandboxFailurePhase, + errorPhaseDebouncePolls: 1, sleep, }); @@ -958,6 +960,58 @@ describe("docker-gpu-patch Error-phase diagnostics (#4316)", () => { expect(sleep).toHaveBeenCalledTimes(1); }); + it("recovers when a transient Error flips to Ready within the debounce window (#6043)", () => { + // DGX Spark repro: the gateway re-registers the just-created sandbox and + // `sandbox list` briefly reports Error before flipping to Ready. The + // default debounce must tolerate the transient rather than fast-failing. + const outputs = [ + "my-sandbox Provisioning 1s ago", + "my-sandbox Error 3s ago", + "my-sandbox Error 5s ago", + "my-sandbox Ready 7s ago", + ]; + let i = 0; + const runCaptureOpenshell = vi.fn(() => outputs[Math.min(i++, outputs.length - 1)]); + const sleep = vi.fn(); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: "my-sandbox", + timeoutSecs: 600, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + sleep, + }); + + expect(ready).toEqual({ ready: true, reason: "ready", failurePhase: null }); + expect(runCaptureOpenshell).toHaveBeenCalledTimes(4); + }); + + it("still fails terminally after sustained Error exceeds the debounce window (#6043)", () => { + const runCaptureOpenshell = vi.fn(() => "my-sandbox Error 3s ago"); + const sleep = vi.fn(); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: "my-sandbox", + timeoutSecs: 600, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + errorPhaseDebouncePolls: 3, + sleep, + }); + + expect(ready).toEqual({ + ready: false, + reason: "terminal_failure_phase", + failurePhase: "Error", + }); + // 3 consecutive Error polls trigger the terminal failure; the wait sleeps + // twice between the first three polls and stops before the full timeout. + expect(runCaptureOpenshell).toHaveBeenCalledTimes(3); + expect(sleep).toHaveBeenCalledTimes(2); + }); + it("short-circuits the supervisor-reconnect wait when the sandbox enters Error phase", () => { // Without the short-circuit, a patched container that crashes on startup // leaves users waiting the full 900s+ supervisor-reconnect timeout before diff --git a/src/lib/onboard/sandbox-readiness-tracing.ts b/src/lib/onboard/sandbox-readiness-tracing.ts index 0aeeb54208b..45d494209a6 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.ts @@ -1,10 +1,42 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { envInt } from "./env"; import { addTraceEvent, withDashboardReadinessTrace, withSandboxReadinessTrace } from "./tracing"; type RunCaptureOpenshell = (args: string[], options?: { ignoreError?: boolean }) => string; +export const SANDBOX_READY_ERROR_DEBOUNCE_ENV = "NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE"; + +// Consecutive Error-phase polls required before the create/readiness wait +// treats the phase as terminal. The readiness loop polls `openshell sandbox +// list` every 2 seconds, so the default of 30 tolerates ~60s of sustained +// Error before failing. +// +// Why debounce at all: on a fresh onboard the gateway may (re)start its +// supervisor session and re-register the just-created sandbox (observed on +// DGX Spark, where the dashboard port fallback + supervisor restart race the +// sandbox bootstrap — #6043). During that window `sandbox list` can briefly +// report the sandbox in Error phase before it flips to Ready. Fast-failing on +// the first Error poll turns that transient into a terminal onboard failure. +// The debounce mirrors the Docker GPU supervisor-reconnect path +// (docker-gpu-supervisor-reconnect.ts), which tolerates the same transient +// while the recreated GPU container reconnects. +// +// This does NOT hide terminal failures: a sandbox that stays in Error still +// fast-fails after the bounded debounce window (well before the full readiness +// timeout), and the caller still captures full failure diagnostics. +const SANDBOX_READY_ERROR_PHASE_DEFAULT_DEBOUNCE_POLLS = 30; + +export function getSandboxReadyErrorDebouncePolls( + env: Record = process.env, +): number { + return Math.max( + 1, + envInt(SANDBOX_READY_ERROR_DEBOUNCE_ENV, SANDBOX_READY_ERROR_PHASE_DEFAULT_DEBOUNCE_POLLS, env), + ); +} + export type CreatedSandboxReadinessResult = | { ready: true; reason: "ready"; failurePhase: null } | { ready: false; reason: "terminal_failure_phase"; failurePhase: string | null } @@ -81,6 +113,13 @@ export function waitForCreatedSandboxReadyWithTrace(options: { * timeout window before reporting "did not become ready" (#4316). */ getSandboxFailurePhase?: (output: string, sandboxName: string) => string | null; + /** + * Consecutive Error-phase polls required before the wait treats the phase as + * terminal. Defaults to {@link getSandboxReadyErrorDebouncePolls}. Pass 1 to + * restore the original fast-fail-on-first-Error behavior (used by callers + * that have already ruled out the transient supervisor-reconnect race). + */ + errorPhaseDebouncePolls?: number; sleep: (seconds: number) => void; }): CreatedSandboxReadinessResult { const { @@ -91,8 +130,14 @@ export function waitForCreatedSandboxReadyWithTrace(options: { getSandboxFailurePhase, sleep, } = options; + const errorPhaseDebouncePolls = + options.errorPhaseDebouncePolls == null || !Number.isFinite(options.errorPhaseDebouncePolls) + ? getSandboxReadyErrorDebouncePolls() + : Math.max(1, Math.trunc(options.errorPhaseDebouncePolls)); return withSandboxReadinessTrace(sandboxName, { timeout_seconds: timeoutSecs }, () => { const readyAttempts = Math.max(1, Math.ceil(timeoutSecs / 2)); + let consecutiveFailurePolls = 0; + let lastFailurePhase: string | null = null; for (let i = 0; i < readyAttempts; i++) { const list = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); if (isSandboxReady(list, sandboxName)) { @@ -101,12 +146,30 @@ export function waitForCreatedSandboxReadyWithTrace(options: { } const failurePhase = getSandboxFailurePhase?.(list, sandboxName) ?? null; if (failurePhase) { - addTraceEvent("terminal_failure_phase", { attempt: i + 1, failure_phase: failurePhase }); - return { ready: false, reason: "terminal_failure_phase", failurePhase }; + consecutiveFailurePolls += 1; + lastFailurePhase = failurePhase; + // Sustained Error is terminal; a transient Error while the gateway + // re-registers the sandbox recovers on a later poll (#6043). + if (consecutiveFailurePolls >= errorPhaseDebouncePolls) { + addTraceEvent("terminal_failure_phase", { + attempt: i + 1, + failure_phase: failurePhase, + consecutive_polls: consecutiveFailurePolls, + }); + return { ready: false, reason: "terminal_failure_phase", failurePhase }; + } + addTraceEvent("transient_failure_phase", { + attempt: i + 1, + failure_phase: failurePhase, + consecutive_polls: consecutiveFailurePolls, + debounce_polls: errorPhaseDebouncePolls, + }); + } else { + consecutiveFailurePolls = 0; } if (i < readyAttempts - 1) sleep(2); } - addTraceEvent("not_ready", { attempts: readyAttempts }); + addTraceEvent("not_ready", { attempts: readyAttempts, last_failure_phase: lastFailurePhase }); return { ready: false, reason: "timeout", failurePhase: null }; }); } From 435373d396e08947cc44992a9266789cb7fa23ee Mon Sep 17 00:00:00 2001 From: Yimo Jiang Date: Thu, 2 Jul 2026 05:22:12 +0000 Subject: [PATCH 2/6] fix(onboard): scope readiness Error debounce and add focused tests Address review feedback on the #6043 create/readiness Error debounce: - Scope the debounce to the transient "Error" phase only. "Failed" and "CrashLoopBackOff" are genuinely terminal and now fast-fail immediately instead of burning the debounce window (CodeRabbit r3510182513, advisor PRA-2). - Add a source-of-truth / removal-contract comment block mirroring docker-gpu-supervisor-reconnect.ts: invalid state, OpenShell sandbox-list cache boundary, why tolerated locally, regression evidence, removal condition (advisor PRA-3). - Move the readiness-wait tests into a focused sandbox-readiness-tracing.test.ts (out of the docker-gpu-patch.test.ts hotspot) and add direct env-contract coverage (default 30, override, empty/non-finite/NaN/Infinity fallback, clamp-to-1, fractional round/truncate), non-Error immediate-terminal, counter reset on flap, and a deterministic replay of the reporter's DGX Spark sandbox-list sequence through the real waiter (advisor PRA-4, PRA-5). Signed-off-by: Yimo Jiang --- src/lib/onboard/docker-gpu-patch.test.ts | 85 +----- .../onboard/sandbox-readiness-tracing.test.ts | 283 ++++++++++++++++++ src/lib/onboard/sandbox-readiness-tracing.ts | 70 +++-- 3 files changed, 337 insertions(+), 101 deletions(-) create mode 100644 src/lib/onboard/sandbox-readiness-tracing.test.ts diff --git a/src/lib/onboard/docker-gpu-patch.test.ts b/src/lib/onboard/docker-gpu-patch.test.ts index 5937c7fcf4d..936ed2faa1b 100644 --- a/src/lib/onboard/docker-gpu-patch.test.ts +++ b/src/lib/onboard/docker-gpu-patch.test.ts @@ -6,7 +6,7 @@ import os from "node:os"; import path from "node:path"; import { describe, expect, it, vi } from "vitest"; -import { getSandboxFailurePhase, isSandboxReady } from "../state/gateway"; +import { getSandboxFailurePhase } from "../state/gateway"; import { buildDockerGpuCloneRunArgs, buildDockerGpuCloneRunOptions, @@ -27,7 +27,6 @@ import { shouldApplyDockerGpuPatch, waitForOpenShellSupervisorReconnect, } from "./docker-gpu-patch"; -import { waitForCreatedSandboxReadyWithTrace } from "./sandbox-readiness-tracing"; function inspectFixture(): DockerContainerInspect { return { @@ -931,86 +930,8 @@ describe("docker-gpu-patch Error-phase diagnostics (#4316)", () => { expect(getSandboxFailurePhase("", "my-sandbox")).toBeNull(); }); - it("short-circuits the readiness wait when the sandbox enters Error phase (K=1 opt-out)", () => { - const outputs = ["my-sandbox Provisioning 1s ago", "my-sandbox Error 3s ago"]; - let i = 0; - const runCaptureOpenshell = vi.fn(() => outputs[Math.min(i++, outputs.length - 1)]); - const sleep = vi.fn(); - - const ready = waitForCreatedSandboxReadyWithTrace({ - sandboxName: "my-sandbox", - // 600 / 2 = 300 readyAttempts. Without short-circuit we'd loop 300 - // times. With the K=1 (no-debounce) opt-out we bail out after the 2nd - // poll, preserving the original fast-fail intent. - timeoutSecs: 600, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, - errorPhaseDebouncePolls: 1, - sleep, - }); - - expect(ready).toEqual({ - ready: false, - reason: "terminal_failure_phase", - failurePhase: "Error", - }); - expect(runCaptureOpenshell).toHaveBeenCalledTimes(2); - // Should not sleep after detecting the terminal phase. - expect(sleep).toHaveBeenCalledTimes(1); - }); - - it("recovers when a transient Error flips to Ready within the debounce window (#6043)", () => { - // DGX Spark repro: the gateway re-registers the just-created sandbox and - // `sandbox list` briefly reports Error before flipping to Ready. The - // default debounce must tolerate the transient rather than fast-failing. - const outputs = [ - "my-sandbox Provisioning 1s ago", - "my-sandbox Error 3s ago", - "my-sandbox Error 5s ago", - "my-sandbox Ready 7s ago", - ]; - let i = 0; - const runCaptureOpenshell = vi.fn(() => outputs[Math.min(i++, outputs.length - 1)]); - const sleep = vi.fn(); - - const ready = waitForCreatedSandboxReadyWithTrace({ - sandboxName: "my-sandbox", - timeoutSecs: 600, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, - sleep, - }); - - expect(ready).toEqual({ ready: true, reason: "ready", failurePhase: null }); - expect(runCaptureOpenshell).toHaveBeenCalledTimes(4); - }); - - it("still fails terminally after sustained Error exceeds the debounce window (#6043)", () => { - const runCaptureOpenshell = vi.fn(() => "my-sandbox Error 3s ago"); - const sleep = vi.fn(); - - const ready = waitForCreatedSandboxReadyWithTrace({ - sandboxName: "my-sandbox", - timeoutSecs: 600, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, - errorPhaseDebouncePolls: 3, - sleep, - }); - - expect(ready).toEqual({ - ready: false, - reason: "terminal_failure_phase", - failurePhase: "Error", - }); - // 3 consecutive Error polls trigger the terminal failure; the wait sleeps - // twice between the first three polls and stops before the full timeout. - expect(runCaptureOpenshell).toHaveBeenCalledTimes(3); - expect(sleep).toHaveBeenCalledTimes(2); - }); + // Create/readiness-wait Error-phase behavior (including the #6043 transient + // debounce and its env contract) lives in sandbox-readiness-tracing.test.ts. it("short-circuits the supervisor-reconnect wait when the sandbox enters Error phase", () => { // Without the short-circuit, a patched container that crashes on startup diff --git a/src/lib/onboard/sandbox-readiness-tracing.test.ts b/src/lib/onboard/sandbox-readiness-tracing.test.ts new file mode 100644 index 00000000000..4d2aa23bd24 --- /dev/null +++ b/src/lib/onboard/sandbox-readiness-tracing.test.ts @@ -0,0 +1,283 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { getSandboxFailurePhase, isSandboxReady } from "../state/gateway"; +import { + formatCreatedSandboxReadinessFailureMessage, + getSandboxReadyErrorDebouncePolls, + SANDBOX_READY_ERROR_DEBOUNCE_ENV, + waitForCreatedSandboxReadyWithTrace, +} from "./sandbox-readiness-tracing"; + +const NAME = "my-sandbox"; + +function replay(outputs: readonly string[]) { + let i = 0; + const runCaptureOpenshell = vi.fn(() => outputs[Math.min(i++, outputs.length - 1)]); + const sleep = vi.fn(); + return { runCaptureOpenshell, sleep, polls: () => i }; +} + +describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { + it("fast-fails on the first Error poll when the debounce is opted out (K=1)", () => { + const { runCaptureOpenshell, sleep } = replay([ + `${NAME} Provisioning 1s ago`, + `${NAME} Error 3s ago`, + ]); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + // 600 / 2 = 300 readyAttempts. With the K=1 (no-debounce) opt-out we bail + // out after the 2nd poll, preserving the original fast-fail intent. + timeoutSecs: 600, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + errorPhaseDebouncePolls: 1, + sleep, + }); + + expect(ready).toEqual({ + ready: false, + reason: "terminal_failure_phase", + failurePhase: "Error", + }); + expect(runCaptureOpenshell).toHaveBeenCalledTimes(2); + // Should not sleep after detecting the terminal phase. + expect(sleep).toHaveBeenCalledTimes(1); + }); + + it("recovers when a transient Error flips to Ready within the debounce window (#6043)", () => { + // DGX Spark repro: the gateway re-registers the just-created sandbox and + // `sandbox list` briefly reports Error before flipping to Ready. The + // default debounce must tolerate the transient rather than fast-failing. + const { runCaptureOpenshell, sleep } = replay([ + `${NAME} Provisioning 1s ago`, + `${NAME} Error 3s ago`, + `${NAME} Error 5s ago`, + `${NAME} Ready 7s ago`, + ]); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 600, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + sleep, + }); + + expect(ready).toEqual({ ready: true, reason: "ready", failurePhase: null }); + expect(runCaptureOpenshell).toHaveBeenCalledTimes(4); + }); + + it("resets the debounce counter when a non-Error poll interrupts the Error streak", () => { + // Flapping Error must not accumulate toward the terminal threshold. + const { runCaptureOpenshell, sleep } = replay([ + `${NAME} Error 1s ago`, + `${NAME} Provisioning 3s ago`, + `${NAME} Error 5s ago`, + `${NAME} Ready 7s ago`, + ]); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 600, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + errorPhaseDebouncePolls: 2, + sleep, + }); + + // Never two consecutive Error polls, so it never crosses the threshold. + expect(ready).toEqual({ ready: true, reason: "ready", failurePhase: null }); + }); + + it("still fails terminally after sustained Error exceeds the debounce window (#6043)", () => { + const { runCaptureOpenshell, sleep } = replay([`${NAME} Error 3s ago`]); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 600, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + errorPhaseDebouncePolls: 3, + sleep, + }); + + expect(ready).toEqual({ + ready: false, + reason: "terminal_failure_phase", + failurePhase: "Error", + }); + // 3 consecutive Error polls trigger the terminal failure; the wait sleeps + // twice between the first three polls and stops before the full timeout. + expect(runCaptureOpenshell).toHaveBeenCalledTimes(3); + expect(sleep).toHaveBeenCalledTimes(2); + }); + + it.each([ + "Failed", + "CrashLoopBackOff", + ])("fast-fails immediately on genuinely terminal phase %s even with a large debounce", (phase) => { + const { runCaptureOpenshell, sleep } = replay([ + `${NAME} Provisioning 1s ago`, + `${NAME} ${phase} 3s ago`, + ]); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 600, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + // Even with a very large debounce, non-Error terminal phases must not + // be debounced (#6043 CodeRabbit/advisor: debounce is Error-only). + errorPhaseDebouncePolls: 999, + sleep, + }); + + expect(ready).toEqual({ ready: false, reason: "terminal_failure_phase", failurePhase: phase }); + expect(runCaptureOpenshell).toHaveBeenCalledTimes(2); + expect(sleep).toHaveBeenCalledTimes(1); + }); + + it("truncates a fractional debounce override toward zero (2.9 -> 2)", () => { + const { runCaptureOpenshell, sleep } = replay([`${NAME} Error 3s ago`]); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 600, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + errorPhaseDebouncePolls: 2.9, + sleep, + }); + + expect(ready).toEqual({ + ready: false, + reason: "terminal_failure_phase", + failurePhase: "Error", + }); + // trunc(2.9) === 2, so the 2nd consecutive Error poll is terminal. + expect(runCaptureOpenshell).toHaveBeenCalledTimes(2); + }); + + it("ignores a non-finite debounce override and falls back to the env/default", () => { + // NaN is not finite, so the override is dropped and the default (30) is + // used: a 4-poll transient Error still recovers to Ready. + const { runCaptureOpenshell } = replay([ + `${NAME} Error 1s ago`, + `${NAME} Error 3s ago`, + `${NAME} Error 5s ago`, + `${NAME} Ready 7s ago`, + ]); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 600, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + errorPhaseDebouncePolls: Number.NaN, + sleep: () => {}, + }); + + expect(ready).toEqual({ ready: true, reason: "ready", failurePhase: null }); + }); +}); + +describe("getSandboxReadyErrorDebouncePolls env contract", () => { + it("defaults to 30 when the env var is unset", () => { + expect(getSandboxReadyErrorDebouncePolls({})).toBe(30); + }); + + it("honors a valid override", () => { + expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "12" })).toBe( + 12, + ); + }); + + it("falls back to the default for empty or non-numeric values", () => { + expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "" })).toBe(30); + expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "abc" })).toBe( + 30, + ); + expect( + getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "Infinity" }), + ).toBe(30); + expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "NaN" })).toBe( + 30, + ); + }); + + it("clamps to a minimum of 1 poll", () => { + expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "0" })).toBe(1); + expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "-5" })).toBe(1); + // envInt rounds 0.4 -> 0, then the clamp lifts it to 1. + expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "0.4" })).toBe( + 1, + ); + }); + + it("rounds fractional env values (envInt semantics)", () => { + expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "2.6" })).toBe( + 3, + ); + }); +}); + +// PRA-5 acceptance: deterministic replay of the reporter's DGX Spark +// gateway/port-fallback create sequence through the real readiness waiter. DGX +// Spark hardware is unavailable, so this checked-in replay is the acceptance +// gate: it proves the pre-fix fast-fail regressed on the exact reporter signal +// and that the shipped default recovers. +describe("DGX Spark fresh-onboard readiness replay (#6043)", () => { + // Rows as `openshell sandbox list` reports them while the gateway supervisor + // restarts (dashboard port fallback 18789 -> 18794) and re-registers the + // just-created sandbox before it settles to Ready. + const reporterSequence = [ + `${NAME} Provisioning 2s ago`, + `${NAME} Error 6s ago`, + `${NAME} Error 8s ago`, + `${NAME} Error 10s ago`, + `${NAME} Ready 14s ago`, + ] as const; + + it("regressed pre-fix: fast-fail (K=1) surfaces the exact reporter failure line", () => { + const { runCaptureOpenshell, sleep } = replay(reporterSequence); + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 1500, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + errorPhaseDebouncePolls: 1, + sleep, + }); + + expect(ready.ready).toBe(false); + expect(formatCreatedSandboxReadinessFailureMessage(NAME, ready, 1500)).toContain( + "entered Error phase before it became ready (waited up to 1500s)", + ); + }); + + it("recovers with the shipped default debounce: onboard continues to Ready", () => { + const { runCaptureOpenshell, sleep } = replay(reporterSequence); + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 1500, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + sleep, + }); + + expect(ready).toEqual({ ready: true, reason: "ready", failurePhase: null }); + }); +}); diff --git a/src/lib/onboard/sandbox-readiness-tracing.ts b/src/lib/onboard/sandbox-readiness-tracing.ts index 45d494209a6..90b1650cb05 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.ts @@ -8,24 +8,48 @@ type RunCaptureOpenshell = (args: string[], options?: { ignoreError?: boolean }) export const SANDBOX_READY_ERROR_DEBOUNCE_ENV = "NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE"; -// Consecutive Error-phase polls required before the create/readiness wait -// treats the phase as terminal. The readiness loop polls `openshell sandbox -// list` every 2 seconds, so the default of 30 tolerates ~60s of sustained -// Error before failing. -// -// Why debounce at all: on a fresh onboard the gateway may (re)start its -// supervisor session and re-register the just-created sandbox (observed on -// DGX Spark, where the dashboard port fallback + supervisor restart race the -// sandbox bootstrap — #6043). During that window `sandbox list` can briefly -// report the sandbox in Error phase before it flips to Ready. Fast-failing on -// the first Error poll turns that transient into a terminal onboard failure. -// The debounce mirrors the Docker GPU supervisor-reconnect path -// (docker-gpu-supervisor-reconnect.ts), which tolerates the same transient -// while the recreated GPU container reconnects. -// -// This does NOT hide terminal failures: a sandbox that stays in Error still -// fast-fails after the bounded debounce window (well before the full readiness -// timeout), and the caller still captures full failure diagnostics. +/* + * Create/readiness Error-phase debounce. + * + * Invalid state + * ------------- + * On a fresh onboard the OpenShell gateway may (re)start its supervisor + * session and re-register the just-created sandbox. During that window + * `openshell sandbox list` briefly reports the sandbox in the transient + * "Error" phase before it flips to Ready. Observed on DGX Spark, where the + * dashboard port fallback (18789 -> 18794) and supervisor restart race the + * sandbox bootstrap (#6043). Fast-failing on the first Error poll turns that + * recoverable transient into a terminal onboard failure. + * + * Source-of-truth boundary + * ------------------------ + * The transient lives in the OpenShell gateway's `sandbox list` cache: the + * preferred fix is upstream — `sandbox list` should not report a terminal + * phase for a sandbox the gateway is still registering. Until that ships, + * NemoClaw tolerates the transient at this layer via a consecutive-Error-poll + * debounce, mirroring the Docker GPU supervisor-reconnect path + * (docker-gpu-supervisor-reconnect.ts), which tolerates the same class of + * transient while a recreated GPU container reconnects. + * + * Scope + * ----- + * Only the "Error" phase is debounced. "Failed" and "CrashLoopBackOff" are + * genuinely terminal and still fast-fail immediately. A sandbox that stays in + * Error also fast-fails after the bounded debounce window (well before the + * full readiness timeout), and the caller still captures full failure + * diagnostics — this does NOT hide terminal failures. + * + * Regression evidence / removal condition + * --------------------------------------- + * Delete this debounce once OpenShell guarantees `sandbox list` skips the + * brief Error transition during a known registration. The runtime evidence + * required is a fresh-onboard reproduction (DGX Spark, or the deterministic + * `sandbox list` replay in sandbox-readiness-tracing.test.ts) showing a + * transient create-time Error that recovers to Ready. + * + * The readiness loop polls `sandbox list` every 2 seconds, so the default of + * 30 tolerates ~60s of sustained Error before failing. + */ const SANDBOX_READY_ERROR_PHASE_DEFAULT_DEBOUNCE_POLLS = 30; export function getSandboxReadyErrorDebouncePolls( @@ -145,7 +169,15 @@ export function waitForCreatedSandboxReadyWithTrace(options: { return { ready: true, reason: "ready", failurePhase: null }; } const failurePhase = getSandboxFailurePhase?.(list, sandboxName) ?? null; - if (failurePhase) { + // Only the transient "Error" phase is debounced — it is the phase the + // gateway briefly reports while re-registering the just-created sandbox + // (#6043). "Failed" and "CrashLoopBackOff" are genuinely terminal and + // must still fast-fail immediately rather than burn the debounce window. + if (failurePhase && failurePhase !== "Error") { + addTraceEvent("terminal_failure_phase", { attempt: i + 1, failure_phase: failurePhase }); + return { ready: false, reason: "terminal_failure_phase", failurePhase }; + } + if (failurePhase === "Error") { consecutiveFailurePolls += 1; lastFailurePhase = failurePhase; // Sustained Error is terminal; a transient Error while the gateway From 80cdef4b504ed05c5bbdc60a107b544eada252b2 Mon Sep 17 00:00:00 2001 From: Yimo Jiang Date: Thu, 2 Jul 2026 05:40:10 +0000 Subject: [PATCH 3/6] fix(onboard): report stuck Error at timeout and document debounce contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address second-round review on the #6043 readiness Error debounce: - Surface the terminal Error phase (not a phase-less timeout) when the sandbox is still in Error on the final poll — happens when the debounce window outlasts a low readiness timeout, which previously misreported a stuck Error as "did not become ready" and dropped the phase (advisor PRA-1). - Document the ~60s default-debounce latency trade-off and the intentional trunc-vs-envInt-rounding difference on the errorPhaseDebouncePolls JSDoc (advisor PRA-4, PRA-6). - Add a maintainer-enableable removal-signal test (upstream_openshell_sandbox_list_error_transient_fixed) and a real-DGX E2E follow-up note on the replay fixture (advisor PRA-3, PRA-5). Signed-off-by: Yimo Jiang --- .../onboard/sandbox-readiness-tracing.test.ts | 43 +++++++++++++++++++ src/lib/onboard/sandbox-readiness-tracing.ts | 24 ++++++++++- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/lib/onboard/sandbox-readiness-tracing.test.ts b/src/lib/onboard/sandbox-readiness-tracing.test.ts index 4d2aa23bd24..00be88ded6e 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.test.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.test.ts @@ -120,6 +120,28 @@ describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { expect(sleep).toHaveBeenCalledTimes(2); }); + it("reports the Error phase (not a generic timeout) when the debounce outlasts the timeout", () => { + // Small readiness timeout (1 poll) with the default debounce (30): a stuck + // Error can never reach the debounce threshold, but it must still surface + // the terminal phase rather than a phase-less timeout (#6043 review PRA-1). + const { runCaptureOpenshell, sleep } = replay([`${NAME} Error 3s ago`]); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 2, // -> readyAttempts = 1, far below the default 30-poll debounce + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + sleep, + }); + + expect(ready).toEqual({ + ready: false, + reason: "terminal_failure_phase", + failurePhase: "Error", + }); + }); + it.each([ "Failed", "CrashLoopBackOff", @@ -280,4 +302,25 @@ describe("DGX Spark fresh-onboard readiness replay (#6043)", () => { expect(ready).toEqual({ ready: true, reason: "ready", failurePhase: null }); }); + + // Follow-up: when DGX Spark (or an equivalent ARM64 GPU) CI runner becomes + // available, replace/augment this replay with a live fresh-onboard E2E on + // that hardware (tracked on #6043). A real worktree-CLI onboard on a healthy + // non-DGX host was validated for the happy path, but cannot force the + // transient Error branch this replay exercises. + + // Removal signal for the debounce workaround (see the source-of-truth block + // in sandbox-readiness-tracing.ts). A maintainer enables this once OpenShell + // guarantees `sandbox list` no longer reports a transient Error while the + // gateway re-registers a just-created sandbox: if the raw upstream sequence + // contains no Error rows, the debounce in waitForCreatedSandboxReadyWithTrace + // can be deleted. + it.skip("upstream_openshell_sandbox_list_error_transient_fixed", () => { + // Replace `reporterSequence` with a captured `sandbox list` trace from a + // fixed OpenShell during a fresh GPU onboard, then assert no Error rows. + const hasTransientError = reporterSequence.some( + (row) => getSandboxFailurePhase(row, NAME) === "Error", + ); + expect(hasTransientError).toBe(false); + }); }); diff --git a/src/lib/onboard/sandbox-readiness-tracing.ts b/src/lib/onboard/sandbox-readiness-tracing.ts index 90b1650cb05..65b0e8802d7 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.ts @@ -139,7 +139,13 @@ export function waitForCreatedSandboxReadyWithTrace(options: { getSandboxFailurePhase?: (output: string, sandboxName: string) => string | null; /** * Consecutive Error-phase polls required before the wait treats the phase as - * terminal. Defaults to {@link getSandboxReadyErrorDebouncePolls}. Pass 1 to + * terminal. Defaults to {@link getSandboxReadyErrorDebouncePolls} (30 polls / + * ~60s at the 2s poll interval). Trade-off: a genuinely stuck Error is + * reported ~60s later than a fast-fail; the window is intentionally bounded + * (and far below the readiness timeout) so it never masks a terminal failure. + * Fractional values are truncated toward zero (Math.trunc), matching the + * override contract in docker-gpu-supervisor-reconnect.ts; the env-var path + * ({@link getSandboxReadyErrorDebouncePolls}) rounds via envInt. Pass 1 to * restore the original fast-fail-on-first-Error behavior (used by callers * that have already ruled out the transient supervisor-reconnect race). */ @@ -201,6 +207,22 @@ export function waitForCreatedSandboxReadyWithTrace(options: { } if (i < readyAttempts - 1) sleep(2); } + // If the sandbox is still in Error on the final poll, surface the terminal + // phase instead of a generic timeout. This happens when the configured + // debounce window is larger than the readiness timeout allows (e.g. a low + // NEMOCLAW_SANDBOX_READY_TIMEOUT with the default 30-poll debounce), so a + // genuinely stuck Error would otherwise be misreported as "did not become + // ready" and drop the phase (#6043 review). + if (consecutiveFailurePolls > 0 && lastFailurePhase) { + addTraceEvent("terminal_failure_phase", { + attempts: readyAttempts, + failure_phase: lastFailurePhase, + consecutive_polls: consecutiveFailurePolls, + debounce_polls: errorPhaseDebouncePolls, + note: "debounce_window_exceeded_timeout", + }); + return { ready: false, reason: "terminal_failure_phase", failurePhase: lastFailurePhase }; + } addTraceEvent("not_ready", { attempts: readyAttempts, last_failure_phase: lastFailurePhase }); return { ready: false, reason: "timeout", failurePhase: null }; }); From eb8c59a996e4af8b1e520798cc82f5d6096ff16b Mon Sep 17 00:00:00 2001 From: Yimo Jiang Date: Thu, 2 Jul 2026 05:49:11 +0000 Subject: [PATCH 4/6] fix(onboard): unify debounce rounding and document removal tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third-round review (PR Review Advisor Nemotron): - Round the programmatic errorPhaseDebouncePolls override (Math.round) so it matches the env-var path's envInt rounding — one consistent rule across both entry points (PRA-3). - Document the removal-signal tracking mechanism in the source-of-truth block: the maintainer-enabled upstream_openshell_sandbox_list_error_transient_fixed test is the executable checkpoint; escalate to a tracking issue if the workaround outlives a release cycle (PRA-2). - Expand the errorPhaseDebouncePolls JSDoc to call out the fresh-create latency trade-off and justify the conservative 30-poll default (re-registration scales with host/gateway speed; a too-low default risks re-introducing #6043; env-tunable) (PRA-4). Signed-off-by: Yimo Jiang --- .../onboard/sandbox-readiness-tracing.test.ts | 10 +++--- src/lib/onboard/sandbox-readiness-tracing.ts | 32 +++++++++++++++---- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/lib/onboard/sandbox-readiness-tracing.test.ts b/src/lib/onboard/sandbox-readiness-tracing.test.ts index 00be88ded6e..19a30ef8b48 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.test.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.test.ts @@ -168,7 +168,7 @@ describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { expect(sleep).toHaveBeenCalledTimes(1); }); - it("truncates a fractional debounce override toward zero (2.9 -> 2)", () => { + it("rounds a fractional debounce override (2.6 -> 3), matching envInt semantics", () => { const { runCaptureOpenshell, sleep } = replay([`${NAME} Error 3s ago`]); const ready = waitForCreatedSandboxReadyWithTrace({ @@ -177,7 +177,7 @@ describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { runCaptureOpenshell, isSandboxReady, getSandboxFailurePhase, - errorPhaseDebouncePolls: 2.9, + errorPhaseDebouncePolls: 2.6, sleep, }); @@ -186,8 +186,10 @@ describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { reason: "terminal_failure_phase", failurePhase: "Error", }); - // trunc(2.9) === 2, so the 2nd consecutive Error poll is terminal. - expect(runCaptureOpenshell).toHaveBeenCalledTimes(2); + // round(2.6) === 3 (truncation would give 2), so the 3rd consecutive Error + // poll is terminal — the same rounding rule as the + // NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE env path. + expect(runCaptureOpenshell).toHaveBeenCalledTimes(3); }); it("ignores a non-finite debounce override and falls back to the env/default", () => { diff --git a/src/lib/onboard/sandbox-readiness-tracing.ts b/src/lib/onboard/sandbox-readiness-tracing.ts index 65b0e8802d7..d185a9cc7f8 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.ts @@ -47,6 +47,14 @@ export const SANDBOX_READY_ERROR_DEBOUNCE_ENV = "NEMOCLAW_SANDBOX_READY_ERROR_DE * `sandbox list` replay in sandbox-readiness-tracing.test.ts) showing a * transient create-time Error that recovers to Ready. * + * Tracking mechanism: the maintainer-enabled removal-signal test + * `upstream_openshell_sandbox_list_error_transient_fixed` + * (sandbox-readiness-tracing.test.ts, currently `it.skip`) is the executable + * checkpoint — point it at a captured `sandbox list` trace from a fixed + * OpenShell and, once it passes (no transient Error), this debounce can be + * removed. Escalate to a GitHub tracking issue against the OpenShell fix if the + * workaround outlives a release cycle. + * * The readiness loop polls `sandbox list` every 2 seconds, so the default of * 30 tolerates ~60s of sustained Error before failing. */ @@ -140,12 +148,20 @@ export function waitForCreatedSandboxReadyWithTrace(options: { /** * Consecutive Error-phase polls required before the wait treats the phase as * terminal. Defaults to {@link getSandboxReadyErrorDebouncePolls} (30 polls / - * ~60s at the 2s poll interval). Trade-off: a genuinely stuck Error is - * reported ~60s later than a fast-fail; the window is intentionally bounded - * (and far below the readiness timeout) so it never masks a terminal failure. - * Fractional values are truncated toward zero (Math.trunc), matching the - * override contract in docker-gpu-supervisor-reconnect.ts; the env-var path - * ({@link getSandboxReadyErrorDebouncePolls}) rounds via envInt. Pass 1 to + * ~60s at the 2s poll interval). + * + * Trade-off: on a fresh create — the path this waiter guards — a healthy + * sandbox that briefly transits Error costs nothing (it flips to Ready and + * the wait returns on that poll), while a genuinely stuck Error is reported + * ~60s later than a fast-fail would. The default is deliberately conservative + * rather than tuned to the shortest observed transient: the re-registration + * window scales with host/gateway speed (slower on ARM64/DGX-class hosts), so + * a too-low default risks re-introducing #6043. The window is bounded and far + * below the readiness timeout, so it never masks a terminal failure; operators + * who want a tighter bound set NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE. + * + * Fractional values are rounded (Math.round), matching the env-var path's + * envInt rounding for one consistent rule across both entry points. Pass 1 to * restore the original fast-fail-on-first-Error behavior (used by callers * that have already ruled out the transient supervisor-reconnect race). */ @@ -163,7 +179,9 @@ export function waitForCreatedSandboxReadyWithTrace(options: { const errorPhaseDebouncePolls = options.errorPhaseDebouncePolls == null || !Number.isFinite(options.errorPhaseDebouncePolls) ? getSandboxReadyErrorDebouncePolls() - : Math.max(1, Math.trunc(options.errorPhaseDebouncePolls)); + : // Round (not truncate) so a fractional override matches the env-var + // path's envInt rounding — one consistent rule for both entry points. + Math.max(1, Math.round(options.errorPhaseDebouncePolls)); return withSandboxReadinessTrace(sandboxName, { timeout_seconds: timeoutSecs }, () => { const readyAttempts = Math.max(1, Math.ceil(timeoutSecs / 2)); let consecutiveFailurePolls = 0; From e4a785dd9b10d831be51d72877e06fc715961973 Mon Sep 17 00:00:00 2001 From: Yimo Jiang Date: Thu, 2 Jul 2026 06:04:25 +0000 Subject: [PATCH 5/6] fix(onboard): align sibling debounce rounding and reference removal tracker Fourth-round review (PR Review Advisor Nemotron): - Align the docker-gpu-supervisor-reconnect.ts programmatic override to Math.round so both onboard debounce modules and the env-var path share one rounding rule (PRA-5). - Reference NemoClaw #6043 as the removal tracker in the source-of-truth block and the removal-signal test comment, so the workaround has a concrete tracking handle without inventing a duplicate issue (PRA-2, PRA-3). Signed-off-by: Yimo Jiang --- src/lib/onboard/docker-gpu-supervisor-reconnect.ts | 4 +++- src/lib/onboard/sandbox-readiness-tracing.test.ts | 11 ++++++----- src/lib/onboard/sandbox-readiness-tracing.ts | 9 +++++---- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/lib/onboard/docker-gpu-supervisor-reconnect.ts b/src/lib/onboard/docker-gpu-supervisor-reconnect.ts index abd59a8c1d3..4d078f2ad21 100644 --- a/src/lib/onboard/docker-gpu-supervisor-reconnect.ts +++ b/src/lib/onboard/docker-gpu-supervisor-reconnect.ts @@ -120,7 +120,9 @@ export function waitForOpenShellSupervisorReconnect( const errorPhaseDebouncePolls = deps.errorPhaseDebouncePolls == null || !Number.isFinite(deps.errorPhaseDebouncePolls) ? getDockerGpuSupervisorReconnectErrorDebouncePolls() - : Math.max(1, Math.trunc(deps.errorPhaseDebouncePolls)); + : // Round (not truncate) to match the env-var path's envInt rounding and + // the sibling create/readiness debounce in sandbox-readiness-tracing.ts. + Math.max(1, Math.round(deps.errorPhaseDebouncePolls)); let consecutiveErrorPolls = 0; while (Date.now() <= deadline) { const result = deps.runOpenshell(["sandbox", "exec", "-n", sandboxName, "--", "true"], { diff --git a/src/lib/onboard/sandbox-readiness-tracing.test.ts b/src/lib/onboard/sandbox-readiness-tracing.test.ts index 19a30ef8b48..ed219fbafbc 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.test.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.test.ts @@ -312,11 +312,12 @@ describe("DGX Spark fresh-onboard readiness replay (#6043)", () => { // transient Error branch this replay exercises. // Removal signal for the debounce workaround (see the source-of-truth block - // in sandbox-readiness-tracing.ts). A maintainer enables this once OpenShell - // guarantees `sandbox list` no longer reports a transient Error while the - // gateway re-registers a just-created sandbox: if the raw upstream sequence - // contains no Error rows, the debounce in waitForCreatedSandboxReadyWithTrace - // can be deleted. + // in sandbox-readiness-tracing.ts). Removal is tracked on NemoClaw #6043 + // (which owns the pending upstream OpenShell `sandbox list` fix). A maintainer + // enables this once OpenShell guarantees `sandbox list` no longer reports a + // transient Error while the gateway re-registers a just-created sandbox: if + // the raw upstream sequence contains no Error rows, the debounce in + // waitForCreatedSandboxReadyWithTrace can be deleted. it.skip("upstream_openshell_sandbox_list_error_transient_fixed", () => { // Replace `reporterSequence` with a captured `sandbox list` trace from a // fixed OpenShell during a fresh GPU onboard, then assert no Error rows. diff --git a/src/lib/onboard/sandbox-readiness-tracing.ts b/src/lib/onboard/sandbox-readiness-tracing.ts index d185a9cc7f8..ef5acb354d6 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.ts @@ -47,13 +47,14 @@ export const SANDBOX_READY_ERROR_DEBOUNCE_ENV = "NEMOCLAW_SANDBOX_READY_ERROR_DE * `sandbox list` replay in sandbox-readiness-tracing.test.ts) showing a * transient create-time Error that recovers to Ready. * - * Tracking mechanism: the maintainer-enabled removal-signal test - * `upstream_openshell_sandbox_list_error_transient_fixed` + * Tracking mechanism: removal is tracked on NemoClaw #6043 (which owns the + * pending OpenShell `sandbox list` fix). The maintainer-enabled removal-signal + * test `upstream_openshell_sandbox_list_error_transient_fixed` * (sandbox-readiness-tracing.test.ts, currently `it.skip`) is the executable * checkpoint — point it at a captured `sandbox list` trace from a fixed * OpenShell and, once it passes (no transient Error), this debounce can be - * removed. Escalate to a GitHub tracking issue against the OpenShell fix if the - * workaround outlives a release cycle. + * removed. Escalate to a dedicated OpenShell-fix tracking issue (referenced + * here and in the test) if the workaround outlives a release cycle. * * The readiness loop polls `sandbox list` every 2 seconds, so the default of * 30 tolerates ~60s of sustained Error before failing. From 1c7c955123ed16feeb7c963490248fd56751d614 Mon Sep 17 00:00:00 2001 From: Yimo Jiang Date: Thu, 2 Jul 2026 06:14:48 +0000 Subject: [PATCH 6/6] docs(onboard): document NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fifth-round review (PR Review Advisor Nemotron): - Document the new NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE env var (default 30, Error-only scope, K=1 fast-fail opt-out) in the commands + commands-nemohermes env-var tables, and add a troubleshooting entry for the "entered Error phase before it became ready" message — mirroring the existing sibling docs for NEMOCLAW_DOCKER_GPU_SUPERVISOR_RECONNECT_ERROR_DEBOUNCE (PRA-5). - Add the full NemoClaw #6043 issue URL to the source-of-truth block and the removal-signal test comment as the removal tracker (PRA-2, PRA-3). Signed-off-by: Yimo Jiang --- docs/reference/commands-nemohermes.mdx | 1 + docs/reference/commands.mdx | 1 + docs/reference/troubleshooting.mdx | 26 +++++++++++++++++++ .../onboard/sandbox-readiness-tracing.test.ts | 3 ++- src/lib/onboard/sandbox-readiness-tracing.ts | 5 ++-- 5 files changed, 33 insertions(+), 3 deletions(-) diff --git a/docs/reference/commands-nemohermes.mdx b/docs/reference/commands-nemohermes.mdx index 3d54a051797..ea46c719c2d 100644 --- a/docs/reference/commands-nemohermes.mdx +++ b/docs/reference/commands-nemohermes.mdx @@ -2103,6 +2103,7 @@ Set them before running `nemohermes onboard` if a slow connection or large model | `NEMOCLAW_OLLAMA_PULL_TIMEOUT` | `1800` (30 minutes) | Wall-clock timeout for `ollama pull` during onboard, in seconds. Accepts integer or float values. Already-downloaded layers are kept; re-running the pull resumes them. | | `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT` | `180` | Wall-clock timeout for the inference-server validation probe during onboard, in seconds. Raise on slow networks or for very large prompts. | | `NEMOCLAW_SANDBOX_READY_TIMEOUT` | `180` | Wall-clock timeout for the post-create readiness wait, in seconds. Raise when the sandbox image build, gateway upload, or in-sandbox boot exceeds the default (typical on 70B+ models, first-time gateway uploads over slow links, or DGX Station / remote-VM first runs). When the deadline expires onboarding deletes the orphaned sandbox and prints the retry hint. | +| `NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE` | `30` | Consecutive `Error`-phase polls (2s apart, so ~60s by default) the post-create readiness wait tolerates before treating `Error` as terminal. The gateway can briefly report a just-created sandbox in `Error` while it re-registers the sandbox (seen on DGX Spark); the debounce lets that transient recover to `Ready`. `Failed` and `CrashLoopBackOff` always fail immediately. Set to `1` to restore fast-fail on the first `Error` poll. | ```bash export NEMOCLAW_OLLAMA_PULL_TIMEOUT=3600 diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 926127f9078..db3f88cf890 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -2588,6 +2588,7 @@ Set them before running `$$nemoclaw onboard` if a slow connection or large model | `NEMOCLAW_OLLAMA_PULL_TIMEOUT` | `1800` (30 minutes) | Wall-clock timeout for `ollama pull` during onboard, in seconds. Accepts integer or float values. Already-downloaded layers are kept; re-running the pull resumes them. | | `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT` | `180` | Wall-clock timeout for the inference-server validation probe during onboard, in seconds. Raise on slow networks or for very large prompts. | | `NEMOCLAW_SANDBOX_READY_TIMEOUT` | `180` | Wall-clock timeout for the post-create readiness wait, in seconds. Raise when the sandbox image build, gateway upload, or in-sandbox boot exceeds the default (typical on 70B+ models, first-time gateway uploads over slow links, or DGX Station / remote-VM first runs). When the deadline expires onboarding deletes the orphaned sandbox and prints the retry hint. | +| `NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE` | `30` | Consecutive `Error`-phase polls (2s apart, so ~60s by default) the post-create readiness wait tolerates before treating `Error` as terminal. The gateway can briefly report a just-created sandbox in `Error` while it re-registers the sandbox (seen on DGX Spark); the debounce lets that transient recover to `Ready`. `Failed` and `CrashLoopBackOff` always fail immediately. Set to `1` to restore fast-fail on the first `Error` poll. | ```bash export NEMOCLAW_OLLAMA_PULL_TIMEOUT=3600 diff --git a/docs/reference/troubleshooting.mdx b/docs/reference/troubleshooting.mdx index 55a28ee2d49..2dc623349c3 100644 --- a/docs/reference/troubleshooting.mdx +++ b/docs/reference/troubleshooting.mdx @@ -977,6 +977,32 @@ openshell sandbox list $$nemoclaw status ``` +### Sandbox onboard fails with "entered Error phase before it became ready" + +Onboarding ends with: + +```text + Sandbox 'my-assistant' entered Error phase before it became ready (waited up to 180s). +``` + +On a fresh onboard the OpenShell gateway can (re)start its supervisor session and re-register the just-created sandbox. During that window `openshell sandbox list` briefly reports the sandbox in the transient `Error` phase before it flips to `Ready` — seen on DGX Spark, where the dashboard port fallback and supervisor restart race the sandbox bootstrap. + +NemoClaw tolerates a bounded run of consecutive `Error` polls (default 30 polls / ~60s) so this transient recovers on its own; only `Error` that persists past the debounce window is treated as terminal. `Failed` and `CrashLoopBackOff` are always terminal and fail immediately. + +If your host needs a longer window (slower re-registration), raise the debounce; to fail fast on the first `Error` poll, set it to `1`: + +```bash +export NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE=60 # tolerate ~120s of transient Error +$$nemoclaw onboard +``` + +If the failure persists after the debounce, the sandbox is genuinely stuck — inspect the retained diagnostics and gateway state: + +```bash +openshell sandbox list +$$nemoclaw status +``` + ### Agent fails at runtime after onboarding succeeds with a compatible endpoint Some OpenAI-compatible servers (such as SGLang) expose `/v1/responses` but their diff --git a/src/lib/onboard/sandbox-readiness-tracing.test.ts b/src/lib/onboard/sandbox-readiness-tracing.test.ts index ed219fbafbc..de68cddfb15 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.test.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.test.ts @@ -313,7 +313,8 @@ describe("DGX Spark fresh-onboard readiness replay (#6043)", () => { // Removal signal for the debounce workaround (see the source-of-truth block // in sandbox-readiness-tracing.ts). Removal is tracked on NemoClaw #6043 - // (which owns the pending upstream OpenShell `sandbox list` fix). A maintainer + // (https://github.com/NVIDIA/NemoClaw/issues/6043), which owns the pending + // upstream OpenShell `sandbox list` fix. A maintainer // enables this once OpenShell guarantees `sandbox list` no longer reports a // transient Error while the gateway re-registers a just-created sandbox: if // the raw upstream sequence contains no Error rows, the debounce in diff --git a/src/lib/onboard/sandbox-readiness-tracing.ts b/src/lib/onboard/sandbox-readiness-tracing.ts index ef5acb354d6..aeef5a08ad4 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.ts @@ -47,8 +47,9 @@ export const SANDBOX_READY_ERROR_DEBOUNCE_ENV = "NEMOCLAW_SANDBOX_READY_ERROR_DE * `sandbox list` replay in sandbox-readiness-tracing.test.ts) showing a * transient create-time Error that recovers to Ready. * - * Tracking mechanism: removal is tracked on NemoClaw #6043 (which owns the - * pending OpenShell `sandbox list` fix). The maintainer-enabled removal-signal + * Tracking mechanism: removal is tracked on NemoClaw #6043 + * (https://github.com/NVIDIA/NemoClaw/issues/6043), which owns the pending + * OpenShell `sandbox list` fix. The maintainer-enabled removal-signal * test `upstream_openshell_sandbox_list_error_transient_fixed` * (sandbox-readiness-tracing.test.ts, currently `it.skip`) is the executable * checkpoint — point it at a captured `sandbox list` trace from a fixed