diff --git a/scripts/checks/run-managed-image-openshell-e2e.ts b/scripts/checks/run-managed-image-openshell-e2e.ts index 917cc682d1..349a8f4000 100644 --- a/scripts/checks/run-managed-image-openshell-e2e.ts +++ b/scripts/checks/run-managed-image-openshell-e2e.ts @@ -9,6 +9,7 @@ import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { resolveAgent } from "../../src/lib/agent/onboard.ts"; import { parseOpenShellSandboxId } from "../../src/lib/adapters/openshell/sandbox-identity.ts"; +import { createCliOpenShellSandboxObserverFromRunner } from "../../src/lib/adapters/openshell/sandbox-observer-cli.ts"; import { isValidName, NAME_ALLOWED_FORMAT } from "../../src/lib/name-validation.ts"; import { type StopHostGatewayResult, @@ -1049,6 +1050,7 @@ async function run) => string, + gatewayName?: string, ): string { - const output = runCaptureOpenshell(["sandbox", "get", sandboxName], { - ignoreError: false, - }); + const output = runCaptureOpenshell( + ["sandbox", "get", ...(gatewayName ? ["-g", gatewayName] : []), sandboxName], + { + ignoreError: false, + }, + ); const sandboxId = parseOpenShellSandboxId(output); if (!sandboxId) { throw new Error( diff --git a/src/lib/adapters/openshell/sandbox-observer-cli.test.ts b/src/lib/adapters/openshell/sandbox-observer-cli.test.ts index d355f9f618..0ad4ff7547 100644 --- a/src/lib/adapters/openshell/sandbox-observer-cli.test.ts +++ b/src/lib/adapters/openshell/sandbox-observer-cli.test.ts @@ -4,8 +4,10 @@ import { describe, expect, it, vi } from "vitest"; import { + createCliOpenShellLegacyPodReadinessProbe, createCliOpenShellSandboxLookup, createCliOpenShellSandboxObserver as createObserver, + createCliOpenShellSandboxObserverFromRunner, type CapturedSandboxCommandResult, type CliOpenShellSandboxObserverDeps, parseCliOpenShellSandboxInventory, @@ -94,6 +96,24 @@ describe("CLI OpenShell sandbox observer", () => { }); }); + it("parses the captured DGX Spark readiness sequence inside the CLI implementation (#9803)", () => { + const rows = [ + "my-sandbox Provisioning 2s ago", + "my-sandbox Error 6s ago", + "my-sandbox Error 8s ago", + "my-sandbox Error 10s ago", + "my-sandbox Ready 14s ago", + ]; + + expect(rows.map((row) => parseCliOpenShellSandboxInventory(row).sandboxes[0]?.phase)).toEqual([ + "Provisioning", + "Error", + "Error", + "Error", + "Ready", + ]); + }); + it("parses successful list output from stdout without treating stderr as inventory (#9803)", async () => { const observer = createCliOpenShellSandboxObserver({ capture: () => captured(0, "alpha Ready", "warning text"), @@ -219,4 +239,79 @@ describe("CLI OpenShell sandbox observer", () => { error: { kind: "timeout", message: "OpenShell sandbox observation timed out." }, }); }); + + it("normalizes a structured runner without exposing runner output to consumers (#9803)", async () => { + const run = vi.fn(() => ({ + status: 0, + stdout: Buffer.from("alpha Ready"), + stderr: Buffer.from("warning text"), + })); + const observer = createCliOpenShellSandboxObserverFromRunner(run, 9_000); + + await expect( + observer.listSandboxes({ target: namedOpenShellGateway("nemoclaw") }), + ).resolves.toEqual({ + ok: true, + value: { + sandboxes: [{ name: "alpha", phase: "Ready", readiness: "ready" }], + }, + }); + expect(run).toHaveBeenCalledWith(["sandbox", "list", "-g", "nemoclaw"], { + ignoreError: true, + suppressOutput: true, + timeout: 9_000, + }); + }); + + it("keeps the legacy Kubernetes readiness command and phase parsing in the CLI implementation (#9803)", async () => { + const capture = vi.fn(() => captured(0, "Running")); + const probe = createCliOpenShellLegacyPodReadinessProbe({ + capture, + defaultTimeoutMs: 7_000, + }); + + await expect( + probe({ + target: namedOpenShellGateway("nemoclaw"), + sandboxName: "alpha", + }), + ).resolves.toEqual({ ok: true, value: "ready" }); + expect(capture).toHaveBeenCalledWith( + [ + "doctor", + "exec", + "--", + "kubectl", + "-n", + "openshell", + "get", + "pod", + "alpha", + "-o", + "jsonpath={.status.phase}", + ], + { + ignoreError: true, + includeStderr: true, + includeStreams: true, + timeout: 7_000, + }, + ); + }); + + it("returns a typed legacy Kubernetes observation failure (#9803)", async () => { + const probe = createCliOpenShellLegacyPodReadinessProbe({ + capture: () => captured(1, "", "authentication failed"), + }); + + await expect( + probe({ target: selectedOpenShellGateway(), sandboxName: "alpha" }), + ).resolves.toEqual({ + ok: false, + error: { + kind: "authentication", + message: "OpenShell could not authenticate the sandbox observation.", + }, + }); + }); }); diff --git a/src/lib/adapters/openshell/sandbox-observer-cli.ts b/src/lib/adapters/openshell/sandbox-observer-cli.ts index 6a95929d07..1bcc36bb01 100644 --- a/src/lib/adapters/openshell/sandbox-observer-cli.ts +++ b/src/lib/adapters/openshell/sandbox-observer-cli.ts @@ -10,6 +10,7 @@ import { type OpenShellSandboxLookup, type OpenShellSandboxObservation, type OpenShellSandboxObserver, + type OpenShellSandboxReadinessProbe, type OpenShellSandboxResult, } from "./sandbox-observer"; import { OPENSHELL_PROBE_TIMEOUT_MS } from "./timeouts"; @@ -68,6 +69,16 @@ export type CliOpenShellSandboxObserverDeps = Readonly<{ defaultTimeoutMs?: number; }>; +export type RunSandboxCommand = ( + args: string[], + options: { ignoreError: true; suppressOutput: true; timeout: number }, +) => Readonly<{ + status?: number | null; + stdout?: string | Buffer | null; + stderr?: string | Buffer | null; + error?: Error | null; +}>; + export type CliOpenShellSandboxLookupResult = Readonly<{ result: OpenShellSandboxResult; displayOutput: string; @@ -209,6 +220,68 @@ function failure(error: OpenShellSandboxError): OpenShellSandboxResult { return { ok: false, error }; } +function streamText(value: string | Buffer | null | undefined): string { + return value == null ? "" : String(value); +} + +/** Normalize structured runner results inside the CLI implementation. */ +export function createCliOpenShellSandboxObserverFromRunner( + run: RunSandboxCommand, + defaultTimeoutMs?: number, +): OpenShellSandboxObserver { + return createCliOpenShellSandboxObserver({ + capture: (args, options) => { + const result = run(args, { + ignoreError: true, + suppressOutput: true, + timeout: options.timeout, + }); + const stdout = streamText(result.stdout); + const stderr = streamText(result.stderr); + return { + status: result.status ?? null, + output: `${stdout}${stderr}`.trim(), + stdout, + stderr, + ...(result.error ? { error: result.error } : {}), + }; + }, + ...(defaultTimeoutMs === undefined ? {} : { defaultTimeoutMs }), + }); +} + +/** CLI-only fallback for legacy gateways that publish readiness through Kubernetes pod phase. */ +export function createCliOpenShellLegacyPodReadinessProbe( + deps: CliOpenShellSandboxObserverDeps, +): OpenShellSandboxReadinessProbe { + return async (request) => { + const result = await deps.capture( + [ + "doctor", + "exec", + "--", + "kubectl", + "-n", + "openshell", + "get", + "pod", + request.sandboxName, + "-o", + "jsonpath={.status.phase}", + ], + { + ignoreError: true, + includeStderr: true, + includeStreams: true, + timeout: request.timeoutMs ?? deps.defaultTimeoutMs ?? OPENSHELL_PROBE_TIMEOUT_MS, + }, + ); + const error = classifyCliOpenShellCommandError(result); + if (error) return failure(error); + return success(successfulCommandOutput(result).trim() === "Running" ? "ready" : "not_ready"); + }; +} + /** * CLI-only compatibility lookup for the legacy status display. Presence and * phase decisions must use `result`; `displayOutput` remains a CLI-only diff --git a/src/lib/adapters/openshell/sandbox-observer.ts b/src/lib/adapters/openshell/sandbox-observer.ts index 45102d10b3..b2f1cebb35 100644 --- a/src/lib/adapters/openshell/sandbox-observer.ts +++ b/src/lib/adapters/openshell/sandbox-observer.ts @@ -58,6 +58,10 @@ export type LookupOpenShellSandboxRequest = ListOpenShellSandboxesRequest & sandboxName: string; }>; +export type OpenShellSandboxReadinessProbe = ( + request: LookupOpenShellSandboxRequest, +) => Promise>; + /** Transport-neutral sandbox observation capabilities used by NemoClaw. */ export interface OpenShellSandboxObserver { listSandboxes( diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index fa2242bc60..cc66150e6e 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -721,10 +721,10 @@ const { // Gateway state functions — delegated to src/lib/state/gateway.ts const { isSandboxReady, parseSandboxStatus, getSandboxStateFromOutputs } = gatewayState; -const waitForSandboxReady = sandboxReadinessTracing.createSandboxReadyWaiter({ - runCaptureOpenshell, - isSandboxReady, +const waitForSandboxReady = sandboxReadinessTracing.createCliSandboxReadyWaiter({ isLinuxDockerDriverGatewayEnabled, + capture: captureOpenshell, + getGatewayName: () => GATEWAY_NAME, sleep: sleepSeconds, }); const { hasStaleGateway, isSelectedGateway, isGatewayHealthy, getGatewayReuseState } = diff --git a/src/lib/onboard/__test-helpers__/sandbox-gpu-create-flow.ts b/src/lib/onboard/__test-helpers__/sandbox-gpu-create-flow.ts index 444781b3b0..9d13143e74 100644 --- a/src/lib/onboard/__test-helpers__/sandbox-gpu-create-flow.ts +++ b/src/lib/onboard/__test-helpers__/sandbox-gpu-create-flow.ts @@ -7,6 +7,7 @@ import path from "node:path"; import { expect, vi } from "vitest"; +import { createCliOpenShellSandboxObserver } from "../../adapters/openshell/sandbox-observer-cli"; import type { CheckpointPortableRuntimeAuthority } from "../../state/onboard-checkpoint-types"; import type { SandboxGpuProofResult } from "../../state/registry"; import type { ManagedBootstrapRuntimeCreateLifecycleInput } from "../managed-bootstrap/runtime-create"; @@ -57,18 +58,57 @@ export function createGpuFlowInput(): SandboxGpuCreateFlowInput { }; } -export function createGpuFlowDeps(sandboxId = "alpha-sandbox-id"): SandboxGpuCreateFlowDeps { +export function createGpuFlowDeps(sandboxId?: string): SandboxGpuCreateFlowDeps; +export function createGpuFlowDeps( + expectedGatewayName: string, + requireTargetedSandboxProbes: boolean, +): SandboxGpuCreateFlowDeps; +export function createGpuFlowDeps( + sandboxIdOrGatewayName = "alpha-sandbox-id", + expectedGatewayNameOrRequireTargetedProbes: string | boolean = "nemoclaw", +): SandboxGpuCreateFlowDeps { + const requiresTargetedSandboxProbes = + typeof expectedGatewayNameOrRequireTargetedProbes === "boolean"; + const sandboxId = requiresTargetedSandboxProbes ? "alpha-sandbox-id" : sandboxIdOrGatewayName; + const expectedGatewayName = requiresTargetedSandboxProbes + ? sandboxIdOrGatewayName + : expectedGatewayNameOrRequireTargetedProbes; + const assertSandboxProbeTarget = (args: readonly string[]) => { + if (!requiresTargetedSandboxProbes) return; + if (args[0] !== "sandbox" || !["exec", "get", "list"].includes(args[1] ?? "")) return; + const gatewayFlag = args.indexOf("-g"); + expect(gatewayFlag).toBeGreaterThan(1); + expect(args[gatewayFlag + 1]).toBe(expectedGatewayName); + }; + const runCaptureOpenshell = vi.fn((args: string[], _options?: Record) => { + assertSandboxProbeTarget(args); + if (args[0] === "sandbox" && args[1] === "get") { + return `Name: alpha\nId: ${sandboxId}\nState: Ready\n`; + } + if (args[0] === "sandbox" && args[1] === "list") return "alpha Ready"; + return ""; + }); return { - runOpenshell: vi.fn((args: string[]) => - args[0] === "sandbox" && args[1] === "get" + runOpenshell: vi.fn((args: string[]) => { + assertSandboxProbeTarget(args); + return args[0] === "sandbox" && args[1] === "get" ? { status: 0, stdout: `Name: alpha\nId: ${sandboxId}\nState: Ready\n`, stderr: "", } - : { status: 0, stdout: "", stderr: "" }, - ), - runCaptureOpenshell: vi.fn(() => "alpha Ready"), + : { status: 0, stdout: "", stderr: "" }; + }), + runCaptureOpenshell, + sandboxObserver: createCliOpenShellSandboxObserver({ + capture: (args, options) => { + const stdout = runCaptureOpenshell(args, { + ignoreError: true, + timeout: options.timeout, + }); + return { status: 0, output: stdout, stdout, stderr: "" }; + }, + }), sleep: vi.fn(), openshellArgv: vi.fn((args: string[]) => ["openshell", ...args]), verifyDirectSandboxGpu: vi.fn(() => VERIFIED_GPU_PROOF), diff --git a/src/lib/onboard/__test-helpers__/sandbox-observer-replay.ts b/src/lib/onboard/__test-helpers__/sandbox-observer-replay.ts new file mode 100644 index 0000000000..cca8563505 --- /dev/null +++ b/src/lib/onboard/__test-helpers__/sandbox-observer-replay.ts @@ -0,0 +1,48 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { vi } from "vitest"; + +import type { + OpenShellSandboxObserver, + OpenShellSandboxReadiness, +} from "../../adapters/openshell/sandbox-observer"; + +export type SandboxObservationFrame = Readonly<{ + phase: string; + readiness: OpenShellSandboxReadiness; +}> | null; + +export function readySandboxFrame(phase = "Ready"): SandboxObservationFrame { + return { phase, readiness: "ready" }; +} + +export function pendingSandboxFrame(phase: string): SandboxObservationFrame { + return { phase, readiness: "not_ready" }; +} + +export function terminalSandboxFrame(phase: string): SandboxObservationFrame { + return { phase, readiness: "terminal" }; +} + +export function replaySandboxObservations( + sandboxName: string, + frames: readonly SandboxObservationFrame[], +) { + let index = 0; + const listSandboxes = vi.fn(async () => { + const frame = frames[Math.min(index++, frames.length - 1)] ?? null; + return { + ok: true, + value: { + sandboxes: frame ? [{ name: sandboxName, ...frame }] : [], + }, + }; + }); + return { + observer: { listSandboxes }, + listSandboxes, + sleep: vi.fn(), + polls: () => index, + }; +} diff --git a/src/lib/onboard/experimental/hermes-portable-onboarding-created-identity.test.ts b/src/lib/onboard/experimental/hermes-portable-onboarding-created-identity.test.ts index 1993cabb03..efd9b80726 100644 --- a/src/lib/onboard/experimental/hermes-portable-onboarding-created-identity.test.ts +++ b/src/lib/onboard/experimental/hermes-portable-onboarding-created-identity.test.ts @@ -53,6 +53,51 @@ describe("Hermes Portable created-identity capture", () => { expect(capture).toHaveBeenCalledExactlyOnceWith(args); }); + it("accepts the observer's exact named-gateway readiness list (#9803)", () => { + const capture = vi.fn(() => ({ + status: 0, + stdout: Buffer.from("alpha Ready"), + stderr: Buffer.alloc(0), + })); + const run = createHermesPortableReadyRunner("alpha", "nemoclaw", capture); + const args = ["sandbox", "list", "-g", "nemoclaw"]; + + expect(run(args).status).toBe(0); + expect(capture).toHaveBeenCalledExactlyOnceWith(args); + }); + + it("accepts the exact named-gateway readiness exec (#9803)", () => { + const capture = vi.fn(() => ({ + status: 0, + stdout: Buffer.alloc(0), + stderr: Buffer.alloc(0), + })); + const run = createHermesPortableReadyRunner("alpha", "nemoclaw", capture); + const args = ["sandbox", "exec", "-g", "nemoclaw", "--name", "alpha", "--", "true"]; + + expect(run(args).status).toBe(0); + expect(capture).toHaveBeenCalledExactlyOnceWith(args); + }); + + it.each([ + ["another gateway", ["sandbox", "exec", "-g", "other", "--name", "alpha", "--", "true"]], + ["another sandbox", ["sandbox", "exec", "-g", "nemoclaw", "--name", "beta", "--", "true"]], + ])("rejects a readiness exec for %s before capture (#9803)", (_case, args) => { + const capture = vi.fn(); + const run = createHermesPortableReadyRunner("alpha", "nemoclaw", capture); + + expect(() => run(args)).toThrow("unsupported OpenShell command"); + expect(capture).not.toHaveBeenCalled(); + }); + + it("rejects a readiness list for another gateway before capture (#9803)", () => { + const capture = vi.fn(); + const run = createHermesPortableReadyRunner("alpha", "nemoclaw", capture); + + expect(() => run(["sandbox", "list", "-g", "other"])).toThrow("unsupported OpenShell command"); + expect(capture).not.toHaveBeenCalled(); + }); + it.each([ ["wrong gateway", ["sandbox", "get", "-g", "other", "alpha"]], ["wrong sandbox", ["sandbox", "get", "-g", "nemoclaw", "beta"]], diff --git a/src/lib/onboard/experimental/hermes-portable-onboarding.ts b/src/lib/onboard/experimental/hermes-portable-onboarding.ts index 081034c4ac..7e0d156467 100644 --- a/src/lib/onboard/experimental/hermes-portable-onboarding.ts +++ b/src/lib/onboard/experimental/hermes-portable-onboarding.ts @@ -315,6 +315,45 @@ function scopeHermesPortableReadyGetArgs( return null; } +function scopeHermesPortableReadyListArgs(args: string[], gatewayName: string): string[] | null { + if (args.length === 2 && args[0] === "sandbox" && args[1] === "list") { + return ["sandbox", "list", "-g", gatewayName]; + } + if ( + args.length === 4 && + args[0] === "sandbox" && + args[1] === "list" && + args[2] === "-g" && + args[3] === gatewayName + ) { + return ["sandbox", "list", "-g", gatewayName]; + } + return null; +} + +function scopeHermesPortableReadyExecArgs( + args: string[], + sandboxName: string, + gatewayName: string, +): string[] | null { + const scoped = ["sandbox", "exec", "-g", gatewayName, "--name", sandboxName, "--", "true"]; + if ( + args.length === 6 && + args[0] === "sandbox" && + args[1] === "exec" && + args[2] === "--name" && + args[3] === sandboxName && + args[4] === "--" && + args[5] === "true" + ) { + return scoped; + } + if (args.length === scoped.length && args.every((value, index) => value === scoped[index])) { + return scoped; + } + return null; +} + /** Route create readiness and failed-create cleanup through exact schema-5 authority. */ export function createHermesPortableReadyRunner( sandboxName: string, @@ -325,22 +364,11 @@ export function createHermesPortableReadyRunner( const scoped = scopeHermesPortableCreatedIdentityArgs(args, gatewayName) ?? scopeHermesPortableReadyGetArgs(args, sandboxName, gatewayName) ?? - (args[0] === "sandbox" && args[1] === "list" && args.length === 2 - ? ["sandbox", "list", "-g", gatewayName] - : args[0] === "sandbox" && - args[1] === "delete" && - args.length === 3 && - args[2] === sandboxName - ? ["sandbox", "delete", "-g", gatewayName, args[2]!] - : args.length === 6 && - args[0] === "sandbox" && - args[1] === "exec" && - args[2] === "--name" && - args[3] === sandboxName && - args[4] === "--" && - args[5] === "true" - ? ["sandbox", "exec", "-g", gatewayName, "--name", args[3]!, "--", "true"] - : null); + scopeHermesPortableReadyListArgs(args, gatewayName) ?? + scopeHermesPortableReadyExecArgs(args, sandboxName, gatewayName) ?? + (args[0] === "sandbox" && args[1] === "delete" && args.length === 3 && args[2] === sandboxName + ? ["sandbox", "delete", "-g", gatewayName, args[2]!] + : null); if (!scoped) fail("create lifecycle attempted an unsupported OpenShell command"); return capture(scoped); }; diff --git a/src/lib/onboard/policy-selection-application.test.ts b/src/lib/onboard/policy-selection-application.test.ts index 279129ec1c..17911ca4db 100644 --- a/src/lib/onboard/policy-selection-application.test.ts +++ b/src/lib/onboard/policy-selection-application.test.ts @@ -56,7 +56,11 @@ describe("onboarding policy application", () => { sandboxCancelRollback: { markCancelled: vi.fn() }, useColor: false, withSandboxMutationLock, - waitForSandboxReady: vi.fn(() => true), + waitForSandboxReady: vi.fn(async () => ({ + ready: true as const, + reason: "ready" as const, + error: null, + })), waitForSandboxControlPlaneReady: vi.fn(() => true), setPolicyTier: vi.fn(), getRecordedPolicyTier: vi.fn(() => null), @@ -102,12 +106,19 @@ describe("onboarding policy application", () => { sandboxCancelRollback: { markCancelled: vi.fn() }, useColor: false, withSandboxMutationLock: async (_sandboxName, action) => await action(), - waitForSandboxReady: vi.fn(() => true), + waitForSandboxReady: vi.fn(async () => ({ + ready: true as const, + reason: "ready" as const, + error: null, + })), waitForSandboxControlPlaneReady: vi.fn(() => true), setPolicyTier: vi.fn(), getRecordedPolicyTier: vi.fn(() => "balanced"), parsePolicyPresetEnv: vi.fn((value: string) => - value.split(",").map((name) => name.trim()).filter(Boolean), + value + .split(",") + .map((name) => name.trim()) + .filter(Boolean), ), env, }); diff --git a/src/lib/onboard/policy-selection-host-local-route.test.ts b/src/lib/onboard/policy-selection-host-local-route.test.ts index 8fea60dfc8..d085356513 100644 --- a/src/lib/onboard/policy-selection-host-local-route.test.ts +++ b/src/lib/onboard/policy-selection-host-local-route.test.ts @@ -33,7 +33,11 @@ function createHarness() { step: vi.fn(), note: vi.fn(), isNonInteractive: vi.fn(() => true), - waitForSandboxReady: vi.fn(() => true), + waitForSandboxReady: vi.fn(async () => ({ + ready: true as const, + reason: "ready" as const, + error: null, + })), waitForSandboxControlPlaneReady: vi.fn(() => true), syncPresetSelection, selectPolicyTier: vi.fn(async () => "balanced"), diff --git a/src/lib/onboard/policy-selection-recorded-tier.test.ts b/src/lib/onboard/policy-selection-recorded-tier.test.ts index 33787a5678..5e163e2cf6 100644 --- a/src/lib/onboard/policy-selection-recorded-tier.test.ts +++ b/src/lib/onboard/policy-selection-recorded-tier.test.ts @@ -9,7 +9,11 @@ function createPolicySelectionHarness(controlPlaneReady = true) { const selectPolicyTier = vi.fn(async () => "balanced"); const setPolicyTier = vi.fn(); const syncPresetSelection = vi.fn(); - const waitForSandboxReady = vi.fn(() => true); + const waitForSandboxReady = vi.fn(async () => ({ + ready: true, + reason: "ready", + error: null, + })); const waitForSandboxControlPlaneReady = vi.fn(() => controlPlaneReady); const onSelection = vi.fn(); const deps = { @@ -130,6 +134,35 @@ describe("policy selection after interrupted onboarding", () => { expect(exit).toHaveBeenCalledWith(1); }); + it("reports an observation failure before policy application instead of calling it not ready (#9803)", async () => { + const exit = vi.spyOn(process, "exit").mockImplementation((code) => { + throw new Error(`process.exit(${code})`); + }); + const errorLine = vi.spyOn(console, "error").mockImplementation(() => {}); + const { deps, syncPresetSelection, waitForSandboxReady } = createPolicySelectionHarness(); + waitForSandboxReady.mockResolvedValueOnce({ + ready: false, + reason: "observation_failed", + error: { + kind: "authentication", + message: "OpenShell could not authenticate the sandbox observation.", + }, + }); + + await expect(setupPoliciesWithSelection(deps, "alpha", setupOptions)).rejects.toThrow( + "process.exit(1)", + ); + + expect(syncPresetSelection).not.toHaveBeenCalled(); + expect(errorLine).toHaveBeenCalledWith( + " NemoClaw could not observe sandbox 'alpha' before policy application.", + ); + expect(errorLine).toHaveBeenCalledWith( + " OpenShell could not authenticate the sandbox observation.", + ); + expect(exit).toHaveBeenCalledWith(1); + }); + it("does not persist the selection when gateway synchronization fails", async () => { const syncFailure = new Error("policy removal failed"); const { deps, onSelection, syncPresetSelection } = createPolicySelectionHarness(); diff --git a/src/lib/onboard/policy-selection.ts b/src/lib/onboard/policy-selection.ts index 0b25475a2d..2c7d2b3d38 100644 --- a/src/lib/onboard/policy-selection.ts +++ b/src/lib/onboard/policy-selection.ts @@ -40,6 +40,7 @@ import { type PreparedPolicyResumeSelection, preparePolicyPresetResumeSelection, } from "./policy-resume-selection"; +import type { SandboxReadyWaitResult } from "./sandbox-readiness-tracing"; import { createPolicySelectionPromptHelpers, type PolicySelectionPromptDeps, @@ -63,7 +64,7 @@ export type OnboardPolicyApplicationDeps = Omit< step: (number: number, total: number, title: string) => void; localInferenceProviders: readonly string[]; withSandboxMutationLock: typeof import("../state/mcp-lifecycle-lock").withSandboxMutationLock; - waitForSandboxReady(sandboxName: string): boolean; + waitForSandboxReady(sandboxName: string): Promise; waitForSandboxControlPlaneReady(sandboxName: string): boolean; setPolicyTier(sandboxName: string, tierName: string): void; getRecordedPolicyTier(sandboxName: string): string | null | undefined; @@ -132,7 +133,7 @@ export type SetupPolicySelectionDeps = { step: (number: number, total: number, title: string) => void; note: (message: string) => void; isNonInteractive: () => boolean; - waitForSandboxReady: (sandboxName: string) => boolean; + waitForSandboxReady: (sandboxName: string) => Promise; waitForSandboxControlPlaneReady: (sandboxName: string) => boolean; syncPresetSelection: ( sandboxName: string, @@ -350,12 +351,20 @@ export async function setupPoliciesWithSelection( return chosen; } -function requireSandboxReady( +async function requireSandboxReady( deps: SetupPolicySelectionDeps, sandboxName: string, stage: "before" | "after", -): void { - if (!deps.waitForSandboxReady(sandboxName)) { +): Promise { + const readiness = await deps.waitForSandboxReady(sandboxName); + if (!readiness.ready) { + if (readiness.reason === "observation_failed") { + console.error( + ` NemoClaw could not observe sandbox '${sandboxName}' ${stage} policy application.`, + ); + console.error(` ${readiness.error.message}`); + process.exit(1); + } console.error(` Sandbox '${sandboxName}' was not ready ${stage} policy application.`); process.exit(1); } @@ -508,13 +517,13 @@ async function setupPoliciesWithSelectionInner( if (selectedPresets !== null) { const resumeSelection = chosen || []; refuseInPlacePersonalRemoval(personalAlreadyActive, resumeSelection); - requireSandboxReady(deps, sandboxName, "before"); + await requireSandboxReady(deps, sandboxName, "before"); deps.note(` [resume] Reapplying policy presets: ${resumeSelection.join(", ")}`); options.revalidatePolicyRequirements?.( `reapply recorded policy presets to sandbox '${sandboxName}'`, ); deps.syncPresetSelection(sandboxName, currentAppliedPresets, resumeSelection); - requireSandboxReady(deps, sandboxName, "after"); + await requireSandboxReady(deps, sandboxName, "after"); if (onSelection) onSelection(resumeSelection); return resumeSelection; } @@ -586,7 +595,7 @@ async function setupPoliciesWithSelectionInner( retainedPresets.some((name, index) => name !== currentAppliedPresets[index]); if (selectionChanged) { refuseInPlacePersonalRemoval(personalAlreadyActive, retainedPresets); - requireSandboxReady(deps, sandboxName, "before"); + await requireSandboxReady(deps, sandboxName, "before"); deps.note( personalTier ? " [non-interactive] Applying the Personal tier requirement while skipping optional policy presets." @@ -596,7 +605,7 @@ async function setupPoliciesWithSelectionInner( `apply retained policy presets to sandbox '${sandboxName}'`, ); deps.syncPresetSelection(sandboxName, currentAppliedPresets, retainedPresets); - requireSandboxReady(deps, sandboxName, "after"); + await requireSandboxReady(deps, sandboxName, "after"); if (onSelection) onSelection(retainedPresets); return retainedPresets; } @@ -677,13 +686,13 @@ async function setupPoliciesWithSelectionInner( } refuseInPlacePersonalRemoval(personalAlreadyActive, chosen); - requireSandboxReady(deps, sandboxName, "before"); + await requireSandboxReady(deps, sandboxName, "before"); deps.note(` [non-interactive] Applying policy presets: ${chosen.join(", ")}`); options.revalidatePolicyRequirements?.( `apply non-interactive policy presets to sandbox '${sandboxName}'`, ); deps.syncPresetSelection(sandboxName, currentAppliedPresets, chosen); - requireSandboxReady(deps, sandboxName, "after"); + await requireSandboxReady(deps, sandboxName, "after"); if (onSelection) onSelection(chosen); return chosen; } @@ -723,7 +732,7 @@ async function setupPoliciesWithSelectionInner( ); refuseInPlacePersonalRemoval(personalAlreadyActive, interactiveChoice); - requireSandboxReady(deps, sandboxName, "before"); + await requireSandboxReady(deps, sandboxName, "before"); const accessByName: Record = {}; const interactiveChoiceNames = new Set(interactiveChoice); @@ -732,7 +741,7 @@ async function setupPoliciesWithSelectionInner( } options.revalidatePolicyRequirements?.(`apply policy presets to sandbox '${sandboxName}'`); deps.syncPresetSelection(sandboxName, currentAppliedPresets, interactiveChoice, accessByName); - requireSandboxReady(deps, sandboxName, "after"); + await requireSandboxReady(deps, sandboxName, "after"); if (onSelection) onSelection(interactiveChoice); return interactiveChoice; } diff --git a/src/lib/onboard/sandbox-create/orchestration.ts b/src/lib/onboard/sandbox-create/orchestration.ts index 1cffc904a7..39f2f16258 100644 --- a/src/lib/onboard/sandbox-create/orchestration.ts +++ b/src/lib/onboard/sandbox-create/orchestration.ts @@ -12,6 +12,7 @@ import { } from "../../adapters/openshell/policy-authority"; import type { SandboxPolicyAuthority } from "../../adapters/openshell/policy-authority"; import { HERMES_PORTABLE_OPENSHELL_VERSION } from "../../adapters/openshell/resolve-shared"; +import { createCliOpenShellSandboxObserverFromRunner } from "../../adapters/openshell/sandbox-observer-cli"; import { NEMOCLAW_CREATE_ATTEMPT_LABEL } from "../../adapters/openshell/sandbox-identity"; import type { AgentDefinition } from "../../agent/defs"; import type { WebSearchConfig } from "../../inference/web-search"; @@ -2794,6 +2795,9 @@ export function createSandboxWithBaseImageResolution(runtime: SandboxCreateOrche { runOpenshell: hermesPortableReadyRunner ?? runOpenshell, runCaptureOpenshell: hermesPortableReadyCapture ?? runCaptureOpenshell, + sandboxObserver: createCliOpenShellSandboxObserverFromRunner( + hermesPortableReadyRunner ?? runOpenshell, + ), sleep: sleepSeconds, openshellArgv, verifyDirectSandboxGpu: createGpuVerifier, diff --git a/src/lib/onboard/sandbox-gpu-create-flow.test.ts b/src/lib/onboard/sandbox-gpu-create-flow.test.ts index a039df5bad..cfc08cd7bf 100644 --- a/src/lib/onboard/sandbox-gpu-create-flow.test.ts +++ b/src/lib/onboard/sandbox-gpu-create-flow.test.ts @@ -405,7 +405,10 @@ describe("runSandboxGpuCreateFlow provider-owned managed create", () => { vi.mocked(deps.runCaptureOpenshell).mockClear(); await expect(runSandboxGpuCreateFlow(input, deps)).resolves.toMatchObject({ route: "none" }); - expect(deps.runCaptureOpenshell).toHaveBeenCalledWith(["sandbox", "list"], READY_CHECK_OPTIONS); + expect(deps.runCaptureOpenshell).toHaveBeenCalledWith( + ["sandbox", "list", "-g", "nemoclaw"], + READY_CHECK_OPTIONS, + ); expect(vi.mocked(console.warn).mock.calls.flat().join("\n")).toContain( "unrelated sandbox 'bravo'", @@ -432,6 +435,7 @@ describe("runSandboxGpuCreateFlow provider-owned managed create", () => { it("reports the terminal phase when an incomplete managed create cannot become ready (#9819)", async () => { const input = createInput(); + input.gatewayName = "nemoclaw-18080"; const bootstrapIdentity = "e".repeat(64); input.managedBootstrap = { bootstrapIdentity, @@ -472,11 +476,12 @@ describe("runSandboxGpuCreateFlow provider-owned managed create", () => { reason: "terminal_failure_phase", failurePhase: "Failed", }); - await expect(runSandboxGpuCreateFlow(input, deps)).rejects.toThrow( "Sandbox 'alpha' entered Failed phase before it became ready (waited up to 60s).", ); - expect(mocks.waitForCreatedSandboxReadyWithTrace).toHaveBeenCalledOnce(); + expect(mocks.waitForCreatedSandboxReadyWithTrace).toHaveBeenCalledWith( + expect.objectContaining({ target: { kind: "named", gatewayName: input.gatewayName } }), + ); }); }); describe("runSandboxGpuCreateFlow proof authorization", () => { @@ -617,10 +622,12 @@ describe("runSandboxGpuCreateFlow native failure and readiness", () => { expect(args[3].readyCheck()).toBe(true); return { status: 0, output: "Created sandbox: alpha", sawProgress: true }; }); - const result = await runSandboxGpuCreateFlow(createInput(), deps); expect(result).toMatchObject({ route: "native" }); - expect(deps.runCaptureOpenshell).toHaveBeenCalledWith(["sandbox", "list"], READY_CHECK_OPTIONS); + expect(deps.runCaptureOpenshell).toHaveBeenCalledWith( + ["sandbox", "list", "-g", "nemoclaw"], + READY_CHECK_OPTIONS, + ); }); it("defers restart-safe no-GPU recreation until the create process exits (#8720)", async () => { @@ -707,9 +714,9 @@ describe("runSandboxGpuCreateFlow native failure and readiness", () => { const deps = createDeps(); vi.mocked(deps.runOpenshell).mockImplementation( createSequencedOpenShellRunner([ - ["sandbox get alpha", [readySandboxGetResult(), readySandboxGetResult()]], + ["sandbox get -g nemoclaw alpha", [readySandboxGetResult(), readySandboxGetResult()]], [ - "sandbox exec --name alpha -- true", + "sandbox exec -g nemoclaw --name alpha -- true", [{ status: 1, stdout: "", stderr: "permission denied" }], ], ]), @@ -760,11 +767,11 @@ describe("runSandboxGpuCreateFlow native failure and readiness", () => { vi.mocked(deps.runOpenshell).mockImplementation( createSequencedOpenShellRunner([ [ - "sandbox get alpha", + "sandbox get -g nemoclaw alpha", [readySandboxGetResult(), readySandboxGetResult(), readySandboxGetResult()], ], [ - "sandbox exec --name alpha -- true", + "sandbox exec -g nemoclaw --name alpha -- true", [ { status: 1, @@ -791,7 +798,9 @@ describe("runSandboxGpuCreateFlow native failure and readiness", () => { expect( vi .mocked(deps.runOpenshell) - .mock.calls.filter(([args]) => args.join(" ") === "sandbox exec --name alpha -- true"), + .mock.calls.filter( + ([args]) => args.join(" ") === "sandbox exec -g nemoclaw --name alpha -- true", + ), ).toHaveLength(2); expect(patch.rollbackManagedStartupAfterCreateFailure).not.toHaveBeenCalled(); expect(deps.runOpenshell).not.toHaveBeenCalledWith( diff --git a/src/lib/onboard/sandbox-gpu-create-flow.ts b/src/lib/onboard/sandbox-gpu-create-flow.ts index 8b5b9bc719..0e1a83fb3c 100644 --- a/src/lib/onboard/sandbox-gpu-create-flow.ts +++ b/src/lib/onboard/sandbox-gpu-create-flow.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import type { AgentDefinition } from "../agent/defs"; +import type { OpenShellSandboxObserver } from "../adapters/openshell/sandbox-observer"; import { NEMOCLAW_CREATE_ATTEMPT_LABEL } from "../adapters/openshell/sandbox-identity"; import type { StreamSandboxCreateResult } from "../sandbox/create-stream"; import { redactFull } from "../security/redact"; @@ -292,6 +293,7 @@ export function refuseApfMutableNameFallbackCleanup(sandboxName: string) { export interface SandboxGpuCreateFlowDeps { runOpenshell: RunOpenshell; runCaptureOpenshell: RunCaptureOpenshell; + sandboxObserver: OpenShellSandboxObserver; sleep: Sleep; openshellArgv(args: string[]): string[]; verifyDirectSandboxGpu(sandboxName: string): SandboxGpuProofResult; diff --git a/src/lib/onboard/sandbox-gpu-create-identity-gate.test.ts b/src/lib/onboard/sandbox-gpu-create-identity-gate.test.ts index 1432e27f91..1c2ba29b1a 100644 --- a/src/lib/onboard/sandbox-gpu-create-identity-gate.test.ts +++ b/src/lib/onboard/sandbox-gpu-create-identity-gate.test.ts @@ -121,10 +121,48 @@ beforeEach(() => setupGpuFlowMocks(mocks)); afterEach(resetGpuFlowMocks); describe("created sandbox identity gate", () => { + it("keeps fresh-create readiness probes on the owning gateway (#9803)", async () => { + const gatewayName = "nemoclaw-18080"; + const input = createGpuFlowInput(); + input.gatewayName = gatewayName; + const deps = createGpuFlowDeps(gatewayName, true); + mocks.streamSandboxCreate.mockImplementationOnce(async (...args) => { + expect(args[3].readyCheck()).toBe(true); + return { status: 0, output: "Created sandbox: alpha", sawProgress: true }; + }); + mocks.waitForCreatedSandboxReadyWithTrace.mockImplementationOnce(async (options) => { + expect(options.target).toEqual({ kind: "named", gatewayName }); + await expect( + options.observer.listSandboxes({ target: options.target }), + ).resolves.toMatchObject({ ok: true }); + expect(options.checkReadyIdentity?.()).toBe("ready"); + return { ready: true, reason: "ready", failurePhase: null }; + }); + + await expect(runSandboxGpuCreateFlow(input, deps)).resolves.toMatchObject({ + route: "native", + }); + + expect(deps.runCaptureOpenshell).toHaveBeenCalledWith(["sandbox", "list", "-g", gatewayName], { + ignoreError: true, + timeout: 5_000, + }); + expect(deps.runOpenshell).toHaveBeenCalledWith( + ["sandbox", "get", "-g", gatewayName, "alpha"], + expect.objectContaining({ ignoreError: true, suppressOutput: true }), + ); + expect(deps.runOpenshell).toHaveBeenCalledWith( + ["sandbox", "exec", "-g", gatewayName, "--name", "alpha", "--", "true"], + expect.objectContaining({ ignoreError: true, suppressOutput: true }), + ); + }); + it("resumes the exact verified sandbox without issuing another create (#9833)", async () => { const events: string[] = []; const sandboxId = "alpha-sandbox-id"; + const gatewayName = "nemoclaw-18080"; const input = noGpuInput(); + input.gatewayName = gatewayName; input.resumeVerifiedCreate = { route: "none", liveIdentityFingerprint: fingerprintSandboxRecreateValue(sandboxId), @@ -154,7 +192,7 @@ describe("created sandbox identity gate", () => { }); const deps = createGpuFlowDeps(); vi.mocked(deps.runOpenshell).mockImplementation((args) => - args.join(" ") === "sandbox get alpha" + args.join(" ") === `sandbox get -g ${gatewayName} alpha` ? { status: 0, stdout: `Name: alpha\nId: ${sandboxId}\nState: Ready\n`, stderr: "" } : { status: 0, stdout: "", stderr: "" }, ); @@ -169,6 +207,10 @@ describe("created sandbox identity gate", () => { }); expect(mocks.streamSandboxCreate).not.toHaveBeenCalled(); + expect(deps.runOpenshell).toHaveBeenCalledWith( + ["sandbox", "get", "-g", gatewayName, "alpha"], + expect.objectContaining({ ignoreError: true, suppressOutput: true }), + ); expect(events).toEqual([ "verify-created", "revalidate:activate managed sandbox network for 'alpha'", @@ -199,7 +241,7 @@ describe("created sandbox identity gate", () => { mocks.createDockerGpuSandboxCreatePatch.mockReturnValue(patch); const deps = createGpuFlowDeps(); vi.mocked(deps.runOpenshell).mockImplementation((args) => - args.join(" ") === "sandbox get alpha" + args.join(" ") === "sandbox get -g nemoclaw alpha" ? { status: 0, stdout: "Name: alpha\nId: replacement-id\nState: Ready\n", stderr: "" } : { status: 0, stdout: "", stderr: "" }, ); @@ -373,7 +415,7 @@ describe("created sandbox identity gate", () => { expect(firstIdentityTimeout as number).toBeGreaterThan(0); expect(firstIdentityTimeout as number).toBeLessThanOrEqual(30_000); expect(deps.runCaptureOpenshell).not.toHaveBeenCalledWith( - ["sandbox", "get", "alpha"], + ["sandbox", "get", "-g", "nemoclaw", "alpha"], expect.anything(), ); }); diff --git a/src/lib/onboard/sandbox-gpu-create-run-attempt.ts b/src/lib/onboard/sandbox-gpu-create-run-attempt.ts index 9204e0deab..3ded03a116 100644 --- a/src/lib/onboard/sandbox-gpu-create-run-attempt.ts +++ b/src/lib/onboard/sandbox-gpu-create-run-attempt.ts @@ -19,7 +19,7 @@ import { import { printSandboxCreateRecoveryHints } from "../build-context"; import { streamSandboxCreate, type StreamSandboxCreateResult } from "../sandbox/create-stream"; import { getReadyCheckOutputPatternsForAgent } from "../sandbox/create-stream-ready-gate"; -import { getSandboxFailurePhase, isSandboxReady } from "../state/gateway"; +import { isSandboxReady } from "../state/gateway"; import type { SandboxGpuProofResult } from "../state/registry"; import { classifySandboxCreateFailure } from "../validation"; import { reportSandboxCreateFailure } from "./created-sandbox-failure"; @@ -211,12 +211,13 @@ function remainingReadinessProbeTimeout(getRemainingMs: () => number): number | function probeExactOpenShellSandboxId( sandboxName: string, + gatewayName: string, deps: SandboxGpuCreateFlowDeps, getRemainingMs: () => number = () => SANDBOX_RECREATE_PROBE_TIMEOUT_MS, ): OpenShellSandboxIdentityProbe { const timeout = remainingReadinessProbeTimeout(getRemainingMs); if (timeout === null) return { state: "not_ready" }; - const result = deps.runOpenshell(["sandbox", "get", sandboxName], { + const result = deps.runOpenshell(["sandbox", "get", "-g", gatewayName, sandboxName], { ignoreError: true, suppressOutput: true, timeout, @@ -320,41 +321,47 @@ function waitForCreatedOpenShellSandboxPublication( function checkRecreatedSandboxReadyIdentity( sandboxName: string, + gatewayName: string, expectedSandboxId: string, deps: SandboxGpuCreateFlowDeps, getRemainingMs: () => number, ): ReturnType { - const identity = probeExactOpenShellSandboxId(sandboxName, deps, getRemainingMs); + const identity = probeExactOpenShellSandboxId(sandboxName, gatewayName, deps, getRemainingMs); if (identity.state === "not_ready") return "not_ready"; if (identity.state === "failed") return "probe_failed"; if (identity.sandboxId !== expectedSandboxId) return "identity_changed"; - return checkSandboxExecutableReadiness(sandboxName, deps, getRemainingMs); + return checkSandboxExecutableReadiness(sandboxName, gatewayName, deps, getRemainingMs); } function checkCreatedSandboxReadyIdentity( sandboxName: string, + gatewayName: string, deps: SandboxGpuCreateFlowDeps, getRemainingMs: () => number, ): ReturnType { - const identity = probeExactOpenShellSandboxId(sandboxName, deps, getRemainingMs); + const identity = probeExactOpenShellSandboxId(sandboxName, gatewayName, deps, getRemainingMs); if (identity.state === "not_ready") return "not_ready"; if (identity.state === "failed") return "probe_failed"; - return checkSandboxExecutableReadiness(sandboxName, deps, getRemainingMs); + return checkSandboxExecutableReadiness(sandboxName, gatewayName, deps, getRemainingMs); } function checkSandboxExecutableReadiness( sandboxName: string, + gatewayName: string, deps: SandboxGpuCreateFlowDeps, getRemainingMs: () => number, ): ReturnType { const timeout = remainingReadinessProbeTimeout(getRemainingMs); if (timeout === null) return "not_ready"; - const result = deps.runOpenshell(["sandbox", "exec", "--name", sandboxName, "--", "true"], { - ignoreError: true, - suppressOutput: true, - timeout, - killSignal: "SIGKILL", - }); + const result = deps.runOpenshell( + ["sandbox", "exec", "-g", gatewayName, "--name", sandboxName, "--", "true"], + { + ignoreError: true, + suppressOutput: true, + timeout, + killSignal: "SIGKILL", + }, + ); if (result.status === 0 && !result.error) return "ready"; if (result.error || result.status === null || ("signal" in result && result.signal)) { return "probe_failed"; @@ -603,7 +610,7 @@ export function createSandboxGpuCreateAttemptRunner( streamSandboxCreate(createExecutable, createExecutableArgs, createEnv, { ...(input.createWorkingDirectory ? { cwd: input.createWorkingDirectory } : {}), readyCheck: () => { - const list = deps.runCaptureOpenshell(["sandbox", "list"], { + const list = deps.runCaptureOpenshell(["sandbox", "list", "-g", input.gatewayName], { ignoreError: true, timeout: SANDBOX_READY_PROBE_TIMEOUT_MS, }); @@ -642,7 +649,7 @@ export function createSandboxGpuCreateAttemptRunner( if (route !== input.resumeVerifiedCreate.route) { throw new Error("Verified sandbox recovery route changed before continuation."); } - const identity = probeExactOpenShellSandboxId(input.sandboxName, deps); + const identity = probeExactOpenShellSandboxId(input.sandboxName, input.gatewayName, deps); if (identity.state !== "identified") { throw new Error( `Cannot resume sandbox '${input.sandboxName}': its exact live identity is unavailable.`, @@ -686,12 +693,11 @@ export function createSandboxGpuCreateAttemptRunner( throw new ManagedBootstrapCreateStreamFailure(result); } if (createFailure?.kind === "sandbox_create_incomplete") { - const readiness = sandboxReadinessTracing.waitForCreatedSandboxReadyWithTrace({ + const readiness = await sandboxReadinessTracing.waitForCreatedSandboxReadyWithTrace({ sandboxName: input.sandboxName, timeoutSecs: input.sandboxReadyTimeoutSecs, - runCaptureOpenshell: deps.runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer: deps.sandboxObserver, + target: { kind: "named", gatewayName: input.gatewayName }, stableReadyPolls: REPLACEMENT_STABLE_READY_POLLS, sleep: deps.sleep, }); @@ -708,11 +714,22 @@ export function createSandboxGpuCreateAttemptRunner( ); } } else { - const list = deps.runCaptureOpenshell(["sandbox", "list"], { - ignoreError: true, - timeout: SANDBOX_READY_PROBE_TIMEOUT_MS, - }); - if (!isSandboxReady(list, input.sandboxName)) { + const observation = await sandboxReadinessTracing.observeOpenShellSandbox( + deps.sandboxObserver, + { kind: "named", gatewayName: input.gatewayName }, + input.sandboxName, + SANDBOX_READY_PROBE_TIMEOUT_MS, + ); + if (!observation.ok) { + if (createAttemptNonce) persistIdentitySettlementRecovery(); + throw new Error( + `Managed bootstrap create completed, but NemoClaw could not observe the sandbox. ${observation.error.message}`, + ); + } + if ( + observation.value.state !== "present" || + observation.value.sandbox.readiness !== "ready" + ) { if (createAttemptNonce) persistIdentitySettlementRecovery(); throw new Error( "Managed bootstrap create completed without an authoritative Ready sandbox.", @@ -729,7 +746,11 @@ export function createSandboxGpuCreateAttemptRunner( runCaptureOpenshell: deps.runCaptureOpenshell, sleep: (milliseconds) => deps.sleep(milliseconds / 1000), }) - : resolveOpenShellSandboxId(input.sandboxName, deps.runCaptureOpenshell); + : resolveOpenShellSandboxId( + input.sandboxName, + deps.runCaptureOpenshell, + input.gatewayName, + ); } catch (error) { if (createAttemptNonce) persistIdentitySettlementRecovery(); throw new Error( @@ -888,7 +909,7 @@ export function createSandboxGpuCreateAttemptRunner( } const preRecreateIdentity = deferRestartSafeCutover && !resumedSandboxId - ? probeExactOpenShellSandboxId(input.sandboxName, deps) + ? probeExactOpenShellSandboxId(input.sandboxName, input.gatewayName, deps) : null; const expectedRecreatedSandboxId = resumedSandboxId ?? @@ -913,12 +934,11 @@ export function createSandboxGpuCreateAttemptRunner( await runtimePatch.waitForSupervisorReconnectIfNeeded(); revalidatePostCreateEffect(`reconnect sandbox supervisor for '${input.sandboxName}'`); console.log(" Waiting for sandbox to become ready..."); - const readiness = sandboxReadinessTracing.waitForCreatedSandboxReadyWithTrace({ + const readiness = await sandboxReadinessTracing.waitForCreatedSandboxReadyWithTrace({ sandboxName: input.sandboxName, timeoutSecs: input.sandboxReadyTimeoutSecs, - runCaptureOpenshell: deps.runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer: deps.sandboxObserver, + target: { kind: "named", gatewayName: input.gatewayName }, stableReadyPolls: compatibility || managedBootstrap || expectedRecreatedSandboxId ? REPLACEMENT_STABLE_READY_POLLS @@ -927,6 +947,7 @@ export function createSandboxGpuCreateAttemptRunner( ? (getRemainingMs = () => SANDBOX_RECREATE_PROBE_TIMEOUT_MS) => checkRecreatedSandboxReadyIdentity( input.sandboxName, + input.gatewayName, expectedRecreatedSandboxId, deps, getRemainingMs, @@ -934,7 +955,12 @@ export function createSandboxGpuCreateAttemptRunner( : input.terminalAgent ? undefined : (getRemainingMs = () => SANDBOX_RECREATE_PROBE_TIMEOUT_MS) => - checkCreatedSandboxReadyIdentity(input.sandboxName, deps, getRemainingMs), + checkCreatedSandboxReadyIdentity( + input.sandboxName, + input.gatewayName, + deps, + getRemainingMs, + ), sleep: deps.sleep, }); if (!readiness.ready) { diff --git a/src/lib/onboard/sandbox-readiness-stability.test.ts b/src/lib/onboard/sandbox-readiness-stability.test.ts index 4fba29d0ff..c8e0a9f6cf 100644 --- a/src/lib/onboard/sandbox-readiness-stability.test.ts +++ b/src/lib/onboard/sandbox-readiness-stability.test.ts @@ -3,58 +3,58 @@ import { describe, expect, it, vi } from "vitest"; -import { getSandboxFailurePhase, isSandboxReady } from "../state/gateway"; +import { namedOpenShellGateway } from "../adapters/openshell/sandbox-observer"; +import { + readySandboxFrame, + replaySandboxObservations, + terminalSandboxFrame, +} from "./__test-helpers__/sandbox-observer-replay"; import { waitForCreatedSandboxReadyWithTrace } from "./sandbox-readiness-tracing"; const NAME = "my-sandbox"; - -function replay(outputs: readonly string[]) { - let index = 0; - const runCaptureOpenshell = vi.fn(() => outputs[Math.min(index++, outputs.length - 1)] ?? ""); - return { runCaptureOpenshell, sleep: vi.fn() }; -} +const TARGET = namedOpenShellGateway("nemoclaw"); describe("created sandbox Ready stability", () => { - it("preserves single-poll Ready acceptance by default", () => { - const { runCaptureOpenshell, sleep } = replay([`${NAME} Ready 1s ago`]); + it("preserves single-poll Ready acceptance by default", async () => { + const { observer, listSandboxes, sleep } = replaySandboxObservations(NAME, [ + readySandboxFrame(), + ]); - const ready = waitForCreatedSandboxReadyWithTrace({ + const ready = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 600, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer, + target: TARGET, sleep, }); expect(ready).toEqual({ ready: true, reason: "ready", failurePhase: null }); - expect(runCaptureOpenshell).toHaveBeenCalledOnce(); + expect(listSandboxes).toHaveBeenCalledOnce(); expect(sleep).not.toHaveBeenCalled(); }); - it("rejects a stale Ready row until compatibility recreation reaches stable Ready", () => { + it("rejects a stale Ready row until compatibility recreation reaches stable Ready", async () => { // Exact fallback-run ordering from 28817562371: after a successful // supervisor exec, sandbox-list first retained the old container's Ready // row, then published the recreated supervisor's Error -> Ready sequence. - const { runCaptureOpenshell, sleep } = replay([ - `${NAME} Ready old-container`, - `${NAME} Error replacement-registering`, - `${NAME} Ready replacement-connected`, - `${NAME} Ready replacement-stable`, + const { observer, listSandboxes, sleep } = replaySandboxObservations(NAME, [ + readySandboxFrame(), + terminalSandboxFrame("Error"), + readySandboxFrame(), + readySandboxFrame(), ]); - const ready = waitForCreatedSandboxReadyWithTrace({ + const ready = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 600, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer, + target: TARGET, stableReadyPolls: 2, sleep, }); expect(ready).toEqual({ ready: true, reason: "ready", failurePhase: null }); - expect(runCaptureOpenshell).toHaveBeenCalledTimes(4); + expect(listSandboxes).toHaveBeenCalledTimes(4); expect(sleep).toHaveBeenCalledTimes(3); expect(sleep).toHaveBeenNthCalledWith(1, 2); }); diff --git a/src/lib/onboard/sandbox-readiness-tracing.test.ts b/src/lib/onboard/sandbox-readiness-tracing.test.ts index 52b9459fc4..ccdd9eff2a 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.test.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.test.ts @@ -3,8 +3,20 @@ import { describe, expect, it, vi } from "vitest"; -import { getSandboxFailurePhase, isSandboxReady } from "../state/gateway"; import { + namedOpenShellGateway, + type OpenShellSandboxObserver, + type OpenShellSandboxReadinessProbe, +} from "../adapters/openshell/sandbox-observer"; +import { + pendingSandboxFrame, + readySandboxFrame, + replaySandboxObservations, + terminalSandboxFrame, + type SandboxObservationFrame, +} from "./__test-helpers__/sandbox-observer-replay"; +import { + createCliSandboxReadyWaiter, createSandboxReadyWaiter, formatCreatedSandboxReadinessFailureMessage, getSandboxReadyErrorDebouncePolls, @@ -15,122 +27,305 @@ import { } 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 }; +const TARGET = namedOpenShellGateway("nemoclaw"); +const REJECTED_OBSERVATION_ERROR = { + kind: "command" as const, + reason: "failed" as const, + message: "OpenShell sandbox observation failed before returning a result.", +}; + +function replay(frames: readonly SandboxObservationFrame[]) { + return replaySandboxObservations(NAME, frames); } describe("createSandboxReadyWaiter", () => { - it("uses the legacy Docker-driver poll settings as an adaptive deadline budget", () => { - const runCaptureOpenshell = vi.fn(() => `${NAME} Provisioning`); + it("reads the authoritative gateway name for each CLI-backed wait (#9803)", async () => { + let gatewayName = "nemoclaw"; + const capture = vi.fn((_args: string[]) => ({ + status: 0, + output: `${NAME} Ready`, + stdout: `${NAME} Ready`, + stderr: "", + })); + const waitForSandboxReady = createCliSandboxReadyWaiter({ + capture, + getGatewayName: () => gatewayName, + isLinuxDockerDriverGatewayEnabled: () => true, + sleep: vi.fn(), + }); + + await expect(waitForSandboxReady(NAME, 1, 0)).resolves.toEqual({ + ready: true, + reason: "ready", + error: null, + }); + gatewayName = "nemoclaw-18080"; + await expect(waitForSandboxReady(NAME, 1, 0)).resolves.toEqual({ + ready: true, + reason: "ready", + error: null, + }); + + expect(capture.mock.calls.map(([args]) => args)).toEqual([ + ["sandbox", "list", "-g", "nemoclaw"], + ["sandbox", "list", "-g", "nemoclaw-18080"], + ]); + }); + + it("uses the legacy Docker-driver poll settings as an adaptive deadline budget", async () => { + const { observer, listSandboxes } = replay([pendingSandboxFrame("Provisioning")]); const sleep = vi.fn(); const waitForSandboxReady = createSandboxReadyWaiter({ - runCaptureOpenshell, - isSandboxReady, + observer, + target: TARGET, isLinuxDockerDriverGatewayEnabled: () => true, sleep, }); - expect(waitForSandboxReady(NAME, 2, 3)).toBe(false); - expect(runCaptureOpenshell).toHaveBeenCalledTimes(7); + await expect(waitForSandboxReady(NAME, 2, 3)).resolves.toEqual({ + ready: false, + reason: "timeout", + error: null, + }); + expect(listSandboxes).toHaveBeenCalledTimes(7); + const observationTimeouts = listSandboxes.mock.calls.map(([request]) => request.timeoutMs); + expect(listSandboxes).toHaveBeenCalledWith(expect.objectContaining({ target: TARGET })); + expect(observationTimeouts[0]).toBe(6_000); + expect( + observationTimeouts.every( + (timeoutMs, index) => index === 0 || timeoutMs! < observationTimeouts[index - 1]!, + ), + ).toBe(true); expect(sleep).toHaveBeenCalledTimes(7); expect(sleep).toHaveBeenNthCalledWith(1, 0.25); expect(sleep.mock.calls.reduce((total, [seconds]) => total + seconds, 0)).toBeCloseTo(6, 2); expect(Math.max(...sleep.mock.calls.map(([seconds]) => seconds))).toBeLessThanOrEqual(3); }); - it("uses the same deadline for the legacy Kubernetes pod fallback", () => { - const runCaptureOpenshell = vi - .fn() - .mockReturnValueOnce(`${NAME} Provisioning`) - .mockReturnValueOnce("Pending"); + it("uses the same deadline for the legacy Kubernetes pod fallback", async () => { + const { observer, listSandboxes } = replay([pendingSandboxFrame("Provisioning")]); + const fallbackReadinessProbe = vi.fn(async () => ({ + ok: true, + value: "not_ready", + })); const sleep = vi.fn(); const waitForSandboxReady = createSandboxReadyWaiter({ - runCaptureOpenshell, - isSandboxReady, + observer, + target: TARGET, + fallbackReadinessProbe, isLinuxDockerDriverGatewayEnabled: () => false, sleep, }); - expect(waitForSandboxReady(NAME, 1, 2)).toBe(false); - expect(runCaptureOpenshell).toHaveBeenCalledTimes(8); - expect(runCaptureOpenshell.mock.calls[1]?.[0]).toContain("kubectl"); + await expect(waitForSandboxReady(NAME, 1, 2)).resolves.toEqual({ + ready: false, + reason: "timeout", + error: null, + }); + expect(listSandboxes).toHaveBeenCalledTimes(4); + expect(fallbackReadinessProbe).toHaveBeenCalledTimes(4); + expect(fallbackReadinessProbe).toHaveBeenCalledWith( + expect.objectContaining({ + target: TARGET, + sandboxName: NAME, + timeoutMs: expect.any(Number), + }), + ); + const listTimeouts = listSandboxes.mock.calls.map(([request]) => request.timeoutMs); + const fallbackTimeouts = fallbackReadinessProbe.mock.calls.map( + ([request]) => request.timeoutMs, + ); + expect( + fallbackTimeouts.every( + (timeoutMs, index) => timeoutMs! > 0 && timeoutMs! <= listTimeouts[index]!, + ), + ).toBe(true); expect(sleep).toHaveBeenCalledTimes(4); expect(sleep.mock.calls.reduce((total, [seconds]) => total + seconds, 0)).toBeCloseTo(2, 2); }); - it("keeps the traced waiter within its deadline without an extra final delay", () => { - const runCaptureOpenshell = vi - .fn() - .mockReturnValueOnce(`${NAME} Provisioning`) - .mockReturnValueOnce("Pending"); + it("keeps the traced waiter within its deadline without an extra final delay", async () => { + const { observer } = replay([pendingSandboxFrame("Provisioning")]); + const fallbackReadinessProbe = vi.fn(async () => ({ + ok: true as const, + value: "not_ready" as const, + })); const sleep = vi.fn(); - expect( + await expect( waitForSandboxReadyWithTrace({ sandboxName: NAME, attempts: 1, delaySeconds: 2, - runCaptureOpenshell, - isSandboxReady, + observer, + target: TARGET, + fallbackReadinessProbe, isLinuxDockerDriverGatewayEnabled: () => false, sleep, }), - ).toBe(false); + ).resolves.toEqual({ ready: false, reason: "timeout", error: null }); expect(sleep).toHaveBeenCalledTimes(4); expect(sleep.mock.calls.reduce((total, [seconds]) => total + seconds, 0)).toBeCloseTo(2, 2); }); + + it("stops on an authentication failure without using the readiness fallback (#9803)", async () => { + const error = { + kind: "authentication" as const, + message: "OpenShell could not authenticate the sandbox observation.", + }; + const listSandboxes = vi.fn(async () => ({ + ok: false, + error, + })); + const fallbackReadinessProbe = vi.fn(); + const sleep = vi.fn(); + + await expect( + waitForSandboxReadyWithTrace({ + sandboxName: NAME, + attempts: 10, + delaySeconds: 2, + observer: { listSandboxes }, + target: TARGET, + fallbackReadinessProbe, + isLinuxDockerDriverGatewayEnabled: () => false, + sleep, + now: () => 1_000, + }), + ).resolves.toEqual({ ready: false, reason: "observation_failed", error }); + expect(listSandboxes).toHaveBeenCalledOnce(); + expect(listSandboxes).toHaveBeenCalledWith({ target: TARGET, timeoutMs: 20_000 }); + expect(fallbackReadinessProbe).not.toHaveBeenCalled(); + expect(sleep).not.toHaveBeenCalled(); + }); + + it("returns a typed failure when the sandbox observer rejects (#9803)", async () => { + const listSandboxes = vi + .fn() + .mockRejectedValue(new Error("untrusted observer diagnostic")); + + const result = await waitForSandboxReadyWithTrace({ + sandboxName: NAME, + attempts: 1, + delaySeconds: 2, + observer: { listSandboxes }, + target: TARGET, + isLinuxDockerDriverGatewayEnabled: () => true, + sleep: vi.fn(), + }); + + expect(result).toEqual({ + ready: false, + reason: "observation_failed", + error: REJECTED_OBSERVATION_ERROR, + }); + expect(JSON.stringify(result)).not.toContain("untrusted observer diagnostic"); + }); + + it("returns a typed failure when the legacy readiness probe rejects (#9803)", async () => { + const { observer } = replay([pendingSandboxFrame("Provisioning")]); + const fallbackReadinessProbe = vi + .fn() + .mockRejectedValue(new Error("untrusted legacy probe diagnostic")); + + const result = await waitForSandboxReadyWithTrace({ + sandboxName: NAME, + attempts: 1, + delaySeconds: 2, + observer, + target: TARGET, + fallbackReadinessProbe, + isLinuxDockerDriverGatewayEnabled: () => false, + sleep: vi.fn(), + }); + + expect(result).toEqual({ + ready: false, + reason: "observation_failed", + error: REJECTED_OBSERVATION_ERROR, + }); + expect(JSON.stringify(result)).not.toContain("untrusted legacy probe diagnostic"); + }); + + it("retries an unreachable gateway observation before accepting Ready (#9803)", async () => { + const listSandboxes = vi + .fn() + .mockResolvedValueOnce({ + ok: false, + error: { + kind: "transport", + reason: "unreachable", + message: "OpenShell could not reach the selected gateway.", + }, + }) + .mockResolvedValue({ + ok: true, + value: { sandboxes: [{ name: NAME, phase: "Ready", readiness: "ready" }] }, + }); + const sleep = vi.fn(); + + await expect( + waitForSandboxReadyWithTrace({ + sandboxName: NAME, + attempts: 2, + delaySeconds: 1, + observer: { listSandboxes }, + target: TARGET, + isLinuxDockerDriverGatewayEnabled: () => true, + sleep, + }), + ).resolves.toEqual({ ready: true, reason: "ready", error: null }); + expect(listSandboxes).toHaveBeenCalledTimes(2); + expect(sleep).toHaveBeenCalledOnce(); + }); }); describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { - it("waits for the exact recreated sandbox to become executable before accepting stable Ready (#9050)", () => { - const { runCaptureOpenshell, sleep } = replay([`${NAME} Ready`]); + it("waits for the exact recreated sandbox to become executable before accepting stable Ready (#9050)", async () => { + const { observer, listSandboxes, sleep } = replay([readySandboxFrame()]); const checkReadyIdentity = vi.fn().mockReturnValueOnce("not_ready").mockReturnValue("ready"); - expect( + await expect( waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 30, - runCaptureOpenshell, - isSandboxReady, + observer, + target: TARGET, stableReadyPolls: 2, checkReadyIdentity, sleep, }), - ).toEqual({ ready: true, reason: "ready", failurePhase: null }); + ).resolves.toEqual({ ready: true, reason: "ready", failurePhase: null }); expect(checkReadyIdentity).toHaveBeenCalledTimes(3); - expect(runCaptureOpenshell).toHaveBeenCalledTimes(3); + expect(listSandboxes).toHaveBeenCalledTimes(3); expect(sleep).toHaveBeenCalledTimes(2); }); - it("stops when the recreated sandbox identity changes (#9050)", () => { - const { runCaptureOpenshell, sleep } = replay([`${NAME} Ready`]); + it("stops when the recreated sandbox identity changes (#9050)", async () => { + const { observer, listSandboxes, sleep } = replay([readySandboxFrame()]); - expect( + await expect( waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 30, - runCaptureOpenshell, - isSandboxReady, + observer, + target: TARGET, checkReadyIdentity: () => "identity_changed", sleep, }), - ).toEqual({ ready: false, reason: "identity_changed", failurePhase: null }); - expect(runCaptureOpenshell).toHaveBeenCalledOnce(); + ).resolves.toEqual({ ready: false, reason: "identity_changed", failurePhase: null }); + expect(listSandboxes).toHaveBeenCalledOnce(); expect(sleep).not.toHaveBeenCalled(); }); - it("stops after an unknown durable-identity probe failure (#9050)", () => { - const { runCaptureOpenshell, sleep } = replay([`${NAME} Ready`]); + it("stops after an unknown durable-identity probe failure (#9050)", async () => { + const { observer, listSandboxes, sleep } = replay([readySandboxFrame()]); - const readiness = waitForCreatedSandboxReadyWithTrace({ + const readiness = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 30, - runCaptureOpenshell, - isSandboxReady, + observer, + target: TARGET, checkReadyIdentity: () => "probe_failed", sleep, }); @@ -142,39 +337,120 @@ describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { expect(formatCreatedSandboxReadinessFailureMessage(NAME, readiness, 30)).toBe( ` NemoClaw could not verify that sandbox '${NAME}' returned a durable ID and accepted commands.`, ); - expect(runCaptureOpenshell).toHaveBeenCalledOnce(); + expect(listSandboxes).toHaveBeenCalledOnce(); expect(sleep).not.toHaveBeenCalled(); }); - it("does not probe when the readiness deadline is zero (#3768)", () => { - const { runCaptureOpenshell, sleep } = replay([`${NAME} Ready`]); + it("does not probe when the readiness deadline is zero (#3768)", async () => { + const { observer, listSandboxes, sleep } = replay([readySandboxFrame()]); - expect( + await expect( waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 0, - runCaptureOpenshell, - isSandboxReady, + observer, + target: TARGET, sleep, }), - ).toEqual({ ready: false, reason: "timeout", failurePhase: null }); - expect(runCaptureOpenshell).not.toHaveBeenCalled(); + ).resolves.toEqual({ ready: false, reason: "timeout", failurePhase: null }); + expect(listSandboxes).not.toHaveBeenCalled(); + }); + + it("stops on a typed observation failure instead of treating it as missing", async () => { + const error = { + kind: "authentication" as const, + message: "OpenShell could not authenticate the sandbox observation.", + }; + const listSandboxes = vi.fn(async () => ({ + ok: false, + error, + })); + const sleep = vi.fn(); + + const readiness = await waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 30, + observer: { listSandboxes }, + target: TARGET, + sleep, + now: () => 1_000, + }); + + expect(readiness).toEqual({ + ready: false, + reason: "observation_failed", + failurePhase: null, + error, + }); + expect(formatCreatedSandboxReadinessFailureMessage(NAME, readiness, 30)).toContain( + error.message, + ); + expect(listSandboxes).toHaveBeenCalledOnce(); + expect(listSandboxes).toHaveBeenCalledWith({ target: TARGET, timeoutMs: 30_000 }); + expect(sleep).not.toHaveBeenCalled(); + }); + + it("returns a typed failure when the created-sandbox observer rejects (#9803)", async () => { + const listSandboxes = vi + .fn() + .mockRejectedValue(new Error("untrusted created-sandbox diagnostic")); + + const result = await waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 30, + observer: { listSandboxes }, + target: TARGET, + sleep: vi.fn(), + }); + + expect(result).toEqual({ + ready: false, + reason: "observation_failed", + failurePhase: null, + error: REJECTED_OBSERVATION_ERROR, + }); + expect(JSON.stringify(result)).not.toContain("untrusted created-sandbox diagnostic"); + }); + + it("retries a timed-out observation before accepting created-sandbox Ready (#9803)", async () => { + const listSandboxes = vi + .fn() + .mockResolvedValueOnce({ + ok: false, + error: { kind: "timeout", message: "OpenShell sandbox observation timed out." }, + }) + .mockResolvedValue({ + ok: true, + value: { sandboxes: [{ name: NAME, phase: "Ready", readiness: "ready" }] }, + }); + const sleep = vi.fn(); + + await expect( + waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 30, + observer: { listSandboxes }, + target: TARGET, + sleep, + }), + ).resolves.toEqual({ ready: true, reason: "ready", failurePhase: null }); + expect(listSandboxes).toHaveBeenCalledTimes(2); + expect(sleep).toHaveBeenCalledOnce(); }); - 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`, + it("fast-fails on the first Error poll when the debounce is opted out (K=1)", async () => { + const { observer, listSandboxes, sleep } = replay([ + pendingSandboxFrame("Provisioning"), + terminalSandboxFrame("Error"), ]); - const ready = waitForCreatedSandboxReadyWithTrace({ + const ready = await 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, + observer, + target: TARGET, errorPhaseDebouncePolls: 1, sleep, }); @@ -184,50 +460,48 @@ describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { reason: "terminal_failure_phase", failurePhase: "Error", }); - expect(runCaptureOpenshell).toHaveBeenCalledTimes(2); + expect(listSandboxes).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)", () => { + it("recovers when a transient Error flips to Ready within the debounce window (#6043)", async () => { // 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 { observer, listSandboxes, sleep } = replay([ + pendingSandboxFrame("Provisioning"), + terminalSandboxFrame("Error"), + terminalSandboxFrame("Error"), + readySandboxFrame(), ]); - const ready = waitForCreatedSandboxReadyWithTrace({ + const ready = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 600, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer, + target: TARGET, sleep, }); expect(ready).toEqual({ ready: true, reason: "ready", failurePhase: null }); - expect(runCaptureOpenshell).toHaveBeenCalledTimes(4); + expect(listSandboxes).toHaveBeenCalledTimes(4); }); - it("resets the debounce counter when a non-Error poll interrupts the Error streak", () => { + it("resets the debounce counter when a non-Error poll interrupts the Error streak", async () => { // 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 { observer, sleep } = replay([ + terminalSandboxFrame("Error"), + pendingSandboxFrame("Provisioning"), + terminalSandboxFrame("Error"), + readySandboxFrame(), ]); - const ready = waitForCreatedSandboxReadyWithTrace({ + const ready = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 600, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer, + target: TARGET, errorPhaseDebouncePolls: 2, sleep, }); @@ -236,15 +510,14 @@ describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { 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`]); + it("still fails terminally after sustained Error exceeds the debounce window (#6043)", async () => { + const { observer, listSandboxes, sleep } = replay([terminalSandboxFrame("Error")]); - const ready = waitForCreatedSandboxReadyWithTrace({ + const ready = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 600, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer, + target: TARGET, errorPhaseDebouncePolls: 3, sleep, }); @@ -256,22 +529,21 @@ describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { }); // 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(listSandboxes).toHaveBeenCalledTimes(3); expect(sleep).toHaveBeenCalledTimes(2); }); - it("reports the Error phase (not a generic timeout) when the debounce outlasts the timeout", () => { + it("reports the Error phase (not a generic timeout) when the debounce outlasts the timeout", async () => { // 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 { observer, sleep } = replay([terminalSandboxFrame("Error")]); - const ready = waitForCreatedSandboxReadyWithTrace({ + const ready = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 2, // -> readyAttempts = 1, far below the default 30-poll debounce - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer, + target: TARGET, sleep, }); @@ -282,41 +554,43 @@ describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { }); }); - 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, - }); + it.each(["Failed", "CrashLoopBackOff"])( + "fast-fails immediately on genuinely terminal phase %s even with a large debounce", + async (phase) => { + const { observer, listSandboxes, sleep } = replay([ + pendingSandboxFrame("Provisioning"), + terminalSandboxFrame(phase), + ]); - expect(ready).toEqual({ ready: false, reason: "terminal_failure_phase", failurePhase: phase }); - expect(runCaptureOpenshell).toHaveBeenCalledTimes(2); - expect(sleep).toHaveBeenCalledTimes(1); - }); - - it("rounds a fractional debounce override (2.6 -> 3), matching envInt semantics", () => { - const { runCaptureOpenshell, sleep } = replay([`${NAME} Error 3s ago`]); - - const ready = waitForCreatedSandboxReadyWithTrace({ + const ready = await waitForCreatedSandboxReadyWithTrace({ + sandboxName: NAME, + timeoutSecs: 600, + observer, + target: TARGET, + // 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(listSandboxes).toHaveBeenCalledTimes(2); + expect(sleep).toHaveBeenCalledTimes(1); + }, + ); + + it("rounds a fractional debounce override (2.6 -> 3), matching envInt semantics", async () => { + const { observer, listSandboxes, sleep } = replay([terminalSandboxFrame("Error")]); + + const ready = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 600, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer, + target: TARGET, errorPhaseDebouncePolls: 2.6, sleep, }); @@ -329,25 +603,24 @@ describe("waitForCreatedSandboxReadyWithTrace terminal-phase handling", () => { // 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); + expect(listSandboxes).toHaveBeenCalledTimes(3); }); - it("ignores a non-finite debounce override and falls back to the env/default", () => { + it("ignores a non-finite debounce override and falls back to the env/default", async () => { // 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 { observer } = replay([ + terminalSandboxFrame("Error"), + terminalSandboxFrame("Error"), + terminalSandboxFrame("Error"), + readySandboxFrame(), ]); - const ready = waitForCreatedSandboxReadyWithTrace({ + const ready = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 600, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer, + target: TARGET, errorPhaseDebouncePolls: Number.NaN, sleep: () => {}, }); @@ -486,31 +759,29 @@ describe("getSandboxReadyErrorDebouncePolls env contract", () => { }); }); -// PRA-5 acceptance: deterministic replay of the reporter's DGX Spark -// gateway/port-fallback create sequence through the real readiness waiter. DGX +// PRA-5 acceptance: typed replay of the reporter's DGX Spark +// gateway/port-fallback phase 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`, + // The CLI-adapter test owns the captured table layout that produced these + // phases. This action test owns the debounce decision for typed observations. + const reporterPhaseSequence = [ + pendingSandboxFrame("Provisioning"), + terminalSandboxFrame("Error"), + terminalSandboxFrame("Error"), + terminalSandboxFrame("Error"), + readySandboxFrame(), ] as const; - it("regressed pre-fix: fast-fail (K=1) surfaces the exact reporter failure line", () => { - const { runCaptureOpenshell, sleep } = replay(reporterSequence); - const ready = waitForCreatedSandboxReadyWithTrace({ + it("regressed pre-fix: fast-fail (K=1) surfaces the reporter failure phase", async () => { + const { observer, sleep } = replay(reporterPhaseSequence); + const ready = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 1500, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer, + target: TARGET, errorPhaseDebouncePolls: 1, sleep, }); @@ -531,14 +802,13 @@ describe("DGX Spark fresh-onboard readiness replay (#6043)", () => { ).toContain("entered Failed 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({ + it("recovers with the shipped default debounce: onboard continues to Ready", async () => { + const { observer, sleep } = replay(reporterPhaseSequence); + const ready = await waitForCreatedSandboxReadyWithTrace({ sandboxName: NAME, timeoutSecs: 1500, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, + observer, + target: TARGET, sleep, }); @@ -557,14 +827,11 @@ describe("DGX Spark fresh-onboard readiness replay (#6043)", () => { // 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 + // the typed upstream sequence contains no Error phase, 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", - ); + // Replace `reporterPhaseSequence` with phases from a fixed OpenShell trace. + const hasTransientError = reporterPhaseSequence.some((frame) => frame?.phase === "Error"); expect(hasTransientError).toBe(false); }); }); diff --git a/src/lib/onboard/sandbox-readiness-tracing.ts b/src/lib/onboard/sandbox-readiness-tracing.ts index 200dd09448..9e88e19798 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.ts @@ -1,7 +1,21 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { waitUntil } from "../core/wait"; +import type { + OpenShellGatewayTarget, + OpenShellSandboxError, + OpenShellSandboxLookup, + OpenShellSandboxObserver, + OpenShellSandboxReadinessProbe, + OpenShellSandboxResult, +} from "../adapters/openshell/sandbox-observer"; +import { namedOpenShellGateway } from "../adapters/openshell/sandbox-observer"; +import { + createCliOpenShellLegacyPodReadinessProbe, + createCliOpenShellSandboxObserver, + type CliOpenShellSandboxObserverDeps, +} from "../adapters/openshell/sandbox-observer-cli"; +import { waitUntil, waitUntilAsync } from "../core/wait"; import { envInt } from "./env"; import { createReadinessWaitOptions, @@ -49,9 +63,9 @@ export const SANDBOX_READY_ERROR_DEBOUNCE_ENV = "NEMOCLAW_SANDBOX_READY_ERROR_DE * --------------------------------------- * 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. + * required is a fresh-onboard reproduction that shows a transient create-time + * Error which recovers to Ready. The CLI-adapter test owns the captured table + * layout, and sandbox-readiness-tracing.test.ts owns the typed phase replay. * * Tracking mechanism: removal is tracked on NemoClaw #6043 * (https://github.com/NVIDIA/NemoClaw/issues/6043), which owns the pending @@ -83,15 +97,27 @@ export type CreatedSandboxReadinessResult = | { ready: false; reason: "terminal_failure_phase"; failurePhase: string | null } | { ready: false; reason: "identity_changed"; failurePhase: null } | { ready: false; reason: "identity_probe_failed"; failurePhase: null } + | { + ready: false; + reason: "observation_failed"; + failurePhase: null; + error: OpenShellSandboxError; + } | { ready: false; reason: "timeout"; failurePhase: null }; +export type SandboxReadyWaitResult = + | { ready: true; reason: "ready"; error: null } + | { ready: false; reason: "observation_failed"; error: OpenShellSandboxError } + | { ready: false; reason: "timeout"; error: null }; + export type CreatedSandboxReadyIdentityCheck = ( getRemainingMs?: () => number, ) => "ready" | "not_ready" | "identity_changed" | "probe_failed"; export interface SandboxReadyWaitDeps { - runCaptureOpenshell: RunCaptureOpenshell; - isSandboxReady: (output: string, sandboxName: string) => boolean; + observer: OpenShellSandboxObserver; + target: OpenShellGatewayTarget; + fallbackReadinessProbe?: OpenShellSandboxReadinessProbe; isLinuxDockerDriverGatewayEnabled: () => boolean; sleep: (seconds: number) => void; now?: () => number; @@ -103,17 +129,61 @@ export interface SandboxReadyWaitOptions extends SandboxReadyWaitDeps { delaySeconds: number; } -function pollSandboxReady( +export async function observeOpenShellSandbox( + observer: OpenShellSandboxObserver, + target: OpenShellGatewayTarget, + sandboxName: string, + timeoutMs?: number, +): Promise> { + const result = await observer.listSandboxes({ + target, + ...(timeoutMs === undefined ? {} : { timeoutMs }), + }); + if (!result.ok) return result; + const sandbox = result.value.sandboxes.find((candidate) => candidate.name === sandboxName); + return sandbox + ? { ok: true, value: { state: "present", sandbox } } + : { ok: true, value: { state: "missing" } }; +} + +function isTransientObservationError(error: OpenShellSandboxError): boolean { + return error.kind === "timeout" || (error.kind === "transport" && error.reason === "unreachable"); +} + +function remainingObservationTimeoutMs(deadlineMs?: number, now?: () => number): number { + if (deadlineMs === undefined || now === undefined) return 1; + return Math.max(1, Math.floor(deadlineMs - now())); +} + +async function settleSandboxObservation( + observe: () => Promise>, +): Promise> { + try { + return await observe(); + } catch { + return { + ok: false, + error: { + kind: "command", + reason: "failed", + message: "OpenShell sandbox observation failed before returning a result.", + }, + }; + } +} + +async function pollSandboxReady( options: SandboxReadyWaitOptions & { trace?: (event: string, attributes: Record) => void; }, -): boolean { +): Promise { const { sandboxName, attempts, delaySeconds, - runCaptureOpenshell, - isSandboxReady, + observer, + target, + fallbackReadinessProbe, isLinuxDockerDriverGatewayEnabled, sleep, } = options; @@ -128,48 +198,103 @@ function pollSandboxReady( }); if (!waitOptions) { options.trace?.("not_ready", { attempts: 0, deadline_ms: budgetMs }); - return false; + return { ready: false, reason: "timeout", error: null }; } - const ready = waitUntil(() => { + let result: SandboxReadyWaitResult | null = null; + const transient = { error: null as OpenShellSandboxError | null }; + await waitUntilAsync(async () => { attempt += 1; - const list = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); - if (isSandboxReady(list, sandboxName)) { + const observation = await settleSandboxObservation(() => + observeOpenShellSandbox( + observer, + target, + sandboxName, + remainingObservationTimeoutMs(waitOptions.deadlineMs, waitOptions.now), + ), + ); + if (!observation.ok) { + if (isTransientObservationError(observation.error)) { + transient.error = observation.error; + options.trace?.("observation_retry", { + attempt, + error_kind: observation.error.kind, + error_reason: "reason" in observation.error ? observation.error.reason : null, + }); + return false; + } + result = { ready: false, reason: "observation_failed", error: observation.error }; + options.trace?.("observation_failed", { + attempt, + error_kind: observation.error.kind, + error_reason: "reason" in observation.error ? observation.error.reason : null, + }); + return true; + } + transient.error = null; + if (observation.value.state === "present" && observation.value.sandbox.readiness === "ready") { options.trace?.("ready", { attempt, source: "sandbox_list" }); + result = { ready: true, reason: "ready", error: null }; return true; } - // Package-managed OpenShell gateways report readiness through - // `sandbox list`; legacy Kubernetes gateways may still expose pod state. + // Compatibility boundary: new readiness behavior must use `sandbox list`. + // Only existing non-Docker legacy gateways may fall back to Kubernetes pod + // phase. #9811 removes this probe after every supported gateway reports + // readiness through `sandbox list`. if (isLinuxDockerDriverGatewayEnabled()) { return false; } - const podPhase = runCaptureOpenshell( - [ - "doctor", - "exec", - "--", - "kubectl", - "-n", - "openshell", - "get", - "pod", - sandboxName, - "-o", - "jsonpath={.status.phase}", - ], - { ignoreError: true }, - ); - if (podPhase === "Running") { + const fallback = fallbackReadinessProbe + ? await settleSandboxObservation(() => + fallbackReadinessProbe({ + target, + sandboxName, + timeoutMs: remainingObservationTimeoutMs(waitOptions.deadlineMs, waitOptions.now), + }), + ) + : undefined; + if (fallback && !fallback.ok) { + if (isTransientObservationError(fallback.error)) { + transient.error = fallback.error; + options.trace?.("observation_retry", { + attempt, + error_kind: fallback.error.kind, + error_reason: "reason" in fallback.error ? fallback.error.reason : null, + }); + return false; + } + result = { ready: false, reason: "observation_failed", error: fallback.error }; + options.trace?.("observation_failed", { + attempt, + error_kind: fallback.error.kind, + error_reason: "reason" in fallback.error ? fallback.error.reason : null, + }); + return true; + } + if (fallback?.ok && fallback.value === "ready") { options.trace?.("ready", { attempt, source: "pod_phase" }); + result = { ready: true, reason: "ready", error: null }; return true; } return false; }, waitOptions); - if (!ready) options.trace?.("not_ready", { attempts: attempt, deadline_ms: budgetMs }); - return ready; + if (result) return result; + if (transient.error) { + options.trace?.("observation_failed", { + attempts: attempt, + error_kind: transient.error.kind, + error_reason: "reason" in transient.error ? transient.error.reason : null, + note: "readiness_deadline_exhausted", + }); + return { ready: false, reason: "observation_failed", error: transient.error }; + } + options.trace?.("not_ready", { attempts: attempt, deadline_ms: budgetMs }); + return { ready: false, reason: "timeout", error: null }; } -export function waitForSandboxReadyWithTrace(options: SandboxReadyWaitOptions): boolean { +export function waitForSandboxReadyWithTrace( + options: SandboxReadyWaitOptions, +): Promise { return withSandboxReadinessTrace( options.sandboxName, { attempts: options.attempts, delay_seconds: options.delaySeconds }, @@ -179,7 +304,11 @@ export function waitForSandboxReadyWithTrace(options: SandboxReadyWaitOptions): export function createSandboxReadyWaiter( deps: SandboxReadyWaitDeps, -): (sandboxName: string, attempts?: number, delaySeconds?: number) => boolean { +): ( + sandboxName: string, + attempts?: number, + delaySeconds?: number, +) => Promise { return (sandboxName, attempts = 10, delaySeconds = 2) => pollSandboxReady({ sandboxName, @@ -189,18 +318,35 @@ export function createSandboxReadyWaiter( }); } +export function createCliSandboxReadyWaiter(options: { + capture: CliOpenShellSandboxObserverDeps["capture"]; + getGatewayName: () => string; + isLinuxDockerDriverGatewayEnabled: () => boolean; + sleep: (seconds: number) => void; + now?: () => number; +}): ReturnType { + const cliDeps = { capture: options.capture }; + const observer = createCliOpenShellSandboxObserver(cliDeps); + const fallbackReadinessProbe = createCliOpenShellLegacyPodReadinessProbe(cliDeps); + return (sandboxName, attempts = 10, delaySeconds = 2) => + pollSandboxReady({ + sandboxName, + attempts, + delaySeconds, + observer, + target: namedOpenShellGateway(options.getGatewayName()), + fallbackReadinessProbe, + isLinuxDockerDriverGatewayEnabled: options.isLinuxDockerDriverGatewayEnabled, + sleep: options.sleep, + ...(options.now ? { now: options.now } : {}), + }); +} + export function waitForCreatedSandboxReadyWithTrace(options: { sandboxName: string; timeoutSecs: number; - runCaptureOpenshell: RunCaptureOpenshell; - isSandboxReady: (output: string, sandboxName: string) => boolean; - /** - * Optional terminal-failure-phase classifier. When provided, the waiter - * short-circuits as soon as the sandbox enters a terminal failure phase - * (e.g. Error / Failed / CrashLoopBackOff) rather than burning the full - * timeout window before reporting "did not become ready" (#4316). - */ - getSandboxFailurePhase?: (output: string, sandboxName: string) => string | null; + observer: OpenShellSandboxObserver; + target: OpenShellGatewayTarget; /** * Consecutive Ready polls required before returning success. Defaults to 1. * The Docker GPU compatibility recreate passes 2 because the OpenShell @@ -240,15 +386,8 @@ export function waitForCreatedSandboxReadyWithTrace(options: { errorPhaseDebouncePolls?: number; sleep: (seconds: number) => void; now?: () => number; -}): CreatedSandboxReadinessResult { - const { - sandboxName, - timeoutSecs, - runCaptureOpenshell, - isSandboxReady, - getSandboxFailurePhase, - sleep, - } = options; +}): Promise { + const { sandboxName, timeoutSecs, observer, target, sleep } = options; const errorPhaseDebouncePolls = options.errorPhaseDebouncePolls == null || !Number.isFinite(options.errorPhaseDebouncePolls) ? getSandboxReadyErrorDebouncePolls() @@ -259,138 +398,197 @@ export function waitForCreatedSandboxReadyWithTrace(options: { options.stableReadyPolls == null || !Number.isFinite(options.stableReadyPolls) ? 1 : Math.max(1, Math.round(options.stableReadyPolls)); - return withSandboxReadinessTrace(sandboxName, { timeout_seconds: timeoutSecs }, () => { - const budgetMs = Math.max(0, timeoutSecs * 1000); - const waitOptions = createReadinessWaitOptions({ - budgetMs, - initialIntervalMs: stableReadyPolls > 1 ? 2_000 : undefined, - maxIntervalMs: 2_000, - now: options.now, - sleep: (ms) => sleep(ms / 1000), - }); - if (!waitOptions) { - addTraceEvent("not_ready", { attempts: 0, deadline_ms: budgetMs }); - return { ready: false, reason: "timeout", failurePhase: null }; - } - const readinessDeadlineMs = waitOptions.deadlineMs; - const readinessNow = waitOptions.now; - if (readinessDeadlineMs === undefined || readinessNow === undefined) { - throw new Error("Created sandbox readiness requires a deadline and clock."); - } - const getRemainingMs = () => Math.max(0, readinessDeadlineMs - readinessNow()); - let consecutiveReadyPolls = 0; - let consecutiveFailurePolls = 0; - let lastFailurePhase: string | null = null; - let attempt = 0; - let result: CreatedSandboxReadinessResult | null = null; - waitUntil(() => { - attempt += 1; - const list = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); - if (isSandboxReady(list, sandboxName)) { - const identity = options.checkReadyIdentity?.(getRemainingMs) ?? "ready"; - if (identity === "identity_changed") { - addTraceEvent("identity_changed", { attempt }); - result = { - ready: false, - reason: "identity_changed", - failurePhase: null, - }; - return true; - } - if (identity === "probe_failed") { - addTraceEvent("identity_probe_failed", { attempt }); + return withSandboxReadinessTrace( + sandboxName, + { timeout_seconds: timeoutSecs }, + async (): Promise => { + const budgetMs = Math.max(0, timeoutSecs * 1000); + const waitOptions = createReadinessWaitOptions({ + budgetMs, + initialIntervalMs: stableReadyPolls > 1 ? 2_000 : undefined, + maxIntervalMs: 2_000, + now: options.now, + sleep: (ms) => sleep(ms / 1000), + }); + if (!waitOptions) { + addTraceEvent("not_ready", { attempts: 0, deadline_ms: budgetMs }); + return { ready: false, reason: "timeout", failurePhase: null }; + } + const readinessDeadlineMs = waitOptions.deadlineMs; + const readinessNow = waitOptions.now; + if (readinessDeadlineMs === undefined || readinessNow === undefined) { + throw new Error("Created sandbox readiness requires a deadline and clock."); + } + const getRemainingMs = () => Math.max(0, readinessDeadlineMs - readinessNow()); + let consecutiveReadyPolls = 0; + let consecutiveFailurePolls = 0; + let lastFailurePhase: string | null = null; + const transient = { error: null as OpenShellSandboxError | null }; + let attempt = 0; + let result: CreatedSandboxReadinessResult | null = null; + await waitUntilAsync(async () => { + attempt += 1; + const observation = await settleSandboxObservation(() => + observeOpenShellSandbox( + observer, + target, + sandboxName, + remainingObservationTimeoutMs(readinessDeadlineMs, readinessNow), + ), + ); + if (!observation.ok) { + if (isTransientObservationError(observation.error)) { + consecutiveReadyPolls = 0; + consecutiveFailurePolls = 0; + lastFailurePhase = null; + transient.error = observation.error; + addTraceEvent("observation_retry", { + attempt, + error_kind: observation.error.kind, + error_reason: "reason" in observation.error ? observation.error.reason : null, + }); + return false; + } + addTraceEvent("observation_failed", { + attempt, + error_kind: observation.error.kind, + error_reason: "reason" in observation.error ? observation.error.reason : null, + }); result = { ready: false, - reason: "identity_probe_failed", + reason: "observation_failed", failurePhase: null, + error: observation.error, }; return true; } - if (identity === "not_ready") { - consecutiveReadyPolls = 0; + transient.error = null; + const sandbox = observation.value.state === "present" ? observation.value.sandbox : null; + if (sandbox?.readiness === "ready") { + const identity = options.checkReadyIdentity?.(getRemainingMs) ?? "ready"; + if (identity === "identity_changed") { + addTraceEvent("identity_changed", { attempt }); + result = { + ready: false, + reason: "identity_changed", + failurePhase: null, + }; + return true; + } + if (identity === "probe_failed") { + addTraceEvent("identity_probe_failed", { attempt }); + result = { + ready: false, + reason: "identity_probe_failed", + failurePhase: null, + }; + return true; + } + if (identity === "not_ready") { + consecutiveReadyPolls = 0; + consecutiveFailurePolls = 0; + lastFailurePhase = null; + addTraceEvent("ready_identity_pending", { attempt }); + return false; + } + consecutiveReadyPolls += 1; consecutiveFailurePolls = 0; lastFailurePhase = null; - addTraceEvent("ready_identity_pending", { attempt }); - return false; - } - consecutiveReadyPolls += 1; - consecutiveFailurePolls = 0; - lastFailurePhase = null; - if (consecutiveReadyPolls >= stableReadyPolls) { - addTraceEvent("ready", { + if (consecutiveReadyPolls >= stableReadyPolls) { + addTraceEvent("ready", { + attempt, + consecutive_polls: consecutiveReadyPolls, + }); + result = { ready: true, reason: "ready", failurePhase: null }; + return true; + } + addTraceEvent("ready_pending_stability", { attempt, consecutive_polls: consecutiveReadyPolls, + required_polls: stableReadyPolls, }); - result = { ready: true, reason: "ready", failurePhase: null }; + return false; + } + consecutiveReadyPolls = 0; + const failurePhase = + sandbox?.phase === "Error" || + sandbox?.phase === "Failed" || + sandbox?.phase === "CrashLoopBackOff" + ? sandbox.phase + : null; + // 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, failure_phase: failurePhase }); + result = { ready: false, reason: "terminal_failure_phase", failurePhase }; return true; } - addTraceEvent("ready_pending_stability", { - attempt, - consecutive_polls: consecutiveReadyPolls, - required_polls: stableReadyPolls, - }); - return false; - } - consecutiveReadyPolls = 0; - const failurePhase = getSandboxFailurePhase?.(list, sandboxName) ?? null; - // 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, failure_phase: failurePhase }); - result = { ready: false, reason: "terminal_failure_phase", failurePhase }; - return true; - } - if (failurePhase === "Error") { - 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", { + if (failurePhase === "Error") { + 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, + failure_phase: failurePhase, + consecutive_polls: consecutiveFailurePolls, + }); + result = { ready: false, reason: "terminal_failure_phase", failurePhase }; + return true; + } + addTraceEvent("transient_failure_phase", { attempt, failure_phase: failurePhase, consecutive_polls: consecutiveFailurePolls, + debounce_polls: errorPhaseDebouncePolls, }); - result = { ready: false, reason: "terminal_failure_phase", failurePhase }; - return true; + } else { + consecutiveFailurePolls = 0; } - addTraceEvent("transient_failure_phase", { - attempt, - failure_phase: failurePhase, + return false; + }, waitOptions); + if (result) return result; + if (transient.error) { + addTraceEvent("observation_failed", { + attempts: attempt, + error_kind: transient.error.kind, + error_reason: "reason" in transient.error ? transient.error.reason : null, + note: "readiness_deadline_exhausted", + }); + return { + ready: false, + reason: "observation_failed", + failurePhase: null, + error: transient.error, + }; + } + // 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: attempt, + failure_phase: lastFailurePhase, consecutive_polls: consecutiveFailurePolls, debounce_polls: errorPhaseDebouncePolls, + note: "debounce_window_exceeded_timeout", }); - } else { - consecutiveFailurePolls = 0; + return { ready: false, reason: "terminal_failure_phase", failurePhase: lastFailurePhase }; } - return false; - }, waitOptions); - if (result) return result; - // 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", { + addTraceEvent("not_ready", { attempts: attempt, - failure_phase: lastFailurePhase, - consecutive_polls: consecutiveFailurePolls, - debounce_polls: errorPhaseDebouncePolls, - note: "debounce_window_exceeded_timeout", + deadline_ms: budgetMs, + last_failure_phase: lastFailurePhase, }); - return { ready: false, reason: "terminal_failure_phase", failurePhase: lastFailurePhase }; - } - addTraceEvent("not_ready", { - attempts: attempt, - deadline_ms: budgetMs, - last_failure_phase: lastFailurePhase, - }); - return { ready: false, reason: "timeout", failurePhase: null }; - }); + return { ready: false, reason: "timeout", failurePhase: null }; + }, + ); } /** @@ -414,6 +612,9 @@ export function formatCreatedSandboxReadinessFailureMessage( if (readiness.reason === "identity_probe_failed") { return ` NemoClaw could not verify that sandbox '${sandboxName}' returned a durable ID and accepted commands.`; } + if (readiness.reason === "observation_failed") { + return ` NemoClaw could not observe readiness for sandbox '${sandboxName}'. ${readiness.error.message}`; + } return ` Sandbox '${sandboxName}' was created but did not become ready within ${timeoutSecs}s.`; } diff --git a/test/onboarding/onboard-policy-application-wiring.test.ts b/test/onboarding/onboard-policy-application-wiring.test.ts index 1dc98f1d16..f7722b82b3 100644 --- a/test/onboarding/onboard-policy-application-wiring.test.ts +++ b/test/onboarding/onboard-policy-application-wiring.test.ts @@ -39,9 +39,9 @@ describe("onboarding policy application production wiring", () => { return { policyTier: "restricted" }; }); const updateSandbox = vi.fn(); - const waitForSandboxReady = vi.fn(() => { + const waitForSandboxReady = vi.fn(async () => { events.push("sandbox ready"); - return true; + return { ready: true as const, reason: "ready" as const, error: null }; }); const waitForSandboxControlPlaneReady = vi.fn(() => { events.push("control plane ready"); @@ -70,7 +70,8 @@ describe("onboarding policy application production wiring", () => { const registryPath = require.resolve("../../src/lib/state/registry.js"); const lockPath = require.resolve("../../src/lib/state/mcp-lifecycle-lock.js"); const readinessPath = require.resolve("../../src/lib/onboard/sandbox-readiness-tracing.js"); - const finalFlowPath = require.resolve("../../src/lib/onboard/machine/final-flow-composition.js"); + const finalFlowPath = + require.resolve("../../src/lib/onboard/machine/final-flow-composition.js"); try { const policy = require(policyPath) as Record; @@ -107,7 +108,7 @@ describe("onboarding policy application production wiring", () => { const readiness = require(readinessPath) as Record; replaceCachedExports(readinessPath, { ...readiness, - createSandboxReadyWaiter: vi.fn(() => waitForSandboxReady), + createCliSandboxReadyWaiter: vi.fn(() => waitForSandboxReady), }); const finalFlow = require(finalFlowPath) as { finalizationHandlerDeps: Record; diff --git a/test/onboarding/onboard-preset-diff.test.ts b/test/onboarding/onboard-preset-diff.test.ts index 4f0c2952e7..e824b648cf 100644 --- a/test/onboarding/onboard-preset-diff.test.ts +++ b/test/onboarding/onboard-preset-diff.test.ts @@ -86,7 +86,7 @@ async function runPolicyScenario({ step: () => undefined, note: () => undefined, isNonInteractive: () => true, - waitForSandboxReady: () => true, + waitForSandboxReady: async () => ({ ready: true, reason: "ready", error: null }), waitForSandboxControlPlaneReady: () => true, syncPresetSelection: (_sandboxName, current, selected) => { const currentSet = new Set(current); diff --git a/test/runtime/policy/policy-tiers-onboard.test.ts b/test/runtime/policy/policy-tiers-onboard.test.ts index 82ca2e1327..94a1b58fb1 100644 --- a/test/runtime/policy/policy-tiers-onboard.test.ts +++ b/test/runtime/policy/policy-tiers-onboard.test.ts @@ -145,7 +145,7 @@ function createSetupHarness({ step: () => undefined, note: (message) => notes.push(message), isNonInteractive: () => nonInteractive, - waitForSandboxReady: () => true, + waitForSandboxReady: async () => ({ ready: true, reason: "ready", error: null }), waitForSandboxControlPlaneReady: () => true, syncPresetSelection: (sandboxName, current, selected, accessByName) => { syncCalls.push({