From 689894b4c8320bb697dbc4100e74e575f9fc4ad4 Mon Sep 17 00:00:00 2001 From: umi008 Date: Fri, 10 Jul 2026 20:23:18 -0600 Subject: [PATCH 1/7] fix(terminal): tell model when user pressed Abort on a running command When the user clicks Abort on a running shell command, the terminal previously emitted shell_execution_complete with exitCode: 0 and no indication of user intervention. The model would see a successful exit code or SIGKILL and suspect the Linux OOM-killer, leading to incorrect diagnosis and wrong follow-up actions. Now the ExitCodeDetails interface carries an aborted: true flag when the user explicitly stopped the process. formatExitStatus() checks this first and returns 'Command was aborted by the user.' so the model immediately knows what happened and doesn't speculate about OOM-killer. Closes #833 --- src/core/tools/ExecuteCommandTool.ts | 4 ++++ src/integrations/terminal/ExecaTerminalProcess.ts | 2 +- src/integrations/terminal/types.ts | 6 ++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index 58b0055b4c..5066ee166a 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -571,6 +571,10 @@ function formatExitStatus(exitDetails: ExitCodeDetails | undefined): string { 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/integrations/terminal/ExecaTerminalProcess.ts b/src/integrations/terminal/ExecaTerminalProcess.ts index cde5a1251f..bec0b01082 100644 --- a/src/integrations/terminal/ExecaTerminalProcess.ts +++ b/src/integrations/terminal/ExecaTerminalProcess.ts @@ -130,7 +130,7 @@ 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}`) diff --git a/src/integrations/terminal/types.ts b/src/integrations/terminal/types.ts index 8224875b60..be494a1e2f 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 user explicitly pressed the Abort button + * to terminate the command, as opposed to the process exiting on its own + * or being killed by the OS (e.g., OOM-killer). + */ + aborted?: boolean } From 5f21e21f3a85c4dc9cc57869d3e0a88fcc9feb8c Mon Sep 17 00:00:00 2001 From: umi008 Date: Fri, 10 Jul 2026 20:43:24 -0600 Subject: [PATCH 2/7] test(terminal): update ExecaTerminalProcess test for aborted field The shell_execution_complete emission now includes aborted: false for normal process exits. Update the test expectation to match. --- .../terminal/__tests__/ExecaTerminalProcess.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts index c7f3ee2145..331d9864e0 100644 --- a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts @@ -129,7 +129,7 @@ describe("ExecaTerminalProcess", () => { 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 () => { From 053f12a3aa5b8d32ada0cc83ea7ea2cdf20f92b4 Mon Sep 17 00:00:00 2001 From: umi008 Date: Sun, 12 Jul 2026 21:43:24 -0600 Subject: [PATCH 3/7] fix(terminal): propagate aborted flag in catch block error emissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When abort() sends SIGKILL and the subprocess throws ExecaError before the for-await loop checks this.aborted, the catch block was emitting shell_execution_complete without aborted: this.aborted. This caused formatExitStatus to return 'Process terminated by signal SIGKILL' — the ambiguous message this PR was designed to eliminate. Both error paths now propagate aborted: this.aborted. --- src/integrations/terminal/ExecaTerminalProcess.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/integrations/terminal/ExecaTerminalProcess.ts b/src/integrations/terminal/ExecaTerminalProcess.ts index bec0b01082..9f7db549db 100644 --- a/src/integrations/terminal/ExecaTerminalProcess.ts +++ b/src/integrations/terminal/ExecaTerminalProcess.ts @@ -134,13 +134,13 @@ export class ExecaTerminalProcess extends BaseTerminalProcess { } 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 } From f296dcace7a3815790d4757269f602c6be5ab794 Mon Sep 17 00:00:00 2001 From: umi008 Date: Fri, 17 Jul 2026 03:30:56 +0000 Subject: [PATCH 4/7] test(terminal): add coverage for abort status and error-path emissions - Export formatExitStatus from ExecuteCommandTool for testing - Add tests for formatExitStatus: aborted=true returns user message, aborted=false/signal/undefined all produce correct output - Add ExecaTerminalProcess error-path tests: both ExecaError and generic error catch paths assert aborted: false in emission - Rename existing test to reflect aborted: false assertion Increases codecov patch coverage from 20% to 100% Addresses PR #880 review comment from edelauna --- src/core/tools/ExecuteCommandTool.ts | 2 +- .../__tests__/executeCommandTool.spec.ts | 30 +++++++++++ .../__tests__/ExecaTerminalProcess.spec.ts | 54 ++++++++++++++++++- 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index 5066ee166a..55c230c97f 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -566,7 +566,7 @@ 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: " } 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/__tests__/ExecaTerminalProcess.spec.ts b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts index 331d9864e0..0b19ee83b9 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,7 +125,7 @@ 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") @@ -146,6 +146,56 @@ describe("ExecaTerminalProcess", () => { }) }) + describe("error-path abort emission", () => { + 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) => + (async function* () { + throw Object.assign(new ExecaError("exec failed"), { + exitCode: 1, + signal: "SIGTERM", + }) + })(), + kill: vitest.fn(), + }) + }) + + 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) => + (async function* () { + throw new Error("generic error") + })(), + kill: vitest.fn(), + }) + }) + + 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 From 6d9a6b40fe4ab7ac4b7b827bf388b0905d088c62 Mon Sep 17 00:00:00 2001 From: umi008 Date: Fri, 17 Jul 2026 03:33:36 +0000 Subject: [PATCH 5/7] test(terminal): add abort regression test + fix abort docstring wording - Add test that calls abort() during execution and verifies shell_execution_complete includes aborted: true - Fix docstring wording: abort() is also called by timeout and task teardown paths, not just user button press Addresses PR #880 review comments from edelauna (2 threads) --- .../__tests__/ExecaTerminalProcess.spec.ts | 48 ++++++++++++++++++- src/integrations/terminal/types.ts | 6 +-- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts index 0b19ee83b9..635ac48628 100644 --- a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts @@ -146,8 +146,52 @@ describe("ExecaTerminalProcess", () => { }) }) - describe("error-path abort emission", () => { - it("should emit aborted: false in ExecaError catch path", async () => { + 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(), + }) + }) + + 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[]) => ({ diff --git a/src/integrations/terminal/types.ts b/src/integrations/terminal/types.ts index be494a1e2f..f11645ec2b 100644 --- a/src/integrations/terminal/types.ts +++ b/src/integrations/terminal/types.ts @@ -73,9 +73,9 @@ export interface ExitCodeDetails { signalName?: string coreDumpPossible?: boolean /** - * Set to true when the user explicitly pressed the Abort button - * to terminate the command, as opposed to the process exiting on its own - * or being killed by the OS (e.g., OOM-killer). + * 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 } From 317e842fc29477bfd13ab34906a98c8f36264a5e Mon Sep 17 00:00:00 2001 From: umi008 Date: Fri, 17 Jul 2026 04:05:55 +0000 Subject: [PATCH 6/7] fix(terminal): suppress eslint require-yield warnings in error-path generators add eslint-disable-next-line comments before the async generator functions that throw immediately (no yield needed) to pass the CI compile/lint check --- .../__tests__/ExecaTerminalProcess.spec.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts index 635ac48628..b71ec84af7 100644 --- a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts @@ -196,9 +196,10 @@ describe("ExecaTerminalProcess", () => { execaMock.mockImplementation((options: any) => { return (_template: TemplateStringsArray, ...args: any[]) => ({ pid: mockPid, - iterable: (_opts: any) => - (async function* () { - throw Object.assign(new ExecaError("exec failed"), { + iterable: (_opts: any) => + // eslint-disable-next-line require-yield + (async function* () { + throw Object.assign(new ExecaError("exec failed"), { exitCode: 1, signal: "SIGTERM", }) @@ -222,9 +223,10 @@ describe("ExecaTerminalProcess", () => { execaMock.mockImplementation((options: any) => { return (_template: TemplateStringsArray, ...args: any[]) => ({ pid: mockPid, - iterable: (_opts: any) => - (async function* () { - throw new Error("generic error") + iterable: (_opts: any) => + // eslint-disable-next-line require-yield + (async function* () { + throw new Error("generic error") })(), kill: vitest.fn(), }) From 836ff76b7fee8b96fc71a96b2de9f814e36c4966 Mon Sep 17 00:00:00 2001 From: umi008 Date: Fri, 17 Jul 2026 04:31:25 +0000 Subject: [PATCH 7/7] fix(terminal): add as any casts to all mockImplementations for TS check-types compliance --- .../__tests__/ExecaTerminalProcess.spec.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts index b71ec84af7..050b0e7bc6 100644 --- a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts @@ -154,7 +154,7 @@ describe("ExecaTerminalProcess", () => { resolveBlock = resolve }) - execaMock.mockImplementation((options: any) => { + execaMock.mockImplementation(((options: any) => { return (_template: TemplateStringsArray, ...args: any[]) => ({ pid: mockPid, iterable: (_opts: any) => @@ -165,7 +165,7 @@ describe("ExecaTerminalProcess", () => { })(), kill: vitest.fn(), }) - }) + }) as any) const spy = vitest.fn() terminalProcess.on("shell_execution_complete", spy) @@ -193,20 +193,20 @@ describe("ExecaTerminalProcess", () => { it("should emit aborted: false in ExecaError catch path", async () => { const execaMock = vitest.mocked(execa) - execaMock.mockImplementation((options: any) => { + 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("exec failed"), { + 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) @@ -220,7 +220,7 @@ describe("ExecaTerminalProcess", () => { it("should emit aborted: false in generic error catch path", async () => { const execaMock = vitest.mocked(execa) - execaMock.mockImplementation((options: any) => { + execaMock.mockImplementation(((options: any) => { return (_template: TemplateStringsArray, ...args: any[]) => ({ pid: mockPid, iterable: (_opts: any) => @@ -230,7 +230,7 @@ describe("ExecaTerminalProcess", () => { })(), kill: vitest.fn(), }) - }) + }) as any) const spy = vitest.fn() terminalProcess.on("shell_execution_complete", spy)