fix(coding-agent): fail signal-terminated shell commands
Map signal termination to conventional nonzero exit codes and reject null exit codes from custom shell operations while preserving partial output. fixes #9577
This commit is contained in:
@@ -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)).
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user