diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3bef17718..5c5fc4523 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -19,6 +19,7 @@ ### Fixed +- Fixed signal-terminated local shell commands being reported as successful with partial output ([#9577](https://github.com/earendil-works/pi/issues/9577)). - Fixed local clipboard failures reporting success when the terminal ignored the fallback OSC 52 write, and added platform-specific setup guidance when no clipboard backend works ([#9618](https://github.com/earendil-works/pi/issues/9618)). - Capped agent-level retry backoff at `retry.maxAgentDelayMs` (60s by default) so long retry runs stay responsive during prolonged transient outages ([#8826](https://github.com/earendil-works/pi/issues/8826)). - Fixed direct RPC `steer` and `follow_up` commands bypassing extension `input` handlers ([#8718](https://github.com/earendil-works/pi/issues/8718)). diff --git a/packages/coding-agent/src/core/tools/bash.ts b/packages/coding-agent/src/core/tools/bash.ts index b7c9973a4..bde6ecb91 100644 --- a/packages/coding-agent/src/core/tools/bash.ts +++ b/packages/coding-agent/src/core/tools/bash.ts @@ -1,5 +1,6 @@ import { constants } from "node:fs"; import { access as fsAccess } from "node:fs/promises"; +import { constants as osConstants } from "node:os"; import type { AgentTool } from "@earendil-works/pi-agent-core"; import { spawn } from "child_process"; import { type Static, Type } from "typebox"; @@ -61,7 +62,8 @@ export interface BashOperations { * @param command The command to execute * @param cwd Working directory * @param options Execution options - * @returns Promise resolving to exit code (null if killed) + * @returns Promise resolving to the exit code. Report signal terminations as 128 + signal number; + * a null exit code is treated as a failed command. */ exec: ( command: string, @@ -134,7 +136,10 @@ export function createLocalShellOperations(shellName: string, resolveShellConfig if (timedOut) { throw new Error(`timeout:${timeout}`); } - return { exitCode }; + // A signal-killed shell has no exit code. Use the standard shell convention so + // callers do not mistake the termination for a successful command. + const signalCode = child.signalCode; + return { exitCode: exitCode ?? (signalCode ? 128 + (osConstants.signals[signalCode] ?? 0) : 1) }; } finally { if (child.pid) untrackDetachedChildPid(child.pid); if (timeoutHandle) clearTimeout(timeoutHandle); @@ -360,7 +365,10 @@ export function createShellToolDefinition( const snapshot = await finishOutput(); const { text: outputText, details } = formatOutput(snapshot); - if (exitCode !== 0 && exitCode !== null) { + if (exitCode === null) { + throw new Error(appendStatus(outputText, "Command terminated without an exit code")); + } + if (exitCode !== 0) { throw new Error(appendStatus(outputText, `Command exited with code ${exitCode}`)); } return { content: [{ type: "text", text: outputText }], details }; diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index bb71dfead..e63b6747e 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -496,6 +496,52 @@ describe("Coding Agent Tools", () => { ); }); + // Regression tests for https://github.com/earendil-works/pi/issues/9577 + it.skipIf(process.platform === "win32")( + "should map signal-killed commands to 128 plus the signal number", + async () => { + const operations = createLocalBashOperations(); + for (const { signal, exitCode } of [ + { signal: "KILL", exitCode: 137 }, + { signal: "TERM", exitCode: 143 }, + ]) { + const result = await operations.exec(`kill -${signal} $$`, testDir, { onData: () => {} }); + expect(result.exitCode).toBe(exitCode); + } + }, + ); + + it.skipIf(process.platform === "win32")( + "should reject signal-killed commands while preserving partial output", + async () => { + for (const { signal, exitCode } of [ + { signal: "KILL", exitCode: 137 }, + { signal: "TERM", exitCode: 143 }, + ]) { + const execution = bashTool.execute(`test-call-signal-${signal}`, { + command: `printf 'before-kill\\n'; kill -${signal} $$`, + }); + await expect(execution).rejects.toThrow( + new RegExp(`before-kill\\s+Command exited with code ${exitCode}$`), + ); + } + }, + ); + + it("should reject a null exit code from custom operations", async () => { + const operations: BashOperations = { + exec: async (_command, _cwd, { onData }) => { + onData(Buffer.from("partial\n", "utf-8")); + return { exitCode: null }; + }, + }; + const bash = createBashTool(testDir, { operations }); + + await expect(bash.execute("test-call-null-exit", { command: "remote" })).rejects.toThrow( + /partial\s+Command terminated without an exit code$/, + ); + }); + it("should respect timeout", async () => { const command = `${JSON.stringify(process.execPath)} -e ${JSON.stringify("setInterval(() => {}, 1000)")}`; await expect(bashTool.execute("test-call-10", { command, timeout: 0.05 })).rejects.toThrow(/timed out/i);