diff --git a/src/lib/actions/sandbox/snapshot-auto-create-failure.test.ts b/src/lib/actions/sandbox/snapshot-auto-create-failure.test.ts new file mode 100644 index 0000000000..1bdc602cee --- /dev/null +++ b/src/lib/actions/sandbox/snapshot-auto-create-failure.test.ts @@ -0,0 +1,141 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; +import type { SnapshotStreamSandboxCreateMock } from "./snapshot-create-stream-test-types"; + +const captureOpenshellMock = vi.fn(() => ({ status: 0, output: "alpha Ready\n" })); +const getSandboxMock = vi.fn((name?: string) => + name === "alpha" + ? { + name: "alpha", + agent: "openclaw", + gatewayName: "nemoclaw", + imageTag: "nemoclaw-alpha:test", + openshellDriver: "docker", + provider: "nvidia-nim", + model: "nvidia/model-a", + } + : null, +); +const registerSandboxMock = vi.fn(); +const restoreSandboxStateMock = vi.fn(); +const streamSandboxCreateMock = vi.fn(async () => ({ + status: 7, + output: "create failed before registry write", + sawProgress: false, + forcedReady: false, +})); + +vi.mock("../../adapters/docker", () => ({ dockerCapture: vi.fn(() => "") })); +vi.mock("../../adapters/openshell/runtime", () => ({ + captureOpenshell: captureOpenshellMock, + getOpenshellBinary: vi.fn(() => "openshell"), + runOpenshell: vi.fn(() => ({ status: 0, output: "" })), +})); +vi.mock("../../credentials/store", () => ({ prompt: vi.fn() })); +vi.mock("../../domain/sandbox/destroy", () => ({ + getSandboxDeleteOutcome: vi.fn(() => ({ alreadyGone: false, gatewayUnreachable: false })), +})); +vi.mock("../../inference/gateway-route-compatibility", () => ({ + checkGatewayRouteCompatibility: vi.fn(() => ({ ok: true })), + formatGatewayRouteConflict: vi.fn(() => "route conflict"), +})); +vi.mock("../../inference/gateway-route-mutation-lock", () => ({ + withGatewayRouteMutationLock: vi.fn((_gateway, fn) => fn()), +})); +vi.mock("../../inference/nim", () => ({ + stopNimContainer: vi.fn(), + stopNimContainerByName: vi.fn(), +})); +vi.mock("../../messaging/channels", () => ({ listMessagingProviderSuffixes: vi.fn(() => []) })); +vi.mock("../../policy", () => ({ + applyPreset: vi.fn(() => true), + applyPresetContent: vi.fn(() => true), + getAppliedPresets: vi.fn(() => []), + getCustomPolicies: vi.fn(() => []), + getPresetContentGatewayState: vi.fn(() => "absent"), + loadPresetForSandbox: vi.fn(() => null), + removePreset: vi.fn(() => true), +})); +vi.mock("../../runner", () => ({ + ROOT: "/repo", + run: vi.fn(() => ({ status: 0 })), + shellQuote: (value: string) => `'${value}'`, + validateName: vi.fn((value: string) => value), +})); +vi.mock("../../runtime-recovery", () => ({ + parseLiveSandboxNames: vi.fn(() => new Set(["alpha"])), +})); +vi.mock("../../sandbox/create-stream", () => ({ streamSandboxCreate: streamSandboxCreateMock })); +vi.mock("../../shields", () => ({ + get isShieldsDown() { + return true; + }, + repairMutableConfigPerms: vi.fn(() => ({ applied: true, verified: true, errors: [] })), + shieldsUp: vi.fn(), +})); +vi.mock("../../shields/timer-bound-lock", () => ({ + withTimerBoundShieldsMutationLock: vi.fn((_sandbox, _command, fn) => fn()), +})); +vi.mock("../../shields/timer-control", () => ({ readTimerMarker: vi.fn(() => null) })); +vi.mock("../../state/gateway", () => ({ + isGatewayHealthy: vi.fn(() => true), + isSandboxReady: vi.fn((output: string, sandboxName: string) => + output.includes(`${sandboxName} Ready`), + ), +})); +vi.mock("../../state/mcp-lifecycle-lock", () => ({ + withSandboxMutationLock: vi.fn((_sandbox, fn) => fn()), +})); +vi.mock("../../state/registry", () => ({ + getSandbox: getSandboxMock, + listSandboxes: vi.fn(() => ({ sandboxes: [getSandboxMock("alpha")], defaultSandbox: "alpha" })), + registerSandbox: registerSandboxMock, + removeSandbox: vi.fn(), + updateSandbox: vi.fn(), +})); +vi.mock("../../state/sandbox", () => ({ + backupSandboxState: vi.fn(), + findBackup: vi.fn(() => ({ match: null })), + getLatestBackup: vi.fn(() => ({ + timestamp: "2026-06-15T00:00:00.000Z", + backupPath: "/tmp/backup-alpha", + })), + listBackups: vi.fn(() => []), + restoreSandboxState: restoreSandboxStateMock, +})); +vi.mock("./destroy", () => ({ + cleanupShieldsDestroyArtifacts: vi.fn(), + removeSandboxRegistryEntry: vi.fn(), +})); +vi.mock("./sandbox-gateway-routing", () => ({ + probeGatewayRunning: vi.fn(() => true), + selectSandboxGatewayIfRegistered: vi.fn(() => true), + usesGatewayMetadataProbe: vi.fn( + (driver?: string | null) => driver === "docker" || driver === "vm", + ), +})); + +describe("snapshot restore auto-create failures", () => { + it("does not register a ghost sandbox when auto-create fails", async () => { + vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(console, "log").mockImplementation(() => {}); + const { runSandboxSnapshot } = await import("./snapshot"); + + await expect( + runSandboxSnapshot("alpha", { kind: "restore", to: "beta" }), + ).rejects.toMatchObject({ + exitCode: 1, + }); + + expect(streamSandboxCreateMock).toHaveBeenCalledWith( + "openshell", + expect.arrayContaining(["sandbox", "create", "--name", "beta"]), + expect.any(Object), + expect.objectContaining({ initialPhase: "create" }), + ); + expect(registerSandboxMock).not.toHaveBeenCalled(); + expect(restoreSandboxStateMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/actions/sandbox/snapshot-create-stream-test-types.ts b/src/lib/actions/sandbox/snapshot-create-stream-test-types.ts new file mode 100644 index 0000000000..726d74c1bd --- /dev/null +++ b/src/lib/actions/sandbox/snapshot-create-stream-test-types.ts @@ -0,0 +1,14 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { + StreamSandboxCreateOptions, + StreamSandboxCreateResult, +} from "../../sandbox/create-stream"; + +export type SnapshotStreamSandboxCreateMock = ( + command: string, + args: readonly string[], + env?: NodeJS.ProcessEnv, + options?: StreamSandboxCreateOptions, +) => Promise; diff --git a/src/lib/actions/sandbox/snapshot.test.ts b/src/lib/actions/sandbox/snapshot.test.ts index f77c00ad72..0507721038 100644 --- a/src/lib/actions/sandbox/snapshot.test.ts +++ b/src/lib/actions/sandbox/snapshot.test.ts @@ -1,12 +1,12 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 - import { spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { SANDBOX_EXEC_STARTED_MARKER } from "./sandbox-exec-output"; +import type { SnapshotStreamSandboxCreateMock } from "./snapshot-create-stream-test-types"; type OpenshellCaptureResult = { status: number | null; @@ -16,16 +16,7 @@ type OpenshellCaptureResult = { error?: Error; signal?: NodeJS.Signals | null; }; -type SandboxRecord = { - name: string; - agent?: string | null; - gatewayName?: string | null; - imageTag?: string | null; - openshellDriver?: string | null; - observabilityEnabled?: boolean; - provider?: string | null; - model?: string | null; -}; +type SandboxRecord = { name: string; observabilityEnabled?: boolean } & Record; type DcodeProbeState = "active" | "idle" | "unverifiable" | "no-runtime"; function dcodeProbeOutput(state: DcodeProbeState, extra = ""): string { @@ -139,13 +130,12 @@ const runOpenshellMock = vi.fn((args: string[]) => { args[0] === "sandbox" && args[1] === "delete" && lifecycleMock.events.push("delete"); return { status: 0, output: "" }; }); -const streamSandboxCreateMock = vi.fn( - async (_command: string, _env: NodeJS.ProcessEnv, _options?: Record) => ({ - status: 0, - output: "", - forcedReady: false, - }), -); +const streamSandboxCreateMock = vi.fn(async () => ({ + status: 0, + output: "", + sawProgress: false, + forcedReady: false, +})); const dcodeSandboxEntry = { name: "alpha", agent: "langchain-deepagents-code", @@ -984,9 +974,11 @@ describe("runSandboxSnapshot", () => { getLatestBackupMock.mockReturnValue({ ...latestBackupFixture }); const { runSandboxSnapshot } = await import("./snapshot"); await runSandboxSnapshot("alpha", { kind: "restore", to: "beta" }); - const [createCommandValue, createEnv] = streamSandboxCreateMock.mock.calls[0] ?? []; - const createCommand = String(createCommandValue ?? ""); - expect(createCommand).toContain(`'NEMOCLAW_OBSERVABILITY=${expectedValue}'`); + const createCall = streamSandboxCreateMock.mock.calls[0] ?? []; + const createArgs = createCall[1] as readonly string[]; + const createEnv = createCall[2] as NodeJS.ProcessEnv | undefined; + expect(createCall[0]).toBe("openshell"); + expect(createArgs).toContain(`NEMOCLAW_OBSERVABILITY=${expectedValue}`); expect(createEnv?.NEMOCLAW_OBSERVABILITY).toBeUndefined(); expect(registerSandboxMock).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/src/lib/actions/sandbox/snapshot.ts b/src/lib/actions/sandbox/snapshot.ts index ba525d55e3..b070e3d24b 100644 --- a/src/lib/actions/sandbox/snapshot.ts +++ b/src/lib/actions/sandbox/snapshot.ts @@ -28,7 +28,7 @@ import { } from "../../onboard/observability-policy-presets"; import { normalizePolicyTierName } from "../../onboard/policy-tier-suppression"; import * as policies from "../../policy"; -import { ROOT, run, shellQuote, validateName } from "../../runner"; +import { ROOT, run, validateName } from "../../runner"; import { parseLiveSandboxNames } from "../../runtime-recovery"; import { streamSandboxCreate } from "../../sandbox/create-stream"; import * as shields from "../../shields"; @@ -240,8 +240,8 @@ async function autoCreateSandboxFromSource( const createEnv = { ...process.env }; delete createEnv.NEMOCLAW_OBSERVABILITY; - const cmdParts = [ - openshellBin, + const command = openshellBin; + const commandArgs = [ "sandbox", "create", "--name", @@ -253,12 +253,11 @@ async function autoCreateSandboxFromSource( "--auto-providers", "--", ...startupCommand, - ].map((p) => shellQuote(p)); - const command = `${cmdParts.join(" ")} 2>&1`; + ]; console.log(` '${dstName}' does not exist. Creating from '${srcName}' image (${fromImage})...`); - const createResult = await streamSandboxCreate(command, createEnv, { + const createResult = await streamSandboxCreate(command, commandArgs, createEnv, { // Use a pre-built image, so skip build+push and jump to pod creation. initialPhase: "create", // Wait until the sandbox actually reaches Ready state, not just appears in the list. diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 166e0d7dd7..e0972d4a66 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -118,6 +118,13 @@ const { const { finalizeCreatedSandbox, }: typeof import("./onboard/created-sandbox-finalization") = require("./onboard/created-sandbox-finalization"); +const { + reportSandboxCreateFailure, + reportSandboxReadinessFailure, +}: typeof import("./onboard/created-sandbox-failure") = require("./onboard/created-sandbox-failure"); +const { + runSandboxCreateStep, +}: typeof import("./onboard/sandbox-create-step") = require("./onboard/sandbox-create-step"); const providerKeyBridge: typeof import("./onboard/provider-key-bridge") = require("./onboard/provider-key-bridge"); const { isLinuxDockerDriverGatewayEnabled, @@ -2800,41 +2807,39 @@ async function createSandboxWithBaseImageResolution( gatewayPort: GATEWAY_PORT, }); const sandboxReadyTimeoutSecs = getSandboxReadyTimeoutSecs(effectiveSandboxGpuConfig); - const { createCommand, effectiveDashboardPort, prebuild, sandboxEnv, sandboxStartupCommand } = - await sandboxCreateLaunch.prepareSandboxCreateLaunchWithPrebuild({ - agent, - observabilityEnabled: createIntent?.observabilityEnabled === true, - chatUiUrl, - createArgs, - sandboxName, - env: process.env, - extraPlaceholderKeys, - getDashboardForwardPort, - hermesDashboardState, - manageDashboard, - openshellShellCommand, - prebuild: { buildCtx, buildId, dockerDriverGateway, origin }, - }); - const dockerGpuCreatePatch = dockerGpuSandboxCreate.createDockerGpuSandboxCreatePatch({ - enabled: useDockerGpuPatch, - sandboxName, - gpuDevice: effectiveSandboxGpuConfig.sandboxGpuDevice, - openshellSandboxCommand: sandboxStartupCommand, - timeoutSecs: sandboxReadyTimeoutSecs, - backend: effectiveSandboxGpuConfig.hostGpuPlatform === "jetson" ? "jetson" : "generic", - deps: { runOpenshell, runCaptureOpenshell, sleep: sleepSeconds }, - }); - const createResult = await streamSandboxCreate(createCommand, sandboxEnv, { - readyCheck: () => { - const list = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); - if (isSandboxReady(list, sandboxName)) return true; - dockerGpuCreatePatch.maybeApplyDuringCreate(); - return false; - }, - readyCheckOutputPatterns: agentDefs.isTerminalAgent(agent) ? [] : undefined, - failureCheck: dockerGpuCreatePatch.createFailureMessage, - traceEvent: onboardTracing.addTraceEvent, - }); + const { createResult, prebuild, effectiveDashboardPort, dockerGpuCreatePatch } = + await runSandboxCreateStep( + { + agent, + observabilityEnabled: createIntent?.observabilityEnabled === true, + chatUiUrl, + createArgs, + sandboxName, + env: process.env, + extraPlaceholderKeys, + getDashboardForwardPort, + hermesDashboardState, + manageDashboard, + openshellShellCommand, + openshellArgv, + prebuild: { buildCtx, buildId, dockerDriverGateway, origin }, + useDockerGpuPatch, + gpuDevice: effectiveSandboxGpuConfig.sandboxGpuDevice, + gpuBackend: effectiveSandboxGpuConfig.hostGpuPlatform === "jetson" ? "jetson" : "generic", + timeoutSecs: sandboxReadyTimeoutSecs, + }, + { + prepareCreateLaunch: sandboxCreateLaunch.prepareSandboxCreateLaunchWithPrebuild, + createDockerGpuPatch: dockerGpuSandboxCreate.createDockerGpuSandboxCreatePatch, + streamCreate: streamSandboxCreate, + isSandboxReady, + isTerminalAgent: agentDefs.isTerminalAgent, + addTraceEvent: onboardTracing.addTraceEvent, + runOpenshell, + runCaptureOpenshell, + sleepSeconds, + }, + ); if (initialSandboxPolicy.cleanup && initialSandboxPolicy.cleanup()) { process.removeListener("exit", initialSandboxPolicy.cleanup); } @@ -2854,30 +2859,24 @@ async function createSandboxWithBaseImageResolution( pendingStateRestore?.manifest?.backupPath ?? pendingStateRestoreBackupPath; if (createResult.status !== 0) { - const failure = classifySandboxCreateFailure(createResult.output); - if (failure.kind === "sandbox_create_incomplete") { - // The sandbox was created in the gateway but the create stream exited - // with a non-zero code (e.g. SSH 255). Fall through to the ready-wait - // loop — the sandbox may still reach Ready on its own. - console.warn(""); - console.warn( - ` Create stream exited with code ${createResult.status} after sandbox was created.`, - ); - console.warn(" Checking whether the sandbox reaches Ready state..."); - } else { - console.error(""); - console.error(` Sandbox creation failed (exit ${createResult.status}).`); - if (createResult.output) { - console.error(""); - console.error(createResult.output); - } - sandboxCreateFailureDiagnostics.printSandboxCreateFailureDiagnostics(sandboxName, { - backupPath: restoreBackupPath, - }); - console.error(" Try: openshell sandbox list # check gateway state"); - printSandboxCreateRecoveryHints(createResult.output, { createArgs: prebuild.createArgs }); - process.exit(createResult.status || 1); - } + reportSandboxCreateFailure( + { + sandboxName, + createStatus: createResult.status, + createOutput: createResult.output, + restoreBackupPath, + createArgs: prebuild.createArgs, + }, + { + classifyCreateFailure: classifySandboxCreateFailure, + printCreateFailureDiagnostics: (name, options) => + sandboxCreateFailureDiagnostics.printSandboxCreateFailureDiagnostics(name, options), + printRecoveryHints: printSandboxCreateRecoveryHints, + warn: (message) => console.warn(message), + error: (message) => console.error(message), + exitProcess: (code) => process.exit(code), + }, + ); } dockerGpuCreatePatch.ensureApplied(); @@ -2898,26 +2897,27 @@ async function createSandboxWithBaseImageResolution( }); if (!readiness.ready) { - console.error(""); - sandboxReadinessTracing.printReadinessFailure(readiness, sandboxName, sandboxReadyTimeoutSecs); - sandboxCreateFailureDiagnostics.printSandboxCreateFailureDiagnostics(sandboxName, { - backupPath: restoreBackupPath, - }); - if (useDockerGpuPatch) { - dockerGpuCreatePatch.printReadinessFailureIfEnabled(); - } else { - // Clean up non-GPU failures after preserving local diagnostics so the - // next onboard retry with the same name does not fail on "sandbox already exists". - const delResult = runOpenshell(["sandbox", "delete", sandboxName], { ignoreError: true }); - if (delResult.status === 0) { - console.error(" The failed sandbox has been removed; retry will recreate it."); - } else { - console.error(" Could not remove the failed sandbox. Manual cleanup:"); - console.error(` openshell sandbox delete "${sandboxName}"`); - } - } - console.error(` Retry: ${cliName()} onboard`); - process.exit(1); + reportSandboxReadinessFailure( + { + sandboxName, + readiness, + createStatus: createResult.status, + timeoutSecs: sandboxReadyTimeoutSecs, + restoreBackupPath, + useDockerGpuPatch, + }, + { + printReadinessFailure: (result, name, timeoutSecs) => + sandboxReadinessTracing.printReadinessFailure(result, name, timeoutSecs), + printCreateFailureDiagnostics: (name, options) => + sandboxCreateFailureDiagnostics.printSandboxCreateFailureDiagnostics(name, options), + printDockerGpuReadinessFailure: () => dockerGpuCreatePatch.printReadinessFailureIfEnabled(), + deleteSandbox: (name) => runOpenshell(["sandbox", "delete", name], { ignoreError: true }), + cliName, + error: (message) => console.error(message), + exitProcess: (code) => process.exit(code), + }, + ); } if (manageDashboard) { diff --git a/src/lib/onboard/created-sandbox-failure.test.ts b/src/lib/onboard/created-sandbox-failure.test.ts new file mode 100644 index 0000000000..7e03bbfa92 --- /dev/null +++ b/src/lib/onboard/created-sandbox-failure.test.ts @@ -0,0 +1,231 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { + reportSandboxCreateFailure, + reportSandboxReadinessFailure, + type SandboxCreateFailureReportDeps, + type SandboxCreateFailureReportOptions, + type SandboxReadinessFailureReportDeps, + type SandboxReadinessFailureReportOptions, +} from "./created-sandbox-failure"; +import type { CreatedSandboxReadinessResult } from "./sandbox-readiness-tracing"; + +class ExitSignal extends Error { + constructor(readonly code: number) { + super(`exit:${code}`); + } +} + +function createFailureDeps( + overrides: Partial = {}, +): SandboxCreateFailureReportDeps { + return { + classifyCreateFailure: vi.fn(() => ({ kind: "unknown" })), + printCreateFailureDiagnostics: vi.fn(), + printRecoveryHints: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + exitProcess: vi.fn((code: number): never => { + throw new ExitSignal(code); + }), + ...overrides, + }; +} + +function createFailureOptions( + overrides: Partial = {}, +): SandboxCreateFailureReportOptions { + return { + sandboxName: "alpha", + createStatus: 3, + createOutput: "boom", + restoreBackupPath: null, + createArgs: ["sandbox", "create", "alpha"], + ...overrides, + }; +} + +describe("reportSandboxCreateFailure", () => { + it("warns and returns (does not exit) when the create is merely incomplete", () => { + const deps = createFailureDeps({ + classifyCreateFailure: vi.fn(() => ({ kind: "sandbox_create_incomplete" })), + }); + expect(() => reportSandboxCreateFailure(createFailureOptions(), deps)).not.toThrow(); + expect(deps.warn).toHaveBeenCalledWith( + " Create stream exited with code 3 after sandbox was created.", + ); + expect(deps.printCreateFailureDiagnostics).not.toHaveBeenCalled(); + expect(deps.printRecoveryHints).not.toHaveBeenCalled(); + expect(deps.exitProcess).not.toHaveBeenCalled(); + }); + + it("prints diagnostics + recovery hints and exits with the create status on a hard failure", () => { + const deps = createFailureDeps(); + expect(() => + reportSandboxCreateFailure( + createFailureOptions({ createStatus: 42, restoreBackupPath: "/tmp/backup" }), + deps, + ), + ).toThrow(ExitSignal); + expect(deps.printCreateFailureDiagnostics).toHaveBeenCalledWith("alpha", { + backupPath: "/tmp/backup", + }); + expect(deps.printRecoveryHints).toHaveBeenCalledWith("boom", { + createArgs: ["sandbox", "create", "alpha"], + }); + expect(deps.exitProcess).toHaveBeenCalledWith(42); + expect(deps.warn).not.toHaveBeenCalled(); + }); + + it("redacts create output before classification and echoing", () => { + // With output: leading blank + headline + blank + output echo + "Try:" hint = 5 error() calls. + const withOutput = createFailureDeps(); + expect(() => + reportSandboxCreateFailure( + createFailureOptions({ createOutput: "failed with Authorization: Bearer secret-token" }), + withOutput, + ), + ).toThrow(ExitSignal); + expect(withOutput.classifyCreateFailure).toHaveBeenCalledWith( + "failed with Authorization: Bearer secr********", + ); + expect(withOutput.error).toHaveBeenCalledWith("failed with Authorization: Bearer secr********"); + expect(withOutput.error).not.toHaveBeenCalledWith( + "failed with Authorization: Bearer secret-token", + ); + expect(withOutput.printRecoveryHints).toHaveBeenCalledWith( + "failed with Authorization: Bearer secr********", + expect.any(Object), + ); + expect(withOutput.error).toHaveBeenCalledTimes(5); + + // Without output: the echo block is skipped, so only 3 error() calls remain. + const noOutput = createFailureDeps(); + expect(() => + reportSandboxCreateFailure(createFailureOptions({ createOutput: "" }), noOutput), + ).toThrow(ExitSignal); + expect(noOutput.error).toHaveBeenCalledTimes(3); + // still exits (createStatus || 1) + expect(noOutput.exitProcess).toHaveBeenCalledWith(3); + }); + + it("redacts multiple known token formats in create output", () => { + const deps = createFailureDeps(); + const createOutput = [ + "Authorization: Bearer secret-token", + "github ghp_abcdefghijklmnopqrstuvwxyz1234567890", + "openai sk-abcdefghijklmnopqrstuvwxyz1234567890", + "aws AKIAABCDEFGHIJKLMNOP", + ].join("\n"); + + expect(() => reportSandboxCreateFailure(createFailureOptions({ createOutput }), deps)).toThrow( + ExitSignal, + ); + + const echoed = (deps.error as ReturnType).mock.calls + .map((call) => String(call[0])) + .join("\n"); + expect(echoed).not.toContain("secret-token"); + expect(echoed).not.toContain("ghp_abcdefghijklmnopqrstuvwxyz1234567890"); + expect(echoed).not.toContain("sk-abcdefghijklmnopqrstuvwxyz1234567890"); + expect(echoed).not.toContain("AKIAABCDEFGHIJKLMNOP"); + const hinted = (deps.printRecoveryHints as ReturnType).mock.calls + .map((call) => String(call[0])) + .join("\n"); + expect(hinted).not.toContain("secret-token"); + expect(hinted).not.toContain("ghp_abcdefghijklmnopqrstuvwxyz1234567890"); + expect(hinted).not.toContain("sk-abcdefghijklmnopqrstuvwxyz1234567890"); + expect(hinted).not.toContain("AKIAABCDEFGHIJKLMNOP"); + }); + + it("falls back to exit code 1 when the create status is zero", () => { + const deps = createFailureDeps(); + expect(() => + reportSandboxCreateFailure(createFailureOptions({ createStatus: 0 }), deps), + ).toThrow(ExitSignal); + expect(deps.exitProcess).toHaveBeenCalledWith(1); + }); +}); + +const NOT_READY: CreatedSandboxReadinessResult = { + ready: false, + reason: "timeout", + failurePhase: null, +}; + +function readinessDeps( + overrides: Partial = {}, +): SandboxReadinessFailureReportDeps { + return { + printReadinessFailure: vi.fn(), + printCreateFailureDiagnostics: vi.fn(), + printDockerGpuReadinessFailure: vi.fn(), + deleteSandbox: vi.fn(() => ({ status: 0 })), + cliName: vi.fn(() => "nemoclaw"), + error: vi.fn(), + exitProcess: vi.fn((code: number): never => { + throw new ExitSignal(code); + }), + ...overrides, + }; +} + +function readinessOptions( + overrides: Partial = {}, +): SandboxReadinessFailureReportOptions { + return { + sandboxName: "alpha", + readiness: NOT_READY, + createStatus: 0, + timeoutSecs: 300, + restoreBackupPath: null, + useDockerGpuPatch: false, + ...overrides, + }; +} + +describe("reportSandboxReadinessFailure", () => { + it("deletes the failed sandbox on the non-GPU path and exits 1", () => { + const deps = readinessDeps(); + expect(() => reportSandboxReadinessFailure(readinessOptions(), deps)).toThrow(ExitSignal); + expect(deps.printReadinessFailure).toHaveBeenCalledWith(NOT_READY, "alpha", 300); + expect(deps.printCreateFailureDiagnostics).toHaveBeenCalledWith("alpha", { backupPath: null }); + expect(deps.deleteSandbox).toHaveBeenCalledWith("alpha"); + expect(deps.printDockerGpuReadinessFailure).not.toHaveBeenCalled(); + expect(deps.error).toHaveBeenCalledWith( + " The failed sandbox has been removed; retry will recreate it.", + ); + expect(deps.error).toHaveBeenCalledWith(" Retry: nemoclaw onboard"); + expect(deps.exitProcess).toHaveBeenCalledWith(1); + }); + + it("surfaces manual cleanup when deletion fails", () => { + const deps = readinessDeps({ deleteSandbox: vi.fn(() => ({ status: 1 })) }); + expect(() => reportSandboxReadinessFailure(readinessOptions(), deps)).toThrow(ExitSignal); + expect(deps.error).toHaveBeenCalledWith( + " Could not remove the failed sandbox. Manual cleanup:", + ); + expect(deps.error).toHaveBeenCalledWith(' openshell sandbox delete "alpha"'); + }); + + it("defers cleanup to the Docker-GPU patch and never deletes the sandbox", () => { + const deps = readinessDeps(); + expect(() => + reportSandboxReadinessFailure(readinessOptions({ useDockerGpuPatch: true }), deps), + ).toThrow(ExitSignal); + expect(deps.printDockerGpuReadinessFailure).toHaveBeenCalledTimes(1); + expect(deps.deleteSandbox).not.toHaveBeenCalled(); + expect(deps.exitProcess).toHaveBeenCalledWith(1); + }); + + it("preserves a non-zero create-stream status when readiness later fails", () => { + const deps = readinessDeps(); + expect(() => + reportSandboxReadinessFailure(readinessOptions({ createStatus: 255 }), deps), + ).toThrow(ExitSignal); + expect(deps.exitProcess).toHaveBeenCalledWith(255); + }); +}); diff --git a/src/lib/onboard/created-sandbox-failure.ts b/src/lib/onboard/created-sandbox-failure.ts new file mode 100644 index 0000000000..a067aa9b56 --- /dev/null +++ b/src/lib/onboard/created-sandbox-failure.ts @@ -0,0 +1,120 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { redact } from "../security/redact"; +import type { CreatedSandboxReadinessResult } from "./sandbox-readiness-tracing"; + +export type SandboxCreateFailureReportOptions = { + sandboxName: string; + /** Non-zero exit status from the create stream. */ + createStatus: number; + /** Raw create-stream output, used for failure classification and recovery hints. */ + createOutput: string; + /** Pre-recreate/pre-upgrade state backup path to surface in diagnostics, if any. */ + restoreBackupPath: string | null; + /** Resolved `openshell sandbox create` args, so recovery hints stay aligned with --from. */ + createArgs: readonly string[]; +}; + +export type SandboxCreateFailureReportDeps = { + classifyCreateFailure(output: string): { kind: string }; + printCreateFailureDiagnostics(sandboxName: string, options: { backupPath: string | null }): void; + printRecoveryHints(output: string, options: { createArgs: readonly string[] }): void; + warn(message: string): void; + error(message: string): void; + exitProcess(code: number): never; +}; + +/** + * Report a non-zero sandbox create-stream exit. A mere "create incomplete" + * (the sandbox exists in the gateway but the stream exited non-zero, e.g. SSH + * 255) warns and returns so the caller can fall through to the ready-wait loop; + * any other failure prints diagnostics + recovery hints and exits. + */ +export function reportSandboxCreateFailure( + options: SandboxCreateFailureReportOptions, + deps: SandboxCreateFailureReportDeps, +): void { + const redactedCreateOutput = redact(options.createOutput); + const failure = deps.classifyCreateFailure(redactedCreateOutput); + if (failure.kind === "sandbox_create_incomplete") { + // The sandbox was created in the gateway but the create stream exited + // with a non-zero code (e.g. SSH 255). Fall through to the ready-wait + // loop — the sandbox may still reach Ready on its own. + deps.warn(""); + deps.warn( + ` Create stream exited with code ${options.createStatus} after sandbox was created.`, + ); + deps.warn(" Checking whether the sandbox reaches Ready state..."); + return; + } + deps.error(""); + deps.error(` Sandbox creation failed (exit ${options.createStatus}).`); + if (options.createOutput) { + deps.error(""); + deps.error(redactedCreateOutput); + } + deps.printCreateFailureDiagnostics(options.sandboxName, { + backupPath: options.restoreBackupPath, + }); + deps.error(" Try: openshell sandbox list # check gateway state"); + deps.printRecoveryHints(redactedCreateOutput, { createArgs: options.createArgs }); + return deps.exitProcess(options.createStatus === 0 ? 1 : options.createStatus); +} + +export type SandboxReadinessFailureReportOptions = { + sandboxName: string; + readiness: CreatedSandboxReadinessResult; + /** Exit status reported by the sandbox create stream before readiness polling. */ + createStatus: number; + timeoutSecs: number; + restoreBackupPath: string | null; + /** When the Docker-GPU create patch is active, cleanup is deferred to the patch. */ + useDockerGpuPatch: boolean; +}; + +export type SandboxReadinessFailureReportDeps = { + printReadinessFailure( + readiness: CreatedSandboxReadinessResult, + sandboxName: string, + timeoutSecs: number, + ): void; + printCreateFailureDiagnostics(sandboxName: string, options: { backupPath: string | null }): void; + printDockerGpuReadinessFailure(): void; + deleteSandbox(sandboxName: string): { status: number | null }; + cliName(): string; + error(message: string): void; + exitProcess(code: number): never; +}; + +/** + * Report a sandbox that never reached Ready: print the readiness failure and + * create diagnostics, then either defer cleanup to the Docker-GPU patch or + * delete the failed sandbox so a same-name retry does not collide, and exit. + */ +export function reportSandboxReadinessFailure( + options: SandboxReadinessFailureReportOptions, + deps: SandboxReadinessFailureReportDeps, +): never { + deps.error(""); + deps.printReadinessFailure(options.readiness, options.sandboxName, options.timeoutSecs); + deps.printCreateFailureDiagnostics(options.sandboxName, { + backupPath: options.restoreBackupPath, + }); + if (options.useDockerGpuPatch) { + deps.printDockerGpuReadinessFailure(); + } else { + // Clean up non-GPU failures after preserving local diagnostics so the + // next onboard retry with the same name does not fail on "sandbox already exists". + const delResult = deps.deleteSandbox(options.sandboxName); + if (delResult.status === 0) { + deps.error(" The failed sandbox has been removed; retry will recreate it."); + } else { + deps.error(" Could not remove the failed sandbox. Manual cleanup:"); + deps.error(` openshell sandbox delete "${options.sandboxName}"`); + } + } + deps.error(` Retry: ${deps.cliName()} onboard`); + const exitCode = options.createStatus === 0 ? 1 : options.createStatus; + return deps.exitProcess(exitCode); +} diff --git a/src/lib/onboard/sandbox-create-launch.test.ts b/src/lib/onboard/sandbox-create-launch.test.ts index b61ec66104..e07a10f93e 100644 --- a/src/lib/onboard/sandbox-create-launch.test.ts +++ b/src/lib/onboard/sandbox-create-launch.test.ts @@ -86,6 +86,7 @@ describe("prepareSandboxCreateLaunch", () => { expect(result.createCommand).toBe( `openshell sandbox create --from /tmp/build/Dockerfile --name demo -- ${result.sandboxStartupCommand.join(" ")} 2>&1`, ); + expect(result.createArgv).toEqual(["bash", "-lc", result.createCommand]); }); it("forwards only the allowlisted OpenClaw auto-pair runtime controls", () => { @@ -256,6 +257,7 @@ describe("prepareSandboxCreateLaunch", () => { getDashboardForwardPort: () => "19000", hermesDashboardState: disabledHermesDashboardState, openshellShellCommand: helpers.openshellShellCommand, + openshellArgv: helpers.openshellArgv, buildEnv: () => ({}), }); @@ -279,6 +281,7 @@ describe("prepareSandboxCreateLaunch", () => { expect(fs.existsSync(injectedFromPath)).toBe(false); expect(fs.existsSync(injectedUrlPath)).toBe(false); expect(fs.existsSync(injectedProxyPath)).toBe(false); + expect(result.createArgv).toEqual([fakeOpenshell, ...capturedArgs]); } finally { fs.rmSync(tmpDir, { recursive: true, force: true }); } diff --git a/src/lib/onboard/sandbox-create-launch.ts b/src/lib/onboard/sandbox-create-launch.ts index 7b904c24c9..6d780d0a13 100644 --- a/src/lib/onboard/sandbox-create-launch.ts +++ b/src/lib/onboard/sandbox-create-launch.ts @@ -17,6 +17,7 @@ import { } from "./sandbox-prebuild"; type OpenshellShellCommand = (args: string[]) => string; +type OpenshellArgv = (args: string[]) => string[]; // These non-secret scheduler controls are intentionally forwarded for bounded // live-test and operator tuning. Keep this as an exact allowlist: the host's @@ -52,11 +53,13 @@ export interface SandboxCreateLaunchInput { hermesDashboardState: HermesDashboardOnboardState; manageDashboard?: boolean; openshellShellCommand: OpenshellShellCommand; + openshellArgv?: OpenshellArgv; buildEnv?(): Record; } export interface SandboxCreateLaunch { createCommand: string; + createArgv: string[]; effectiveDashboardPort: string; envArgs: string[]; sandboxEnv: Record; @@ -139,16 +142,15 @@ export function prepareSandboxCreateLaunch(input: SandboxCreateLaunchInput): San // command (awk, always 0) unless pipefail is set. Removing the pipe // lets the real exit code flow through to run(). const sandboxStartupCommand = ["env", ...envArgs, "nemoclaw-start"]; - const createCommand = `${input.openshellShellCommand([ - "sandbox", - "create", - ...input.createArgs, - "--", - ...sandboxStartupCommand, - ])} 2>&1`; + const openshellArgs = ["sandbox", "create", ...input.createArgs, "--", ...sandboxStartupCommand]; + const createCommand = `${input.openshellShellCommand(openshellArgs)} 2>&1`; + const createArgv = input.openshellArgv + ? input.openshellArgv(openshellArgs) + : ["bash", "-lc", createCommand]; return { createCommand, + createArgv, effectiveDashboardPort, envArgs, sandboxEnv, diff --git a/src/lib/onboard/sandbox-create-step.test.ts b/src/lib/onboard/sandbox-create-step.test.ts new file mode 100644 index 0000000000..e183f4a57a --- /dev/null +++ b/src/lib/onboard/sandbox-create-step.test.ts @@ -0,0 +1,300 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { streamSandboxCreate } from "../sandbox/create-stream"; +import { + dockerEnv, + FakeChild, + makePollingOptions, + vmEnv, +} from "../sandbox/create-stream-test-fixtures"; +import { + runSandboxCreateStep, + type SandboxCreateStepContext, + type SandboxCreateStepDeps, +} from "./sandbox-create-step"; + +function makeLaunch(overrides: Record = {}) { + return { + createCommand: "openshell sandbox create alpha", + effectiveDashboardPort: "18789", + createArgv: ["openshell", "sandbox", "create", "alpha"], + envArgs: [], + sandboxEnv: { FOO: "bar" }, + sandboxStartupCommand: ["run", "alpha"], + prebuild: { imageRef: "img:tag", createArgs: ["sandbox", "create", "alpha"] }, + ...overrides, + }; +} + +function makePatch() { + return { + maybeApplyDuringCreate: vi.fn(), + createFailureMessage: vi.fn(() => null), + ensureApplied: vi.fn(), + }; +} + +function makeContext(overrides: Partial = {}): SandboxCreateStepContext { + // Cast once at the boundary: hermesDashboardState / openshellShellCommand / + // prebuild are structural seams this orchestration test does not exercise. + const base = { + agent: null, + observabilityEnabled: false, + chatUiUrl: "", + createArgs: ["sandbox", "create", "alpha"], + sandboxName: "alpha", + env: {}, + extraPlaceholderKeys: [], + getDashboardForwardPort: () => "18789", + hermesDashboardState: null, + manageDashboard: false, + openshellShellCommand: null, + prebuild: { buildCtx: "/tmp/ctx", buildId: "b1", dockerDriverGateway: null, origin: "local" }, + useDockerGpuPatch: false, + gpuDevice: null, + gpuBackend: "generic" as const, + timeoutSecs: 300, + }; + return { ...base, ...overrides } as unknown as SandboxCreateStepContext; +} + +function makeDeps( + launch: ReturnType, + patch: ReturnType, + createResult: { status: number; output: string }, + overrides: Partial = {}, +): SandboxCreateStepDeps { + return { + prepareCreateLaunch: vi.fn(async () => launch), + createDockerGpuPatch: vi.fn(() => patch), + streamCreate: vi.fn(async () => createResult), + isSandboxReady: vi.fn(() => false), + isTerminalAgent: vi.fn(() => false), + addTraceEvent: vi.fn(), + runOpenshell: vi.fn(() => ({ status: 0, output: "" })), + runCaptureOpenshell: vi.fn(() => "sandbox-list"), + sleepSeconds: vi.fn(), + ...overrides, + } as unknown as SandboxCreateStepDeps; +} + +describe("runSandboxCreateStep", () => { + it("threads the prebuild handoff into launch, GPU patch, and stream, and returns the handles", async () => { + const launch = makeLaunch(); + const patch = makePatch(); + const createResult = { status: 0, output: "created" }; + const deps = makeDeps(launch, patch, createResult); + + const result = await runSandboxCreateStep( + makeContext({ + useDockerGpuPatch: true, + gpuDevice: "nvidia.com/gpu=all", + gpuBackend: "jetson", + }), + deps, + ); + + // prepareCreateLaunch receives the assembled launch input incl. the prebuild handoff. + expect(deps.prepareCreateLaunch).toHaveBeenCalledWith( + expect.objectContaining({ + sandboxName: "alpha", + prebuild: { + buildCtx: "/tmp/ctx", + buildId: "b1", + dockerDriverGateway: null, + origin: "local", + }, + }), + ); + // GPU patch is created with the startup command from the launch result + backend/device. + expect(deps.createDockerGpuPatch).toHaveBeenCalledWith( + expect.objectContaining({ + enabled: true, + openshellSandboxCommand: ["run", "alpha"], + gpuDevice: "nvidia.com/gpu=all", + backend: "jetson", + }), + ); + // stream is fed the launch command + env. + expect(deps.streamCreate).toHaveBeenCalledWith( + "openshell", + ["sandbox", "create", "alpha"], + { FOO: "bar" }, + expect.objectContaining({ traceEvent: deps.addTraceEvent }), + ); + // Handles returned for downstream consumers. + expect(result).toEqual({ + createResult, + prebuild: launch.prebuild, + effectiveDashboardPort: "18789", + dockerGpuCreatePatch: patch, + }); + }); + + it("separates readiness detection from GPU patch polling", async () => { + const launch = makeLaunch(); + const patch = makePatch(); + const deps = makeDeps( + launch, + patch, + { status: 0, output: "" }, + { isSandboxReady: vi.fn(() => true) }, + ); + + await runSandboxCreateStep(makeContext(), deps); + const streamOpts = (deps.streamCreate as unknown as { mock: { calls: unknown[][] } }).mock + .calls[0][3] as { readyCheck: () => boolean; onPoll: () => void }; + + expect(streamOpts.readyCheck()).toBe(true); + expect(patch.maybeApplyDuringCreate).not.toHaveBeenCalled(); + + (deps.isSandboxReady as unknown as ReturnType).mockReturnValue(false); + expect(streamOpts.readyCheck()).toBe(false); + expect(patch.maybeApplyDuringCreate).not.toHaveBeenCalled(); + + streamOpts.onPoll(); + expect(patch.maybeApplyDuringCreate).toHaveBeenCalledTimes(1); + }); + + it("threads the terminal-agent early-ready gate into stream options", async () => { + const terminalDeps = makeDeps( + makeLaunch(), + makePatch(), + { status: 0, output: "" }, + { + isTerminalAgent: vi.fn(() => true), + }, + ); + await runSandboxCreateStep(makeContext(), terminalDeps); + expect( + (terminalDeps.streamCreate as unknown as { mock: { calls: unknown[][] } }).mock.calls[0][3], + ).toMatchObject({ readyCheckOutputPatterns: [] }); + + const nonTerminalDeps = makeDeps(makeLaunch({ sandboxEnv: vmEnv }), makePatch(), { + status: 0, + output: "", + }); + await runSandboxCreateStep(makeContext(), nonTerminalDeps); + expect( + (nonTerminalDeps.streamCreate as unknown as { mock: { calls: unknown[][] } }).mock + .calls[0][3], + ).toMatchObject({ readyCheckOutputPatterns: [expect.any(RegExp)] }); + }); + + it.each([ + ["terminal VM", true, vmEnv], + ["terminal Docker", true, dockerEnv], + ["non-terminal Docker", false, dockerEnv], + ])("detaches immediately for %s", async (_label, isTerminalAgent, env) => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const logLine = vi.fn(); + const streamOptions = makePollingOptions(child, { logLine }); + const deps = makeDeps( + makeLaunch({ sandboxEnv: env }), + makePatch(), + { status: 0, output: "" }, + { + streamCreate: ((command, args, sandboxEnv, options) => + streamSandboxCreate(command, args, sandboxEnv, { + ...options, + ...streamOptions, + })) as SandboxCreateStepDeps["streamCreate"], + isTerminalAgent: vi.fn(() => isTerminalAgent), + }, + ); + let ready = false; + deps.isSandboxReady = vi.fn(() => ready); + deps.addTraceEvent = vi.fn(); + + const promise = runSandboxCreateStep(makeContext(), deps); + child.stdout.emit("data", Buffer.from("Created sandbox: alpha\n")); + ready = true; + await vi.advanceTimersByTimeAsync(6); + + expect(logLine).not.toHaveBeenCalledWith( + " Sandbox reported Ready; waiting for startup command output before detaching.", + ); + await expect(promise).resolves.toMatchObject({ + createResult: expect.objectContaining({ status: 0, forcedReady: true }), + }); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + vi.useRealTimers(); + }); + + it("waits for startup output for non-terminal VM creates", async () => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const logLine = vi.fn(); + const streamOptions = makePollingOptions(child, { logLine }); + const deps = makeDeps( + makeLaunch({ sandboxEnv: vmEnv }), + makePatch(), + { status: 0, output: "" }, + { + streamCreate: ((command, args, sandboxEnv, options) => + streamSandboxCreate(command, args, sandboxEnv, { + ...options, + ...streamOptions, + })) as SandboxCreateStepDeps["streamCreate"], + }, + ); + let ready = false; + deps.isSandboxReady = vi.fn(() => ready); + deps.addTraceEvent = vi.fn(); + + const promise = runSandboxCreateStep(makeContext(), deps); + child.stdout.emit("data", Buffer.from("Created sandbox: alpha\n")); + ready = true; + await vi.advanceTimersByTimeAsync(6); + + expect(child.kill).not.toHaveBeenCalled(); + expect(logLine).toHaveBeenCalledWith( + " Sandbox reported Ready; waiting for startup command output before detaching.", + ); + + child.stderr.emit("data", Buffer.from("Setting up NemoClaw (Hermes)...\n")); + await vi.advanceTimersByTimeAsync(6); + + await expect(promise).resolves.toMatchObject({ + createResult: expect.objectContaining({ status: 0, forcedReady: true }), + }); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + vi.useRealTimers(); + }); + + it("recovers SSH 255 exits when the sandbox is ready", async () => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const streamOptions = makePollingOptions(child, { pollIntervalMs: 60_000 }); + const deps = makeDeps( + makeLaunch({ sandboxEnv: dockerEnv }), + makePatch(), + { status: 0, output: "" }, + { + streamCreate: ((command, args, sandboxEnv, options) => + streamSandboxCreate(command, args, sandboxEnv, { + ...options, + ...streamOptions, + })) as SandboxCreateStepDeps["streamCreate"], + isSandboxReady: vi.fn(() => true), + }, + ); + + const promise = runSandboxCreateStep(makeContext(), deps); + child.stdout.emit("data", Buffer.from("Created sandbox: alpha\n")); + child.emit("close", 255); + await vi.runOnlyPendingTimersAsync(); + + await expect(promise).resolves.toMatchObject({ + createResult: expect.objectContaining({ status: 0, forcedReady: true }), + }); + vi.useRealTimers(); + }); +}); diff --git a/src/lib/onboard/sandbox-create-step.ts b/src/lib/onboard/sandbox-create-step.ts new file mode 100644 index 0000000000..870d9d390e --- /dev/null +++ b/src/lib/onboard/sandbox-create-step.ts @@ -0,0 +1,129 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { AgentDefinition } from "../agent/defs"; +import type { + StreamSandboxCreateOptions, + StreamSandboxCreateResult, + streamSandboxCreate, +} from "../sandbox/create-stream"; +import { getReadyCheckOutputPatternsForAgent } from "../sandbox/create-stream-ready-gate"; +import type { + createDockerGpuSandboxCreatePatch, + DockerGpuSandboxCreatePatch, +} from "./docker-gpu-sandbox-create"; +import type { + prepareSandboxCreateLaunchWithPrebuild, + SandboxCreateLaunchWithPrebuild, + SandboxCreateLaunchWithPrebuildInput, +} from "./sandbox-create-launch"; + +type LaunchInput = SandboxCreateLaunchWithPrebuildInput; +type GpuPatchDeps = Parameters[0]["deps"]; + +export type SandboxCreateStepContext = { + agent: LaunchInput["agent"]; + observabilityEnabled: boolean; + chatUiUrl: string; + createArgs: LaunchInput["createArgs"]; + sandboxName: string; + env: NodeJS.ProcessEnv; + extraPlaceholderKeys: LaunchInput["extraPlaceholderKeys"]; + getDashboardForwardPort: LaunchInput["getDashboardForwardPort"]; + hermesDashboardState: LaunchInput["hermesDashboardState"]; + manageDashboard: boolean; + openshellShellCommand: LaunchInput["openshellShellCommand"]; + openshellArgv?: LaunchInput["openshellArgv"]; + prebuild: LaunchInput["prebuild"]; + useDockerGpuPatch: boolean; + gpuDevice: string | null | undefined; + gpuBackend: "jetson" | "generic"; + timeoutSecs: number; +}; + +export type SandboxCreateStepDeps = { + prepareCreateLaunch: typeof prepareSandboxCreateLaunchWithPrebuild; + createDockerGpuPatch: typeof createDockerGpuSandboxCreatePatch; + streamCreate: typeof streamSandboxCreate; + isSandboxReady(output: string, sandboxName: string): boolean; + isTerminalAgent(agent: AgentDefinition | null | undefined): boolean; + addTraceEvent: NonNullable; + runOpenshell: GpuPatchDeps["runOpenshell"]; + runCaptureOpenshell: NonNullable; + sleepSeconds: GpuPatchDeps["sleep"]; +}; + +export type SandboxCreateStepResult = { + createResult: StreamSandboxCreateResult; + prebuild: SandboxCreateLaunchWithPrebuild["prebuild"]; + effectiveDashboardPort: string; + dockerGpuCreatePatch: DockerGpuSandboxCreatePatch; +}; + +/** + * Resolve the BuildKit prebuild handoff into the launch command, provision the + * Docker-GPU create patch, and stream the sandbox create. Returns the create + * result plus the handles the caller needs downstream (prebuild identity, the + * dashboard port, and the GPU patch for its ready/verify hooks). Build-context + * and exit-listener cleanup stay with the caller that armed them. + */ +export async function runSandboxCreateStep( + context: SandboxCreateStepContext, + deps: SandboxCreateStepDeps, +): Promise { + const { + createCommand, + createArgv, + effectiveDashboardPort, + prebuild, + sandboxEnv, + sandboxStartupCommand, + } = await deps.prepareCreateLaunch({ + agent: context.agent, + observabilityEnabled: context.observabilityEnabled, + chatUiUrl: context.chatUiUrl, + createArgs: context.createArgs, + sandboxName: context.sandboxName, + env: context.env, + extraPlaceholderKeys: context.extraPlaceholderKeys, + getDashboardForwardPort: context.getDashboardForwardPort, + hermesDashboardState: context.hermesDashboardState, + manageDashboard: context.manageDashboard, + openshellShellCommand: context.openshellShellCommand, + openshellArgv: context.openshellArgv, + prebuild: context.prebuild, + }); + const dockerGpuCreatePatch = deps.createDockerGpuPatch({ + enabled: context.useDockerGpuPatch, + sandboxName: context.sandboxName, + gpuDevice: context.gpuDevice, + openshellSandboxCommand: sandboxStartupCommand, + timeoutSecs: context.timeoutSecs, + backend: context.gpuBackend, + deps: { + runOpenshell: deps.runOpenshell, + runCaptureOpenshell: deps.runCaptureOpenshell, + sleep: deps.sleepSeconds, + }, + }); + const [createExecutable, ...createExecutableArgs] = createArgv; + const createResult = await deps.streamCreate( + createExecutable ?? createCommand, + createExecutableArgs, + sandboxEnv, + { + readyCheck: () => { + const list = deps.runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); + return deps.isSandboxReady(list, context.sandboxName); + }, + onPoll: () => dockerGpuCreatePatch.maybeApplyDuringCreate(), + readyCheckOutputPatterns: getReadyCheckOutputPatternsForAgent( + deps.isTerminalAgent(context.agent), + sandboxEnv, + ), + failureCheck: dockerGpuCreatePatch.createFailureMessage, + traceEvent: deps.addTraceEvent, + }, + ); + return { createResult, prebuild, effectiveDashboardPort, dockerGpuCreatePatch }; +} diff --git a/src/lib/sandbox/create-stream-argv.test.ts b/src/lib/sandbox/create-stream-argv.test.ts new file mode 100644 index 0000000000..6b117f4029 --- /dev/null +++ b/src/lib/sandbox/create-stream-argv.test.ts @@ -0,0 +1,65 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { streamSandboxCreate } from "./create-stream"; +import { dockerEnv, FakeChild } from "./create-stream-test-fixtures"; + +describe("sandbox-create-stream argv boundary", () => { + it("keeps the legacy string-command overload shell compatible", async () => { + const child = new FakeChild(); + const spawnImpl = vi.fn(() => child); + const promise = streamSandboxCreate("echo create", dockerEnv, { + spawnImpl, + logLine: vi.fn(), + }); + + child.emit("close", 0); + await expect(promise).resolves.toMatchObject({ status: 0 }); + expect(spawnImpl).toHaveBeenCalledWith( + "bash", + ["-lc", "echo create"], + expect.not.objectContaining({ shell: true }), + ); + }); + + it("spawns argv directly without shell wrapping", async () => { + const child = new FakeChild(); + const spawnImpl = vi.fn(() => child); + const promise = streamSandboxCreate( + "openshell", + ["sandbox", "create", "alpha; rm -rf /", "--", "nemoclaw-start"], + dockerEnv, + { + spawnImpl, + logLine: vi.fn(), + }, + ); + + child.emit("close", 0); + await expect(promise).resolves.toMatchObject({ status: 0 }); + expect(spawnImpl).toHaveBeenCalledWith( + "openshell", + ["sandbox", "create", "alpha; rm -rf /", "--", "nemoclaw-start"], + expect.not.objectContaining({ shell: true }), + ); + }); + + it("inherits process env when argv callers omit env", async () => { + const child = new FakeChild(); + const spawnImpl = vi.fn(() => child); + const promise = streamSandboxCreate("openshell", ["sandbox", "create"], undefined, { + spawnImpl, + logLine: vi.fn(), + }); + + child.emit("close", 0); + await expect(promise).resolves.toMatchObject({ status: 0 }); + expect(spawnImpl).toHaveBeenCalledWith( + "openshell", + ["sandbox", "create"], + expect.objectContaining({ env: process.env }), + ); + }); +}); diff --git a/src/lib/sandbox/create-stream-progress.ts b/src/lib/sandbox/create-stream-progress.ts new file mode 100644 index 0000000000..63692ed59f --- /dev/null +++ b/src/lib/sandbox/create-stream-progress.ts @@ -0,0 +1,50 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export type CreatePhase = "pull" | "build" | "upload" | "create" | "ready"; + +export const BUILD_PROGRESS_PATTERNS: readonly RegExp[] = [ + /^ {2}Building image /, + /^ {2}Step \d+\/\d+ : /, + /^#\d+ \[/, + /^#\d+ (DONE|CACHED)\b/, +]; + +export const UPLOAD_PROGRESS_PATTERNS: readonly RegExp[] = [ + /^ {2}Pushing image /, + /^\s*\[progress\]/, + /^\s*(?:✓\s*)?Image .*available in the gateway/, +]; + +// Pull-phase indicators. Detect classic Docker pull output (`: Pulling +// from `, `: Pulling fs layer / Downloading / Extracting / Pull +// complete`, `Status: Downloaded`, `Digest:`) plus BuildKit pull progress +// (`#N resolve `, `#N sha256: / `). The tag prefix +// regex uses [^:\s]+ so non-lowercase tags (`v1.2.3`, `cuda-12.5`, `12.4`) +// also match. See #1829. +export const PULL_PROGRESS_PATTERNS: readonly RegExp[] = [ + /^\s*(?:[^:\s]+:\s+)?Pulling from \S+/, + /^\s*[a-f0-9]{6,}: (?:Pulling fs layer|Waiting|Downloading|Extracting|Pull complete|Verifying Checksum|Download complete)\b/, + /^\s*Status: (?:Downloaded|Image is up to date)/, + /^\s*Digest: sha256:[a-f0-9]{8,}/, + /^\s*#\d+\s+(?:resolve\s+\S+|sha256:[a-f0-9]+\s+[\d.]+\s*(?:B|KB|MB|GB)\s*\/)/, +]; + +export const VISIBLE_PROGRESS_PATTERNS: readonly RegExp[] = [ + ...BUILD_PROGRESS_PATTERNS, + /^ {2}Context: /, + /^ {2}Gateway: /, + /^Successfully built /, + /^Successfully tagged /, + /^ {2}Built image /, + ...UPLOAD_PROGRESS_PATTERNS, + ...PULL_PROGRESS_PATTERNS, + /^Created sandbox: /, + /^Creating sandbox/i, + /^Starting sandbox/i, + /^✓ /, +]; + +export function matchesAny(line: string, patterns: readonly RegExp[]) { + return patterns.some((pattern) => pattern.test(line)); +} diff --git a/src/lib/sandbox/create-stream-ready-gate.test.ts b/src/lib/sandbox/create-stream-ready-gate.test.ts new file mode 100644 index 0000000000..b946a0bb29 --- /dev/null +++ b/src/lib/sandbox/create-stream-ready-gate.test.ts @@ -0,0 +1,201 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { streamSandboxCreate } from "./create-stream"; +import { dockerEnv, FakeChild, makePollingOptions, vmEnv } from "./create-stream-test-fixtures"; +import { + getReadyCheckOutputPatterns, + getReadyCheckOutputPatternsForAgent, +} from "./create-stream-ready-gate"; + +describe("sandbox-create-stream ready gate", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it.each([ + ["explicit empty gate on VM", vmEnv, []], + ["explicit empty gate on Docker", dockerEnv, []], + ["default Docker gate", dockerEnv, undefined], + ])("detaches immediately for %s", async (_label, env, readyCheckOutputPatterns) => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const logLine = vi.fn(); + const promise = streamSandboxCreate( + "echo create", + env, + makePollingOptions(child, { + readyCheck: () => true, + readyCheckOutputPatterns, + logLine, + }), + ); + + child.stdout.emit("data", Buffer.from("Created sandbox: demo\n")); + await vi.advanceTimersByTimeAsync(6); + + expect(logLine).not.toHaveBeenCalledWith( + " Sandbox reported Ready; waiting for startup command output before detaching.", + ); + await expect(promise).resolves.toMatchObject({ + status: 0, + forcedReady: true, + output: expect.stringContaining("Sandbox reported Ready before create stream exited"), + }); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + }); + + it.each([ + ["non-terminal Docker", false, dockerEnv], + ["non-terminal VM", false, vmEnv], + ["terminal Docker", true, dockerEnv], + ["terminal VM", true, vmEnv], + ])("keeps agent and env ready gates equivalent for %s", (_label, isTerminalAgent, env) => { + const explicitPatterns = getReadyCheckOutputPatternsForAgent(isTerminalAgent, env); + expect(getReadyCheckOutputPatterns(env, explicitPatterns)).toEqual(explicitPatterns); + }); + + it("ignores process env driver overrides when explicit env is supplied", () => { + vi.stubEnv("OPENSHELL_DRIVERS", "vm"); + expect(getReadyCheckOutputPatterns(dockerEnv, undefined)).toEqual([]); + }); + + it("waits for startup output before detaching with the default VM gate", async () => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const logLine = vi.fn(); + let resolved = false; + const promise = streamSandboxCreate( + "echo create", + vmEnv, + makePollingOptions(child, { readyCheck: () => true, logLine }), + ).then((result) => { + resolved = true; + return result; + }); + + child.stdout.emit("data", Buffer.from("Created sandbox: demo\n")); + await vi.advanceTimersByTimeAsync(6); + + expect(resolved).toBe(false); + expect(child.kill).not.toHaveBeenCalled(); + expect(logLine).toHaveBeenCalledWith( + " Sandbox reported Ready; waiting for startup command output before detaching.", + ); + + child.stderr.emit("data", Buffer.from("Setting up NemoClaw (Hermes)...\n")); + await vi.advanceTimersByTimeAsync(6); + + await expect(promise).resolves.toMatchObject({ + status: 0, + forcedReady: true, + output: expect.stringContaining("Sandbox reported Ready before create stream exited"), + }); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + }); + + it("rejects poll side effects without a terminal failure check", () => { + expect(() => + streamSandboxCreate( + "echo create", + dockerEnv, + makePollingOptions(new FakeChild(), { readyCheck: () => false, onPoll: () => {} }), + ), + ).toThrow( + "streamSandboxCreate onPoll requires failureCheck (e.g., dockerGpuCreatePatch.createFailureMessage)", + ); + }); + + it("runs poll side effects only after a not-ready poll", async () => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const onPoll = vi.fn(); + let ready = false; + const promise = streamSandboxCreate( + "echo create", + dockerEnv, + makePollingOptions(child, { readyCheck: () => ready, onPoll, failureCheck: () => null }), + ); + + await vi.advanceTimersByTimeAsync(6); + expect(onPoll).toHaveBeenCalledTimes(1); + + ready = true; + await vi.advanceTimersByTimeAsync(6); + await expect(promise).resolves.toMatchObject({ status: 0, forcedReady: true }); + expect(onPoll).toHaveBeenCalledTimes(1); + }); + + it("aborts poll side-effect errors with a generic redacted failure", async () => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const traceEvent = vi.fn(); + const logLine = vi.fn(); + const onPoll = vi.fn(() => { + throw new Error("Authorization: Bearer secret-token"); + }); + const promise = streamSandboxCreate( + "echo create", + dockerEnv, + makePollingOptions(child, { + readyCheck: () => false, + onPoll, + failureCheck: () => null, + traceEvent, + logLine, + }), + ); + + await vi.advanceTimersByTimeAsync(6); + + await expect(promise).resolves.toMatchObject({ + status: 1, + output: expect.stringContaining("Sandbox create poll side effect failed."), + }); + expect(traceEvent).toHaveBeenCalledWith("sandbox_create_poll_error", { + message: "Authorization: Bearer secr********", + }); + expect(logLine).toHaveBeenCalledWith(" Sandbox create poll side effect failed."); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + }); + + it("checks failure after a poll side-effect error in the same tick", async () => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const traceEvent = vi.fn(); + const logLine = vi.fn(); + const promise = streamSandboxCreate( + "echo create", + dockerEnv, + makePollingOptions(child, { + readyCheck: () => false, + onPoll: () => { + throw new Error("Authorization: Bearer secret-token"); + }, + failureCheck: () => "Docker GPU patch failed", + traceEvent, + logLine, + }), + ); + + await vi.advanceTimersByTimeAsync(6); + + await expect(promise).resolves.toMatchObject({ + status: 1, + output: expect.stringContaining("Docker GPU patch failed"), + }); + expect(traceEvent).toHaveBeenCalledWith("sandbox_create_poll_error", { + message: "Authorization: Bearer secr********", + }); + expect(logLine).toHaveBeenCalledWith(" Docker GPU patch failed"); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(child.unref).toHaveBeenCalled(); + }); +}); diff --git a/src/lib/sandbox/create-stream-ready-gate.ts b/src/lib/sandbox/create-stream-ready-gate.ts new file mode 100644 index 0000000000..c54e46f402 --- /dev/null +++ b/src/lib/sandbox/create-stream-ready-gate.ts @@ -0,0 +1,27 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export const VM_READY_DETACH_OUTPUT_PATTERNS: readonly RegExp[] = [/Setting up NemoClaw/]; + +function selectedDrivers(env: NodeJS.ProcessEnv): string[] { + const raw = env.OPENSHELL_DRIVERS ?? (process.platform === "darwin" ? "vm" : "docker"); + return raw + .split(",") + .map((driver) => driver.trim()) + .filter(Boolean); +} + +export function getReadyCheckOutputPatterns( + env: NodeJS.ProcessEnv, + patterns: readonly RegExp[] | undefined, +): readonly RegExp[] { + if (patterns) return patterns; + return selectedDrivers(env).includes("vm") ? VM_READY_DETACH_OUTPUT_PATTERNS : []; +} + +export function getReadyCheckOutputPatternsForAgent( + isTerminalAgent: boolean, + env: NodeJS.ProcessEnv, +): readonly RegExp[] { + return isTerminalAgent ? [] : getReadyCheckOutputPatterns(env, undefined); +} diff --git a/src/lib/sandbox/create-stream-test-fixtures.ts b/src/lib/sandbox/create-stream-test-fixtures.ts new file mode 100644 index 0000000000..ab37715901 --- /dev/null +++ b/src/lib/sandbox/create-stream-test-fixtures.ts @@ -0,0 +1,53 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { EventEmitter } from "node:events"; + +import { vi } from "vitest"; + +import type { + StreamSandboxCreateOptions, + StreamableChildProcess, + StreamableReadable, +} from "./create-stream"; + +export class FakeReadable extends EventEmitter implements StreamableReadable { + destroy(): void {} +} + +export class FakeChild extends EventEmitter implements StreamableChildProcess { + stdout = new FakeReadable(); + stderr = new FakeReadable(); + kill = vi.fn(); + unref = vi.fn(); +} + +export const dockerEnv = { ...process.env, OPENSHELL_DRIVERS: "docker" }; +export const vmEnv = { ...process.env, OPENSHELL_DRIVERS: "vm" }; + +export function makeSpawnImpl(child: StreamableChildProcess = new FakeChild()) { + return () => child; +} + +export function makeDefaultStreamOptions( + child: StreamableChildProcess = new FakeChild(), + overrides: StreamSandboxCreateOptions = {}, +): StreamSandboxCreateOptions { + return { + spawnImpl: makeSpawnImpl(child), + heartbeatIntervalMs: 1_000, + silentPhaseMs: 10_000, + logLine: vi.fn(), + ...overrides, + }; +} + +export function makePollingOptions( + child: StreamableChildProcess = new FakeChild(), + overrides: StreamSandboxCreateOptions = {}, +): StreamSandboxCreateOptions { + return makeDefaultStreamOptions(child, { + pollIntervalMs: 5, + ...overrides, + }); +} diff --git a/src/lib/sandbox/create-stream.test.ts b/src/lib/sandbox/create-stream.test.ts index 6b00795212..1c937ca2f1 100644 --- a/src/lib/sandbox/create-stream.test.ts +++ b/src/lib/sandbox/create-stream.test.ts @@ -1,29 +1,15 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { EventEmitter } from "node:events"; - import { afterEach, describe, expect, it, vi } from "vitest"; +import { streamSandboxCreate } from "./create-stream"; import { - type StreamableChildProcess, - type StreamableReadable, - streamSandboxCreate, -} from "./create-stream"; - -class FakeReadable extends EventEmitter implements StreamableReadable { - destroy(): void {} -} - -class FakeChild extends EventEmitter implements StreamableChildProcess { - stdout = new FakeReadable(); - stderr = new FakeReadable(); - kill = vi.fn(); - unref = vi.fn(); -} - -const dockerEnv = { ...process.env, OPENSHELL_DRIVERS: "docker" }; -const vmEnv = { ...process.env, OPENSHELL_DRIVERS: "vm" }; + dockerEnv, + FakeChild, + makeDefaultStreamOptions, + vmEnv, +} from "./create-stream-test-fixtures"; describe("sandbox-create-stream", () => { afterEach(() => { @@ -185,41 +171,38 @@ describe("sandbox-create-stream", () => { expect(child.unref).toHaveBeenCalled(); }); - it("does not detach on Ready until required startup output appears", async () => { + it("traces ready-check errors and keeps polling without forcing ready", async () => { vi.useFakeTimers(); const child = new FakeChild(); - const logLine = vi.fn(); - let resolved = false; - const promise = streamSandboxCreate("echo create", vmEnv, { + const traceEvent = vi.fn(); + const readyCheck = vi + .fn() + .mockImplementationOnce(() => { + throw new Error("Authorization: Bearer secret-token"); + }) + .mockReturnValueOnce(false) + .mockReturnValue(true); + const promise = streamSandboxCreate("echo create", dockerEnv, { spawnImpl: () => child, - readyCheck: () => true, + readyCheck, pollIntervalMs: 5, heartbeatIntervalMs: 1_000, silentPhaseMs: 10_000, - logLine, - }).then((result) => { - resolved = true; - return result; + traceEvent, + logLine: vi.fn(), }); - child.stdout.emit("data", Buffer.from("Created sandbox: demo\n")); - await vi.advanceTimersByTimeAsync(12); - - expect(resolved).toBe(false); + child.stdout.emit("data", Buffer.from(" Building image sandbox\n")); + await vi.advanceTimersByTimeAsync(6); expect(child.kill).not.toHaveBeenCalled(); - expect(logLine).toHaveBeenCalledWith( - " Sandbox reported Ready; waiting for startup command output before detaching.", - ); + expect(traceEvent).toHaveBeenCalledWith("sandbox_create_ready_check_error", { + message: "Authorization: Bearer secr********", + }); - child.stderr.emit("data", Buffer.from("Setting up NemoClaw (Hermes)...\n")); - await vi.advanceTimersByTimeAsync(6); + await vi.advanceTimersByTimeAsync(12); - await expect(promise).resolves.toMatchObject({ - status: 0, - forcedReady: true, - output: expect.stringContaining("Setting up NemoClaw (Hermes)..."), - }); + await expect(promise).resolves.toMatchObject({ status: 0, forcedReady: true }); expect(child.kill).toHaveBeenCalledWith("SIGTERM"); }); @@ -273,10 +256,11 @@ describe("sandbox-create-stream", () => { it("flushes the final partial line before resolving", async () => { const child = new FakeChild(); - const promise = streamSandboxCreate("echo create", process.env, { - spawnImpl: () => child, - logLine: vi.fn(), - }); + const promise = streamSandboxCreate( + "echo create", + process.env, + makeDefaultStreamOptions(child), + ); child.stdout.emit("data", Buffer.from("Created sandbox: demo")); child.emit("close", 0); @@ -288,6 +272,25 @@ describe("sandbox-create-stream", () => { }); }); + it("keeps interleaved stdout and stderr fragments on separate lines", async () => { + const child = new FakeChild(); + const promise = streamSandboxCreate( + "echo create", + process.env, + makeDefaultStreamOptions(child), + ); + + child.stdout.emit("data", Buffer.from("stdout-partial")); + child.stderr.emit("data", Buffer.from("stderr-line\n")); + child.stdout.emit("data", Buffer.from("-complete\n")); + child.emit("close", 0); + + await expect(promise).resolves.toMatchObject({ + status: 0, + output: "stderr-line\nstdout-partial-complete", + }); + }); + it("recovers when sandbox is ready at the moment the stream exits non-zero", async () => { const child = new FakeChild(); const logLine = vi.fn(); @@ -488,10 +491,11 @@ describe("sandbox-create-stream", () => { it("reports spawn errors cleanly", async () => { const child = new FakeChild(); - const promise = streamSandboxCreate("echo create", process.env, { - spawnImpl: () => child, - logLine: vi.fn(), - }); + const promise = streamSandboxCreate( + "echo create", + process.env, + makeDefaultStreamOptions(child), + ); child.emit("error", Object.assign(new Error("ENOENT"), { code: "ENOENT" })); diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index 5311a82418..7096916345 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -3,7 +3,17 @@ import { type SpawnOptions, spawn } from "node:child_process"; +import { redact } from "../security/redact"; import { ROOT } from "../state/paths"; +import { + BUILD_PROGRESS_PATTERNS, + type CreatePhase, + matchesAny, + PULL_PROGRESS_PATTERNS, + UPLOAD_PROGRESS_PATTERNS, + VISIBLE_PROGRESS_PATTERNS, +} from "./create-stream-progress"; +import { getReadyCheckOutputPatterns } from "./create-stream-ready-gate"; export interface StreamSandboxCreateResult { status: number; @@ -14,6 +24,9 @@ export interface StreamSandboxCreateResult { export interface StreamSandboxCreateOptions { readyCheck?: (() => boolean) | null; + // Optional poll side effect. Must be paired with failureCheck so any + // observed side-effect error has an authoritative terminal-state classifier. + onPoll?: (() => void) | null; failureCheck?: (() => string | null | undefined) | null; pollIntervalMs?: number; heartbeatIntervalMs?: number; @@ -54,81 +67,45 @@ export interface StreamableChildProcess { on(event: "close", listener: (code: number | null) => void): this; } -export const BUILD_PROGRESS_PATTERNS: readonly RegExp[] = [ - /^ {2}Building image /, - /^ {2}Step \d+\/\d+ : /, - /^#\d+ \[/, - /^#\d+ (DONE|CACHED)\b/, -]; - -const UPLOAD_PROGRESS_PATTERNS: readonly RegExp[] = [ - /^ {2}Pushing image /, - /^\s*\[progress\]/, - /^\s*(?:✓\s*)?Image .*available in the gateway/, -]; - -// Pull-phase indicators. Detect classic Docker pull output (`: Pulling -// from `, `: Pulling fs layer / Downloading / Extracting / Pull -// complete`, `Status: Downloaded`, `Digest:`) plus BuildKit pull progress -// (`#N resolve `, `#N sha256: / `). The tag prefix -// regex uses [^:\s]+ so non-lowercase tags (`v1.2.3`, `cuda-12.5`, `12.4`) -// also match. See #1829. -const PULL_PROGRESS_PATTERNS: readonly RegExp[] = [ - /^\s*(?:[^:\s]+:\s+)?Pulling from \S+/, - /^\s*[a-f0-9]{6,}: (?:Pulling fs layer|Waiting|Downloading|Extracting|Pull complete|Verifying Checksum|Download complete)\b/, - /^\s*Status: (?:Downloaded|Image is up to date)/, - /^\s*Digest: sha256:[a-f0-9]{8,}/, - /^\s*#\d+\s+(?:resolve\s+\S+|sha256:[a-f0-9]+\s+[\d.]+\s*(?:B|KB|MB|GB)\s*\/)/, -]; - -const VISIBLE_PROGRESS_PATTERNS: readonly RegExp[] = [ - ...BUILD_PROGRESS_PATTERNS, - /^ {2}Context: /, - /^ {2}Gateway: /, - /^Successfully built /, - /^Successfully tagged /, - /^ {2}Built image /, - ...UPLOAD_PROGRESS_PATTERNS, - ...PULL_PROGRESS_PATTERNS, - /^Created sandbox: /, - /^Creating sandbox/i, - /^Starting sandbox/i, - /^✓ /, -]; - -const VM_READY_DETACH_OUTPUT_PATTERNS: readonly RegExp[] = [/Setting up NemoClaw/]; const CLASSIC_DOCKER_STEP_RE = /^\s*Step (\d+)\/(\d+) : (.+)$/; const BUILDKIT_STEP_RE = /^#(\d+)\s+(.+)$/; -function matchesAny(line: string, patterns: readonly RegExp[]) { - return patterns.some((pattern) => pattern.test(line)); -} - -function selectedDrivers(env: NodeJS.ProcessEnv): string[] { - const raw = - env.OPENSHELL_DRIVERS ?? - process.env.OPENSHELL_DRIVERS ?? - (process.platform === "darwin" ? "vm" : "docker"); - return raw - .split(",") - .map((driver) => driver.trim()) - .filter(Boolean); -} - -function getReadyCheckOutputPatterns( - env: NodeJS.ProcessEnv, - patterns: readonly RegExp[] | undefined, -): readonly RegExp[] { - if (patterns) return patterns; - return selectedDrivers(env).includes("vm") ? VM_READY_DETACH_OUTPUT_PATTERNS : []; -} - +/** + * @deprecated Prefer the argv overload `streamSandboxCreate(command, args, env, options)` for + * trusted create paths. This legacy overload preserves shell-compatible callers by executing + * `bash -lc `, so callers must not include unquoted user-controlled input. Remove + * this transitional overload in the #6258 follow-up once external callers have migrated. + */ +export function streamSandboxCreate( + command: string, + env?: NodeJS.ProcessEnv, + options?: StreamSandboxCreateOptions, +): Promise; export function streamSandboxCreate( command: string, - env: NodeJS.ProcessEnv = process.env, - options: StreamSandboxCreateOptions = {}, + args: readonly string[], + env?: NodeJS.ProcessEnv, + options?: StreamSandboxCreateOptions, +): Promise; +export function streamSandboxCreate( + command: string, + argsOrEnv: readonly string[] | NodeJS.ProcessEnv = process.env, + envOrOptions: NodeJS.ProcessEnv | StreamSandboxCreateOptions | undefined = undefined, + maybeOptions: StreamSandboxCreateOptions = {}, ): Promise { - const child: StreamableChildProcess = (options.spawnImpl ?? spawn)("bash", ["-lc", command], { + const hasArgs = Array.isArray(argsOrEnv); + const commandArgs = hasArgs ? argsOrEnv : ["-lc", command]; + const spawnCommand = hasArgs ? command : "bash"; + const env = hasArgs + ? ((envOrOptions ?? process.env) as NodeJS.ProcessEnv) + : (argsOrEnv as NodeJS.ProcessEnv); + const options = hasArgs ? maybeOptions : ((envOrOptions ?? {}) as StreamSandboxCreateOptions); + if (options.onPoll && !options.failureCheck) { + throw new Error( + "streamSandboxCreate onPoll requires failureCheck (e.g., dockerGpuCreatePatch.createFailureMessage)", + ); + } + const child: StreamableChildProcess = (options.spawnImpl ?? spawn)(spawnCommand, commandArgs, { cwd: ROOT, env, stdio: ["ignore", "pipe", "pipe"], @@ -137,7 +114,7 @@ export function streamSandboxCreate( const logLine = options.logLine ?? console.log; const traceEvent = options.traceEvent ?? (() => {}); const lines: string[] = []; - let pending = ""; + const pending = { stdout: "", stderr: "" }; let lastPrintedLine = ""; let sawProgress = false; const readyCheckOutputPatterns = getReadyCheckOutputPatterns( @@ -153,7 +130,6 @@ export function streamSandboxCreate( const silentPhaseMs = options.silentPhaseMs || 15000; const startedAt = Date.now(); let lastOutputAt = startedAt; - type CreatePhase = "pull" | "build" | "upload" | "create" | "ready"; let currentPhase: CreatePhase | null = null; let lastHeartbeatPhase: CreatePhase | null = null; @@ -337,24 +313,26 @@ export function streamSandboxCreate( return matchesAny(line, VISIBLE_PROGRESS_PATTERNS); } - function onChunk(chunk: Buffer | string) { - pending += chunk.toString(); - const parts = pending.split("\n"); - pending = parts.pop() ?? ""; + function onChunk(stream: keyof typeof pending, chunk: Buffer | string) { + pending[stream] += chunk.toString(); + const parts = pending[stream].split("\n"); + pending[stream] = parts.pop() ?? ""; parts.forEach(flushLine); } - function flushPendingLine() { - if (!pending) return; - const trailing = pending; - pending = ""; - flushLine(trailing); + function flushPendingLines() { + for (const stream of ["stdout", "stderr"] as const) { + if (!pending[stream]) continue; + const trailing = pending[stream]; + pending[stream] = ""; + flushLine(trailing); + } } function finish(status: number, overrides: Partial = {}) { if (settled) return; settled = true; - flushPendingLine(); + flushPendingLines(); if (!buildTimingFinished && buildStartedAtMs !== null) { finishBuildTiming(status === 0 ? "completed" : "stopped"); } @@ -378,8 +356,8 @@ export function streamSandboxCreate( child.unref?.(); } - child.stdout?.on("data", onChunk); - child.stderr?.on("data", onChunk); + child.stdout?.on("data", (chunk) => onChunk("stdout", chunk)); + child.stderr?.on("data", (chunk) => onChunk("stderr", chunk)); const readyTimer = options.readyCheck ? setInterval(() => { @@ -389,7 +367,10 @@ export function streamSandboxCreate( let ready = false; try { ready = !!options.readyCheck?.(); - } catch { + } catch (error) { + emitTraceEvent("sandbox_create_ready_check_error", { + message: redact(error instanceof Error ? error.message : String(error)), + }); return; } if (ready) { @@ -418,7 +399,17 @@ export function streamSandboxCreate( return; } - const failure = options.failureCheck?.(); + let pollFailure: string | null | undefined; + try { + options.onPoll?.(); + } catch (error) { + emitTraceEvent("sandbox_create_poll_error", { + message: redact(error instanceof Error ? error.message : String(error)), + }); + pollFailure = options.failureCheck?.() ?? "Sandbox create poll side effect failed."; + } + + const failure = pollFailure ?? options.failureCheck?.(); if (!failure) return; const detail = String(failure); lines.push(detail); @@ -480,7 +471,7 @@ export function streamSandboxCreate( child.on("close", (code) => { // One last ready-check: the sandbox may have become Ready between the // last poll tick and the stream exit (e.g. SSH 255 after "Created sandbox:"). - flushPendingLine(); + flushPendingLines(); if (code && code !== 0 && options.readyCheck) { try { if (options.readyCheck() && readyCheckOutputMatched) {