opencode icon indicating copy to clipboard operation
opencode copied to clipboard

fix(opencode): Improve killing bash tools on aborts

Open dbpolito opened this issue 2 days ago • 2 comments

What does this PR do?

Sometimes when i abort a prompt that has some bash running, like bun dev, it doesn't properly kill the process and keeps running indefinitely.

How did you verify your code works?

TBH i wasn't able to properly reproduce this, so can't be sure this is doing what it should, but the code looks good, this is the reasoning from the IA:

On macOS/Linux there are two different “targets” you can send signals to:

proc.kill(signal) targets one process: the child process with proc.pid.
process.kill(-pid, signal) targets a process group: everything in the group whose id is pid (negative pid means “process group”).
Those behave very differently for bun dev, Electron, watchers, etc.

Why -pid (process group) matters

Dev commands usually spawn child processes (and sometimes grandchildren).
Killing only the parent shell/process (proc.kill(...)) can leave children running.
If you create the child with detached: true, Node/Bun typically makes it the leader of a new process group; then process.kill(-pid, ...) is the right way to kill the whole subtree.
What was already happening before In packages/opencode/src/tool/bash.ts:157, the bash tool spawns with detached: process.platform !== "win32", and Shell.killTree already tries to kill the group via process.kill(-pid, ...), falling back to proc.kill(...) if that fails.

So the “pid vs proc” distinction was already in place.

What I changed (the real difference) The change isn’t “pid vs proc” — it’s the signal sequence:

Before: SIGTERM → wait → SIGKILL
Now: SIGINT → wait → SIGTERM → wait → SIGKILL
A lot of dev tooling behaves like:

handles SIGINT (Ctrl+C) and exits cleanly, including shutting down spawned children
ignores or mishandles SIGTERM (common with some wrappers, Electron tooling, some Node scripts)
if you jump straight to SIGKILL, nothing gets a chance to clean up, and you can more easily end up with orphaned processes / stuck resources
TL;DR

proc.kill() == kill only the immediate child.
process.kill(-pid, ...) == kill the whole process group (better for “stop bun dev”).
The patch mostly improves how we kill (SIGINT-first), not what we kill.

dbpolito avatar Jan 09 '26 18:01 dbpolito