From 23df45be7d4fd275f7cb800892177cf1fcd57f07 Mon Sep 17 00:00:00 2001 From: Dongni Yang Date: Wed, 8 Jul 2026 12:21:15 +0800 Subject: [PATCH 01/23] refactor(onboard): extract sandbox create-failure reporting into a module The sandbox create/readiness failure handling was inlined in the ~4,900-line src/lib/onboard.ts entrypoint. Move both terminal handlers behind a focused boundary in src/lib/onboard/created-sandbox-failure.ts, mirroring the injected- deps shape of created-sandbox-finalization.ts (#6332): reportSandboxCreateFailure warns-and-continues on an incomplete create but prints diagnostics + recovery hints and exits otherwise; reportSandboxReadinessFailure prints the readiness failure, defers cleanup to the Docker-GPU patch or deletes the failed sandbox, then exits. Behavior is unchanged (verified byte-for-byte); onboard.ts ends net-smaller, satisfying the codebase-growth guard without a waiver. First cohesive increment toward the create-orchestration extraction; the remaining prebuild/create/finalize wiring stays for follow-ups. Refs #6258 Signed-off-by: Dongni Yang --- src/lib/onboard.ts | 86 ++++----- .../onboard/created-sandbox-failure.test.ts | 180 ++++++++++++++++++ src/lib/onboard/created-sandbox-failure.ts | 115 +++++++++++ 3 files changed, 337 insertions(+), 44 deletions(-) create mode 100644 src/lib/onboard/created-sandbox-failure.test.ts create mode 100644 src/lib/onboard/created-sandbox-failure.ts diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 6f7da9ffc6..743361fc0b 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -116,6 +116,10 @@ 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 providerKeyBridge: typeof import("./onboard/provider-key-bridge") = require("./onboard/provider-key-bridge"); const { isLinuxDockerDriverGatewayEnabled, @@ -2867,30 +2871,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(); @@ -2911,26 +2909,26 @@ 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, + 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..7a98a3690c --- /dev/null +++ b/src/lib/onboard/created-sandbox-failure.test.ts @@ -0,0 +1,180 @@ +// 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("echoes create output only when present", () => { + // With output: leading blank + headline + blank + output echo + "Try:" hint = 5 error() calls. + const withOutput = createFailureDeps(); + expect(() => + reportSandboxCreateFailure(createFailureOptions({ createOutput: "detail" }), withOutput), + ).toThrow(ExitSignal); + expect(withOutput.error).toHaveBeenCalledWith("detail"); + 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("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, + 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); + }); +}); diff --git a/src/lib/onboard/created-sandbox-failure.ts b/src/lib/onboard/created-sandbox-failure.ts new file mode 100644 index 0000000000..3fe25e676e --- /dev/null +++ b/src/lib/onboard/created-sandbox-failure.ts @@ -0,0 +1,115 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +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 failure = deps.classifyCreateFailure(options.createOutput); + 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(options.createOutput); + } + deps.printCreateFailureDiagnostics(options.sandboxName, { + backupPath: options.restoreBackupPath, + }); + deps.error(" Try: openshell sandbox list # check gateway state"); + deps.printRecoveryHints(options.createOutput, { createArgs: options.createArgs }); + return deps.exitProcess(options.createStatus || 1); +} + +export type SandboxReadinessFailureReportOptions = { + sandboxName: string; + readiness: CreatedSandboxReadinessResult; + 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`); + return deps.exitProcess(1); +} From d08316fe84b1e58fb32bde672e744e5c9d944195 Mon Sep 17 00:00:00 2001 From: Dongni Yang Date: Wed, 8 Jul 2026 12:53:15 +0800 Subject: [PATCH 02/23] refactor(onboard): extract the sandbox create step into a module Move the BuildKit prebuild handoff, Docker-GPU create-patch provisioning, and create-stream out of the src/lib/onboard.ts entrypoint into src/lib/onboard/sandbox-create-step.ts (runSandboxCreateStep), behind a context + injected-deps boundary. Behavior is unchanged (pure move, verified byte-for-byte). The move is line-neutral on onboard.ts, but it gives the create step a name and makes the prepare->patch->stream orchestration unit-testable without standing up the 4,900-line entrypoint. Build-context and exit-listener cleanup stay with the caller that armed them. Second cohesive increment for #6258. Refs #6258 Signed-off-by: Dongni Yang --- src/lib/onboard.ts | 70 ++++---- src/lib/onboard/sandbox-create-step.test.ts | 173 ++++++++++++++++++++ src/lib/onboard/sandbox-create-step.ts | 112 +++++++++++++ 3 files changed, 320 insertions(+), 35 deletions(-) create mode 100644 src/lib/onboard/sandbox-create-step.test.ts create mode 100644 src/lib/onboard/sandbox-create-step.ts diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 743361fc0b..3a1ac7a027 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -120,6 +120,9 @@ 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, @@ -2817,41 +2820,38 @@ 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, + 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); } 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..8b881f104e --- /dev/null +++ b/src/lib/onboard/sandbox-create-step.test.ts @@ -0,0 +1,173 @@ +// 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 { + runSandboxCreateStep, + type SandboxCreateStepContext, + type SandboxCreateStepDeps, +} from "./sandbox-create-step"; + +function makeLaunch(overrides: Record = {}) { + return { + createCommand: "openshell sandbox create alpha", + effectiveDashboardPort: "18789", + 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("readyCheck detaches on Ready and otherwise applies the GPU patch during create", 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][2] as { readyCheck: () => boolean }; + + // Ready → true, no patch application. + expect(streamOpts.readyCheck()).toBe(true); + expect(patch.maybeApplyDuringCreate).not.toHaveBeenCalled(); + + // Not ready → apply patch during create, return false. + (deps.isSandboxReady as unknown as ReturnType).mockReturnValue(false); + expect(streamOpts.readyCheck()).toBe(false); + expect(patch.maybeApplyDuringCreate).toHaveBeenCalledTimes(1); + }); + + it("gates the early-ready escape hatch for terminal agents only", 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][2], + ).toMatchObject({ readyCheckOutputPatterns: [] }); + + const nonTerminalDeps = makeDeps(makeLaunch(), makePatch(), { status: 0, output: "" }); + await runSandboxCreateStep(makeContext(), nonTerminalDeps); + expect( + (nonTerminalDeps.streamCreate as unknown as { mock: { calls: unknown[][] } }).mock + .calls[0][2], + ).toMatchObject({ readyCheckOutputPatterns: undefined }); + }); +}); diff --git a/src/lib/onboard/sandbox-create-step.ts b/src/lib/onboard/sandbox-create-step.ts new file mode 100644 index 0000000000..6d74662a83 --- /dev/null +++ b/src/lib/onboard/sandbox-create-step.ts @@ -0,0 +1,112 @@ +// 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 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"]; + 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, 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, + 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 createResult = await deps.streamCreate(createCommand, sandboxEnv, { + readyCheck: () => { + const list = deps.runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); + if (deps.isSandboxReady(list, context.sandboxName)) return true; + dockerGpuCreatePatch.maybeApplyDuringCreate(); + return false; + }, + readyCheckOutputPatterns: deps.isTerminalAgent(context.agent) ? [] : undefined, + failureCheck: dockerGpuCreatePatch.createFailureMessage, + traceEvent: deps.addTraceEvent, + }); + return { createResult, prebuild, effectiveDashboardPort, dockerGpuCreatePatch }; +} From e27855184db157b82b60f8975e140ac7a007ae7c Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 21:17:11 -0400 Subject: [PATCH 03/23] fix(onboard): preserve sandbox readiness failure status Signed-off-by: Julie Yaunches --- src/lib/onboard.ts | 1 + .../onboard/created-sandbox-failure.test.ts | 23 ++++++++++++++++--- src/lib/onboard/created-sandbox-failure.ts | 10 +++++--- src/lib/onboard/sandbox-create-step.ts | 2 ++ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 3d11df9e51..9036697a9f 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2909,6 +2909,7 @@ async function createSandboxWithBaseImageResolution( { sandboxName, readiness, + createStatus: createResult.status, timeoutSecs: sandboxReadyTimeoutSecs, restoreBackupPath, useDockerGpuPatch, diff --git a/src/lib/onboard/created-sandbox-failure.test.ts b/src/lib/onboard/created-sandbox-failure.test.ts index 7a98a3690c..ab377a39e9 100644 --- a/src/lib/onboard/created-sandbox-failure.test.ts +++ b/src/lib/onboard/created-sandbox-failure.test.ts @@ -80,13 +80,21 @@ describe("reportSandboxCreateFailure", () => { expect(deps.warn).not.toHaveBeenCalled(); }); - it("echoes create output only when present", () => { + it("echoes redacted create output only when present", () => { // With output: leading blank + headline + blank + output echo + "Try:" hint = 5 error() calls. const withOutput = createFailureDeps(); expect(() => - reportSandboxCreateFailure(createFailureOptions({ createOutput: "detail" }), withOutput), + reportSandboxCreateFailure( + createFailureOptions({ createOutput: "failed with Authorization: Bearer secret-token" }), + withOutput, + ), ).toThrow(ExitSignal); - expect(withOutput.error).toHaveBeenCalledWith("detail"); + expect(withOutput.error).toHaveBeenCalledWith( + "failed with Authorization: Bearer secr********", + ); + expect(withOutput.error).not.toHaveBeenCalledWith( + "failed with Authorization: Bearer secret-token", + ); expect(withOutput.error).toHaveBeenCalledTimes(5); // Without output: the echo block is skipped, so only 3 error() calls remain. @@ -137,6 +145,7 @@ function readinessOptions( return { sandboxName: "alpha", readiness: NOT_READY, + createStatus: 0, timeoutSecs: 300, restoreBackupPath: null, useDockerGpuPatch: false, @@ -177,4 +186,12 @@ describe("reportSandboxReadinessFailure", () => { 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 index 3fe25e676e..95541620ba 100644 --- a/src/lib/onboard/created-sandbox-failure.ts +++ b/src/lib/onboard/created-sandbox-failure.ts @@ -1,6 +1,7 @@ // 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 = { @@ -50,19 +51,21 @@ export function reportSandboxCreateFailure( deps.error(` Sandbox creation failed (exit ${options.createStatus}).`); if (options.createOutput) { deps.error(""); - deps.error(options.createOutput); + deps.error(redact(options.createOutput)); } deps.printCreateFailureDiagnostics(options.sandboxName, { backupPath: options.restoreBackupPath, }); deps.error(" Try: openshell sandbox list # check gateway state"); deps.printRecoveryHints(options.createOutput, { createArgs: options.createArgs }); - return deps.exitProcess(options.createStatus || 1); + 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. */ @@ -111,5 +114,6 @@ export function reportSandboxReadinessFailure( } } deps.error(` Retry: ${deps.cliName()} onboard`); - return deps.exitProcess(1); + const exitCode = options.createStatus === 0 ? 1 : options.createStatus; + return deps.exitProcess(exitCode); } diff --git a/src/lib/onboard/sandbox-create-step.ts b/src/lib/onboard/sandbox-create-step.ts index 6d74662a83..926dbcc421 100644 --- a/src/lib/onboard/sandbox-create-step.ts +++ b/src/lib/onboard/sandbox-create-step.ts @@ -104,6 +104,8 @@ export async function runSandboxCreateStep( dockerGpuCreatePatch.maybeApplyDuringCreate(); return false; }, + // Terminal agents do not emit the normal VM startup marker; [] keeps the + // output-pattern gate open while undefined preserves driver defaults. readyCheckOutputPatterns: deps.isTerminalAgent(context.agent) ? [] : undefined, failureCheck: dockerGpuCreatePatch.createFailureMessage, traceEvent: deps.addTraceEvent, From 1287ab29c1cbb77f2450d40f0820f3a27ac49bf6 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 21:31:27 -0400 Subject: [PATCH 04/23] style(onboard): format sandbox failure test Signed-off-by: Julie Yaunches --- src/lib/onboard/created-sandbox-failure.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/lib/onboard/created-sandbox-failure.test.ts b/src/lib/onboard/created-sandbox-failure.test.ts index ab377a39e9..a0f2446cdf 100644 --- a/src/lib/onboard/created-sandbox-failure.test.ts +++ b/src/lib/onboard/created-sandbox-failure.test.ts @@ -89,9 +89,7 @@ describe("reportSandboxCreateFailure", () => { withOutput, ), ).toThrow(ExitSignal); - expect(withOutput.error).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", ); From b7e27f4f154e03b840aad6e7eac277603ea27eea Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 21:50:00 -0400 Subject: [PATCH 05/23] test(onboard): cover terminal ready detach gate Signed-off-by: Julie Yaunches --- src/lib/sandbox/create-stream.test.ts | 29 +++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/lib/sandbox/create-stream.test.ts b/src/lib/sandbox/create-stream.test.ts index 6b00795212..8f738a09d0 100644 --- a/src/lib/sandbox/create-stream.test.ts +++ b/src/lib/sandbox/create-stream.test.ts @@ -223,6 +223,35 @@ describe("sandbox-create-stream", () => { expect(child.kill).toHaveBeenCalledWith("SIGTERM"); }); + it("lets an explicit empty startup-output gate detach immediately on VM", async () => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const logLine = vi.fn(); + const promise = streamSandboxCreate("echo create", vmEnv, { + spawnImpl: () => child, + readyCheck: () => true, + readyCheckOutputPatterns: [], + pollIntervalMs: 5, + heartbeatIntervalMs: 1_000, + silentPhaseMs: 10_000, + logLine, + }); + + child.stdout.emit("data", Buffer.from("Created sandbox: demo\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"); + expect(logLine).not.toHaveBeenCalledWith( + " Sandbox reported Ready; waiting for startup command output before detaching.", + ); + }); + it("does not recover a non-zero close before required startup output appears", async () => { const child = new FakeChild(); const promise = streamSandboxCreate("echo create", vmEnv, { From a10b2fcfae0047eb3d61f68832bf0bb285a28cfc Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 22:07:34 -0400 Subject: [PATCH 06/23] fix(onboard): isolate create poll side effects Signed-off-by: Julie Yaunches --- src/lib/onboard/sandbox-create-step.test.ts | 9 +++--- src/lib/onboard/sandbox-create-step.ts | 5 ++- src/lib/sandbox/create-stream.test.ts | 35 +++++++++++++++++++-- src/lib/sandbox/create-stream.ts | 7 +++++ 4 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/lib/onboard/sandbox-create-step.test.ts b/src/lib/onboard/sandbox-create-step.test.ts index 8b881f104e..b83058f737 100644 --- a/src/lib/onboard/sandbox-create-step.test.ts +++ b/src/lib/onboard/sandbox-create-step.test.ts @@ -125,7 +125,7 @@ describe("runSandboxCreateStep", () => { }); }); - it("readyCheck detaches on Ready and otherwise applies the GPU patch during create", async () => { + it("separates readiness detection from GPU patch polling", async () => { const launch = makeLaunch(); const patch = makePatch(); const deps = makeDeps( @@ -137,15 +137,16 @@ describe("runSandboxCreateStep", () => { await runSandboxCreateStep(makeContext(), deps); const streamOpts = (deps.streamCreate as unknown as { mock: { calls: unknown[][] } }).mock - .calls[0][2] as { readyCheck: () => boolean }; + .calls[0][2] as { readyCheck: () => boolean; onPoll: () => void }; - // Ready → true, no patch application. expect(streamOpts.readyCheck()).toBe(true); expect(patch.maybeApplyDuringCreate).not.toHaveBeenCalled(); - // Not ready → apply patch during create, return false. (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); }); diff --git a/src/lib/onboard/sandbox-create-step.ts b/src/lib/onboard/sandbox-create-step.ts index 926dbcc421..bf350122f0 100644 --- a/src/lib/onboard/sandbox-create-step.ts +++ b/src/lib/onboard/sandbox-create-step.ts @@ -100,10 +100,9 @@ export async function runSandboxCreateStep( const createResult = await deps.streamCreate(createCommand, sandboxEnv, { readyCheck: () => { const list = deps.runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); - if (deps.isSandboxReady(list, context.sandboxName)) return true; - dockerGpuCreatePatch.maybeApplyDuringCreate(); - return false; + return deps.isSandboxReady(list, context.sandboxName); }, + onPoll: () => dockerGpuCreatePatch.maybeApplyDuringCreate(), // Terminal agents do not emit the normal VM startup marker; [] keeps the // output-pattern gate open while undefined preserves driver defaults. readyCheckOutputPatterns: deps.isTerminalAgent(context.agent) ? [] : undefined, diff --git a/src/lib/sandbox/create-stream.test.ts b/src/lib/sandbox/create-stream.test.ts index 8f738a09d0..68196a0a81 100644 --- a/src/lib/sandbox/create-stream.test.ts +++ b/src/lib/sandbox/create-stream.test.ts @@ -223,15 +223,19 @@ describe("sandbox-create-stream", () => { expect(child.kill).toHaveBeenCalledWith("SIGTERM"); }); - it("lets an explicit empty startup-output gate detach immediately on VM", async () => { + 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", vmEnv, { + const promise = streamSandboxCreate("echo create", env, { spawnImpl: () => child, readyCheck: () => true, - readyCheckOutputPatterns: [], + readyCheckOutputPatterns, pollIntervalMs: 5, heartbeatIntervalMs: 1_000, silentPhaseMs: 10_000, @@ -252,6 +256,31 @@ describe("sandbox-create-stream", () => { ); }); + 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, { + spawnImpl: () => child, + readyCheck: () => ready, + onPoll, + pollIntervalMs: 5, + heartbeatIntervalMs: 1_000, + silentPhaseMs: 10_000, + logLine: vi.fn(), + }); + + 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("does not recover a non-zero close before required startup output appears", async () => { const child = new FakeChild(); const promise = streamSandboxCreate("echo create", vmEnv, { diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index 5311a82418..63ab9a57fc 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -14,6 +14,7 @@ export interface StreamSandboxCreateResult { export interface StreamSandboxCreateOptions { readyCheck?: (() => boolean) | null; + onPoll?: (() => void) | null; failureCheck?: (() => string | null | undefined) | null; pollIntervalMs?: number; heartbeatIntervalMs?: number; @@ -418,6 +419,12 @@ export function streamSandboxCreate( return; } + try { + options.onPoll?.(); + } catch { + return; + } + const failure = options.failureCheck?.(); if (!failure) return; const detail = String(failure); From 9c4edb3fe37d175272705b1aaab543d7af032130 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 22:20:16 -0400 Subject: [PATCH 07/23] test(sandbox): cover create stream poll errors Signed-off-by: Julie Yaunches --- .../sandbox/create-stream-test-fixtures.ts | 22 ++++++++ src/lib/sandbox/create-stream.test.ts | 55 +++++++++++-------- src/lib/sandbox/create-stream.ts | 5 +- 3 files changed, 59 insertions(+), 23 deletions(-) create mode 100644 src/lib/sandbox/create-stream-test-fixtures.ts 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..0b08140e1f --- /dev/null +++ b/src/lib/sandbox/create-stream-test-fixtures.ts @@ -0,0 +1,22 @@ +// 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 { 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" }; diff --git a/src/lib/sandbox/create-stream.test.ts b/src/lib/sandbox/create-stream.test.ts index 68196a0a81..d7ebf2dbf8 100644 --- a/src/lib/sandbox/create-stream.test.ts +++ b/src/lib/sandbox/create-stream.test.ts @@ -1,29 +1,10 @@ // 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 { - 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" }; +import { streamSandboxCreate } from "./create-stream"; +import { dockerEnv, FakeChild, vmEnv } from "./create-stream-test-fixtures"; describe("sandbox-create-stream", () => { afterEach(() => { @@ -185,7 +166,7 @@ describe("sandbox-create-stream", () => { expect(child.unref).toHaveBeenCalled(); }); - it("does not detach on Ready until required startup output appears", async () => { + it("does not detach on Ready with default VM gate until required startup output appears", async () => { vi.useFakeTimers(); const child = new FakeChild(); @@ -281,6 +262,36 @@ describe("sandbox-create-stream", () => { expect(onPoll).toHaveBeenCalledTimes(1); }); + it("traces poll side-effect errors and keeps polling", async () => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const traceEvent = vi.fn(); + const onPoll = vi.fn(() => { + throw new Error("patch unavailable"); + }); + let ready = false; + const promise = streamSandboxCreate("echo create", dockerEnv, { + spawnImpl: () => child, + readyCheck: () => ready, + onPoll, + traceEvent, + pollIntervalMs: 5, + heartbeatIntervalMs: 1_000, + silentPhaseMs: 10_000, + logLine: vi.fn(), + }); + + await vi.advanceTimersByTimeAsync(6); + expect(traceEvent).toHaveBeenCalledWith("sandbox_create_poll_error", { + message: "patch unavailable", + }); + + ready = true; + await vi.advanceTimersByTimeAsync(6); + await expect(promise).resolves.toMatchObject({ status: 0, forcedReady: true }); + }); + it("does not recover a non-zero close before required startup output appears", async () => { const child = new FakeChild(); const promise = streamSandboxCreate("echo create", vmEnv, { diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index 63ab9a57fc..55feaf0aae 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -421,7 +421,10 @@ export function streamSandboxCreate( try { options.onPoll?.(); - } catch { + } catch (error) { + emitTraceEvent("sandbox_create_poll_error", { + message: error instanceof Error ? error.message : String(error), + }); return; } From 0bb30ac2e4e4453b8f251cb1382985a58a390b3b Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 22:41:25 -0400 Subject: [PATCH 08/23] test(onboard): cover driver-aware create detach Signed-off-by: Julie Yaunches --- src/lib/onboard/sandbox-create-step.test.ts | 60 ++++++- src/lib/onboard/sandbox-create-step.ts | 5 +- .../sandbox/create-stream-ready-gate.test.ts | 109 +++++++++++++ .../sandbox/create-stream-test-fixtures.ts | 33 +++- src/lib/sandbox/create-stream.test.ts | 151 ++---------------- src/lib/sandbox/create-stream.ts | 14 +- 6 files changed, 232 insertions(+), 140 deletions(-) create mode 100644 src/lib/sandbox/create-stream-ready-gate.test.ts diff --git a/src/lib/onboard/sandbox-create-step.test.ts b/src/lib/onboard/sandbox-create-step.test.ts index b83058f737..7fc45cd731 100644 --- a/src/lib/onboard/sandbox-create-step.test.ts +++ b/src/lib/onboard/sandbox-create-step.test.ts @@ -3,6 +3,13 @@ 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, @@ -150,7 +157,7 @@ describe("runSandboxCreateStep", () => { expect(patch.maybeApplyDuringCreate).toHaveBeenCalledTimes(1); }); - it("gates the early-ready escape hatch for terminal agents only", async () => { + it("threads the terminal-agent early-ready gate into stream options", async () => { const terminalDeps = makeDeps( makeLaunch(), makePatch(), @@ -171,4 +178,55 @@ describe("runSandboxCreateStep", () => { .calls[0][2], ).toMatchObject({ readyCheckOutputPatterns: undefined }); }); + + it.each([ + ["terminal VM", true, vmEnv, false], + ["terminal Docker", true, dockerEnv, false], + ["non-terminal VM", false, vmEnv, true], + ["non-terminal Docker", false, dockerEnv, false], + ])("applies driver-aware detach behavior for %s", async (_label, isTerminalAgent, env, shouldWaitForStartupOutput) => { + 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, sandboxEnv, options) => + streamSandboxCreate(command, 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); + + const waitMessage = + " Sandbox reported Ready; waiting for startup command output before detaching."; + if (shouldWaitForStartupOutput) { + expect(child.kill).not.toHaveBeenCalled(); + expect(logLine).toHaveBeenCalledWith(waitMessage); + child.stderr.emit("data", Buffer.from("Setting up NemoClaw (Hermes)...\n")); + await vi.advanceTimersByTimeAsync(6); + } else { + expect(logLine).not.toHaveBeenCalledWith(waitMessage); + } + + await expect(promise).resolves.toMatchObject({ + createResult: expect.objectContaining({ status: 0, forcedReady: true }), + }); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + vi.useRealTimers(); + }); }); diff --git a/src/lib/onboard/sandbox-create-step.ts b/src/lib/onboard/sandbox-create-step.ts index bf350122f0..701eb0793b 100644 --- a/src/lib/onboard/sandbox-create-step.ts +++ b/src/lib/onboard/sandbox-create-step.ts @@ -103,8 +103,9 @@ export async function runSandboxCreateStep( return deps.isSandboxReady(list, context.sandboxName); }, onPoll: () => dockerGpuCreatePatch.maybeApplyDuringCreate(), - // Terminal agents do not emit the normal VM startup marker; [] keeps the - // output-pattern gate open while undefined preserves driver defaults. + // Terminal agents use [] to detach as soon as Ready is observed. Non-terminal + // agents pass undefined so streamSandboxCreate applies its driver defaults: + // VM waits for the "Setting up NemoClaw" startup marker, Docker detaches on Ready. readyCheckOutputPatterns: deps.isTerminalAgent(context.agent) ? [] : undefined, failureCheck: dockerGpuCreatePatch.createFailureMessage, traceEvent: deps.addTraceEvent, 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..c58d65527a --- /dev/null +++ b/src/lib/sandbox/create-stream-ready-gate.test.ts @@ -0,0 +1,109 @@ +// 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"; + +describe("sandbox-create-stream ready gate", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it.each([ + ["explicit empty gate on VM", vmEnv, [], false], + ["explicit empty gate on Docker", dockerEnv, [], false], + ["default Docker gate", dockerEnv, undefined, false], + ["default VM gate", vmEnv, undefined, true], + ])( + "handles %s", + async (_label, env, readyCheckOutputPatterns, shouldWaitForStartupOutput) => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const logLine = vi.fn(); + let resolved = false; + const promise = streamSandboxCreate( + "echo create", + env, + makePollingOptions(child, { + readyCheck: () => true, + readyCheckOutputPatterns, + logLine, + }), + ).then((result) => { + resolved = true; + return result; + }); + + child.stdout.emit("data", Buffer.from("Created sandbox: demo\n")); + await vi.advanceTimersByTimeAsync(6); + + const waitMessage = + " Sandbox reported Ready; waiting for startup command output before detaching."; + if (shouldWaitForStartupOutput) { + expect(resolved).toBe(false); + expect(child.kill).not.toHaveBeenCalled(); + expect(logLine).toHaveBeenCalledWith(waitMessage); + child.stderr.emit("data", Buffer.from("Setting up NemoClaw (Hermes)...\n")); + await vi.advanceTimersByTimeAsync(6); + } else { + expect(logLine).not.toHaveBeenCalledWith(waitMessage); + } + + 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("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 }), + ); + + 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("traces redacted poll side-effect errors and keeps polling", async () => { + vi.useFakeTimers(); + + const child = new FakeChild(); + const traceEvent = vi.fn(); + const onPoll = vi.fn(() => { + throw new Error("Authorization: Bearer secret-token"); + }); + let ready = false; + const promise = streamSandboxCreate( + "echo create", + dockerEnv, + makePollingOptions(child, { readyCheck: () => ready, onPoll, traceEvent }), + ); + + await vi.advanceTimersByTimeAsync(6); + expect(traceEvent).toHaveBeenCalledWith("sandbox_create_poll_error", { + message: "Authorization: Bearer secr********", + }); + + ready = true; + await vi.advanceTimersByTimeAsync(6); + await expect(promise).resolves.toMatchObject({ status: 0, forcedReady: true }); + }); +}); diff --git a/src/lib/sandbox/create-stream-test-fixtures.ts b/src/lib/sandbox/create-stream-test-fixtures.ts index 0b08140e1f..ab37715901 100644 --- a/src/lib/sandbox/create-stream-test-fixtures.ts +++ b/src/lib/sandbox/create-stream-test-fixtures.ts @@ -5,7 +5,11 @@ import { EventEmitter } from "node:events"; import { vi } from "vitest"; -import type { StreamableChildProcess, StreamableReadable } from "./create-stream"; +import type { + StreamSandboxCreateOptions, + StreamableChildProcess, + StreamableReadable, +} from "./create-stream"; export class FakeReadable extends EventEmitter implements StreamableReadable { destroy(): void {} @@ -20,3 +24,30 @@ export class FakeChild extends EventEmitter implements StreamableChildProcess { 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 d7ebf2dbf8..f78c18da6a 100644 --- a/src/lib/sandbox/create-stream.test.ts +++ b/src/lib/sandbox/create-stream.test.ts @@ -4,7 +4,12 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { streamSandboxCreate } from "./create-stream"; -import { dockerEnv, FakeChild, vmEnv } from "./create-stream-test-fixtures"; +import { + dockerEnv, + FakeChild, + makeDefaultStreamOptions, + vmEnv, +} from "./create-stream-test-fixtures"; describe("sandbox-create-stream", () => { afterEach(() => { @@ -166,132 +171,6 @@ describe("sandbox-create-stream", () => { expect(child.unref).toHaveBeenCalled(); }); - it("does not detach on Ready with default VM gate until required startup output appears", async () => { - vi.useFakeTimers(); - - const child = new FakeChild(); - const logLine = vi.fn(); - let resolved = false; - const promise = streamSandboxCreate("echo create", vmEnv, { - spawnImpl: () => child, - readyCheck: () => true, - pollIntervalMs: 5, - heartbeatIntervalMs: 1_000, - silentPhaseMs: 10_000, - logLine, - }).then((result) => { - resolved = true; - return result; - }); - - child.stdout.emit("data", Buffer.from("Created sandbox: demo\n")); - await vi.advanceTimersByTimeAsync(12); - - 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("Setting up NemoClaw (Hermes)..."), - }); - expect(child.kill).toHaveBeenCalledWith("SIGTERM"); - }); - - 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, { - spawnImpl: () => child, - readyCheck: () => true, - readyCheckOutputPatterns, - pollIntervalMs: 5, - heartbeatIntervalMs: 1_000, - silentPhaseMs: 10_000, - logLine, - }); - - child.stdout.emit("data", Buffer.from("Created sandbox: demo\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"); - expect(logLine).not.toHaveBeenCalledWith( - " Sandbox reported Ready; waiting for startup command output before detaching.", - ); - }); - - 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, { - spawnImpl: () => child, - readyCheck: () => ready, - onPoll, - pollIntervalMs: 5, - heartbeatIntervalMs: 1_000, - silentPhaseMs: 10_000, - logLine: vi.fn(), - }); - - 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("traces poll side-effect errors and keeps polling", async () => { - vi.useFakeTimers(); - - const child = new FakeChild(); - const traceEvent = vi.fn(); - const onPoll = vi.fn(() => { - throw new Error("patch unavailable"); - }); - let ready = false; - const promise = streamSandboxCreate("echo create", dockerEnv, { - spawnImpl: () => child, - readyCheck: () => ready, - onPoll, - traceEvent, - pollIntervalMs: 5, - heartbeatIntervalMs: 1_000, - silentPhaseMs: 10_000, - logLine: vi.fn(), - }); - - await vi.advanceTimersByTimeAsync(6); - expect(traceEvent).toHaveBeenCalledWith("sandbox_create_poll_error", { - message: "patch unavailable", - }); - - ready = true; - await vi.advanceTimersByTimeAsync(6); - await expect(promise).resolves.toMatchObject({ status: 0, forcedReady: true }); - }); - it("does not recover a non-zero close before required startup output appears", async () => { const child = new FakeChild(); const promise = streamSandboxCreate("echo create", vmEnv, { @@ -342,10 +221,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); @@ -557,10 +437,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 55feaf0aae..e17bc9d805 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -3,6 +3,7 @@ import { type SpawnOptions, spawn } from "node:child_process"; +import { redact } from "../security/redact"; import { ROOT } from "../state/paths"; export interface StreamSandboxCreateResult { @@ -129,6 +130,10 @@ export function streamSandboxCreate( env: NodeJS.ProcessEnv = process.env, options: StreamSandboxCreateOptions = {}, ): Promise { + // Trust boundary: command is assembled by internal sandbox/onboard callers from + // trusted OpenShell CLI fragments. Do not pass user-supplied free-form text to + // this shell boundary; switch the API to direct spawn args if external input + // ever needs to cross it. const child: StreamableChildProcess = (options.spawnImpl ?? spawn)("bash", ["-lc", command], { cwd: ROOT, env, @@ -422,8 +427,15 @@ export function streamSandboxCreate( try { options.onPoll?.(); } catch (error) { + // Localized containment: onPoll bridges optional host-side create + // patching into the stream poll loop. Patch failures are observed + // here but the source of truth remains the dedicated failureCheck; + // keep polling so sandbox readiness/failure can settle deterministically. + // Regression coverage: create-stream.test.ts verifies redacted trace + // emission and continued polling. Remove when onPoll becomes a typed + // result-returning dependency instead of an opportunistic side effect. emitTraceEvent("sandbox_create_poll_error", { - message: error instanceof Error ? error.message : String(error), + message: redact(error instanceof Error ? error.message : String(error)), }); return; } From f3395d2ab3c12bc51094f59bea04cd766a51963b Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 22:55:16 -0400 Subject: [PATCH 09/23] test(onboard): keep ready gate tests linear Signed-off-by: Julie Yaunches --- src/lib/onboard/sandbox-create-step.test.ts | 63 +++++++--- .../sandbox/create-stream-ready-gate.test.ts | 114 ++++++++++-------- 2 files changed, 114 insertions(+), 63 deletions(-) diff --git a/src/lib/onboard/sandbox-create-step.test.ts b/src/lib/onboard/sandbox-create-step.test.ts index 7fc45cd731..d72385d156 100644 --- a/src/lib/onboard/sandbox-create-step.test.ts +++ b/src/lib/onboard/sandbox-create-step.test.ts @@ -180,11 +180,10 @@ describe("runSandboxCreateStep", () => { }); it.each([ - ["terminal VM", true, vmEnv, false], - ["terminal Docker", true, dockerEnv, false], - ["non-terminal VM", false, vmEnv, true], - ["non-terminal Docker", false, dockerEnv, false], - ])("applies driver-aware detach behavior for %s", async (_label, isTerminalAgent, env, shouldWaitForStartupOutput) => { + ["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(); @@ -212,16 +211,50 @@ describe("runSandboxCreateStep", () => { ready = true; await vi.advanceTimersByTimeAsync(6); - const waitMessage = - " Sandbox reported Ready; waiting for startup command output before detaching."; - if (shouldWaitForStartupOutput) { - expect(child.kill).not.toHaveBeenCalled(); - expect(logLine).toHaveBeenCalledWith(waitMessage); - child.stderr.emit("data", Buffer.from("Setting up NemoClaw (Hermes)...\n")); - await vi.advanceTimersByTimeAsync(6); - } else { - expect(logLine).not.toHaveBeenCalledWith(waitMessage); - } + 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, sandboxEnv, options) => + streamSandboxCreate(command, 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 }), diff --git a/src/lib/sandbox/create-stream-ready-gate.test.ts b/src/lib/sandbox/create-stream-ready-gate.test.ts index c58d65527a..f61a3d786b 100644 --- a/src/lib/sandbox/create-stream-ready-gate.test.ts +++ b/src/lib/sandbox/create-stream-ready-gate.test.ts @@ -12,54 +12,72 @@ describe("sandbox-create-stream ready gate", () => { }); it.each([ - ["explicit empty gate on VM", vmEnv, [], false], - ["explicit empty gate on Docker", dockerEnv, [], false], - ["default Docker gate", dockerEnv, undefined, false], - ["default VM gate", vmEnv, undefined, true], - ])( - "handles %s", - async (_label, env, readyCheckOutputPatterns, shouldWaitForStartupOutput) => { - vi.useFakeTimers(); - - const child = new FakeChild(); - const logLine = vi.fn(); - let resolved = false; - const promise = streamSandboxCreate( - "echo create", - env, - makePollingOptions(child, { - readyCheck: () => true, - readyCheckOutputPatterns, - logLine, - }), - ).then((result) => { - resolved = true; - return result; - }); - - child.stdout.emit("data", Buffer.from("Created sandbox: demo\n")); - await vi.advanceTimersByTimeAsync(6); - - const waitMessage = - " Sandbox reported Ready; waiting for startup command output before detaching."; - if (shouldWaitForStartupOutput) { - expect(resolved).toBe(false); - expect(child.kill).not.toHaveBeenCalled(); - expect(logLine).toHaveBeenCalledWith(waitMessage); - child.stderr.emit("data", Buffer.from("Setting up NemoClaw (Hermes)...\n")); - await vi.advanceTimersByTimeAsync(6); - } else { - expect(logLine).not.toHaveBeenCalledWith(waitMessage); - } - - await expect(promise).resolves.toMatchObject({ - status: 0, - forcedReady: true, - output: expect.stringContaining("Sandbox reported Ready before create stream exited"), - }); - expect(child.kill).toHaveBeenCalledWith("SIGTERM"); - }, - ); + ["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("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("runs poll side effects only after a not-ready poll", async () => { vi.useFakeTimers(); From 24ecc5cdf851505bdcae1988fdfde637c099db87 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 23:29:44 -0400 Subject: [PATCH 10/23] fix(onboard): preserve create failure checks Signed-off-by: Julie Yaunches --- src/lib/actions/sandbox/snapshot.ts | 5 +- src/lib/onboard.ts | 1 + .../onboard/created-sandbox-failure.test.ts | 27 +++++++- src/lib/onboard/created-sandbox-failure.ts | 5 +- src/lib/onboard/sandbox-create-launch.ts | 16 +++-- src/lib/onboard/sandbox-create-step.test.ts | 48 ++++++++++--- src/lib/onboard/sandbox-create-step.ts | 68 +++++++++++-------- src/lib/sandbox/create-stream-argv.test.ts | 31 +++++++++ .../sandbox/create-stream-ready-gate.test.ts | 34 ++++++++++ src/lib/sandbox/create-stream-ready-gate.ts | 29 ++++++++ src/lib/sandbox/create-stream.ts | 58 +++++++--------- 11 files changed, 243 insertions(+), 79 deletions(-) create mode 100644 src/lib/sandbox/create-stream-argv.test.ts create mode 100644 src/lib/sandbox/create-stream-ready-gate.ts diff --git a/src/lib/actions/sandbox/snapshot.ts b/src/lib/actions/sandbox/snapshot.ts index ba525d55e3..aa0662c343 100644 --- a/src/lib/actions/sandbox/snapshot.ts +++ b/src/lib/actions/sandbox/snapshot.ts @@ -254,11 +254,12 @@ async function autoCreateSandboxFromSource( "--", ...startupCommand, ].map((p) => shellQuote(p)); - const command = `${cmdParts.join(" ")} 2>&1`; + const command = "bash"; + const commandArgs = ["-lc", `${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 9036697a9f..58e940c61a 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2830,6 +2830,7 @@ async function createSandboxWithBaseImageResolution( hermesDashboardState, manageDashboard, openshellShellCommand, + openshellArgv, prebuild: { buildCtx, buildId, dockerDriverGateway, origin }, useDockerGpuPatch, gpuDevice: effectiveSandboxGpuConfig.sandboxGpuDevice, diff --git a/src/lib/onboard/created-sandbox-failure.test.ts b/src/lib/onboard/created-sandbox-failure.test.ts index a0f2446cdf..71bec38dd1 100644 --- a/src/lib/onboard/created-sandbox-failure.test.ts +++ b/src/lib/onboard/created-sandbox-failure.test.ts @@ -80,7 +80,7 @@ describe("reportSandboxCreateFailure", () => { expect(deps.warn).not.toHaveBeenCalled(); }); - it("echoes redacted create output only when present", () => { + 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(() => @@ -89,6 +89,9 @@ describe("reportSandboxCreateFailure", () => { 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", @@ -105,6 +108,28 @@ describe("reportSandboxCreateFailure", () => { 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"); + }); + it("falls back to exit code 1 when the create status is zero", () => { const deps = createFailureDeps(); expect(() => diff --git a/src/lib/onboard/created-sandbox-failure.ts b/src/lib/onboard/created-sandbox-failure.ts index 95541620ba..03f9df9136 100644 --- a/src/lib/onboard/created-sandbox-failure.ts +++ b/src/lib/onboard/created-sandbox-failure.ts @@ -35,7 +35,8 @@ export function reportSandboxCreateFailure( options: SandboxCreateFailureReportOptions, deps: SandboxCreateFailureReportDeps, ): void { - const failure = deps.classifyCreateFailure(options.createOutput); + 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 @@ -51,7 +52,7 @@ export function reportSandboxCreateFailure( deps.error(` Sandbox creation failed (exit ${options.createStatus}).`); if (options.createOutput) { deps.error(""); - deps.error(redact(options.createOutput)); + deps.error(redactedCreateOutput); } deps.printCreateFailureDiagnostics(options.sandboxName, { backupPath: options.restoreBackupPath, diff --git a/src/lib/onboard/sandbox-create-launch.ts b/src/lib/onboard/sandbox-create-launch.ts index 7b904c24c9..08303b06c6 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 createArgv = input.openshellArgv + ? input.openshellArgv(openshellArgs) + : [input.openshellShellCommand(openshellArgs)]; + const createCommand = `${input.openshellShellCommand(openshellArgs)} 2>&1`; 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 index d72385d156..a95ea8d6e4 100644 --- a/src/lib/onboard/sandbox-create-step.test.ts +++ b/src/lib/onboard/sandbox-create-step.test.ts @@ -20,6 +20,7 @@ function makeLaunch(overrides: Record = {}) { return { createCommand: "openshell sandbox create alpha", effectiveDashboardPort: "18789", + createArgv: ["openshell", "sandbox", "create", "alpha"], envArgs: [], sandboxEnv: { FOO: "bar" }, sandboxStartupCommand: ["run", "alpha"], @@ -119,7 +120,8 @@ describe("runSandboxCreateStep", () => { ); // stream is fed the launch command + env. expect(deps.streamCreate).toHaveBeenCalledWith( - "openshell sandbox create alpha", + "openshell", + ["sandbox", "create", "alpha"], { FOO: "bar" }, expect.objectContaining({ traceEvent: deps.addTraceEvent }), ); @@ -144,7 +146,7 @@ describe("runSandboxCreateStep", () => { await runSandboxCreateStep(makeContext(), deps); const streamOpts = (deps.streamCreate as unknown as { mock: { calls: unknown[][] } }).mock - .calls[0][2] as { readyCheck: () => boolean; onPoll: () => void }; + .calls[0][3] as { readyCheck: () => boolean; onPoll: () => void }; expect(streamOpts.readyCheck()).toBe(true); expect(patch.maybeApplyDuringCreate).not.toHaveBeenCalled(); @@ -168,14 +170,14 @@ describe("runSandboxCreateStep", () => { ); await runSandboxCreateStep(makeContext(), terminalDeps); expect( - (terminalDeps.streamCreate as unknown as { mock: { calls: unknown[][] } }).mock.calls[0][2], + (terminalDeps.streamCreate as unknown as { mock: { calls: unknown[][] } }).mock.calls[0][3], ).toMatchObject({ readyCheckOutputPatterns: [] }); const nonTerminalDeps = makeDeps(makeLaunch(), makePatch(), { status: 0, output: "" }); await runSandboxCreateStep(makeContext(), nonTerminalDeps); expect( (nonTerminalDeps.streamCreate as unknown as { mock: { calls: unknown[][] } }).mock - .calls[0][2], + .calls[0][3], ).toMatchObject({ readyCheckOutputPatterns: undefined }); }); @@ -194,8 +196,8 @@ describe("runSandboxCreateStep", () => { makePatch(), { status: 0, output: "" }, { - streamCreate: ((command, sandboxEnv, options) => - streamSandboxCreate(command, sandboxEnv, { + streamCreate: ((command, args, sandboxEnv, options) => + streamSandboxCreate(command, args, sandboxEnv, { ...options, ...streamOptions, })) as SandboxCreateStepDeps["streamCreate"], @@ -232,8 +234,8 @@ describe("runSandboxCreateStep", () => { makePatch(), { status: 0, output: "" }, { - streamCreate: ((command, sandboxEnv, options) => - streamSandboxCreate(command, sandboxEnv, { + streamCreate: ((command, args, sandboxEnv, options) => + streamSandboxCreate(command, args, sandboxEnv, { ...options, ...streamOptions, })) as SandboxCreateStepDeps["streamCreate"], @@ -262,4 +264,34 @@ describe("runSandboxCreateStep", () => { 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 index 701eb0793b..1dd44e3cc5 100644 --- a/src/lib/onboard/sandbox-create-step.ts +++ b/src/lib/onboard/sandbox-create-step.ts @@ -7,6 +7,7 @@ import type { StreamSandboxCreateResult, streamSandboxCreate, } from "../sandbox/create-stream"; +import { getReadyCheckOutputPatternsForAgent } from "../sandbox/create-stream-ready-gate"; import type { createDockerGpuSandboxCreatePatch, DockerGpuSandboxCreatePatch, @@ -32,6 +33,7 @@ export type SandboxCreateStepContext = { hermesDashboardState: LaunchInput["hermesDashboardState"]; manageDashboard: boolean; openshellShellCommand: LaunchInput["openshellShellCommand"]; + openshellArgv?: LaunchInput["openshellArgv"]; prebuild: LaunchInput["prebuild"]; useDockerGpuPatch: boolean; gpuDevice: string | null | undefined; @@ -69,21 +71,28 @@ export async function runSandboxCreateStep( context: SandboxCreateStepContext, deps: SandboxCreateStepDeps, ): Promise { - const { createCommand, 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, - prebuild: context.prebuild, - }); + 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, @@ -97,18 +106,23 @@ export async function runSandboxCreateStep( sleep: deps.sleepSeconds, }, }); - const createResult = await deps.streamCreate(createCommand, sandboxEnv, { - readyCheck: () => { - const list = deps.runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); - return deps.isSandboxReady(list, context.sandboxName); + 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), + ), + failureCheck: dockerGpuCreatePatch.createFailureMessage, + traceEvent: deps.addTraceEvent, }, - onPoll: () => dockerGpuCreatePatch.maybeApplyDuringCreate(), - // Terminal agents use [] to detach as soon as Ready is observed. Non-terminal - // agents pass undefined so streamSandboxCreate applies its driver defaults: - // VM waits for the "Setting up NemoClaw" startup marker, Docker detaches on Ready. - readyCheckOutputPatterns: deps.isTerminalAgent(context.agent) ? [] : undefined, - 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..ff29da3f18 --- /dev/null +++ b/src/lib/sandbox/create-stream-argv.test.ts @@ -0,0 +1,31 @@ +// 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("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 }), + ); + }); +}); diff --git a/src/lib/sandbox/create-stream-ready-gate.test.ts b/src/lib/sandbox/create-stream-ready-gate.test.ts index f61a3d786b..0e47941236 100644 --- a/src/lib/sandbox/create-stream-ready-gate.test.ts +++ b/src/lib/sandbox/create-stream-ready-gate.test.ts @@ -124,4 +124,38 @@ describe("sandbox-create-stream ready gate", () => { await vi.advanceTimersByTimeAsync(6); await expect(promise).resolves.toMatchObject({ status: 0, forcedReady: true }); }); + + 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..03efb47b6d --- /dev/null +++ b/src/lib/sandbox/create-stream-ready-gate.ts @@ -0,0 +1,29 @@ +// 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.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, +): readonly RegExp[] | undefined { + return isTerminalAgent ? [] : undefined; +} diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index e17bc9d805..905f3810d9 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -5,6 +5,7 @@ import { type SpawnOptions, spawn } from "node:child_process"; import { redact } from "../security/redact"; import { ROOT } from "../state/paths"; +import { getReadyCheckOutputPatterns } from "./create-stream-ready-gate"; export interface StreamSandboxCreateResult { status: number; @@ -98,7 +99,6 @@ const VISIBLE_PROGRESS_PATTERNS: readonly RegExp[] = [ /^✓ /, ]; -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+(.+)$/; @@ -106,35 +106,28 @@ 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 : []; -} - export function streamSandboxCreate( command: string, - env: NodeJS.ProcessEnv = process.env, - options: StreamSandboxCreateOptions = {}, + env?: NodeJS.ProcessEnv, + options?: StreamSandboxCreateOptions, +): Promise; +export function streamSandboxCreate( + command: string, + 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 = {}, + maybeOptions: StreamSandboxCreateOptions = {}, ): Promise { - // Trust boundary: command is assembled by internal sandbox/onboard callers from - // trusted OpenShell CLI fragments. Do not pass user-supplied free-form text to - // this shell boundary; switch the API to direct spawn args if external input - // ever needs to cross it. - const child: StreamableChildProcess = (options.spawnImpl ?? spawn)("bash", ["-lc", command], { + const hasArgs = Array.isArray(argsOrEnv); + const commandArgs = hasArgs ? argsOrEnv : []; + const env = hasArgs ? (envOrOptions as NodeJS.ProcessEnv) : (argsOrEnv as NodeJS.ProcessEnv); + const options = hasArgs ? maybeOptions : (envOrOptions as StreamSandboxCreateOptions); + const child: StreamableChildProcess = (options.spawnImpl ?? spawn)(command, commandArgs, { cwd: ROOT, env, stdio: ["ignore", "pipe", "pipe"], @@ -430,14 +423,15 @@ export function streamSandboxCreate( // Localized containment: onPoll bridges optional host-side create // patching into the stream poll loop. Patch failures are observed // here but the source of truth remains the dedicated failureCheck; - // keep polling so sandbox readiness/failure can settle deterministically. - // Regression coverage: create-stream.test.ts verifies redacted trace - // emission and continued polling. Remove when onPoll becomes a typed - // result-returning dependency instead of an opportunistic side effect. + // keep the dedicated failureCheck active in this same tick so a + // patch-observed terminal failure is not delayed by another poll. + // Regression coverage: create-stream-ready-gate.test.ts verifies + // redacted trace emission, continued polling, and same-tick failure + // detection. Remove when onPoll becomes a typed result-returning + // dependency instead of an opportunistic side effect. emitTraceEvent("sandbox_create_poll_error", { message: redact(error instanceof Error ? error.message : String(error)), }); - return; } const failure = options.failureCheck?.(); From 1e890002ab43eedd8da15967560168f22c1e37ed Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 23:39:52 -0400 Subject: [PATCH 11/23] test(snapshot): expect argv create command Signed-off-by: Julie Yaunches --- src/lib/actions/sandbox/snapshot.test.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/lib/actions/sandbox/snapshot.test.ts b/src/lib/actions/sandbox/snapshot.test.ts index f77c00ad72..91b881f9c7 100644 --- a/src/lib/actions/sandbox/snapshot.test.ts +++ b/src/lib/actions/sandbox/snapshot.test.ts @@ -140,7 +140,12 @@ const runOpenshellMock = vi.fn((args: string[]) => { return { status: 0, output: "" }; }); const streamSandboxCreateMock = vi.fn( - async (_command: string, _env: NodeJS.ProcessEnv, _options?: Record) => ({ + async ( + _command: string, + _args: readonly string[], + _env: NodeJS.ProcessEnv, + _options?: Record, + ) => ({ status: 0, output: "", forcedReady: false, @@ -984,9 +989,9 @@ 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 [, createArgs = [], createEnv] = streamSandboxCreateMock.mock.calls[0] ?? []; + const createShellCommand = String(createArgs[1] ?? ""); + expect(createShellCommand).toContain(`'NEMOCLAW_OBSERVABILITY=${expectedValue}'`); expect(createEnv?.NEMOCLAW_OBSERVABILITY).toBeUndefined(); expect(registerSandboxMock).toHaveBeenCalledWith( expect.objectContaining({ From 393efd15e0a7b254b7c5f46fd0270de2405cbd08 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 23:54:31 -0400 Subject: [PATCH 12/23] test(snapshot): keep file within size budget Signed-off-by: Julie Yaunches --- src/lib/actions/sandbox/snapshot.test.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/lib/actions/sandbox/snapshot.test.ts b/src/lib/actions/sandbox/snapshot.test.ts index 91b881f9c7..eb4dcee7b8 100644 --- a/src/lib/actions/sandbox/snapshot.test.ts +++ b/src/lib/actions/sandbox/snapshot.test.ts @@ -139,18 +139,11 @@ 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, - _args: readonly string[], - _env: NodeJS.ProcessEnv, - _options?: Record, - ) => ({ - status: 0, - output: "", - forcedReady: false, - }), -); +const streamSandboxCreateMock = vi.fn(async (..._args: unknown[]) => ({ + status: 0, + output: "", + forcedReady: false, +})); const dcodeSandboxEntry = { name: "alpha", agent: "langchain-deepagents-code", From 36be6477c7c6f8a282b1651787c33cf4abc03df2 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 9 Jul 2026 00:05:35 -0400 Subject: [PATCH 13/23] test(snapshot): type argv create assertion Signed-off-by: Julie Yaunches --- src/lib/actions/sandbox/snapshot.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/actions/sandbox/snapshot.test.ts b/src/lib/actions/sandbox/snapshot.test.ts index eb4dcee7b8..a3597687d8 100644 --- a/src/lib/actions/sandbox/snapshot.test.ts +++ b/src/lib/actions/sandbox/snapshot.test.ts @@ -982,7 +982,9 @@ describe("runSandboxSnapshot", () => { getLatestBackupMock.mockReturnValue({ ...latestBackupFixture }); const { runSandboxSnapshot } = await import("./snapshot"); await runSandboxSnapshot("alpha", { kind: "restore", to: "beta" }); - const [, createArgs = [], createEnv] = streamSandboxCreateMock.mock.calls[0] ?? []; + const createCall = streamSandboxCreateMock.mock.calls[0] ?? []; + const createArgs = createCall[1] as readonly string[]; + const createEnv = createCall[2] as NodeJS.ProcessEnv | undefined; const createShellCommand = String(createArgs[1] ?? ""); expect(createShellCommand).toContain(`'NEMOCLAW_OBSERVABILITY=${expectedValue}'`); expect(createEnv?.NEMOCLAW_OBSERVABILITY).toBeUndefined(); From 05c0c98e643d7ea9b5f794fd7715d56ed81b3f94 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 9 Jul 2026 00:26:12 -0400 Subject: [PATCH 14/23] fix(onboard): close create security advisor gaps Signed-off-by: Julie Yaunches --- src/lib/actions/sandbox/snapshot.test.ts | 4 ++-- src/lib/actions/sandbox/snapshot.ts | 10 ++++------ src/lib/onboard/created-sandbox-failure.test.ts | 11 +++++++++++ src/lib/onboard/created-sandbox-failure.ts | 2 +- src/lib/onboard/sandbox-create-launch.test.ts | 3 +++ src/lib/onboard/sandbox-create-launch.ts | 4 ++-- src/lib/sandbox/create-stream-argv.test.ts | 17 +++++++++++++++++ src/lib/sandbox/create-stream.ts | 5 +++-- 8 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/lib/actions/sandbox/snapshot.test.ts b/src/lib/actions/sandbox/snapshot.test.ts index a3597687d8..9a61b6f32e 100644 --- a/src/lib/actions/sandbox/snapshot.test.ts +++ b/src/lib/actions/sandbox/snapshot.test.ts @@ -985,8 +985,8 @@ describe("runSandboxSnapshot", () => { const createCall = streamSandboxCreateMock.mock.calls[0] ?? []; const createArgs = createCall[1] as readonly string[]; const createEnv = createCall[2] as NodeJS.ProcessEnv | undefined; - const createShellCommand = String(createArgs[1] ?? ""); - expect(createShellCommand).toContain(`'NEMOCLAW_OBSERVABILITY=${expectedValue}'`); + 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 aa0662c343..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,9 +253,7 @@ async function autoCreateSandboxFromSource( "--auto-providers", "--", ...startupCommand, - ].map((p) => shellQuote(p)); - const command = "bash"; - const commandArgs = ["-lc", `${cmdParts.join(" ")} 2>&1`]; + ]; console.log(` '${dstName}' does not exist. Creating from '${srcName}' image (${fromImage})...`); diff --git a/src/lib/onboard/created-sandbox-failure.test.ts b/src/lib/onboard/created-sandbox-failure.test.ts index 71bec38dd1..7e03bbfa92 100644 --- a/src/lib/onboard/created-sandbox-failure.test.ts +++ b/src/lib/onboard/created-sandbox-failure.test.ts @@ -96,6 +96,10 @@ describe("reportSandboxCreateFailure", () => { 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. @@ -128,6 +132,13 @@ describe("reportSandboxCreateFailure", () => { 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", () => { diff --git a/src/lib/onboard/created-sandbox-failure.ts b/src/lib/onboard/created-sandbox-failure.ts index 03f9df9136..a067aa9b56 100644 --- a/src/lib/onboard/created-sandbox-failure.ts +++ b/src/lib/onboard/created-sandbox-failure.ts @@ -58,7 +58,7 @@ export function reportSandboxCreateFailure( backupPath: options.restoreBackupPath, }); deps.error(" Try: openshell sandbox list # check gateway state"); - deps.printRecoveryHints(options.createOutput, { createArgs: options.createArgs }); + deps.printRecoveryHints(redactedCreateOutput, { createArgs: options.createArgs }); return deps.exitProcess(options.createStatus === 0 ? 1 : options.createStatus); } 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 08303b06c6..6d780d0a13 100644 --- a/src/lib/onboard/sandbox-create-launch.ts +++ b/src/lib/onboard/sandbox-create-launch.ts @@ -143,10 +143,10 @@ export function prepareSandboxCreateLaunch(input: SandboxCreateLaunchInput): San // lets the real exit code flow through to run(). const sandboxStartupCommand = ["env", ...envArgs, "nemoclaw-start"]; const openshellArgs = ["sandbox", "create", ...input.createArgs, "--", ...sandboxStartupCommand]; + const createCommand = `${input.openshellShellCommand(openshellArgs)} 2>&1`; const createArgv = input.openshellArgv ? input.openshellArgv(openshellArgs) - : [input.openshellShellCommand(openshellArgs)]; - const createCommand = `${input.openshellShellCommand(openshellArgs)} 2>&1`; + : ["bash", "-lc", createCommand]; return { createCommand, diff --git a/src/lib/sandbox/create-stream-argv.test.ts b/src/lib/sandbox/create-stream-argv.test.ts index ff29da3f18..0460bb30df 100644 --- a/src/lib/sandbox/create-stream-argv.test.ts +++ b/src/lib/sandbox/create-stream-argv.test.ts @@ -7,6 +7,23 @@ 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); diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index 905f3810d9..689200cc0e 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -124,10 +124,11 @@ export function streamSandboxCreate( maybeOptions: StreamSandboxCreateOptions = {}, ): Promise { const hasArgs = Array.isArray(argsOrEnv); - const commandArgs = hasArgs ? argsOrEnv : []; + const commandArgs = hasArgs ? argsOrEnv : ["-lc", command]; + const spawnCommand = hasArgs ? command : "bash"; const env = hasArgs ? (envOrOptions as NodeJS.ProcessEnv) : (argsOrEnv as NodeJS.ProcessEnv); const options = hasArgs ? maybeOptions : (envOrOptions as StreamSandboxCreateOptions); - const child: StreamableChildProcess = (options.spawnImpl ?? spawn)(command, commandArgs, { + const child: StreamableChildProcess = (options.spawnImpl ?? spawn)(spawnCommand, commandArgs, { cwd: ROOT, env, stdio: ["ignore", "pipe", "pipe"], From 349d1137c6d6af41752f3197eabee21ceb0265ab Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 9 Jul 2026 00:47:44 -0400 Subject: [PATCH 15/23] fix(onboard): unify create ready gate detection Signed-off-by: Julie Yaunches --- src/lib/onboard/sandbox-create-step.test.ts | 7 +++++-- src/lib/onboard/sandbox-create-step.ts | 1 + .../sandbox/create-stream-ready-gate.test.ts | 19 +++++++++++++++++++ src/lib/sandbox/create-stream-ready-gate.ts | 10 ++++------ 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/lib/onboard/sandbox-create-step.test.ts b/src/lib/onboard/sandbox-create-step.test.ts index a95ea8d6e4..e183f4a57a 100644 --- a/src/lib/onboard/sandbox-create-step.test.ts +++ b/src/lib/onboard/sandbox-create-step.test.ts @@ -173,12 +173,15 @@ describe("runSandboxCreateStep", () => { (terminalDeps.streamCreate as unknown as { mock: { calls: unknown[][] } }).mock.calls[0][3], ).toMatchObject({ readyCheckOutputPatterns: [] }); - const nonTerminalDeps = makeDeps(makeLaunch(), makePatch(), { status: 0, output: "" }); + 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: undefined }); + ).toMatchObject({ readyCheckOutputPatterns: [expect.any(RegExp)] }); }); it.each([ diff --git a/src/lib/onboard/sandbox-create-step.ts b/src/lib/onboard/sandbox-create-step.ts index 1dd44e3cc5..870d9d390e 100644 --- a/src/lib/onboard/sandbox-create-step.ts +++ b/src/lib/onboard/sandbox-create-step.ts @@ -119,6 +119,7 @@ export async function runSandboxCreateStep( onPoll: () => dockerGpuCreatePatch.maybeApplyDuringCreate(), readyCheckOutputPatterns: getReadyCheckOutputPatternsForAgent( deps.isTerminalAgent(context.agent), + sandboxEnv, ), failureCheck: dockerGpuCreatePatch.createFailureMessage, traceEvent: deps.addTraceEvent, diff --git a/src/lib/sandbox/create-stream-ready-gate.test.ts b/src/lib/sandbox/create-stream-ready-gate.test.ts index 0e47941236..bd5b84d811 100644 --- a/src/lib/sandbox/create-stream-ready-gate.test.ts +++ b/src/lib/sandbox/create-stream-ready-gate.test.ts @@ -5,6 +5,10 @@ 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(() => { @@ -44,6 +48,21 @@ describe("sandbox-create-stream ready gate", () => { 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(); diff --git a/src/lib/sandbox/create-stream-ready-gate.ts b/src/lib/sandbox/create-stream-ready-gate.ts index 03efb47b6d..c54e46f402 100644 --- a/src/lib/sandbox/create-stream-ready-gate.ts +++ b/src/lib/sandbox/create-stream-ready-gate.ts @@ -4,10 +4,7 @@ export const VM_READY_DETACH_OUTPUT_PATTERNS: readonly RegExp[] = [/Setting up NemoClaw/]; function selectedDrivers(env: NodeJS.ProcessEnv): string[] { - const raw = - env.OPENSHELL_DRIVERS ?? - process.env.OPENSHELL_DRIVERS ?? - (process.platform === "darwin" ? "vm" : "docker"); + const raw = env.OPENSHELL_DRIVERS ?? (process.platform === "darwin" ? "vm" : "docker"); return raw .split(",") .map((driver) => driver.trim()) @@ -24,6 +21,7 @@ export function getReadyCheckOutputPatterns( export function getReadyCheckOutputPatternsForAgent( isTerminalAgent: boolean, -): readonly RegExp[] | undefined { - return isTerminalAgent ? [] : undefined; + env: NodeJS.ProcessEnv, +): readonly RegExp[] { + return isTerminalAgent ? [] : getReadyCheckOutputPatterns(env, undefined); } From 3a6016d4d86f3196daf9b9f817db42e508f0e382 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 9 Jul 2026 01:01:33 -0400 Subject: [PATCH 16/23] fix(sandbox): require failure check for create poll hooks Signed-off-by: Julie Yaunches --- .../sandbox/create-stream-ready-gate.test.ts | 19 +++++++++++++++++-- src/lib/sandbox/create-stream.ts | 5 +++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/lib/sandbox/create-stream-ready-gate.test.ts b/src/lib/sandbox/create-stream-ready-gate.test.ts index bd5b84d811..0edfc5a6a5 100644 --- a/src/lib/sandbox/create-stream-ready-gate.test.ts +++ b/src/lib/sandbox/create-stream-ready-gate.test.ts @@ -98,6 +98,16 @@ describe("sandbox-create-stream ready gate", () => { 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"); + }); + it("runs poll side effects only after a not-ready poll", async () => { vi.useFakeTimers(); @@ -107,7 +117,7 @@ describe("sandbox-create-stream ready gate", () => { const promise = streamSandboxCreate( "echo create", dockerEnv, - makePollingOptions(child, { readyCheck: () => ready, onPoll }), + makePollingOptions(child, { readyCheck: () => ready, onPoll, failureCheck: () => null }), ); await vi.advanceTimersByTimeAsync(6); @@ -131,7 +141,12 @@ describe("sandbox-create-stream ready gate", () => { const promise = streamSandboxCreate( "echo create", dockerEnv, - makePollingOptions(child, { readyCheck: () => ready, onPoll, traceEvent }), + makePollingOptions(child, { + readyCheck: () => ready, + onPoll, + failureCheck: () => null, + traceEvent, + }), ); await vi.advanceTimersByTimeAsync(6); diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index 689200cc0e..33bb8297d9 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -16,6 +16,8 @@ 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; @@ -128,6 +130,9 @@ export function streamSandboxCreate( const spawnCommand = hasArgs ? command : "bash"; const env = hasArgs ? (envOrOptions 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"); + } const child: StreamableChildProcess = (options.spawnImpl ?? spawn)(spawnCommand, commandArgs, { cwd: ROOT, env, From 88a72a32efe957c4469276f59fa193a9c71137b9 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 9 Jul 2026 01:17:03 -0400 Subject: [PATCH 17/23] refactor(sandbox): extract create stream progress patterns Signed-off-by: Julie Yaunches --- src/lib/sandbox/create-stream-progress.ts | 50 +++++++++++++++++++++ src/lib/sandbox/create-stream.ts | 55 ++++------------------- 2 files changed, 58 insertions(+), 47 deletions(-) create mode 100644 src/lib/sandbox/create-stream-progress.ts 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.ts b/src/lib/sandbox/create-stream.ts index 33bb8297d9..99f693ab95 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -5,6 +5,14 @@ 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 { @@ -59,55 +67,9 @@ 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 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)); -} - export function streamSandboxCreate( command: string, env?: NodeJS.ProcessEnv, @@ -158,7 +120,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; From 31b64ed97e5397a6991e85f7e150a2922e5568aa Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 9 Jul 2026 01:31:07 -0400 Subject: [PATCH 18/23] fix(sandbox): abort create poll hook failures Signed-off-by: Julie Yaunches --- .../sandbox/create-stream-ready-gate.test.ts | 18 +++++++++++------- src/lib/sandbox/create-stream.ts | 13 +++---------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/lib/sandbox/create-stream-ready-gate.test.ts b/src/lib/sandbox/create-stream-ready-gate.test.ts index 0edfc5a6a5..63a0fe0e39 100644 --- a/src/lib/sandbox/create-stream-ready-gate.test.ts +++ b/src/lib/sandbox/create-stream-ready-gate.test.ts @@ -129,34 +129,38 @@ describe("sandbox-create-stream ready gate", () => { expect(onPoll).toHaveBeenCalledTimes(1); }); - it("traces redacted poll side-effect errors and keeps polling", async () => { + 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"); }); - let ready = false; const promise = streamSandboxCreate( "echo create", dockerEnv, makePollingOptions(child, { - readyCheck: () => ready, + 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********", }); - - ready = true; - await vi.advanceTimersByTimeAsync(6); - await expect(promise).resolves.toMatchObject({ status: 0, forcedReady: true }); + 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 () => { diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index 99f693ab95..07b4a65305 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -384,24 +384,17 @@ export function streamSandboxCreate( return; } + let pollFailure: string | null | undefined; try { options.onPoll?.(); } catch (error) { - // Localized containment: onPoll bridges optional host-side create - // patching into the stream poll loop. Patch failures are observed - // here but the source of truth remains the dedicated failureCheck; - // keep the dedicated failureCheck active in this same tick so a - // patch-observed terminal failure is not delayed by another poll. - // Regression coverage: create-stream-ready-gate.test.ts verifies - // redacted trace emission, continued polling, and same-tick failure - // detection. Remove when onPoll becomes a typed result-returning - // dependency instead of an opportunistic side effect. 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 = options.failureCheck?.(); + const failure = pollFailure ?? options.failureCheck?.(); if (!failure) return; const detail = String(failure); lines.push(detail); From 0b066c36bf1e473b8da29131a8c17837afbcefa2 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 9 Jul 2026 01:50:40 -0400 Subject: [PATCH 19/23] test(sandbox): trace ready check poll errors Signed-off-by: Julie Yaunches --- src/lib/sandbox/create-stream.test.ts | 35 +++++++++++++++++++++++++++ src/lib/sandbox/create-stream.ts | 5 +++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/lib/sandbox/create-stream.test.ts b/src/lib/sandbox/create-stream.test.ts index f78c18da6a..ef25fde3fe 100644 --- a/src/lib/sandbox/create-stream.test.ts +++ b/src/lib/sandbox/create-stream.test.ts @@ -171,6 +171,41 @@ describe("sandbox-create-stream", () => { expect(child.unref).toHaveBeenCalled(); }); + it("traces ready-check errors and keeps polling without forcing ready", async () => { + vi.useFakeTimers(); + + const child = new FakeChild(); + 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, + pollIntervalMs: 5, + heartbeatIntervalMs: 1_000, + silentPhaseMs: 10_000, + traceEvent, + logLine: vi.fn(), + }); + + child.stdout.emit("data", Buffer.from(" Building image sandbox\n")); + await vi.advanceTimersByTimeAsync(6); + expect(child.kill).not.toHaveBeenCalled(); + expect(traceEvent).toHaveBeenCalledWith("sandbox_create_ready_check_error", { + message: "Authorization: Bearer secr********", + }); + + await vi.advanceTimersByTimeAsync(12); + + await expect(promise).resolves.toMatchObject({ status: 0, forcedReady: true }); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + }); + it("does not recover a non-zero close before required startup output appears", async () => { const child = new FakeChild(); const promise = streamSandboxCreate("echo create", vmEnv, { diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index 07b4a65305..fb2374d56f 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -355,7 +355,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) { From c403937f672cf00a90a64ffd98696f5d3ea77968 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 9 Jul 2026 03:05:00 -0400 Subject: [PATCH 20/23] fix(sandbox): document create stream legacy shell overload --- src/lib/sandbox/create-stream-ready-gate.test.ts | 4 +++- src/lib/sandbox/create-stream.ts | 9 ++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/lib/sandbox/create-stream-ready-gate.test.ts b/src/lib/sandbox/create-stream-ready-gate.test.ts index 63a0fe0e39..b946a0bb29 100644 --- a/src/lib/sandbox/create-stream-ready-gate.test.ts +++ b/src/lib/sandbox/create-stream-ready-gate.test.ts @@ -105,7 +105,9 @@ describe("sandbox-create-stream ready gate", () => { dockerEnv, makePollingOptions(new FakeChild(), { readyCheck: () => false, onPoll: () => {} }), ), - ).toThrow("streamSandboxCreate onPoll requires failureCheck"); + ).toThrow( + "streamSandboxCreate onPoll requires failureCheck (e.g., dockerGpuCreatePatch.createFailureMessage)", + ); }); it("runs poll side effects only after a not-ready poll", async () => { diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index fb2374d56f..327aa8abc4 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -70,6 +70,11 @@ export interface StreamableChildProcess { const CLASSIC_DOCKER_STEP_RE = /^\s*Step (\d+)\/(\d+) : (.+)$/; const BUILDKIT_STEP_RE = /^#(\d+)\s+(.+)$/; +/** + * @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. + */ export function streamSandboxCreate( command: string, env?: NodeJS.ProcessEnv, @@ -93,7 +98,9 @@ export function streamSandboxCreate( const env = hasArgs ? (envOrOptions 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"); + throw new Error( + "streamSandboxCreate onPoll requires failureCheck (e.g., dockerGpuCreatePatch.createFailureMessage)", + ); } const child: StreamableChildProcess = (options.spawnImpl ?? spawn)(spawnCommand, commandArgs, { cwd: ROOT, From 36d3d827e362089c1cb2e118b45e9cd7f4966aa5 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 9 Jul 2026 03:42:20 -0400 Subject: [PATCH 21/23] test(snapshot): guard auto-create failure registry state --- .../snapshot-auto-create-failure.test.ts | 141 ++++++++++++++++++ .../snapshot-create-stream-test-types.ts | 14 ++ src/lib/actions/sandbox/snapshot.test.ts | 16 +- src/lib/sandbox/create-stream.ts | 3 +- 4 files changed, 161 insertions(+), 13 deletions(-) create mode 100644 src/lib/actions/sandbox/snapshot-auto-create-failure.test.ts create mode 100644 src/lib/actions/sandbox/snapshot-create-stream-test-types.ts 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 9a61b6f32e..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,9 +130,10 @@ 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 (..._args: unknown[]) => ({ +const streamSandboxCreateMock = vi.fn(async () => ({ status: 0, output: "", + sawProgress: false, forcedReady: false, })); const dcodeSandboxEntry = { diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index 327aa8abc4..bf9a27038a 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -73,7 +73,8 @@ const BUILDKIT_STEP_RE = /^#(\d+)\s+(.+)$/; /** * @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. + * `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, From c7a9b417af286da1674ad7ebce97d740621c417d Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Thu, 9 Jul 2026 03:59:40 -0400 Subject: [PATCH 22/23] fix(sandbox): inherit env for argv create stream --- src/lib/sandbox/create-stream-argv.test.ts | 17 +++++++++++++++++ src/lib/sandbox/create-stream.ts | 8 +++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/lib/sandbox/create-stream-argv.test.ts b/src/lib/sandbox/create-stream-argv.test.ts index 0460bb30df..6b117f4029 100644 --- a/src/lib/sandbox/create-stream-argv.test.ts +++ b/src/lib/sandbox/create-stream-argv.test.ts @@ -45,4 +45,21 @@ describe("sandbox-create-stream argv boundary", () => { 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.ts b/src/lib/sandbox/create-stream.ts index bf9a27038a..c6d9720388 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -90,14 +90,16 @@ export function streamSandboxCreate( export function streamSandboxCreate( command: string, argsOrEnv: readonly string[] | NodeJS.ProcessEnv = process.env, - envOrOptions: NodeJS.ProcessEnv | StreamSandboxCreateOptions = {}, + envOrOptions: NodeJS.ProcessEnv | StreamSandboxCreateOptions | undefined = undefined, maybeOptions: StreamSandboxCreateOptions = {}, ): Promise { const hasArgs = Array.isArray(argsOrEnv); const commandArgs = hasArgs ? argsOrEnv : ["-lc", command]; const spawnCommand = hasArgs ? command : "bash"; - const env = hasArgs ? (envOrOptions as NodeJS.ProcessEnv) : (argsOrEnv as NodeJS.ProcessEnv); - const options = hasArgs ? maybeOptions : (envOrOptions as StreamSandboxCreateOptions); + 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)", From 04581fcfe0175831bc093dea1b7e6713e2edffd9 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Thu, 9 Jul 2026 01:28:59 -0700 Subject: [PATCH 23/23] fix(cli): keep create output streams separate Signed-off-by: Carlos Villela --- src/lib/sandbox/create-stream.test.ts | 19 +++++++++++++++++ src/lib/sandbox/create-stream.ts | 30 ++++++++++++++------------- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/lib/sandbox/create-stream.test.ts b/src/lib/sandbox/create-stream.test.ts index ef25fde3fe..1c937ca2f1 100644 --- a/src/lib/sandbox/create-stream.test.ts +++ b/src/lib/sandbox/create-stream.test.ts @@ -272,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(); diff --git a/src/lib/sandbox/create-stream.ts b/src/lib/sandbox/create-stream.ts index c6d9720388..7096916345 100644 --- a/src/lib/sandbox/create-stream.ts +++ b/src/lib/sandbox/create-stream.ts @@ -114,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( @@ -313,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"); } @@ -354,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(() => { @@ -469,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) {