Skip to content
6 changes: 5 additions & 1 deletion src/core/tools/ExecuteCommandTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,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: <undefined, notify user>"
}

if (exitDetails.aborted) {
return "Command was aborted by the user."
}

if (exitDetails.signalName) {
let status = `Process terminated by signal ${exitDetails.signalName}`
if (exitDetails.coreDumpPossible) {
Expand Down
30 changes: 30 additions & 0 deletions src/core/tools/__tests__/executeCommandTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,4 +333,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: <undefined, notify user>")
})
})
})
6 changes: 3 additions & 3 deletions src/integrations/terminal/ExecaTerminalProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
102 changes: 99 additions & 3 deletions src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 })
Comment thread
umi008 marked this conversation as resolved.
})

it("should emit completed event with full output", async () => {
Expand All @@ -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<void>((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
Expand Down
6 changes: 6 additions & 0 deletions src/integrations/terminal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading