diff --git a/src/agents/bash-tools.exec-runtime-output.test.ts b/src/agents/bash-tools.exec-runtime-output.test.ts new file mode 100644 index 000000000000..e7d9f4631679 --- /dev/null +++ b/src/agents/bash-tools.exec-runtime-output.test.ts @@ -0,0 +1,365 @@ +import { expectDefined } from "@openclaw/normalization-core"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import type { ManagedRun } from "../process/supervisor/index.js"; +import type { SpawnInput } from "../process/supervisor/types.js"; +import { MAX_SAFE_TIMEOUT_DELAY_MS } from "../utils/timer-delay.js"; + +const requestHeartbeatMock = vi.hoisted(() => vi.fn()); +const enqueueSystemEventWithReceiptMock = vi.hoisted(() => vi.fn()); +const supervisorMock = vi.hoisted(() => ({ + spawn: vi.fn(), +})); + +vi.mock("../infra/heartbeat-wake.js", () => ({ + requestHeartbeat: requestHeartbeatMock, +})); + +vi.mock("../infra/system-events.js", () => ({ + enqueueSystemEventWithReceipt: enqueueSystemEventWithReceiptMock, +})); + +vi.mock("../process/supervisor/index.js", () => ({ + getProcessSupervisor: () => ({ + spawn: supervisorMock.spawn, + }), +})); + +let markBackgrounded: typeof import("./bash-process-registry.js").markBackgrounded; +let resetProcessRegistryForTests: typeof import("./bash-process-registry.test-support.js").resetProcessRegistryForTests; +let runExecProcess: typeof import("./bash-tools.exec-runtime.js").runExecProcess; + +beforeAll(async () => { + ({ markBackgrounded } = await import("./bash-process-registry.js")); + ({ resetProcessRegistryForTests } = await import("./bash-process-registry.test-support.js")); + ({ runExecProcess } = await import("./bash-tools.exec-runtime.js")); +}); + +beforeEach(() => { + resetProcessRegistryForTests(); + requestHeartbeatMock.mockClear(); + enqueueSystemEventWithReceiptMock.mockReset(); + enqueueSystemEventWithReceiptMock.mockReturnValue(vi.fn(() => true)); + supervisorMock.spawn.mockReset(); +}); + +afterEach(() => { + resetProcessRegistryForTests(); +}); + +function successfulSupervisorRun() { + return { + runId: "mock-run", + startedAtMs: Date.now(), + wait: async () => ({ + reason: "exit" as const, + exitCode: 0, + exitSignal: null, + durationMs: 0, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }), + cancel: vi.fn(), + }; +} + +function runtimeManagedRun(input: SpawnInput): ManagedRun { + return { + runId: input.runId ?? "test-run", + pid: 1234, + startedAtMs: Date.now(), + stdin: { write: vi.fn(), end: vi.fn(), destroy: vi.fn() }, + cancel: vi.fn(), + wait: vi.fn(async () => ({ + reason: "exit" as const, + exitCode: 0, + exitSignal: null, + durationMs: 1, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + })), + }; +} + +function requireSystemEventCall(): [string, Record] { + const call = enqueueSystemEventWithReceiptMock.mock.calls[0]; + if (!call) { + throw new Error("expected system event call"); + } + return call as [string, Record]; +} + +function requireHeartbeatCall(): Record { + const call = requestHeartbeatMock.mock.calls[0]; + if (!call) { + throw new Error("expected heartbeat call"); + } + return call[0] as Record; +} + +describe("exec notifyOnExit suppression", () => { + async function runBackgroundedExit(params: { + reason: "manual-cancel" | "overall-timeout"; + stdout?: string; + }) { + supervisorMock.spawn.mockImplementationOnce( + async (input: { onStdout?: (chunk: string) => void }) => { + if (params.stdout) { + input.onStdout?.(params.stdout); + } + return { + runId: "run-1", + startedAtMs: Date.now(), + pid: 123, + wait: async () => { + await new Promise((resolve) => { + setImmediate(resolve); + }); + return { + reason: params.reason, + exitCode: null, + exitSignal: "SIGKILL", + durationMs: 10, + stdout: "", + stderr: "", + timedOut: params.reason === "overall-timeout", + noOutputTimedOut: false, + }; + }, + cancel: vi.fn(), + }; + }, + ); + + const run = await runExecProcess({ + command: "sleep 999", + workdir: "/tmp", + env: {}, + usePty: false, + warnings: [], + maxOutput: 1000, + pendingMaxOutput: 1000, + notifyOnExit: true, + notifyOnExitEmptySuccess: false, + sessionKey: "agent:main:main", + timeoutSec: null, + }); + markBackgrounded(run.session); + return await run.promise; + } + + it("keeps manual-cancelled no-output background execs silent", async () => { + const outcome = await runBackgroundedExit({ reason: "manual-cancel" }); + + expect(outcome.status).toBe("failed"); + expect(enqueueSystemEventWithReceiptMock).not.toHaveBeenCalled(); + expect(requestHeartbeatMock).not.toHaveBeenCalled(); + }); + + it("notifies for manual-cancelled background execs with output", async () => { + await runBackgroundedExit({ reason: "manual-cancel", stdout: "partial output\n" }); + + const [message, options] = requireSystemEventCall(); + expect(message).toContain("partial output"); + expect(options.sessionKey).toBe("agent:main:main"); + expect(requestHeartbeatMock).toHaveBeenCalledTimes(1); + const heartbeat = requireHeartbeatCall(); + expect(heartbeat.coalesceMs).toBe(0); + expect(heartbeat.reason).toBe("exec-event"); + expect(heartbeat.sessionKey).toBe("agent:main:main"); + }); + + it("still notifies for no-output background exec timeouts", async () => { + await runBackgroundedExit({ reason: "overall-timeout" }); + + const [message, options] = requireSystemEventCall(); + expect(message).toContain("Exec failed"); + expect(message).toContain("external side effects may already have completed"); + expect(message).toContain("Verify the resulting state before retrying"); + expect(message).toContain("Do not automatically rerun non-idempotent commands"); + expect(options.sessionKey).toBe("agent:main:main"); + expect(requestHeartbeatMock).toHaveBeenCalledTimes(1); + const heartbeat = requireHeartbeatCall(); + expect(heartbeat.coalesceMs).toBe(0); + expect(heartbeat.reason).toBe("exec-event"); + expect(heartbeat.sessionKey).toBe("agent:main:main"); + }); + + it("keeps background exec exit-notification snippets on a UTF-16 boundary", async () => { + const head = "a".repeat(178); + const overflowingOutput = `${head}🎉${"b".repeat(30)}`; + await runBackgroundedExit({ reason: "manual-cancel", stdout: overflowingOutput }); + + const [message] = requireSystemEventCall(); + const loneSurrogate = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { + const prefix = "a".repeat(101); + const tailHead = "b".repeat(179); + const overflowingOutput = `${prefix}🎉${tailHead}${"c".repeat(220)}`; + await runBackgroundedExit({ reason: "manual-cancel", stdout: overflowingOutput }); + + const [message] = requireSystemEventCall(); + const loneSurrogate = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { + it("normalizes non-finite and oversized exec timeouts before spawning", async () => { + supervisorMock.spawn.mockResolvedValue(successfulSupervisorRun()); + + const baseParams = { + command: "echo test", + workdir: "/tmp", + env: { PATH: "/usr/bin" }, + pathPrepend: [], + usePty: false, + warnings: [], + maxOutput: 1000, + pendingMaxOutput: 1000, + notifyOnExit: false, + }; + + await runExecProcess({ + ...baseParams, + timeoutSec: Number.POSITIVE_INFINITY, + }); + await runExecProcess({ + ...baseParams, + timeoutSec: 3_000_000, + }); + + expect(supervisorMock.spawn.mock.calls[0]?.[0].timeoutMs).toBeUndefined(); + expect(supervisorMock.spawn.mock.calls[1]?.[0].timeoutMs).toBe(MAX_SAFE_TIMEOUT_DELAY_MS); + }); + + it("wraps command with PATH export if OPENCLAW_PREPEND_PATH is present", async () => { + if (process.platform === "win32") { + return; + } + + supervisorMock.spawn.mockResolvedValueOnce(successfulSupervisorRun()); + + await runExecProcess({ + command: "echo test", + workdir: "/tmp", + env: { PATH: "/usr/bin" }, + pathPrepend: ["/custom/bin", "/opt/bin"], + usePty: false, + warnings: [], + maxOutput: 1000, + pendingMaxOutput: 1000, + notifyOnExit: false, + timeoutSec: null, + }); + + const spawnCall = expectDefined( + supervisorMock.spawn.mock.calls[0], + "supervisorMock.spawn.mock.calls[0] test invariant", + )[0]; + expect(spawnCall.argv.join(" ")).toContain( + 'export PATH="${OPENCLAW_PREPEND_PATH}${PATH:+:$PATH}"; unset OPENCLAW_PREPEND_PATH; echo test', + ); + }); + + it("does not wrap command on Windows", async () => { + if (process.platform !== "win32") { + return; + } + + supervisorMock.spawn.mockResolvedValueOnce(successfulSupervisorRun()); + await runExecProcess({ + command: "echo test", + workdir: "C:\\tmp", + env: { Path: "C:\\Windows\\System32" }, + pathPrepend: ["C:\\custom\\bin"], + usePty: false, + warnings: [], + maxOutput: 1000, + pendingMaxOutput: 1000, + notifyOnExit: false, + timeoutSec: null, + }); + + const spawnCall = expectDefined( + supervisorMock.spawn.mock.calls[0], + "supervisorMock.spawn.mock.calls[0] test invariant", + )[0]; + const commandStr = spawnCall.argv.join(" "); + expect(commandStr).not.toContain("export PATH="); + expect(commandStr).toContain("echo test"); + }); +}); + +describe("runExecProcess stream sanitization", () => { + function runStyledExec() { + return runExecProcess({ + command: "printf styled", + workdir: process.cwd(), + env: {}, + usePty: false, + warnings: [], + maxOutput: 20_000, + pendingMaxOutput: 20_000, + notifyOnExit: false, + timeoutSec: 5, + }); + } + + it("sanitizes ANSI and OSC sequences split across stdout chunks", async () => { + supervisorMock.spawn.mockImplementationOnce(async (input: SpawnInput) => { + for (const chunk of [ + "A\u001B]0;title", + "\u0007B", + "C\u001B[31", + "mD", + "E\u009D0;title", + "\u001B\\F", + "G\u009B31", + "mH", + ]) { + input.onStdout?.(chunk); + } + return runtimeManagedRun(input); + }); + + const outcome = await (await runStyledExec()).promise; + expect(outcome.aggregated).toContain("ABCDEFGH"); + expect(outcome.aggregated).not.toContain("\\x1b"); + }); + + it("sanitizes escape sequences split across stderr chunks", async () => { + supervisorMock.spawn.mockImplementationOnce(async (input: SpawnInput) => { + input.onStderr?.("warn: \u001B["); + input.onStderr?.("31mred"); + return runtimeManagedRun(input); + }); + + const outcome = await (await runStyledExec()).promise; + expect(outcome.aggregated).toContain("warn: red"); + expect(outcome.aggregated).not.toContain("\\x1b"); + }); + + it("keeps stdout and stderr parser state independent", async () => { + supervisorMock.spawn.mockImplementationOnce(async (input: SpawnInput) => { + input.onStdout?.("out\u001B["); + input.onStderr?.("err\u001B["); + input.onStdout?.("32mOUT"); + input.onStderr?.("31mERR"); + return runtimeManagedRun(input); + }); + + const outcome = await (await runStyledExec()).promise; + expect(outcome.aggregated).toBe("outerrOUTERR"); + }); +}); diff --git a/src/agents/bash-tools.exec-runtime.test.ts b/src/agents/bash-tools.exec-runtime.test.ts index 6eecccdc613c..b4e6f63d9af3 100644 --- a/src/agents/bash-tools.exec-runtime.test.ts +++ b/src/agents/bash-tools.exec-runtime.test.ts @@ -1,10 +1,9 @@ /** * Exec runtime tests. - * Covers target resolution, cursor mode tracking, exit outcome classification, - * system events, and process lifecycle behavior. + * Covers cursor mode tracking, exit outcome classification, system events, + * sandbox finalization, and process lifecycle behavior. */ -import { expectDefined } from "@openclaw/normalization-core"; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { createDeferred } from "../../test/helpers/promise.js"; import { @@ -17,7 +16,6 @@ import { import type { GatewayActiveWorkInspectors } from "../infra/gateway-active-work.js"; import type { ManagedRun } from "../process/supervisor/index.js"; import type { RunExit, SpawnInput } from "../process/supervisor/types.js"; -import { MAX_SAFE_TIMEOUT_DELAY_MS } from "../utils/timer-delay.js"; import type { BashSandboxConfig } from "./bash-tools.shared.js"; const requestHeartbeatMock = vi.hoisted(() => vi.fn()); @@ -44,7 +42,6 @@ let markBackgrounded: typeof import("./bash-process-registry.js").markBackground let getActiveBackgroundExecSessionCount: typeof import("./bash-process-registry.js").getActiveBackgroundExecSessionCount; let listRunningSessions: typeof import("./bash-process-registry.js").listRunningSessions; let resetProcessRegistryForTests: typeof import("./bash-process-registry.test-support.js").resetProcessRegistryForTests; -let resolveExecTarget: typeof import("./bash-tools.exec-runtime.js").resolveExecTarget; let runExecProcess: typeof import("./bash-tools.exec-runtime.js").runExecProcess; let prepareGatewaySuspend: typeof import("../infra/gateway-suspend-coordinator.js").prepareGatewaySuspend; let resetGatewaySuspendCoordinatorForLifecycleRestart: typeof import("../infra/gateway-suspend-coordinator.js").resetGatewaySuspendCoordinatorForLifecycleRestart; @@ -54,7 +51,7 @@ beforeAll(async () => { ({ getActiveBackgroundExecSessionCount, listRunningSessions, markBackgrounded } = await import("./bash-process-registry.js")); ({ resetProcessRegistryForTests } = await import("./bash-process-registry.test-support.js")); - ({ resolveExecTarget, runExecProcess } = await import("./bash-tools.exec-runtime.js")); + ({ runExecProcess } = await import("./bash-tools.exec-runtime.js")); ({ prepareGatewaySuspend, resetGatewaySuspendCoordinatorForLifecycleRestart, @@ -109,24 +106,6 @@ async function runExecWithExit(params: { return { run, outcome: await run.promise }; } -function successfulSupervisorRun() { - return { - runId: "mock-run", - startedAtMs: Date.now(), - wait: async () => ({ - reason: "exit" as const, - exitCode: 0, - exitSignal: null, - durationMs: 0, - stdout: "", - stderr: "", - timedOut: false, - noOutputTimedOut: false, - }), - cancel: vi.fn(), - }; -} - function runtimeManagedRun(input: SpawnInput, stdout = ""): ManagedRun { if (stdout) { input.onStdout?.(stdout); @@ -177,21 +156,6 @@ function prepareSuspension(requestId: string) { }); } -function expectExecTarget( - actual: ReturnType, - expected: { - configuredTarget: string; - requestedTarget: string | null; - selectedTarget: string; - effectiveHost: string; - }, -) { - expect(actual.configuredTarget).toBe(expected.configuredTarget); - expect(actual.requestedTarget).toBe(expected.requestedTarget); - expect(actual.selectedTarget).toBe(expected.selectedTarget); - expect(actual.effectiveHost).toBe(expected.effectiveHost); -} - function requireSystemEventCall(): [string, Record] { const call = enqueueSystemEventWithReceiptMock.mock.calls[0]; if (!call) { @@ -200,14 +164,6 @@ function requireSystemEventCall(): [string, Record] { return call as [string, Record]; } -function requireHeartbeatCall(): Record { - const call = requestHeartbeatMock.mock.calls[0]; - if (!call) { - throw new Error("expected heartbeat call"); - } - return call[0] as Record; -} - describe("runExecProcess cursor tracking", () => { it.each([ { raw: "hello world", expected: "unknown" }, @@ -234,376 +190,77 @@ describe("runExecProcess cursor tracking", () => { }); }); -describe("resolveExecTarget", () => { - it("keeps implicit auto on sandbox when a sandbox runtime is available", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "auto", - elevatedRequested: false, - sandboxAvailable: true, - }), - { - configuredTarget: "auto", - requestedTarget: null, - selectedTarget: "auto", - effectiveHost: "sandbox", - }, - ); - }); - - it("keeps implicit auto on gateway when no sandbox runtime is available", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "auto", - elevatedRequested: false, - sandboxAvailable: false, - }), - { - configuredTarget: "auto", - requestedTarget: null, - selectedTarget: "auto", - effectiveHost: "gateway", - }, - ); - }); - - it("allows per-call host=node override when configured host is auto", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "auto", - requestedTarget: "node", - elevatedRequested: false, - sandboxAvailable: false, - }), - { - configuredTarget: "auto", - requestedTarget: "node", - selectedTarget: "node", - effectiveHost: "node", - }, - ); - }); - - it("allows per-call host=gateway override when configured host is auto and no sandbox", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "auto", - requestedTarget: "gateway", - elevatedRequested: false, - sandboxAvailable: false, - }), - { - configuredTarget: "auto", - requestedTarget: "gateway", - selectedTarget: "gateway", - effectiveHost: "gateway", - }, - ); - }); - - it("rejects per-call host=gateway override from auto when sandbox is available", () => { - expect(() => - resolveExecTarget({ - configuredTarget: "auto", - requestedTarget: "gateway", - elevatedRequested: false, - sandboxAvailable: true, - }), - ).toThrow( - "exec host not allowed (requested gateway; configured host is auto; set tools.exec.host=gateway to allow this override).", - ); - }); - - it("rejects per-call host=node override from auto when sandbox is available", () => { - expect(() => - resolveExecTarget({ - configuredTarget: "auto", - requestedTarget: "node", - elevatedRequested: false, - sandboxAvailable: true, - }), - ).toThrow( - "exec host not allowed (requested node; configured host is auto; set tools.exec.host=node to allow this override).", - ); - }); - - it("allows per-call host=sandbox override when configured host is auto", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "auto", - requestedTarget: "sandbox", - elevatedRequested: false, - sandboxAvailable: true, - }), - { - configuredTarget: "auto", - requestedTarget: "sandbox", - selectedTarget: "sandbox", - effectiveHost: "sandbox", - }, - ); - }); - - it("rejects cross-host override when configured target is a concrete host", () => { - expect(() => - resolveExecTarget({ - configuredTarget: "node", - requestedTarget: "gateway", - elevatedRequested: false, - sandboxAvailable: false, - }), - ).toThrow( - "exec host not allowed (requested gateway; configured host is node; set tools.exec.host=gateway or auto to allow this override).", - ); - }); - - it("allows explicit auto request when configured host is auto", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "auto", - requestedTarget: "auto", - elevatedRequested: false, - sandboxAvailable: true, - }), - { - configuredTarget: "auto", - requestedTarget: "auto", - selectedTarget: "auto", - effectiveHost: "sandbox", - }, - ); - }); - - it("requires an exact match for non-auto configured targets", () => { - expect(() => - resolveExecTarget({ - configuredTarget: "gateway", - requestedTarget: "auto", - elevatedRequested: false, - sandboxAvailable: true, - }), - ).toThrow( - "exec host not allowed (requested auto; configured host is gateway; set tools.exec.host=auto to allow this override).", - ); - }); - - it("allows exact node matches", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "node", - requestedTarget: "node", - elevatedRequested: false, - sandboxAvailable: true, - }), - { - configuredTarget: "node", - requestedTarget: "node", - selectedTarget: "node", - effectiveHost: "node", - }, - ); - }); - - it("forces elevated requests onto the gateway host when configured target is auto", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "auto", - requestedTarget: "sandbox", - elevatedRequested: true, - sandboxAvailable: true, - }), - { - configuredTarget: "auto", - requestedTarget: "sandbox", - selectedTarget: "gateway", - effectiveHost: "gateway", - }, - ); - }); - - it("keeps explicit node override under elevated requests when configured target is auto", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "auto", - requestedTarget: "node", - elevatedRequested: true, - sandboxAvailable: false, - }), - { - configuredTarget: "auto", - requestedTarget: "node", - selectedTarget: "node", - effectiveHost: "node", - }, - ); - }); - - it("honours node target for elevated requests when configured target is node", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "node", - requestedTarget: "node", - elevatedRequested: true, - sandboxAvailable: false, - }), - { - configuredTarget: "node", - requestedTarget: "node", - selectedTarget: "node", - effectiveHost: "node", - }, - ); - }); - - it("routes to node for elevated when configured=node and no per-call override", () => { - expectExecTarget( - resolveExecTarget({ - configuredTarget: "node", - elevatedRequested: true, - sandboxAvailable: false, - }), - { - configuredTarget: "node", - requestedTarget: null, - selectedTarget: "node", - effectiveHost: "node", - }, - ); - }); - - it("rejects mismatched requestedTarget under elevated+node", () => { - expect(() => - resolveExecTarget({ - configuredTarget: "node", - requestedTarget: "gateway", - elevatedRequested: true, - sandboxAvailable: false, - }), - ).toThrow( - "exec host not allowed (requested gateway; configured host is node; set tools.exec.host=gateway or auto to allow this override).", - ); - }); -}); - -describe("exec notifyOnExit suppression", () => { - async function runBackgroundedExit(params: { - reason: "manual-cancel" | "overall-timeout"; - stdout?: string; - }) { - supervisorMock.spawn.mockImplementationOnce( - async (input: { onStdout?: (chunk: string) => void }) => { - if (params.stdout) { - input.onStdout?.(params.stdout); - } - return { - runId: "run-1", - startedAtMs: Date.now(), - pid: 123, - wait: async () => { - await new Promise((resolve) => { - setImmediate(resolve); - }); - return { - reason: params.reason, - exitCode: null, - exitSignal: "SIGKILL", - durationMs: 10, - stdout: "", - stderr: "", - timedOut: params.reason === "overall-timeout", - noOutputTimedOut: false, - }; - }, - cancel: vi.fn(), - }; - }, - ); - - const run = await runExecProcess({ - command: "sleep 999", - workdir: "/tmp", - env: {}, - usePty: false, - warnings: [], - maxOutput: 1000, - pendingMaxOutput: 1000, - notifyOnExit: true, - notifyOnExitEmptySuccess: false, - sessionKey: "agent:main:main", - timeoutSec: null, +describe("sandbox exec preparation failures", () => { + it("settles the registered session once when buildExecSpec rejects", async () => { + const registry = await import("./bash-process-registry.js"); + const sessionId = "sandbox-preparation-failure"; + const sessionSlug = vi.spyOn(registry, "createSessionSlug").mockReturnValue(sessionId); + const preparation = + createDeferred>>>(); + const finalizeExec = vi.fn>(async () => {}); + const onSettledBeforeNotify = vi.fn(); + const completionEvents: DiagnosticExecProcessCompletedEvent[] = []; + const unsubscribe = onInternalDiagnosticEvent((event) => { + if ( + event.type === "exec.process.completed" && + event.sessionKey === "agent:main:sandbox-preparation" + ) { + completionEvents.push(event); + } }); - markBackgrounded(run.session); - return await run.promise; - } + const failure = new Error("sandbox preparation failed"); - it("keeps manual-cancelled no-output background execs silent", async () => { - const outcome = await runBackgroundedExit({ reason: "manual-cancel" }); + try { + const pending = runExecProcess({ + command: "sandbox-command", + workdir: "/tmp", + env: {}, + sandbox: { + containerName: "sandbox", + workspaceDir: "/workspace", + containerWorkdir: "/workspace", + buildExecSpec: async () => await preparation.promise, + finalizeExec, + }, + usePty: false, + warnings: [], + maxOutput: 1000, + pendingMaxOutput: 1000, + notifyOnExit: false, + sessionKey: "agent:main:sandbox-preparation", + timeoutSec: null, + onSettledBeforeNotify, + }); - expect(outcome.status).toBe("failed"); - expect(enqueueSystemEventWithReceiptMock).not.toHaveBeenCalled(); - expect(requestHeartbeatMock).not.toHaveBeenCalled(); - }); + expect(registry.getSession(sessionId)).toMatchObject({ exited: false }); + preparation.reject(failure); + await expect(pending).rejects.toBe(failure); + await new Promise((resolve) => { + setImmediate(resolve); + }); - it("notifies for manual-cancelled background execs with output", async () => { - await runBackgroundedExit({ reason: "manual-cancel", stdout: "partial output\n" }); - - const [message, options] = requireSystemEventCall(); - expect(message).toContain("partial output"); - expect(options.sessionKey).toBe("agent:main:main"); - expect(requestHeartbeatMock).toHaveBeenCalledTimes(1); - const heartbeat = requireHeartbeatCall(); - expect(heartbeat.coalesceMs).toBe(0); - expect(heartbeat.reason).toBe("exec-event"); - expect(heartbeat.sessionKey).toBe("agent:main:main"); - }); - - it("still notifies for no-output background exec timeouts", async () => { - await runBackgroundedExit({ reason: "overall-timeout" }); - - const [message, options] = requireSystemEventCall(); - expect(message).toContain("Exec failed"); - expect(message).toContain("external side effects may already have completed"); - expect(message).toContain("Verify the resulting state before retrying"); - expect(message).toContain("Do not automatically rerun non-idempotent commands"); - expect(options.sessionKey).toBe("agent:main:main"); - expect(requestHeartbeatMock).toHaveBeenCalledTimes(1); - const heartbeat = requireHeartbeatCall(); - expect(heartbeat.coalesceMs).toBe(0); - expect(heartbeat.reason).toBe("exec-event"); - expect(heartbeat.sessionKey).toBe("agent:main:main"); - }); - - it("keeps background exec exit-notification snippets on a UTF-16 boundary", async () => { - // A backgrounded command whose tail output overflows the 180-char snippet - // cap with an emoji straddling the cut must not deliver a lone surrogate to - // the user's channel. The emoji's high surrogate lands at index 178, so a - // raw slice(0, 179) would keep the dangling half. - const head = "a".repeat(178); - const overflowingOutput = `${head}🎉${"b".repeat(30)}`; - await runBackgroundedExit({ reason: "manual-cancel", stdout: overflowingOutput }); - - const [message] = requireSystemEventCall(); - const loneSurrogate = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { - // The notify path first takes a 400-char tail, then compacts that tail to a - // 180-char snippet. If the 400-char tail starts inside an emoji, the final - // compacted snippet must not preserve the dangling low surrogate. - const prefix = "a".repeat(101); - const tailHead = "b".repeat(179); - const overflowingOutput = `${prefix}🎉${tailHead}${"c".repeat(220)}`; - await runBackgroundedExit({ reason: "manual-cancel", stdout: overflowingOutput }); - - const [message] = requireSystemEventCall(); - const loneSurrogate = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { }); }); -describe("runExecProcess POSIX command wrapper", () => { - it("normalizes non-finite and oversized exec timeouts before spawning", async () => { - supervisorMock.spawn.mockResolvedValue(successfulSupervisorRun()); - - const baseParams = { - command: "echo test", - workdir: "/tmp", - env: { PATH: "/usr/bin" }, - pathPrepend: [], - usePty: false, - warnings: [], - maxOutput: 1000, - pendingMaxOutput: 1000, - notifyOnExit: false, - }; - - await runExecProcess({ - ...baseParams, - timeoutSec: Number.POSITIVE_INFINITY, - }); - await runExecProcess({ - ...baseParams, - timeoutSec: 3_000_000, - }); - - expect(supervisorMock.spawn.mock.calls[0]?.[0].timeoutMs).toBeUndefined(); - expect(supervisorMock.spawn.mock.calls[1]?.[0].timeoutMs).toBe(MAX_SAFE_TIMEOUT_DELAY_MS); - }); - - it("wraps command with PATH export if OPENCLAW_PREPEND_PATH is present", async () => { - if (process.platform === "win32") { - return; - } - - supervisorMock.spawn.mockResolvedValueOnce(successfulSupervisorRun()); - - const ignoredRun = await runExecProcess({ - command: "echo test", - workdir: "/tmp", - env: { PATH: "/usr/bin" }, - pathPrepend: ["/custom/bin", "/opt/bin"], - usePty: false, - warnings: [], - maxOutput: 1000, - pendingMaxOutput: 1000, - notifyOnExit: false, - timeoutSec: null, - }); - void ignoredRun; - - expect(supervisorMock.spawn).toHaveBeenCalledTimes(1); - const spawnCall = expectDefined( - supervisorMock.spawn.mock.calls[0], - "supervisorMock.spawn.mock.calls[0] test invariant", - )[0]; - - const commandStr = spawnCall.argv.join(" "); - expect(commandStr).toContain( - 'export PATH="${OPENCLAW_PREPEND_PATH}${PATH:+:$PATH}"; unset OPENCLAW_PREPEND_PATH; echo test', - ); - }); - - it("does not wrap command on Windows", async () => { - if (process.platform !== "win32") { - return; - } - - supervisorMock.spawn.mockResolvedValueOnce(successfulSupervisorRun()); - - const ignoredRun = await runExecProcess({ - command: "echo test", - workdir: "C:\\tmp", - env: { Path: "C:\\Windows\\System32" }, - pathPrepend: ["C:\\custom\\bin"], - usePty: false, - warnings: [], - maxOutput: 1000, - pendingMaxOutput: 1000, - notifyOnExit: false, - timeoutSec: null, - }); - void ignoredRun; - - expect(supervisorMock.spawn).toHaveBeenCalledTimes(1); - const spawnCall = expectDefined( - supervisorMock.spawn.mock.calls[0], - "supervisorMock.spawn.mock.calls[0] test invariant", - )[0]; - - const commandStr = spawnCall.argv.join(" "); - expect(commandStr).not.toContain("export PATH="); - expect(commandStr).toContain("echo test"); - }); -}); - -describe("runExecProcess stream sanitization", () => { - function runStyledExec() { - return runExecProcess({ - command: "printf styled", - workdir: process.cwd(), - env: {}, - usePty: false, - warnings: [], - maxOutput: 20_000, - pendingMaxOutput: 20_000, - notifyOnExit: false, - timeoutSec: 5, - }); - } - - it("sanitizes ANSI and OSC sequences split across stdout chunks", async () => { - supervisorMock.spawn.mockImplementationOnce(async (input: SpawnInput) => { - for (const chunk of [ - "A\u001B]0;title", - "\u0007B", - "C\u001B[31", - "mD", - "E\u009D0;title", - "\u001B\\F", - "G\u009B31", - "mH", - ]) { - input.onStdout?.(chunk); - } - return runtimeManagedRun(input); - }); - - const outcome = await (await runStyledExec()).promise; - - expect(outcome.aggregated).toContain("ABCDEFGH"); - expect(outcome.aggregated).not.toContain("\\x1b"); - }); - - it("sanitizes escape sequences split across stderr chunks", async () => { - supervisorMock.spawn.mockImplementationOnce(async (input: SpawnInput) => { - input.onStderr?.("warn: \u001B["); - input.onStderr?.("31mred"); - return runtimeManagedRun(input); - }); - - const outcome = await (await runStyledExec()).promise; - - expect(outcome.aggregated).toContain("warn: red"); - expect(outcome.aggregated).not.toContain("\\x1b"); - }); - - it("keeps stdout and stderr parser state independent", async () => { - supervisorMock.spawn.mockImplementationOnce(async (input: SpawnInput) => { - // Both streams leave a sequence dangling; neither may consume the other's tail. - input.onStdout?.("out\u001B["); - input.onStderr?.("err\u001B["); - input.onStdout?.("32mOUT"); - input.onStderr?.("31mERR"); - return runtimeManagedRun(input); - }); - - const outcome = await (await runStyledExec()).promise; - - // Interleaved across both streams, but each stream consumed its own sequence: - // no escape leaks, and neither colour parameter survives as visible text. - expect(outcome.aggregated).toBe("outerrOUTERR"); - }); -}); - describe("runExecProcess PTY fallback", () => { afterEach(() => { resetDiagnosticEventsForTest(); diff --git a/src/agents/bash-tools.exec-runtime.ts b/src/agents/bash-tools.exec-runtime.ts index dde74646bf12..8230006697ba 100644 --- a/src/agents/bash-tools.exec-runtime.ts +++ b/src/agents/bash-tools.exec-runtime.ts @@ -753,13 +753,14 @@ export async function runExecProcess(opts: { const timeoutMs = resolveExecTimeoutMs(opts.timeoutSec); let sandboxFinalizeToken: unknown; + let sandboxPrepared = false; let sandboxFinalized = false; const finalizeSandboxExec = async (params: { status: "completed" | "failed"; exitCode: number | null; timedOut: boolean; }) => { - if (sandboxFinalized || !opts.sandbox?.finalizeExec) { + if (!sandboxPrepared || sandboxFinalized || !opts.sandbox?.finalizeExec) { return; } sandboxFinalized = true; @@ -814,20 +815,7 @@ export async function runExecProcess(opts: { return finalOutcome; }; - const spawnSpec: - | { - mode: "child"; - argv: string[]; - env: NodeJS.ProcessEnv; - stdinMode: "pipe-open" | "pipe-closed"; - } - | { - mode: "pty"; - ptyCommand: string; - childFallbackArgv: string[]; - env: NodeJS.ProcessEnv; - stdinMode: "pipe-open"; - } = await (async () => { + const prepareSpawnSpec = async () => { if (opts.sandbox) { const backendExecSpec = await opts.sandbox.buildExecSpec?.({ command: execCommand, @@ -836,6 +824,9 @@ export async function runExecProcess(opts: { usePty: opts.usePty, }); sandboxFinalizeToken = backendExecSpec?.finalizeToken; + // Cleanup ownership transfers only after buildExecSpec resolves: moving this earlier can + // double-finalize backend failures, while removing it leaks the registered exec session. + sandboxPrepared = true; return { mode: "child" as const, argv: backendExecSpec?.argv ?? [ @@ -886,10 +877,10 @@ export async function runExecProcess(opts: { env: shellRuntimeEnv, stdinMode: "pipe-closed" as const, }; - })(); + }; let managedRun: ManagedRun | null = null; - let usingPty = spawnSpec.mode === "pty"; + let usingPty = opts.usePty && !opts.sandbox; const cursorResponse = buildCursorPositionResponse(); const onSupervisorStdout = (chunk: string) => { @@ -907,6 +898,8 @@ export async function runExecProcess(opts: { }; try { + const spawnSpec = await prepareSpawnSpec(); + usingPty = spawnSpec.mode === "pty"; const spawnBase = { runId: sessionId, sessionId: opts.sessionKey?.trim() || sessionId, @@ -919,89 +912,52 @@ export async function runExecProcess(opts: { onStdout: onSupervisorStdout, onStderr: handleStderr, }; - managedRun = - spawnSpec.mode === "pty" - ? await supervisor.spawn({ - ...spawnBase, - mode: "pty", - ptyCommand: spawnSpec.ptyCommand, - }) - : await supervisor.spawn({ - ...spawnBase, - mode: "child", - argv: spawnSpec.argv, - stdinMode: spawnSpec.stdinMode, - }); - } catch (err) { if (spawnSpec.mode === "pty") { - const warning = `Warning: PTY spawn failed (${String(err)}); retrying without PTY for \`${opts.command}\`.`; - logWarn( - `exec: PTY spawn failed (${String(err)}); retrying without PTY for "${opts.command}".`, - ); - opts.warnings.push(warning); - usingPty = false; try { managedRun = await supervisor.spawn({ - runId: sessionId, - sessionId: opts.sessionKey?.trim() || sessionId, - backendId: "exec-host", - scopeKey: opts.scopeKey, + ...spawnBase, + mode: "pty", + ptyCommand: spawnSpec.ptyCommand, + }); + } catch (err) { + const warning = `Warning: PTY spawn failed (${String(err)}); retrying without PTY for \`${opts.command}\`.`; + logWarn( + `exec: PTY spawn failed (${String(err)}); retrying without PTY for "${opts.command}".`, + ); + opts.warnings.push(warning); + usingPty = false; + managedRun = await supervisor.spawn({ + ...spawnBase, mode: "child", argv: spawnSpec.childFallbackArgv, - cwd: opts.workdir, - env: spawnSpec.env, stdinMode: "pipe-open", - timeoutMs, - captureOutput: false, onStdout: handleStdout, - onStderr: handleStderr, }); - } catch (retryErr) { - markExited(session, null, null, "failed"); - maybeNotifyOnExit(session, "failed"); - await finalizeSandboxExec({ - status: "failed", - exitCode: null, - timedOut: false, - }).catch((finalizeErr: unknown) => { - logWarn(`exec: sandbox finalize after spawn failure failed (${String(finalizeErr)}).`); - }); - emitExecProcessCompleted({ - command: opts.command, - mode: "child", - outcome: buildExecRuntimeErrorOutcome({ - error: retryErr, - aggregated: session.aggregated.trim(), - durationMs: Date.now() - startedAt, - }), - sessionKey: opts.sessionKey, - target: diagnosticTarget, - }); - throw retryErr; } } else { - markExited(session, null, null, "failed"); - maybeNotifyOnExit(session, "failed"); - await finalizeSandboxExec({ - status: "failed", - exitCode: null, - timedOut: false, - }).catch((finalizeErr: unknown) => { - logWarn(`exec: sandbox finalize after spawn failure failed (${String(finalizeErr)}).`); + managedRun = await supervisor.spawn({ + ...spawnBase, + mode: "child", + argv: spawnSpec.argv, + stdinMode: spawnSpec.stdinMode, }); - emitExecProcessCompleted({ - command: opts.command, - mode: spawnSpec.mode, - outcome: buildExecRuntimeErrorOutcome({ - error: err, - aggregated: session.aggregated.trim(), - durationMs: Date.now() - startedAt, - }), - sessionKey: opts.sessionKey, - target: diagnosticTarget, - }); - throw err; } + } catch (error) { + const outcome = await finalizeAndSettleSession( + buildExecRuntimeErrorOutcome({ + error, + aggregated: session.aggregated.trim(), + durationMs: Date.now() - startedAt, + }), + ); + emitExecProcessCompleted({ + command: opts.command, + mode: usingPty ? "pty" : "child", + outcome, + sessionKey: opts.sessionKey, + target: diagnosticTarget, + }); + throw error; } session.stdin = managedRun.stdin; session.pid = managedRun.pid; diff --git a/src/agents/bash-tools.exec-target.test.ts b/src/agents/bash-tools.exec-target.test.ts new file mode 100644 index 000000000000..374066eaff2b --- /dev/null +++ b/src/agents/bash-tools.exec-target.test.ts @@ -0,0 +1,268 @@ +import { describe, expect, it } from "vitest"; +import { resolveExecTarget } from "./bash-tools.exec-runtime.js"; + +function expectExecTarget( + actual: ReturnType, + expected: { + configuredTarget: string; + requestedTarget: string | null; + selectedTarget: string; + effectiveHost: string; + }, +) { + expect(actual.configuredTarget).toBe(expected.configuredTarget); + expect(actual.requestedTarget).toBe(expected.requestedTarget); + expect(actual.selectedTarget).toBe(expected.selectedTarget); + expect(actual.effectiveHost).toBe(expected.effectiveHost); +} + +describe("resolveExecTarget", () => { + it("keeps implicit auto on sandbox when a sandbox runtime is available", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "auto", + elevatedRequested: false, + sandboxAvailable: true, + }), + { + configuredTarget: "auto", + requestedTarget: null, + selectedTarget: "auto", + effectiveHost: "sandbox", + }, + ); + }); + + it("keeps implicit auto on gateway when no sandbox runtime is available", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "auto", + elevatedRequested: false, + sandboxAvailable: false, + }), + { + configuredTarget: "auto", + requestedTarget: null, + selectedTarget: "auto", + effectiveHost: "gateway", + }, + ); + }); + + it("allows per-call host=node override when configured host is auto", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "auto", + requestedTarget: "node", + elevatedRequested: false, + sandboxAvailable: false, + }), + { + configuredTarget: "auto", + requestedTarget: "node", + selectedTarget: "node", + effectiveHost: "node", + }, + ); + }); + + it("allows per-call host=gateway override when configured host is auto and no sandbox", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "auto", + requestedTarget: "gateway", + elevatedRequested: false, + sandboxAvailable: false, + }), + { + configuredTarget: "auto", + requestedTarget: "gateway", + selectedTarget: "gateway", + effectiveHost: "gateway", + }, + ); + }); + + it("rejects per-call host=gateway override from auto when sandbox is available", () => { + expect(() => + resolveExecTarget({ + configuredTarget: "auto", + requestedTarget: "gateway", + elevatedRequested: false, + sandboxAvailable: true, + }), + ).toThrow( + "exec host not allowed (requested gateway; configured host is auto; set tools.exec.host=gateway to allow this override).", + ); + }); + + it("rejects per-call host=node override from auto when sandbox is available", () => { + expect(() => + resolveExecTarget({ + configuredTarget: "auto", + requestedTarget: "node", + elevatedRequested: false, + sandboxAvailable: true, + }), + ).toThrow( + "exec host not allowed (requested node; configured host is auto; set tools.exec.host=node to allow this override).", + ); + }); + + it("allows per-call host=sandbox override when configured host is auto", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "auto", + requestedTarget: "sandbox", + elevatedRequested: false, + sandboxAvailable: true, + }), + { + configuredTarget: "auto", + requestedTarget: "sandbox", + selectedTarget: "sandbox", + effectiveHost: "sandbox", + }, + ); + }); + + it("rejects cross-host override when configured target is a concrete host", () => { + expect(() => + resolveExecTarget({ + configuredTarget: "node", + requestedTarget: "gateway", + elevatedRequested: false, + sandboxAvailable: false, + }), + ).toThrow( + "exec host not allowed (requested gateway; configured host is node; set tools.exec.host=gateway or auto to allow this override).", + ); + }); + + it("allows explicit auto request when configured host is auto", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "auto", + requestedTarget: "auto", + elevatedRequested: false, + sandboxAvailable: true, + }), + { + configuredTarget: "auto", + requestedTarget: "auto", + selectedTarget: "auto", + effectiveHost: "sandbox", + }, + ); + }); + + it("requires an exact match for non-auto configured targets", () => { + expect(() => + resolveExecTarget({ + configuredTarget: "gateway", + requestedTarget: "auto", + elevatedRequested: false, + sandboxAvailable: true, + }), + ).toThrow( + "exec host not allowed (requested auto; configured host is gateway; set tools.exec.host=auto to allow this override).", + ); + }); + + it("allows exact node matches", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "node", + requestedTarget: "node", + elevatedRequested: false, + sandboxAvailable: true, + }), + { + configuredTarget: "node", + requestedTarget: "node", + selectedTarget: "node", + effectiveHost: "node", + }, + ); + }); + + it("forces elevated requests onto the gateway host when configured target is auto", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "auto", + requestedTarget: "sandbox", + elevatedRequested: true, + sandboxAvailable: true, + }), + { + configuredTarget: "auto", + requestedTarget: "sandbox", + selectedTarget: "gateway", + effectiveHost: "gateway", + }, + ); + }); + + it("keeps explicit node override under elevated requests when configured target is auto", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "auto", + requestedTarget: "node", + elevatedRequested: true, + sandboxAvailable: false, + }), + { + configuredTarget: "auto", + requestedTarget: "node", + selectedTarget: "node", + effectiveHost: "node", + }, + ); + }); + + it("honours node target for elevated requests when configured target is node", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "node", + requestedTarget: "node", + elevatedRequested: true, + sandboxAvailable: false, + }), + { + configuredTarget: "node", + requestedTarget: "node", + selectedTarget: "node", + effectiveHost: "node", + }, + ); + }); + + it("routes to node for elevated when configured=node and no per-call override", () => { + expectExecTarget( + resolveExecTarget({ + configuredTarget: "node", + elevatedRequested: true, + sandboxAvailable: false, + }), + { + configuredTarget: "node", + requestedTarget: null, + selectedTarget: "node", + effectiveHost: "node", + }, + ); + }); + + it("rejects mismatched requestedTarget under elevated+node", () => { + expect(() => + resolveExecTarget({ + configuredTarget: "node", + requestedTarget: "gateway", + elevatedRequested: true, + sandboxAvailable: false, + }), + ).toThrow( + "exec host not allowed (requested gateway; configured host is node; set tools.exec.host=gateway or auto to allow this override).", + ); + }); +});