diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index 58b0055b4c..55c230c97f 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -566,11 +566,15 @@ export async function executeCommandInTerminal( /** * Format exit status from ExitCodeDetails */ -function formatExitStatus(exitDetails: ExitCodeDetails | undefined): string { +export function formatExitStatus(exitDetails: ExitCodeDetails | undefined): string { if (exitDetails === undefined) { return "Exit code: " } + if (exitDetails.aborted) { + return "Command was aborted by the user." + } + if (exitDetails.signalName) { let status = `Process terminated by signal ${exitDetails.signalName}` if (exitDetails.coreDumpPossible) { diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index d19906a262..d50a5ab577 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -327,4 +327,34 @@ describe("executeCommandTool", () => { expect(executeCommandModule.resolveAgentTimeoutMs(30)).toBe(30_000) }) }) + + describe("formatExitStatus", () => { + it('should return "Command was aborted by the user." when aborted is true', () => { + expect(executeCommandModule.formatExitStatus({ exitCode: 0, aborted: true })).toBe( + "Command was aborted by the user.", + ) + }) + + it("should return exit code message when aborted is false", () => { + expect(executeCommandModule.formatExitStatus({ exitCode: 0, aborted: false })).toBe("Exit code: 0") + }) + + it("should return exit code message when aborted is undefined", () => { + expect(executeCommandModule.formatExitStatus({ exitCode: 0 })).toBe("Exit code: 0") + }) + + it("should return signal message when signalName is present and not aborted", () => { + expect( + executeCommandModule.formatExitStatus({ + exitCode: undefined, + signalName: "SIGKILL", + aborted: false, + }), + ).toBe("Process terminated by signal SIGKILL") + }) + + it("should handle undefined exitDetails", () => { + expect(executeCommandModule.formatExitStatus(undefined)).toBe("Exit code: ") + }) + }) }) diff --git a/src/integrations/terminal/ExecaTerminalProcess.ts b/src/integrations/terminal/ExecaTerminalProcess.ts index cde5a1251f..9f7db549db 100644 --- a/src/integrations/terminal/ExecaTerminalProcess.ts +++ b/src/integrations/terminal/ExecaTerminalProcess.ts @@ -130,17 +130,17 @@ export class ExecaTerminalProcess extends BaseTerminalProcess { } } - this.emit("shell_execution_complete", { exitCode: 0 }) + this.emit("shell_execution_complete", { exitCode: 0, aborted: this.aborted }) } catch (error) { if (error instanceof ExecaError) { console.error(`[ExecaTerminalProcess#run] shell execution error: ${error.message}`) - this.emit("shell_execution_complete", { exitCode: error.exitCode ?? 0, signalName: error.signal }) + this.emit("shell_execution_complete", { exitCode: error.exitCode ?? 0, signalName: error.signal, aborted: this.aborted }) } else { console.error( `[ExecaTerminalProcess#run] shell execution error: ${error instanceof Error ? error.message : String(error)}`, ) - this.emit("shell_execution_complete", { exitCode: 1 }) + this.emit("shell_execution_complete", { exitCode: 1, aborted: this.aborted }) } this.subprocess = undefined } diff --git a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts index c7f3ee2145..050b0e7bc6 100644 --- a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts @@ -23,7 +23,7 @@ vitest.mock("ps-tree", () => ({ }), })) -import { execa } from "execa" +import { execa, ExecaError } from "execa" import { ExecaTerminalProcess } from "../ExecaTerminalProcess" import { BaseTerminal } from "../BaseTerminal" import type { RooTerminal } from "../types" @@ -125,11 +125,11 @@ describe("ExecaTerminalProcess", () => { expect(terminalProcess.terminal).toBe(mockTerminal) }) - it("should emit shell_execution_complete with exitCode 0", async () => { + it("should emit shell_execution_complete with exitCode 0 and aborted: false", async () => { const spy = vitest.fn() terminalProcess.on("shell_execution_complete", spy) await terminalProcess.run("echo test") - expect(spy).toHaveBeenCalledWith({ exitCode: 0 }) + expect(spy).toHaveBeenCalledWith({ exitCode: 0, aborted: false }) }) it("should emit completed event with full output", async () => { @@ -146,6 +146,102 @@ describe("ExecaTerminalProcess", () => { }) }) + describe("error-path abort emission", () => { + it("should emit aborted: true when abort() is called during execution", async () => { + const execaMock = vitest.mocked(execa) + let resolveBlock: () => void + const blockPromise = new Promise((resolve) => { + resolveBlock = resolve + }) + + execaMock.mockImplementation(((options: any) => { + return (_template: TemplateStringsArray, ...args: any[]) => ({ + pid: mockPid, + iterable: (_opts: any) => + (async function* () { + yield "test output\\n" + // Block the second yield so we can call abort() from outside + await blockPromise + })(), + kill: vitest.fn(), + }) + }) as any) + + const spy = vitest.fn() + terminalProcess.on("shell_execution_complete", spy) + + // Start running - will yield once then block on blockPromise + const runPromise = terminalProcess.run("echo test") + + // Let the first yield process + await new Promise((r) => setTimeout(r, 50)) + + // Now abort while the command is "running" + terminalProcess.abort() + + // Resolve the block so the generator returns, + // then the for-await loop checks this.aborted and breaks + resolveBlock!() + + await runPromise + + expect(spy).toHaveBeenCalledWith({ + exitCode: 0, + aborted: true, + }) + }) + + it("should emit aborted: false in ExecaError catch path", async () => { + const execaMock = vitest.mocked(execa) + execaMock.mockImplementation(((options: any) => { + return (_template: TemplateStringsArray, ...args: any[]) => ({ + pid: mockPid, + iterable: (_opts: any) => + // eslint-disable-next-line require-yield + (async function* () { + throw Object.assign(new (ExecaError as any)("exec failed"), { + exitCode: 1, + signal: "SIGTERM", + }) + })(), + kill: vitest.fn(), + }) + }) as any) + + const spy = vitest.fn() + terminalProcess.on("shell_execution_complete", spy) + await terminalProcess.run("echo test") + expect(spy).toHaveBeenCalledWith({ + exitCode: 1, + signalName: "SIGTERM", + aborted: false, + }) + }) + + it("should emit aborted: false in generic error catch path", async () => { + const execaMock = vitest.mocked(execa) + execaMock.mockImplementation(((options: any) => { + return (_template: TemplateStringsArray, ...args: any[]) => ({ + pid: mockPid, + iterable: (_opts: any) => + // eslint-disable-next-line require-yield + (async function* () { + throw new Error("generic error") + })(), + kill: vitest.fn(), + }) + }) as any) + + const spy = vitest.fn() + terminalProcess.on("shell_execution_complete", spy) + await terminalProcess.run("echo test") + expect(spy).toHaveBeenCalledWith({ + exitCode: 1, + aborted: false, + }) + }) + }) + describe("trimRetrievedOutput", () => { it("clears buffer when all output has been retrieved", () => { // Set up a scenario where all output has been retrieved diff --git a/src/integrations/terminal/types.ts b/src/integrations/terminal/types.ts index 8224875b60..f11645ec2b 100644 --- a/src/integrations/terminal/types.ts +++ b/src/integrations/terminal/types.ts @@ -72,4 +72,10 @@ export interface ExitCodeDetails { signal?: number | undefined signalName?: string coreDumpPossible?: boolean + /** + * Set to true when the process was intentionally aborted (via abort(), + * timeout, or task teardown), as opposed to the process exiting on its + * own or being killed by the OS (e.g., OOM-killer). + */ + aborted?: boolean }