Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fuzzy-otters-signal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@kilocode/cli": patch
---

Report Bash commands terminated by a signal with the conventional 128 + signum exit code (e.g. 139 for SIGSEGV) instead of hanging until the command timeout.
12 changes: 2 additions & 10 deletions packages/core/src/cross-spawn-spawner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as NodePath from "@effect/platform-node/NodePath"
import { prepareCommand as prepareSandbox } from "@kilocode/sandbox" // kilocode_change
import { tap as tapStdio, tapped } from "./kilocode/stdio-tap" // kilocode_change - Bun drops buffered stdio on close
import * as SpawnValidation from "./kilocode/spawn-validation" // kilocode_change
import { settle } from "./kilocode/exit-code" // kilocode_change - settle signal termination as 128 + signum
import * as Deferred from "effect/Deferred"
import * as Effect from "effect/Effect"
import * as Exit from "effect/Exit"
Expand Down Expand Up @@ -441,16 +442,7 @@ export const make = Effect.gen(function* () {
getInputFd: fd.getInputFd,
getOutputFd: fd.getOutputFd,
isRunning: Effect.map(Deferred.isDone(signal), (done) => !done),
exitCode: Effect.flatMap(Deferred.await(signal), ([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,
),
)
}),
exitCode: Effect.flatMap(Deferred.await(signal), settle), // kilocode_change - signal termination settles as 128 + signum
kill: (opts?: ChildProcess.KillOptions) => {
const sig = opts?.killSignal ?? "SIGTERM"
const send = (s: NodeJS.Signals) =>
Expand Down
16 changes: 16 additions & 0 deletions packages/core/src/kilocode/exit-code.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { constants } from "node:os"
import * as Effect from "effect/Effect"
import * as Predicate from "effect/Predicate"
import { ExitCode } from "effect/unstable/process/ChildProcessSpawner"

// A process terminated by a signal produces a null exit code (see the "exit"
// event on node:child_process). Report the conventional 128 + signum code
// (e.g. 139 for SIGSEGV) instead of failing, which left consumers like the
// bash tool waiting on a numeric code that never arrived.
export const settle = ([code, signal]: readonly [code: number | null, signal: NodeJS.Signals | null]) => {
if (Predicate.isNotNull(code)) return Effect.succeed(ExitCode(code))
if (Predicate.isNotNull(signal) && signal in constants.signals) {
return Effect.succeed(ExitCode(128 + constants.signals[signal]))
}
return Effect.succeed(ExitCode(1))
}
30 changes: 30 additions & 0 deletions packages/core/test/kilocode/exit-code.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { describe, expect, test } from "bun:test"
import { Effect } from "effect"
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"
import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
import { settle } from "@opencode-ai/core/kilocode/exit-code"
import { testEffect } from "../lib/effect"

const fx = testEffect(CrossSpawnSpawner.defaultLayer)

const code = (exit: readonly [number | null, NodeJS.Signals | null]) => Effect.runPromise(settle(exit))

describe("exit-code settle", () => {
test("maps exit results to numeric codes", async () => {
expect(await code([0, null])).toBe(ChildProcessSpawner.ExitCode(0))
expect(await code([42, null])).toBe(ChildProcessSpawner.ExitCode(42))
expect(await code([null, "SIGSEGV"])).toBe(ChildProcessSpawner.ExitCode(128 + 11))
expect(await code([null, "SIGTERM"])).toBe(ChildProcessSpawner.ExitCode(128 + 15))
expect(await code([null, null])).toBe(ChildProcessSpawner.ExitCode(1))
expect(await code([null, "SIGWHAT" as NodeJS.Signals])).toBe(ChildProcessSpawner.ExitCode(1))
})

fx.effect(
"reports signal termination as 128 + signum",
Effect.gen(function* () {
if (process.platform === "win32") return
const handle = yield* ChildProcess.make(process.execPath, ["-e", "process.kill(process.pid, 'SIGKILL')"])
expect(yield* handle.exitCode).toBe(ChildProcessSpawner.ExitCode(128 + 9))
}),
)
})
72 changes: 72 additions & 0 deletions packages/opencode/test/kilocode/tool/shell-signal.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Regression tests for Kilo-Org/kilocode#12677.
//
// A shell terminated by a signal (for example `bash -c` exec'ing a binary that
// then segfaults) produces no numeric exit code. The spawner used to fail the
// exit-code effect in that case, so the bash tool's race kept waiting for the
// abort or timeout branches and the call appeared to hang. The spawner now
// settles signal termination as the conventional 128 + signum code.

import { describe, expect } from "bun:test"
import { Effect, Layer } from "effect"
import path from "path"
import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
import { FSUtil } from "@opencode-ai/core/fs-util"
import { Agent } from "../../../src/agent/agent"
import { Config } from "../../../src/config/config"
import { RuntimeFlags } from "../../../src/effect/runtime-flags"
import { Plugin } from "../../../src/plugin"
import { SessionID, MessageID } from "../../../src/session/schema"
import { ShellTool } from "../../../src/tool/shell"
import { Truncate } from "../../../src/tool/truncate"
import { provideInstance, testInstanceStoreLayer } from "../../fixture/fixture"
import { testEffect } from "../../lib/effect"

const layer = Layer.mergeAll(
CrossSpawnSpawner.defaultLayer,
FSUtil.defaultLayer,
Plugin.defaultLayer,
Truncate.defaultLayer,
Config.defaultLayer,
Agent.defaultLayer,
RuntimeFlags.defaultLayer,
testInstanceStoreLayer,
)
const it = testEffect(layer)

const ctx = {
sessionID: SessionID.make("ses_test"),
messageID: MessageID.make("msg_test"),
callID: "",
agent: "code",
abort: AbortSignal.any([]),
messages: [],
metadata: () => Effect.void,
ask: () => Effect.void,
}

const root = path.join(__dirname, "../../..")

describe("shell tool signal termination", () => {
if (process.platform !== "win32") {
it.live("settles with 128 + signum when the shell dies from a signal", () =>
provideInstance(root)(
Effect.gen(function* () {
const tool = yield* ShellTool
const bash = yield* tool.init()
const result = yield* bash
.execute(
{
command: "kill -SEGV $$",
description: "Terminate the shell with SIGSEGV",
timeout: 60_000,
},
ctx,
)
.pipe(Effect.timeout("5 seconds"))
expect(result.metadata.exit).toBe(128 + 11)
expect(result.output).not.toContain("exceeding timeout")
}),
),
)
}
})
Loading