From e001e8f2f8caca764622badb340d981eb45ecba4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 23 Mar 2026 04:47:02 +0000 Subject: [PATCH] test: isolate exec foreground failure coverage --- ...ash-tools.exec-foreground-failures.test.ts | 51 ++++++++++++ src/agents/bash-tools.exec-runtime.test.ts | 79 ++++++++++++++++++- src/agents/bash-tools.test.ts | 25 ------ 3 files changed, 129 insertions(+), 26 deletions(-) create mode 100644 src/agents/bash-tools.exec-foreground-failures.test.ts diff --git a/src/agents/bash-tools.exec-foreground-failures.test.ts b/src/agents/bash-tools.exec-foreground-failures.test.ts new file mode 100644 index 00000000000..c6e2b8203f8 --- /dev/null +++ b/src/agents/bash-tools.exec-foreground-failures.test.ts @@ -0,0 +1,51 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { captureEnv } from "../test-utils/env.js"; +import { resetProcessRegistryForTests } from "./bash-process-registry.js"; +import { createExecTool } from "./bash-tools.exec.js"; +import { resolveShellFromPath } from "./shell-utils.js"; + +const isWin = process.platform === "win32"; +const defaultShell = isWin + ? undefined + : process.env.OPENCLAW_TEST_SHELL || resolveShellFromPath("bash") || process.env.SHELL || "sh"; +const longDelayCmd = isWin ? "Start-Sleep -Milliseconds 72" : "sleep 0.072"; + +describe("exec foreground failures", () => { + let envSnapshot: ReturnType; + + beforeEach(() => { + envSnapshot = captureEnv(["SHELL"]); + if (!isWin && defaultShell) { + process.env.SHELL = defaultShell; + } + resetProcessRegistryForTests(); + }); + + afterEach(() => { + envSnapshot.restore(); + }); + + it("returns a failed text result when the default timeout is exceeded", async () => { + const tool = createExecTool({ + security: "full", + ask: "off", + timeoutSec: 0.05, + backgroundMs: 10, + allowBackground: false, + }); + + const result = await tool.execute("call-timeout", { + command: longDelayCmd, + }); + + expect(result.content[0]).toMatchObject({ type: "text" }); + expect((result.content[0] as { text?: string }).text).toMatch(/timed out/i); + expect((result.content[0] as { text?: string }).text).toMatch(/re-run with a higher timeout/i); + expect(result.details).toMatchObject({ + status: "failed", + exitCode: null, + aggregated: "", + }); + expect((result.details as { durationMs?: number }).durationMs).toEqual(expect.any(Number)); + }); +}); diff --git a/src/agents/bash-tools.exec-runtime.test.ts b/src/agents/bash-tools.exec-runtime.test.ts index 35a38b5483d..fc7bd245cba 100644 --- a/src/agents/bash-tools.exec-runtime.test.ts +++ b/src/agents/bash-tools.exec-runtime.test.ts @@ -10,7 +10,11 @@ vi.mock("../infra/system-events.js", () => ({ import { requestHeartbeatNow } from "../infra/heartbeat-wake.js"; import { enqueueSystemEvent } from "../infra/system-events.js"; -import { emitExecSystemEvent } from "./bash-tools.exec-runtime.js"; +import { + buildExecExitOutcome, + emitExecSystemEvent, + formatExecFailureReason, +} from "./bash-tools.exec-runtime.js"; const requestHeartbeatNowMock = vi.mocked(requestHeartbeatNow); const enqueueSystemEventMock = vi.mocked(enqueueSystemEvent); @@ -62,3 +66,76 @@ describe("emitExecSystemEvent", () => { expect(requestHeartbeatNowMock).not.toHaveBeenCalled(); }); }); + +describe("formatExecFailureReason", () => { + it("formats timeout guidance with the configured timeout", () => { + expect( + formatExecFailureReason({ + failureKind: "overall-timeout", + exitSignal: "SIGKILL", + timeoutSec: 45, + }), + ).toContain("45 seconds"); + }); + + it("formats shell failures without timeout-specific guidance", () => { + expect( + formatExecFailureReason({ + failureKind: "shell-command-not-found", + exitSignal: null, + timeoutSec: 45, + }), + ).toBe("Command not found"); + }); +}); + +describe("buildExecExitOutcome", () => { + it("keeps non-zero normal exits in the completed path", () => { + expect( + buildExecExitOutcome({ + exit: { + reason: "exit", + exitCode: 1, + exitSignal: null, + durationMs: 123, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }, + aggregated: "done", + durationMs: 123, + timeoutSec: 30, + }), + ).toMatchObject({ + status: "completed", + exitCode: 1, + aggregated: "done\n\n(Command exited with code 1)", + }); + }); + + it("classifies timed out exits as failures with a reason", () => { + expect( + buildExecExitOutcome({ + exit: { + reason: "overall-timeout", + exitCode: null, + exitSignal: "SIGKILL", + durationMs: 123, + stdout: "", + stderr: "", + timedOut: true, + noOutputTimedOut: false, + }, + aggregated: "", + durationMs: 123, + timeoutSec: 30, + }), + ).toMatchObject({ + status: "failed", + failureKind: "overall-timeout", + timedOut: true, + reason: expect.stringContaining("30 seconds"), + }); + }); +}); diff --git a/src/agents/bash-tools.test.ts b/src/agents/bash-tools.test.ts index 44c5a369ba7..4929ea23be7 100644 --- a/src/agents/bash-tools.test.ts +++ b/src/agents/bash-tools.test.ts @@ -18,7 +18,6 @@ const defaultShell = isWin // PowerShell: Start-Sleep for delays, ; for command separation, $null for null device const shortDelayCmd = isWin ? "Start-Sleep -Milliseconds 4" : "sleep 0.004"; const yieldDelayCmd = isWin ? "Start-Sleep -Milliseconds 16" : "sleep 0.016"; -const longDelayCmd = isWin ? "Start-Sleep -Milliseconds 72" : "sleep 0.072"; const POLL_INTERVAL_MS = 15; const BACKGROUND_POLL_TIMEOUT_MS = isWin ? 8000 : 1200; const NOTIFY_EVENT_TIMEOUT_MS = isWin ? 12_000 : 5_000; @@ -455,30 +454,6 @@ describe("exec tool backgrounding", () => { expect(sessions.find((s) => s.sessionId === sessionId)?.name).toBe(COMMAND_ECHO_HELLO); }); - it("uses default timeout when timeout is omitted", async () => { - const customBash = createTestExecTool({ - timeoutSec: 0.05, - backgroundMs: 10, - allowBackground: false, - }); - const result = await executeExecCommand(customBash, longDelayCmd); - const text = readTextContent(result.content); - expect(text).toMatch(/timed out/i); - expect(text).toMatch(/re-run with a higher timeout/i); - const details = result.details as { - status?: string; - exitCode?: number | null; - durationMs?: number; - aggregated?: string; - }; - expect(details).toMatchObject({ - status: "failed", - exitCode: null, - aggregated: "", - }); - expect(details.durationMs).toEqual(expect.any(Number)); - }); - it.each(DISALLOWED_ELEVATION_CASES)( "$label", runDisallowedElevationCase,