From 18a98f9c54cde79f7797099fb386f3fddbd7975d Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Thu, 13 Aug 2026 14:00:42 -0700 Subject: [PATCH 1/2] fix(agent): cancel sandbox work on host termination Signed-off-by: Prekshi Vyas --- docs/reference/commands.mdx | 4 +- .../agent/passthrough-dispatch.test.ts | 77 ++++++- .../sandbox/agent/passthrough-dispatch.ts | 160 +++++++++++++- .../sandbox/agent/passthrough-json.test.ts | 195 ++++++++++-------- .../actions/sandbox/agent/passthrough-json.ts | 32 +-- .../actions/sandbox/agent/passthrough.test.ts | 112 +++++----- src/lib/actions/sandbox/agent/passthrough.ts | 62 ++---- 7 files changed, 436 insertions(+), 206 deletions(-) diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 92a26297a2d..247af0400dd 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1277,6 +1277,8 @@ Otherwise, it writes the captured output to the corresponding host streams and r The in-sandbox NemoClaw plugin writes its registration banner to `stderr`, so the banner does not prefix the agent reply on `stdout` in non-JSON mode. Because a delivered turn always writes to one of the two streams, the wrapper reports a dispatch with status `0` and no output as a failure. The wrapper prints recovery guidance to `stderr` and exits with status `1`. +Pressing `Ctrl+C` interrupts the OpenShell child, and sending `SIGTERM` to the host wrapper forwards `SIGTERM` to that child. +NemoClaw waits for OpenShell to stop the in-sandbox turn, replays captured output, and returns status `130` for `SIGINT` or `143` for `SIGTERM`. The diagnostic shell-quotes the sandbox name and forwarded arguments, then redacts detected credential values before writing the recovery command to `stderr`. If redaction changes the recovery command, the diagnostic tells you not to replay it; otherwise, it labels the command as runnable inside the sandbox. For a registered sandbox, both captured paths pin the sandbox's recorded gateway with an explicit `-g`. @@ -4516,7 +4518,7 @@ OpenClaw-specific onboarding configuration: | `NEMOCLAW_WEB_SEARCH_PROVIDER` | `brave`, `tavily`, or `none` | Selects Brave Search or Tavily Search in non-interactive onboarding, or disables web search explicitly. When unset, `BRAVE_API_KEY` implicitly selects Brave before `TAVILY_API_KEY` can implicitly select Tavily. | | `BRAVE_API_KEY` | Brave Search API key | Supplies and implicitly selects Brave Search when no web search provider is set. NemoClaw validates the key and stores it in OpenShell rather than the sandbox. | | `TAVILY_API_KEY` | Tavily Search API key | Supplies and implicitly selects Tavily Search when no provider is set and no Brave key is available. NemoClaw validates the key and stores it in OpenShell rather than the sandbox. | -| `NEMOCLAW_AGENT_TIMEOUT` | positive integer (seconds) | Overrides `agents.defaults.timeoutSeconds` and `models.providers..timeoutSeconds` in the built OpenClaw config. Raise for slow inference. | +| `NEMOCLAW_AGENT_TIMEOUT` | positive integer (seconds) | Build-time setting that overrides `agents.defaults.timeoutSeconds` and `models.providers..timeoutSeconds` in the built OpenClaw config. Set it before onboarding builds the sandbox image. Setting it only for a later `$$nemoclaw agent` invocation does not change the existing image. Raise for slow inference. | | `NEMOCLAW_MCP_SHADOW_DIAGNOSTICS` | literal `1` to enable | Forwards opt-in successful Streamable HTTP MCP timing diagnostics to a newly created or rebuilt OpenClaw sandbox. It does not change timeouts, retries, requests, or responses. Unset it and rebuild after evidence collection to restore failure-only logging. Other values are ignored. | | `NEMOCLAW_AUTO_PAIR_SLOW_INTERVAL_SECS` | positive number of seconds | Sets the post-pairing poll cadence for the in-sandbox OpenClaw auto-pair watcher. Defaults to `5` so late allowlisted CLI and browser scope upgrades are approved before clients time out. Raise only on load-sensitive gateways. | | `NEMOCLAW_AUTO_PAIR_FAST_REENTRY_POLLS` | positive integer | Sets how many fast polls run after the watcher observes a fresh allowlisted scope-upgrade request. Defaults to `5`; set lower only when you need to reduce gateway polling. | diff --git a/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts b/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts index 9409bf0e6ab..1d5744186e5 100644 --- a/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts @@ -1,13 +1,88 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, expect, it } from "vitest"; +import { EventEmitter } from "node:events"; + +import { describe, expect, it, vi } from "vitest"; import { + type AgentDispatchChild, agentDispatchStdio, isSilentAgentDispatch, + runAgentDispatch, SILENT_AGENT_DISPATCH_EXIT_CODE, } from "./passthrough-dispatch"; +import type { SandboxExecSignalSource } from "../exec"; + +function dispatchHarness() { + const childEvents = new EventEmitter(); + const signalEvents = new EventEmitter(); + const stderr = new EventEmitter(); + const stdout = new EventEmitter(); + const child: AgentDispatchChild = { + exitCode: null, + signalCode: null, + kill: vi.fn((signal) => { + child.signalCode = signal; + queueMicrotask(() => childEvents.emit("close", null, signal)); + return true; + }), + once: ((event: string, listener: (...args: unknown[]) => void) => + childEvents.once(event, listener)) as AgentDispatchChild["once"], + stderr, + stdout, + }; + const signalSource: SandboxExecSignalSource = { + add: (signal, listener) => signalEvents.on(signal, listener), + remove: (signal, listener) => signalEvents.off(signal, listener), + }; + return { child, signalEvents, signalSource, stderr, stdout }; +} + +describe("runAgentDispatch", () => { + it("forwards host SIGTERM to OpenShell and captures output before signal exit (#8723)", async () => { + const harness = dispatchHarness(); + const pending = runAgentDispatch( + "openshell", + ["sandbox", "exec", "--name", "alpha", "--", "openclaw", "agent"], + { stdinIsTty: true }, + { signalSource: harness.signalSource, spawnChild: () => harness.child }, + ); + + harness.stdout.emit("data", "partial response\n"); + harness.stderr.emit("data", Buffer.from("gateway timeout pending\n")); + harness.signalEvents.emit("SIGTERM"); + + const result = await pending; + expect(harness.child.kill).toHaveBeenCalledOnce(); + expect(harness.child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(result).toMatchObject({ + status: null, + signal: "SIGTERM", + stdout: "partial response\n", + stderr: "gateway timeout pending\n", + }); + expect(harness.signalEvents.listenerCount("SIGTERM")).toBe(0); + expect(harness.signalEvents.listenerCount("SIGINT")).toBe(0); + }); + + it("terminates the OpenShell child when captured output exceeds its bound", async () => { + const harness = dispatchHarness(); + const pending = runAgentDispatch( + "openshell", + ["sandbox", "exec", "--name", "alpha", "--", "openclaw", "agent"], + { maxBufferBytes: 4, stdinIsTty: false }, + { signalSource: harness.signalSource, spawnChild: () => harness.child }, + ); + + harness.stdout.emit("data", "12345"); + + const result = await pending; + expect(harness.child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(result.error).toEqual(new Error("agent stdout exceeded the 4-byte capture limit")); + expect(result.stdout).toBe(""); + }); +}); describe("isSilentAgentDispatch", () => { it("classifies a zero-exit dispatch with no bytes on either stream as silent", () => { diff --git a/src/lib/actions/sandbox/agent/passthrough-dispatch.ts b/src/lib/actions/sandbox/agent/passthrough-dispatch.ts index bfcf4bef57d..5a08625d981 100644 --- a/src/lib/actions/sandbox/agent/passthrough-dispatch.ts +++ b/src/lib/actions/sandbox/agent/passthrough-dispatch.ts @@ -3,10 +3,10 @@ // Source-of-truth boundary for the agent dispatch contract (#8796). // -// Both `nemoclaw agent` transports capture the child's streams and -// forward its exit code. That makes "the child exited 0" the only success -// signal, so a dispatch that never ran the turn is indistinguishable from a -// turn that ran and answered. +// Both `nemoclaw agent` transports capture the child's streams, forward +// host termination signals, and return the child's exit status. OpenClaw can +// still report status 0 when a dispatch produces no result, so the wrapper +// must classify that ambiguous result before it reports success. // // 6. Empty-dispatch guard (delivery contract). // @@ -41,12 +41,26 @@ // - Removal condition: drop the TTY carve-out if `openclaw agent` gains a // documented interactive stdin mode reachable through this wrapper. // +// 8. Host interruption propagation. +// +// - Invalid state: the former synchronous transports blocked the Node.js +// event loop. A host SIGTERM stopped NemoClaw without notifying the +// OpenShell child, so the in-sandbox agent turn continued until its own +// deadline. +// - Source boundary: OpenShell owns remote command cancellation. NemoClaw +// owns its direct child and uses the shared sandbox exec supervisor to +// forward SIGTERM, wait for OpenShell to exit, and return exit 143. +// - Removal condition: none while NemoClaw owns the host-side OpenShell +// child lifecycle. +// // Regression tests: `passthrough-dispatch.test.ts` owns the classifier and the -// stdio shape; `passthrough-help.test.ts` owns the diagnostic text. +// supervised process lifecycle; `passthrough-help.test.ts` owns the diagnostic +// text. -import type { StdioOptions } from "node:child_process"; +import { spawn, type StdioOptions } from "node:child_process"; import { isStdinTty } from "../../../core/stdin"; +import { runSandboxExecChild, type SandboxExecChild, type SandboxExecSignalSource } from "../exec"; /** * Exit code for a dispatch that reported success without delivering a turn. @@ -54,12 +68,142 @@ import { isStdinTty } from "../../../core/stdin"; */ export const SILENT_AGENT_DISPATCH_EXIT_CODE = 1; -/** The subset of a `spawnSync` return the delivery classifier reads. */ +/** The subset of a child-process result the delivery classifier reads. */ export type AgentDispatchOutcome = { - error?: unknown; + error?: Error; status: number | null; + signal?: NodeJS.Signals | null; +}; + +export type AgentDispatchResult = AgentDispatchOutcome & { + stderr: string; + stdout: string; +}; + +type AgentDispatchReadable = { + on(event: "data", listener: (chunk: Buffer | string) => void): unknown; +}; + +export type AgentDispatchChild = SandboxExecChild & { + stderr: AgentDispatchReadable | null; + stdout: AgentDispatchReadable | null; +}; + +export type AgentDispatchSpawner = ( + binary: string, + args: readonly string[], + stdio: StdioOptions, +) => AgentDispatchChild; + +export type AgentDispatchRunner = ( + binary: string, + args: readonly string[], + options?: { + maxBufferBytes?: number; + stdinIsTty?: boolean; + }, +) => Promise; + +export type AgentDispatchRunDeps = { + signalSource?: SandboxExecSignalSource; + spawnChild?: AgentDispatchSpawner; }; +const DEFAULT_AGENT_DISPATCH_MAX_BUFFER_BYTES = 64 * 1024 * 1024; + +const defaultAgentDispatchSpawner: AgentDispatchSpawner = (binary, args, stdio) => + spawn(binary, [...args], { stdio }) as unknown as AgentDispatchChild; + +function captureAgentDispatchStream( + stream: AgentDispatchReadable | null, + streamName: "stderr" | "stdout", + child: AgentDispatchChild, + chunks: Buffer[], + maxBufferBytes: number, + setOverflowError: (error: Error) => void, +): void { + let size = 0; + let overflowed = false; + stream?.on("data", (chunk) => { + if (overflowed) return; + const data = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + size += data.byteLength; + if (size > maxBufferBytes) { + overflowed = true; + setOverflowError( + new Error(`agent ${streamName} exceeded the ${maxBufferBytes}-byte capture limit`), + ); + if (child.exitCode === null && child.signalCode === null) child.kill("SIGTERM"); + return; + } + chunks.push(data); + }); +} + +/** + * Capture one agent dispatch while the shared sandbox exec supervisor forwards + * host termination signals to OpenShell and waits for the child to exit. + */ +export async function runAgentDispatch( + binary: string, + args: readonly string[], + options: { + maxBufferBytes?: number; + stdinIsTty?: boolean; + } = {}, + deps: AgentDispatchRunDeps = {}, +): Promise { + const stderrChunks: Buffer[] = []; + const stdoutChunks: Buffer[] = []; + let overflowError: Error | undefined; + const maxBufferBytes = options.maxBufferBytes ?? DEFAULT_AGENT_DISPATCH_MAX_BUFFER_BYTES; + const spawnChild = deps.spawnChild ?? defaultAgentDispatchSpawner; + const result = await runSandboxExecChild( + binary, + args, + { tty: false }, + (runBinary, runArgs) => { + const child = spawnChild( + runBinary, + runArgs, + agentDispatchStdio(options.stdinIsTty ?? isStdinTty()), + ); + const setOverflowError = (error: Error) => { + overflowError ??= error; + }; + captureAgentDispatchStream( + child.stdout, + "stdout", + child, + stdoutChunks, + maxBufferBytes, + setOverflowError, + ); + captureAgentDispatchStream( + child.stderr, + "stderr", + child, + stderrChunks, + maxBufferBytes, + setOverflowError, + ); + return child; + }, + deps.signalSource, + ); + try { + return { + status: result.status, + signal: result.signal, + ...(result.error || overflowError ? { error: result.error ?? overflowError } : {}), + stderr: Buffer.concat(stderrChunks).toString("utf-8"), + stdout: Buffer.concat(stdoutChunks).toString("utf-8"), + }; + } finally { + result.releaseSignals?.(); + } +} + /** * Stdio for a non-interactive agent dispatch. An interactive terminal is * withheld from fd 0; a genuine pipe or redirect is still forwarded so diff --git a/src/lib/actions/sandbox/agent/passthrough-json.test.ts b/src/lib/actions/sandbox/agent/passthrough-json.test.ts index 0ba8e3c8119..3fc19c83987 100644 --- a/src/lib/actions/sandbox/agent/passthrough-json.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough-json.test.ts @@ -1,8 +1,6 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import type { SpawnSyncOptions } from "node:child_process"; - import { describe, expect, it, vi } from "vitest"; import { buildOpenshellExecArgs, wrapOpenClawAgentCommandWithRuntimeEnv } from "../exec"; @@ -27,7 +25,7 @@ describe("runAgentJsonPassthrough", () => { }; } - it("preserves OpenClaw JSON stdout and appends failed-tool provenance to stderr", () => { + it("preserves OpenClaw JSON stdout and appends failed-tool provenance to stderr", async () => { const payload = JSON.stringify({ result: { messages: [ @@ -43,7 +41,7 @@ describe("runAgentJsonPassthrough", () => { payloads: [{ text: "Saved successfully." }], }, }); - const spawnSync = vi.fn(() => ({ + const runDispatch = vi.fn(async () => ({ status: 0, signal: null, stdout: payload, @@ -53,27 +51,23 @@ describe("runAgentJsonPassthrough", () => { })); const { exit, proc, stderr, stdout } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "/usr/local/bin/openshell", stdinIsTty: () => false, - spawnSync, + runDispatch, }), - ).toThrow("__exit:0"); + ).rejects.toThrow("__exit:0"); - expect(spawnSync).toHaveBeenCalledWith( + expect(runDispatch).toHaveBeenCalledWith( "/usr/local/bin/openshell", buildOpenshellExecArgs( "alpha", wrapOpenClawAgentCommandWithRuntimeEnv(["openclaw", "agent", "--json"]), { tty: false }, ), - expect.objectContaining({ - encoding: "utf-8", - maxBuffer: 64 * 1024 * 1024, - stdio: ["inherit", "pipe", "pipe"], - }), + { stdinIsTty: false }, ); expect(stdout.join("")).toBe(payload); expect(() => JSON.parse(stdout.join(""))).not.toThrow(); @@ -83,33 +77,33 @@ describe("runAgentJsonPassthrough", () => { expect(exit).toHaveBeenCalledWith(0); }); - it("surfaces spawn errors and exits with the computed transport failure code", () => { - const spawnSync = vi.fn(() => ({ + it("surfaces spawn errors and exits with the computed transport failure code", async () => { + const runDispatch = vi.fn(async () => ({ status: null, signal: null, stdout: "", stderr: "", - error: new Error("spawnSync openshell ENOENT"), + error: new Error("runDispatch openshell ENOENT"), pid: 0, output: [null, "", ""], })); const { exit, proc, stderr } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "openshell", stdinIsTty: () => false, - spawnSync, + runDispatch, }), - ).toThrow("__exit:1"); + ).rejects.toThrow("__exit:1"); expect(stderr.join("")).toContain("Failed to invoke openshell"); - expect(stderr.join("")).toContain("spawnSync openshell ENOENT"); + expect(stderr.join("")).toContain("runDispatch openshell ENOENT"); expect(exit).toHaveBeenCalledWith(1); }); - it("does not treat stderr JSON diagnostics as agent provenance", () => { + it("does not treat stderr JSON diagnostics as agent provenance", async () => { const stdoutPayload = JSON.stringify({ result: { payloads: [{ text: "OK" }] } }); const stderrPayload = JSON.stringify({ messages: [ @@ -123,7 +117,7 @@ describe("runAgentJsonPassthrough", () => { }, ], }); - const spawnSync = vi.fn(() => ({ + const runDispatch = vi.fn(async () => ({ status: 0, signal: null, stdout: stdoutPayload, @@ -133,22 +127,22 @@ describe("runAgentJsonPassthrough", () => { })); const { proc, stderr } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "/usr/local/bin/openshell", stdinIsTty: () => false, - spawnSync, + runDispatch, }), - ).toThrow("__exit:0"); + ).rejects.toThrow("__exit:0"); expect(stderr.join("")).toContain("stderr-diagnostic"); expect(stderr.join("")).not.toContain("[openclaw provenance]"); }); - it("preserves forwarded output and remote exit code when provenance parsing fails", () => { + it("preserves forwarded output and remote exit code when provenance parsing fails", async () => { const stdoutPayload = JSON.stringify({ result: { payloads: [{ text: "OK" }] } }); - const spawnSync = vi.fn(() => ({ + const runDispatch = vi.fn(async () => ({ status: 7, signal: null, stdout: stdoutPayload, @@ -158,7 +152,7 @@ describe("runAgentJsonPassthrough", () => { })); const { exit, proc, stderr, stdout } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "/usr/local/bin/openshell", @@ -166,9 +160,9 @@ describe("runAgentJsonPassthrough", () => { provenanceLines: () => { throw new SyntaxError("Unexpected token in OpenClaw JSON output"); }, - spawnSync, + runDispatch, }), - ).toThrow("__exit:7"); + ).rejects.toThrow("__exit:7"); expect(stdout.join("")).toBe(stdoutPayload); expect(stderr.join("")).toContain("openclaw warning"); @@ -178,28 +172,34 @@ describe("runAgentJsonPassthrough", () => { expect(exit).toHaveBeenCalledWith(7); }); - it("pins the sandbox's owning gateway in the dispatched argv", () => { + it("pins the sandbox's owning gateway in the dispatched argv", async () => { const payload = JSON.stringify({ result: { payloads: [{ text: "OK" }] } }); - const spawnSync = vi.fn((_binary: string, _args: readonly string[], _options: object) => ({ - status: 0, - signal: null, - stdout: payload, - stderr: "openclaw warning\n", - pid: 123, - output: [null, payload, "openclaw warning\n"], - })); + const runDispatch = vi.fn( + async ( + _binary: string, + _args: readonly string[], + _options?: { maxBufferBytes?: number; stdinIsTty?: boolean }, + ) => ({ + status: 0, + signal: null, + stdout: payload, + stderr: "openclaw warning\n", + pid: 123, + output: [null, payload, "openclaw warning\n"], + }), + ); const { proc } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => "nemoclaw-8081", getOpenshellBinary: () => "openshell", - spawnSync, + runDispatch, stdinIsTty: () => false, }), - ).toThrow("__exit:0"); + ).rejects.toThrow("__exit:0"); - expect(spawnSync.mock.calls[0]?.[1].slice(0, 6)).toEqual([ + expect(runDispatch.mock.calls[0]?.[1].slice(0, 6)).toEqual([ "sandbox", "exec", "--name", @@ -209,10 +209,10 @@ describe("runAgentJsonPassthrough", () => { ]); }); - it("withholds an interactive terminal from the non-interactive dispatch", () => { + it("withholds an interactive terminal from the non-interactive dispatch", async () => { const payload = JSON.stringify({ result: { payloads: [{ text: "OK" }] } }); - const spawnSync = vi.fn( - (_binary: string, _args: readonly string[], _options: SpawnSyncOptions) => ({ + const runDispatch = vi.fn( + async (_binary: string, _args: readonly string[], _options?: { stdinIsTty?: boolean }) => ({ status: 0, signal: null, stdout: payload, @@ -223,19 +223,19 @@ describe("runAgentJsonPassthrough", () => { ); const { proc } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "openshell", - spawnSync, + runDispatch, stdinIsTty: () => true, }), - ).toThrow("__exit:0"); + ).rejects.toThrow("__exit:0"); - expect(spawnSync.mock.calls[0]?.[2].stdio).toEqual(["ignore", "pipe", "pipe"]); + expect(runDispatch.mock.calls[0]?.[2]).toEqual({ stdinIsTty: true }); }); - it("exits non-zero for a turn the payload marks incomplete, after preserving the trace", () => { + it("exits non-zero for a turn the payload marks incomplete, after preserving the trace", async () => { const payload = JSON.stringify({ status: "ok", summary: "completed", @@ -249,7 +249,7 @@ describe("runAgentJsonPassthrough", () => { }, }, }); - const spawnSync = vi.fn(() => ({ + const runDispatch = vi.fn(async () => ({ status: 0, signal: null, stdout: payload, @@ -259,14 +259,14 @@ describe("runAgentJsonPassthrough", () => { })); const { exit, proc, stderr, stdout } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "openshell", - spawnSync, + runDispatch, stdinIsTty: () => false, }), - ).toThrow("__exit:1"); + ).rejects.toThrow("__exit:1"); expect(stdout.join("")).toBe(payload); expect(stderr.join("")).toContain("did not complete"); @@ -281,12 +281,12 @@ describe("runAgentJsonPassthrough", () => { expect(exit).toHaveBeenCalledWith(1); }); - it("exits non-zero when an incomplete response omits optional payloads", () => { + it("exits non-zero when an incomplete response omits optional payloads", async () => { const payload = JSON.stringify({ status: "ok", result: { meta: { error: { kind: "incomplete_turn" } } }, }); - const spawnSync = vi.fn(() => ({ + const runDispatch = vi.fn(async () => ({ status: 0, signal: null, stdout: payload, @@ -296,26 +296,26 @@ describe("runAgentJsonPassthrough", () => { })); const { exit, proc, stdout } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "openshell", - spawnSync, + runDispatch, stdinIsTty: () => false, }), - ).toThrow("__exit:1"); + ).rejects.toThrow("__exit:1"); expect(stdout.join("")).toBe(payload); expect(exit).toHaveBeenCalledWith(1); }); - it("keeps a completed turn at exit 0 so the incomplete-turn check does not misfire", () => { + it("keeps a completed turn at exit 0 so the incomplete-turn check does not misfire", async () => { const payload = JSON.stringify({ status: "ok", summary: "completed", result: { payloads: [{ text: "PONG" }], meta: { livenessState: "working" } }, }); - const spawnSync = vi.fn(() => ({ + const runDispatch = vi.fn(async () => ({ status: 0, signal: null, stdout: payload, @@ -325,19 +325,19 @@ describe("runAgentJsonPassthrough", () => { })); const { exit, proc } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "openshell", - spawnSync, + runDispatch, stdinIsTty: () => false, }), - ).toThrow("__exit:0"); + ).rejects.toThrow("__exit:0"); expect(exit).toHaveBeenCalledWith(0); }); - it("keeps a healthy response at exit 0 after a marker-bearing JSON log record", () => { + it("keeps a healthy response at exit 0 after a marker-bearing JSON log record", async () => { const payload = [ JSON.stringify({ event: "progress", meta: { replayInvalid: true } }), JSON.stringify({ @@ -345,7 +345,7 @@ describe("runAgentJsonPassthrough", () => { result: { payloads: [{ text: "done" }], meta: { livenessState: "working" } }, }), ].join("\n"); - const spawnSync = vi.fn(() => ({ + const runDispatch = vi.fn(async () => ({ status: 0, signal: null, stdout: payload, @@ -355,20 +355,20 @@ describe("runAgentJsonPassthrough", () => { })); const { exit, proc, stdout } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "openshell", - spawnSync, + runDispatch, stdinIsTty: () => false, }), - ).toThrow("__exit:0"); + ).rejects.toThrow("__exit:0"); expect(stdout.join("")).toBe(payload); expect(exit).toHaveBeenCalledWith(0); }); - it("keeps a completed turn at exit 0 when a tool result merely contains marker fields", () => { + it("keeps a completed turn at exit 0 when a tool result merely contains marker fields", async () => { const payload = JSON.stringify({ status: "ok", result: { @@ -385,7 +385,7 @@ describe("runAgentJsonPassthrough", () => { payloads: [{ text: "done" }], }, }); - const spawnSync = vi.fn(() => ({ + const runDispatch = vi.fn(async () => ({ status: 0, signal: null, stdout: payload, @@ -395,22 +395,22 @@ describe("runAgentJsonPassthrough", () => { })); const { exit, proc, stdout } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "openshell", - spawnSync, + runDispatch, stdinIsTty: () => false, }), - ).toThrow("__exit:0"); + ).rejects.toThrow("__exit:0"); expect(stdout.join("")).toBe(payload); expect(exit).toHaveBeenCalledWith(0); }); - it("preserves an upstream non-zero code instead of relabelling an incomplete turn", () => { + it("preserves an upstream non-zero code instead of relabelling an incomplete turn", async () => { const payload = JSON.stringify({ result: { meta: { error: { kind: "incomplete_turn" } } } }); - const spawnSync = vi.fn(() => ({ + const runDispatch = vi.fn(async () => ({ status: 7, signal: null, stdout: payload, @@ -420,20 +420,45 @@ describe("runAgentJsonPassthrough", () => { })); const { exit, proc } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "openshell", - spawnSync, + runDispatch, stdinIsTty: () => false, }), - ).toThrow("__exit:7"); + ).rejects.toThrow("__exit:7"); expect(exit).toHaveBeenCalledWith(7); }); - it("fails loud and keeps stdout empty when the dispatch delivers nothing", () => { - const spawnSync = vi.fn(() => ({ + it("returns exit 143 after preserving output from a SIGTERM-interrupted dispatch (#8723)", async () => { + const partial = JSON.stringify({ event: "progress", status: "running" }); + const runDispatch = vi.fn(async () => ({ + status: null, + signal: "SIGTERM" as const, + stdout: partial, + stderr: "agent turn interrupted\n", + })); + const { exit, proc, stderr, stdout } = makeProc(); + + await expect( + runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { + getGatewayName: () => null, + getOpenshellBinary: () => "openshell", + incompleteTurnSignal: () => null, + provenanceLines: () => [], + runDispatch, + }), + ).rejects.toThrow("__exit:143"); + + expect(stdout.join("")).toBe(partial); + expect(stderr.join("")).toContain("agent turn interrupted"); + expect(exit).toHaveBeenCalledWith(143); + }); + + it("fails loud and keeps stdout empty when the dispatch delivers nothing", async () => { + const runDispatch = vi.fn(async () => ({ status: 0, signal: null, stdout: "", @@ -443,14 +468,14 @@ describe("runAgentJsonPassthrough", () => { })); const { exit, proc, stderr, stdout } = makeProc(); - expect(() => + await expect( runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { getGatewayName: () => null, getOpenshellBinary: () => "openshell", - spawnSync, + runDispatch, stdinIsTty: () => false, }), - ).toThrow("__exit:1"); + ).rejects.toThrow("__exit:1"); expect(stdout).toEqual([]); expect(stderr.join("")).toContain("exited 0 without producing any output"); diff --git a/src/lib/actions/sandbox/agent/passthrough-json.ts b/src/lib/actions/sandbox/agent/passthrough-json.ts index dcdc66b3bce..03e0b308d63 100644 --- a/src/lib/actions/sandbox/agent/passthrough-json.ts +++ b/src/lib/actions/sandbox/agent/passthrough-json.ts @@ -1,8 +1,6 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { type SpawnSyncOptions, type SpawnSyncReturns, spawnSync } from "node:child_process"; - import { isStdinTty } from "../../../core/stdin"; import { openClawAgentIncompleteTurnSignal, @@ -16,8 +14,9 @@ import { } from "../exec"; import { getKnownSandboxTargetGatewayName } from "../gateway-target"; import { - agentDispatchStdio, + type AgentDispatchRunner, isSilentAgentDispatch, + runAgentDispatch, SILENT_AGENT_DISPATCH_EXIT_CODE, } from "./passthrough-dispatch"; import { @@ -25,8 +24,6 @@ import { writeSilentAgentDispatchFailure, } from "./passthrough-help"; -const AGENT_JSON_MAX_BUFFER_BYTES = 64 * 1024 * 1024; - /** Exit code for a turn the payload itself marks incomplete or abandoned. */ export const INCOMPLETE_AGENT_TURN_EXIT_CODE = 1; @@ -42,18 +39,9 @@ export type AgentJsonPassthroughDeps = { stdinIsTty?: () => boolean; provenanceLines?: (raw: string) => string[]; incompleteTurnSignal?: (raw: string) => OpenClawIncompleteTurnSignal | null; - spawnSync?: ( - command: string, - args: readonly string[], - options: SpawnSyncOptions, - ) => SpawnSyncReturns; + runDispatch?: AgentDispatchRunner; }; -function text(value: string | Buffer | null | undefined): string { - if (Buffer.isBuffer(value)) return value.toString("utf-8"); - return typeof value === "string" ? value : ""; -} - export function defaultGetOpenshellBinary(): string { // Lazy require keeps this module unit-testable under Vitest's TS loader; the // OpenShell runtime imports runner/platform modules that only exist in built @@ -72,15 +60,14 @@ function writeProvenanceBlock( proc.stderr.write(`${stderr && !stderr.endsWith("\n") ? "\n" : ""}${lines.join("\n")}\n`); } -export function runAgentJsonPassthrough( +export async function runAgentJsonPassthrough( sandboxName: string, command: readonly string[], proc: AgentJsonPassthroughProcess = process, deps: AgentJsonPassthroughDeps = {}, -): never { +): Promise { const binary = (deps.getOpenshellBinary ?? defaultGetOpenshellBinary)(); - const spawnSyncImpl = deps.spawnSync ?? spawnSync; - const result = spawnSyncImpl( + const result = await (deps.runDispatch ?? runAgentDispatch)( binary, buildOpenshellExecArgs( sandboxName, @@ -89,13 +76,10 @@ export function runAgentJsonPassthrough( (deps.getGatewayName ?? getKnownSandboxTargetGatewayName)(sandboxName) ?? undefined, ), { - encoding: "utf-8", - maxBuffer: AGENT_JSON_MAX_BUFFER_BYTES, - stdio: agentDispatchStdio((deps.stdinIsTty ?? isStdinTty)()), + stdinIsTty: (deps.stdinIsTty ?? isStdinTty)(), }, ); - const stdout = text(result.stdout); - const stderr = text(result.stderr); + const { stderr, stdout } = result; // Ahead of the stdout write so machine-readable stdout stays byte-empty and // no provenance line is appended for a turn that never ran. diff --git a/src/lib/actions/sandbox/agent/passthrough.test.ts b/src/lib/actions/sandbox/agent/passthrough.test.ts index b17d6939ec7..abef941e6bb 100644 --- a/src/lib/actions/sandbox/agent/passthrough.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough.test.ts @@ -40,8 +40,8 @@ vi.mock("../exec", () => ({ buildOpenshellExecArgs: vi.fn((_sb: string, cmd: readonly string[]) => cmd), wrapExecCommandWithRuntimeEnv: vi.fn((cmd: readonly string[]) => cmd), wrapOpenClawAgentCommandWithRuntimeEnv: vi.fn((cmd: readonly string[]) => cmd), - computeExitCode: vi.fn((result: { status: number | null }) => ({ - code: result.status ?? 1, + computeExitCode: vi.fn((result: { signal?: NodeJS.Signals | null; status: number | null }) => ({ + code: result.status ?? (result.signal === "SIGTERM" ? 143 : 1), errorMessage: null, })), })); @@ -820,12 +820,12 @@ describe("runAgentNonJsonPassthrough", () => { }; } - function makeSpawnMock( + function makeDispatchMock( stdout: string, stderr: string, status: number | null = 0, - ): NonNullable { - return vi.fn(() => ({ + ): NonNullable { + return vi.fn(async () => ({ stdout, stderr, status, @@ -838,15 +838,15 @@ describe("runAgentNonJsonPassthrough", () => { const stubBinary = () => "/usr/local/bin/openshell"; - it("emits a clean embedded-fallback error and exits 1 when EMBEDDED FALLBACK appears in stdout", () => { + it("emits a clean embedded-fallback error and exits 1 when EMBEDDED FALLBACK appears in stdout", async () => { const { stderrWrites, stdoutWrites, exit, proc } = makeNonJsonProcMock(); - const spawnSyncMock = makeSpawnMock("EMBEDDED FALLBACK: using local model\nPONG\n", "", 0); - expect(() => + const runDispatchMock = makeDispatchMock("EMBEDDED FALLBACK: using local model\nPONG\n", "", 0); + await expect( runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "--agent", "main"], proc, { getOpenshellBinary: stubBinary, - spawnSync: spawnSyncMock, + runDispatch: runDispatchMock, }), - ).toThrow("__exit:1"); + ).rejects.toThrow("__exit:1"); expect(exit).toHaveBeenCalledWith(1); const errText = stderrWrites.join(""); expect(errText).toMatch(/embedded-fallback mode in sandbox 'my-sb'/); @@ -856,59 +856,79 @@ describe("runAgentNonJsonPassthrough", () => { expect(stdoutWrites.join("")).toBe(""); }); - it("emits a clean embedded-fallback error and exits 1 when [agent/embedded] appears in stderr", () => { + it("emits a clean embedded-fallback error and exits 1 when [agent/embedded] appears in stderr", async () => { const { stderrWrites, exit, proc } = makeNonJsonProcMock(); - const spawnSyncMock = makeSpawnMock( + const runDispatchMock = makeDispatchMock( "", "[agent/embedded] transport active\nsome response\n", 0, ); - expect(() => + await expect( runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "--agent", "main"], proc, { getOpenshellBinary: stubBinary, - spawnSync: spawnSyncMock, + runDispatch: runDispatchMock, }), - ).toThrow("__exit:1"); + ).rejects.toThrow("__exit:1"); expect(exit).toHaveBeenCalledWith(1); expect(stderrWrites.join("")).toMatch(/embedded-fallback mode/); }); - it("passes through clean stdout and exits with the real exit code when no embedded-fallback pattern is found", () => { + it("passes through clean stdout and exits with the real exit code when no embedded-fallback pattern is found", async () => { const { stdoutWrites, stderrWrites, exit, proc } = makeNonJsonProcMock(); - const spawnSyncMock = makeSpawnMock("PONG\n", "", 0); - expect(() => + const runDispatchMock = makeDispatchMock("PONG\n", "", 0); + await expect( runAgentNonJsonPassthrough( "my-sb", ["openclaw", "agent", "--agent", "main", "-m", "ping"], proc, { getOpenshellBinary: stubBinary, - spawnSync: spawnSyncMock, + runDispatch: runDispatchMock, }, ), - ).toThrow("__exit:0"); + ).rejects.toThrow("__exit:0"); expect(exit).toHaveBeenCalledWith(0); expect(stdoutWrites.join("")).toBe("PONG\n"); expect(stderrWrites.join("")).toBe(""); }); - it("passes through non-zero exit code on clean failure without embedded-fallback", () => { + it("passes through non-zero exit code on clean failure without embedded-fallback", async () => { const { stderrWrites, exit, proc } = makeNonJsonProcMock(); - const spawnSyncMock = makeSpawnMock("", "Error: agent session not found\n", 1); - expect(() => + const runDispatchMock = makeDispatchMock("", "Error: agent session not found\n", 1); + await expect( runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "--agent", "main"], proc, { getOpenshellBinary: stubBinary, - spawnSync: spawnSyncMock, + runDispatch: runDispatchMock, }), - ).toThrow("__exit:1"); + ).rejects.toThrow("__exit:1"); expect(exit).toHaveBeenCalledWith(1); expect(stderrWrites.join("")).toContain("Error: agent session not found"); }); - it("fails loud instead of reporting success when the dispatch delivers nothing", () => { + it("returns exit 143 after the supervised OpenShell child receives SIGTERM (#8723)", async () => { + const { stderrWrites, exit, proc } = makeNonJsonProcMock(); + const runDispatchMock = vi.fn(async () => ({ + status: null, + signal: "SIGTERM" as const, + stdout: "", + stderr: "agent turn interrupted\n", + })); + + await expect( + runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "--agent", "main"], proc, { + getOpenshellBinary: stubBinary, + runDispatch: runDispatchMock, + }), + ).rejects.toThrow("__exit:143"); + + expect(exit).toHaveBeenCalledWith(143); + expect(stderrWrites.join("")).toContain("agent turn interrupted"); + }); + + it("fails loud instead of reporting success when the dispatch delivers nothing", async () => { const { stdoutWrites, stderrWrites, exit, proc } = makeNonJsonProcMock(); - const spawnSyncMock = makeSpawnMock("", "", 0); - expect(() => + const runDispatchMock = makeDispatchMock("", "", 0); + await expect( runAgentNonJsonPassthrough( "my-sb", ["openclaw", "agent", "--session-key", "agent:main:main", "-m", "ping"], @@ -916,41 +936,41 @@ describe("runAgentNonJsonPassthrough", () => { { getGatewayName: () => null, getOpenshellBinary: stubBinary, - spawnSync: spawnSyncMock, + runDispatch: runDispatchMock, stdinIsTty: () => false, }, ), - ).toThrow("__exit:1"); + ).rejects.toThrow("__exit:1"); expect(exit).toHaveBeenCalledWith(1); expect(stdoutWrites).toEqual([]); expect(stderrWrites.join("")).toContain("without producing any output"); }); - it("keeps a stderr-only turn a success so quiet turns do not misfire", () => { + it("keeps a stderr-only turn a success so quiet turns do not misfire", async () => { const { exit, proc } = makeNonJsonProcMock(); - const spawnSyncMock = makeSpawnMock("", "openclaw warning\n", 0); - expect(() => + const runDispatchMock = makeDispatchMock("", "openclaw warning\n", 0); + await expect( runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "--agent", "main"], proc, { getGatewayName: () => null, getOpenshellBinary: stubBinary, - spawnSync: spawnSyncMock, + runDispatch: runDispatchMock, stdinIsTty: () => false, }), - ).toThrow("__exit:0"); + ).rejects.toThrow("__exit:0"); expect(exit).toHaveBeenCalledWith(0); }); - it("pins the sandbox's owning gateway when building the dispatch argv", () => { + it("pins the sandbox's owning gateway when building the dispatch argv", async () => { const { proc } = makeNonJsonProcMock(); - const spawnSyncMock = makeSpawnMock("PONG\n", "", 0); - expect(() => + const runDispatchMock = makeDispatchMock("PONG\n", "", 0); + await expect( runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "--agent", "main"], proc, { getGatewayName: () => "nemoclaw-8081", getOpenshellBinary: stubBinary, - spawnSync: spawnSyncMock, + runDispatch: runDispatchMock, stdinIsTty: () => false, }), - ).toThrow("__exit:0"); + ).rejects.toThrow("__exit:0"); expect(buildOpenshellExecArgs).toHaveBeenCalledWith( "my-sb", expect.anything(), @@ -959,17 +979,17 @@ describe("runAgentNonJsonPassthrough", () => { ); }); - it("withholds an interactive terminal from the non-interactive dispatch", () => { + it("withholds an interactive terminal from the non-interactive dispatch", async () => { const { proc } = makeNonJsonProcMock(); - const spawnSyncMock = makeSpawnMock("PONG\n", "", 0); - expect(() => + const runDispatchMock = makeDispatchMock("PONG\n", "", 0); + await expect( runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "--agent", "main"], proc, { getGatewayName: () => null, getOpenshellBinary: stubBinary, - spawnSync: spawnSyncMock, + runDispatch: runDispatchMock, stdinIsTty: () => true, }), - ).toThrow("__exit:0"); - expect(vi.mocked(spawnSyncMock).mock.calls[0]?.[2].stdio).toEqual(["ignore", "pipe", "pipe"]); + ).rejects.toThrow("__exit:0"); + expect(vi.mocked(runDispatchMock).mock.calls[0]?.[2]).toEqual({ stdinIsTty: true }); }); }); diff --git a/src/lib/actions/sandbox/agent/passthrough.ts b/src/lib/actions/sandbox/agent/passthrough.ts index eed0235f53e..0cfa6458824 100644 --- a/src/lib/actions/sandbox/agent/passthrough.ts +++ b/src/lib/actions/sandbox/agent/passthrough.ts @@ -1,8 +1,6 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { type SpawnSyncOptions, type SpawnSyncReturns, spawnSync } from "node:child_process"; - // Source-of-truth boundary for the `nemoclaw agent` passthrough. // // The wrapper enforces three host-side mirrors of upstream contracts, one @@ -83,19 +81,18 @@ import { type SpawnSyncOptions, type SpawnSyncReturns, spawnSync } from "node:ch // 6. Dispatch delivery contract and stdin posture. Both captured transports // fail loud when the exec returns success with no bytes on either stream, // and neither hands an interactive terminal to the non-interactive -// dispatch. Both transports also pin the sandbox's owning gateway with an -// explicit `-g`, restoring the per-subprocess authority #7113 established -// for `execSandbox`; PR #8191 dropped it here when it moved this path off -// `execSandbox`, and the JSON path never had it. The source-boundary -// analysis and the classifier live in `passthrough-dispatch.ts`; the -// operator-facing failure text lives beside the help copy in -// `passthrough-help.ts`. +// dispatch. Both transports pin the sandbox's owning gateway and use the +// shared asynchronous exec supervisor, which forwards host termination to +// OpenShell before returning the signal-derived exit status. The complete +// source-boundary analysis and classifier live in +// `passthrough-dispatch.ts`; the operator-facing failure text lives beside +// the help copy in `passthrough-help.ts`. // // Regression tests: `passthrough.test.ts` covers the Hermes redirect, the -// forwarded argv, the registry-miss fallback to OpenClaw, registry and -// manifest-resolution fail-closed paths, quoted manifest command rejection, -// the enforced `--no-tty` argv shape, the non-Ready phase recovery path, the -// unparseable phase fail-closed path, the OpenClaw no-selector rejection, and +// forwarded argv, SIGTERM exit status, the registry-miss fallback to OpenClaw, +// registry and manifest-resolution fail-closed paths, quoted manifest command +// rejection, the enforced `--no-tty` argv shape, the non-Ready phase recovery +// path, the unparseable phase fail-closed path, the OpenClaw no-selector rejection, and // the `--flag=value` selector-acceptance branch, plus the OpenClaw JSON // captured transport path used to append failure provenance without polluting // machine-readable stdout. The focused shields and Ollama modules own their @@ -134,8 +131,9 @@ import { import { ensureLiveSandboxOrExit } from "../gateway-state"; import { getKnownSandboxTargetGatewayName } from "../gateway-target"; import { - agentDispatchStdio, + type AgentDispatchRunner, isSilentAgentDispatch, + runAgentDispatch, SILENT_AGENT_DISPATCH_EXIT_CODE, } from "./passthrough-dispatch"; import { @@ -151,10 +149,7 @@ import { import { OLLAMA_LOCAL_PROVIDER, runOllamaRestartRecovery } from "./passthrough-ollama-recovery"; import { maybeEmitShieldsRelockWarning } from "./passthrough-shields-warning"; -export { - hasAgentPassthroughHelpToken, - printAgentPassthroughHelp, -} from "./passthrough-help"; +export { hasAgentPassthroughHelpToken, printAgentPassthroughHelp } from "./passthrough-help"; const OPENCLAW_AGENT_VALUE_FLAGS = new Set([ "-a", @@ -182,33 +177,21 @@ const OPENCLAW_AGENT_BOOLEAN_FLAGS = new Set(["--deliver"]); const OPENCLAW_EMBEDDED_FALLBACK_PATTERN = /EMBEDDED FALLBACK|\[agent\/embedded\]|fallbackFrom[": ]+gateway|transport[": ]+embedded/i; -const AGENT_NON_JSON_MAX_BUFFER_BYTES = 64 * 1024 * 1024; - -function nonJsonAsText(value: string | Buffer | null | undefined): string { - if (Buffer.isBuffer(value)) return value.toString("utf-8"); - return typeof value === "string" ? value : ""; -} - export type AgentNonJsonPassthroughDeps = { getOpenshellBinary?: () => string; getGatewayName?: (sandboxName: string) => string | null; + runDispatch?: AgentDispatchRunner; stdinIsTty?: () => boolean; - spawnSync?: ( - command: string, - args: readonly string[], - options: SpawnSyncOptions, - ) => SpawnSyncReturns; }; -export function runAgentNonJsonPassthrough( +export async function runAgentNonJsonPassthrough( sandboxName: string, command: readonly string[], proc: NonNullable, deps: AgentNonJsonPassthroughDeps = {}, -): never { +): Promise { const binary = (deps.getOpenshellBinary ?? defaultGetOpenshellBinary)(); - const spawnSyncImpl = deps.spawnSync ?? spawnSync; - const result = spawnSyncImpl( + const result = await (deps.runDispatch ?? runAgentDispatch)( binary, buildOpenshellExecArgs( sandboxName, @@ -217,13 +200,10 @@ export function runAgentNonJsonPassthrough( (deps.getGatewayName ?? getKnownSandboxTargetGatewayName)(sandboxName) ?? undefined, ), { - encoding: "utf-8", - maxBuffer: AGENT_NON_JSON_MAX_BUFFER_BYTES, - stdio: agentDispatchStdio((deps.stdinIsTty ?? isStdinTty)()), + stdinIsTty: (deps.stdinIsTty ?? isStdinTty)(), }, ); - const stdout = nonJsonAsText(result.stdout); - const stderr = nonJsonAsText(result.stderr); + const { stderr, stdout } = result; if (isSilentAgentDispatch(result, stdout, stderr)) { writeSilentAgentDispatchFailure(proc, sandboxName, command); @@ -649,7 +629,7 @@ export async function runAgentPassthrough( } if (isOpenClawPassthroughCommand(command) && requestsOpenClawJsonOutput(extraArgs)) { const execJson = deps.execJson ?? runAgentJsonPassthrough; - execJson(sandboxName, command, { + await execJson(sandboxName, command, { exit: proc.exit.bind(proc), stdout: proc.stdout ?? process.stdout, stderr: proc.stderr, @@ -658,7 +638,7 @@ export async function runAgentPassthrough( } if (isOpenClawPassthroughCommand(command)) { const execNonJson = deps.execNonJson ?? runAgentNonJsonPassthrough; - execNonJson(sandboxName, command, proc); + await execNonJson(sandboxName, command, proc); return; } const exec = deps.exec ?? execSandbox; From 5140eab7d88d9d392e9603ea15774f21275a2603 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Thu, 13 Aug 2026 14:51:34 -0700 Subject: [PATCH 2/2] fix(agent): enforce combined dispatch capture bound Signed-off-by: Prekshi Vyas --- .../agent/passthrough-dispatch.test.ts | 31 +++++++++++++++++-- .../sandbox/agent/passthrough-dispatch.ts | 25 +++++++++------ .../actions/sandbox/agent/passthrough.test.ts | 7 ++--- src/lib/core/process-exit.test.ts | 12 ++++--- 4 files changed, 53 insertions(+), 22 deletions(-) diff --git a/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts b/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts index 1d5744186e5..6405484c2c3 100644 --- a/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts @@ -12,7 +12,7 @@ import { runAgentDispatch, SILENT_AGENT_DISPATCH_EXIT_CODE, } from "./passthrough-dispatch"; -import type { SandboxExecSignalSource } from "../exec"; +import { computeExitCode, type SandboxExecSignalSource } from "../exec"; function dispatchHarness() { const childEvents = new EventEmitter(); @@ -79,9 +79,36 @@ describe("runAgentDispatch", () => { const result = await pending; expect(harness.child.kill).toHaveBeenCalledWith("SIGTERM"); - expect(result.error).toEqual(new Error("agent stdout exceeded the 4-byte capture limit")); + expect(result.error).toEqual( + new Error("agent output exceeded the 4-byte combined capture limit"), + ); + expect(computeExitCode(result)).toEqual({ + code: 1, + errorMessage: "agent output exceeded the 4-byte combined capture limit", + }); expect(result.stdout).toBe(""); }); + + it("enforces one capture bound across stdout and stderr", async () => { + const harness = dispatchHarness(); + const pending = runAgentDispatch( + "openshell", + ["sandbox", "exec", "--name", "alpha", "--", "openclaw", "agent"], + { maxBufferBytes: 6, stdinIsTty: false }, + { signalSource: harness.signalSource, spawnChild: () => harness.child }, + ); + + harness.stdout.emit("data", "1234"); + harness.stderr.emit("data", "567"); + + const result = await pending; + expect(harness.child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(result.error).toEqual( + new Error("agent output exceeded the 6-byte combined capture limit"), + ); + expect(result.stdout).toBe("1234"); + expect(result.stderr).toBe(""); + }); }); describe("isSilentAgentDispatch", () => { diff --git a/src/lib/actions/sandbox/agent/passthrough-dispatch.ts b/src/lib/actions/sandbox/agent/passthrough-dispatch.ts index 5a08625d981..5e25216d9a2 100644 --- a/src/lib/actions/sandbox/agent/passthrough-dispatch.ts +++ b/src/lib/actions/sandbox/agent/passthrough-dispatch.ts @@ -84,6 +84,11 @@ type AgentDispatchReadable = { on(event: "data", listener: (chunk: Buffer | string) => void): unknown; }; +type AgentDispatchCaptureBudget = { + bytes: number; + overflowed: boolean; +}; + export type AgentDispatchChild = SandboxExecChild & { stderr: AgentDispatchReadable | null; stdout: AgentDispatchReadable | null; @@ -116,26 +121,25 @@ const defaultAgentDispatchSpawner: AgentDispatchSpawner = (binary, args, stdio) function captureAgentDispatchStream( stream: AgentDispatchReadable | null, - streamName: "stderr" | "stdout", child: AgentDispatchChild, chunks: Buffer[], maxBufferBytes: number, + budget: AgentDispatchCaptureBudget, setOverflowError: (error: Error) => void, ): void { - let size = 0; - let overflowed = false; stream?.on("data", (chunk) => { - if (overflowed) return; + if (budget.overflowed) return; const data = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); - size += data.byteLength; - if (size > maxBufferBytes) { - overflowed = true; + const nextSize = budget.bytes + data.byteLength; + if (nextSize > maxBufferBytes) { + budget.overflowed = true; setOverflowError( - new Error(`agent ${streamName} exceeded the ${maxBufferBytes}-byte capture limit`), + new Error(`agent output exceeded the ${maxBufferBytes}-byte combined capture limit`), ); if (child.exitCode === null && child.signalCode === null) child.kill("SIGTERM"); return; } + budget.bytes = nextSize; chunks.push(data); }); } @@ -155,6 +159,7 @@ export async function runAgentDispatch( ): Promise { const stderrChunks: Buffer[] = []; const stdoutChunks: Buffer[] = []; + const captureBudget: AgentDispatchCaptureBudget = { bytes: 0, overflowed: false }; let overflowError: Error | undefined; const maxBufferBytes = options.maxBufferBytes ?? DEFAULT_AGENT_DISPATCH_MAX_BUFFER_BYTES; const spawnChild = deps.spawnChild ?? defaultAgentDispatchSpawner; @@ -173,18 +178,18 @@ export async function runAgentDispatch( }; captureAgentDispatchStream( child.stdout, - "stdout", child, stdoutChunks, maxBufferBytes, + captureBudget, setOverflowError, ); captureAgentDispatchStream( child.stderr, - "stderr", child, stderrChunks, maxBufferBytes, + captureBudget, setOverflowError, ); return child; diff --git a/src/lib/actions/sandbox/agent/passthrough.test.ts b/src/lib/actions/sandbox/agent/passthrough.test.ts index abef941e6bb..3ac7c9b11f3 100644 --- a/src/lib/actions/sandbox/agent/passthrough.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough.test.ts @@ -35,15 +35,12 @@ const isTerminalAgentMock = vi.hoisted(() => vi.fn((agent: { runtime?: { kind?: string } }) => agent.runtime?.kind === "terminal"), ); -vi.mock("../exec", () => ({ +vi.mock("../exec", async (importOriginal) => ({ + ...(await importOriginal()), execSandbox: execMock, buildOpenshellExecArgs: vi.fn((_sb: string, cmd: readonly string[]) => cmd), wrapExecCommandWithRuntimeEnv: vi.fn((cmd: readonly string[]) => cmd), wrapOpenClawAgentCommandWithRuntimeEnv: vi.fn((cmd: readonly string[]) => cmd), - computeExitCode: vi.fn((result: { signal?: NodeJS.Signals | null; status: number | null }) => ({ - code: result.status ?? (result.signal === "SIGTERM" ? 143 : 1), - errorMessage: null, - })), })); vi.mock("../gateway-state", () => ({ ensureLiveSandboxOrExit: ensureLiveMock })); vi.mock("../../../state/registry", () => ({ getSandbox: getSandboxMock })); diff --git a/src/lib/core/process-exit.test.ts b/src/lib/core/process-exit.test.ts index 60bfae8ec96..098b4b231bf 100644 --- a/src/lib/core/process-exit.test.ts +++ b/src/lib/core/process-exit.test.ts @@ -9,14 +9,16 @@ describe("spawnExitCode", () => { ["zero status", { status: 0 }, 0], ["nonzero status", { status: 42 }, 42], ["status before signal", { status: 7, signal: "SIGTERM" }, 7], + ["SIGINT", { status: null, signal: "SIGINT" }, 130], ["SIGTERM", { status: null, signal: "SIGTERM" }, 143], ["SIGKILL", { status: null, signal: "SIGKILL" }, 137], ["missing signal", { status: null }, 1], ["null signal", { status: null, signal: null }, 1], ["unknown signal", { status: null, signal: "SIGBOGUS" as NodeJS.Signals }, 1], - ] satisfies Array< - [string, Parameters[0], number] - >)("normalizes %s (#5936)", (_label, result, expected) => { - expect(spawnExitCode(result)).toBe(expected); - }); + ] satisfies Array<[string, Parameters[0], number]>)( + "normalizes %s (#5936)", + (_label, result, expected) => { + expect(spawnExitCode(result)).toBe(expected); + }, + ); });