fix(core): report shell commands killed by a signal (#51145)

This commit is contained in:
Aiden Cline
2026-09-24 11:21:31 -05:00
committed by GitHub
parent 0e4ad0ce76
commit e22c1622e0
7 changed files with 32 additions and 12 deletions
@@ -188,6 +188,7 @@ export type ShellInfo = {
file: string
pid?: number
exit?: number
signal?: string
metadata: { [x: string]: any }
time: { started: number; completed?: number }
}
@@ -395,6 +396,7 @@ export type ShellInfo1 = {
file: string
pid?: number
exit?: number
signal?: string
metadata: { [x: string]: JsonValue }
time: { started: number; completed?: number }
}
+11 -2
View File
@@ -6,6 +6,7 @@ import { ChildProcess } from "effect/unstable/process"
import { produce } from "immer"
import { Shell } from "@opencode/schema/shell"
import { AppProcess } from "@opencode/util/process"
import { CrossSpawnSpawner } from "@opencode/util/cross-spawn-spawner"
import { makeGlobalNode, makeLocationNode } from "@opencode/util/effect/app-node"
import { FSUtil } from "@opencode/util/fs-util"
import { Bus } from "./bus.js"
@@ -345,12 +346,13 @@ const layer = () =>
}),
)
const finish = (status: Info["status"], exit?: number, beforeWait = Effect.void) =>
const finish = (status: Info["status"], exit?: number, beforeWait = Effect.void, signal?: string) =>
Effect.gen(function* () {
if (command.info.status !== "running") return
command.info = produce(command.info, (draft) => {
draft.status = status
if (exit !== undefined) draft.exit = exit
if (signal !== undefined) draft.signal = signal
draft.time.completed = Date.now()
})
yield* beforeWait
@@ -401,7 +403,14 @@ const layer = () =>
runFork(
handle.exitCode.pipe(
Effect.flatMap((code) => finish("exited", code)),
Effect.catch(() => finish("exited")),
Effect.catch((error) =>
finish(
"exited",
undefined,
Effect.void,
error.cause instanceof CrossSpawnSpawner.KilledBySignal ? error.cause.signal : undefined,
),
),
),
)
+5 -2
View File
@@ -7,7 +7,7 @@ export type Result = {
capture: { output: string; truncated: boolean } | undefined
}
type Output = { output: string; truncated: boolean; exit?: number; timeout?: boolean }
type Output = { output: string; truncated: boolean; exit?: number; signal?: string; timeout?: boolean }
const missing = "Shell command output is no longer available."
export const unavailable: Shell.Output = {
@@ -22,12 +22,14 @@ export function output(result: Result): Output {
output: result.capture?.output ?? unavailable.output,
truncated: result.capture?.truncated ?? false,
...(result.info.exit !== undefined ? { exit: result.info.exit } : {}),
...(result.info.signal !== undefined ? { signal: result.info.signal } : {}),
...(result.info.status === "timeout" ? { timeout: true } : {}),
}
}
export function notice(output: Pick<Output, "exit" | "timeout">) {
export function notice(output: Pick<Output, "exit" | "signal" | "timeout">) {
if (output.timeout) return "Timed out before completion"
if (output.signal !== undefined) return `Killed by ${output.signal}`
if (output.exit !== undefined && output.exit !== 0) return `Exited with code ${output.exit}`
}
@@ -35,6 +37,7 @@ export function metadata(output: Output) {
return {
truncated: output.truncated,
...(output.exit !== undefined ? { exit: output.exit } : {}),
...(output.signal !== undefined ? { signal: output.signal } : {}),
...(output.timeout !== undefined ? { timeout: output.timeout } : {}),
}
}
+1
View File
@@ -61,6 +61,7 @@ export const Input = Schema.Struct({
const StructuredOutput = Schema.Struct({
exit: Schema.optionalKey(Schema.Number),
signal: Schema.optionalKey(Schema.String),
shellID: Schema.optionalKey(Schema.String),
truncated: Schema.Boolean,
timeout: Schema.optionalKey(Schema.Boolean),
+1 -1
View File
@@ -1679,7 +1679,7 @@ describe("ShellTool", () => {
process.kill(-info.pid, "SIGTERM")
const result = yield* shell.wait(id).pipe(Effect.timeoutOption(Duration.seconds(1)))
expect(result._tag).toBe("Some")
if (result._tag === "Some") expect(result.value.status).toBe("exited")
if (result._tag === "Some") expect(result.value).toMatchObject({ status: "exited", signal: "SIGTERM" })
expect((yield* shell.list()).map((item) => item.id)).not.toContain(id)
}),
)
+1
View File
@@ -45,6 +45,7 @@ export const Info = Schema.Struct({
file: Schema.String,
pid: optional(NonNegativeInt),
exit: optional(Schema.Finite),
signal: optional(Schema.String),
// Always present; defaults to an empty object when the creator supplies no metadata.
metadata: Metadata,
time: Time,
+11 -7
View File
@@ -85,6 +85,15 @@ const toPlatformError = (
})
}
export class KilledBySignal extends Error {
readonly signal: NodeJS.Signals
constructor(signal: NodeJS.Signals) {
super(`Process interrupted due to receipt of signal: '${signal}'`)
this.signal = signal
}
}
type ExitSignal = Deferred.Deferred<readonly [code: number | null, signal: NodeJS.Signals | null]>
type Spawned = readonly [process: NodeChildProcess.ChildProcess, closed: ExitSignal, exited: ExitSignal]
@@ -471,13 +480,8 @@ const makeCrossSpawnSpawner = Effect.gen(function* () {
}),
exitCode: Effect.flatMap(completion, ([code, signal]) => {
if (Predicate.isNotNull(code)) return Effect.succeed(ExitCode(code))
return Effect.fail(
toPlatformError(
"exitCode",
new Error(`Process interrupted due to receipt of signal: '${signal}'`),
command,
),
)
// Node reports a signal whenever the exit code is null.
return Effect.fail(toPlatformError("exitCode", new KilledBySignal(signal!), command))
}),
kill: (opts?: ChildProcess.KillOptions) => stop(command, proc, closed, stopOutput, opts),
unref: Effect.sync(() => {