Skip to content

Commit 805ce1a

Browse files
committed
fix(core): resolve spawn on exit not close to avoid bg-child hang
The cross-spawn spawner only resolved its exit Deferred on the Node `close` event, which waits for all stdio descriptors to drain. When a spawned command forks a background grandchild that inherits those descriptors (e.g. `cmd &`, daemonized servers, gradle daemon, MCP stdio servers), `close` never fires and the shell tool hangs until the configured timeout (~2 minutes). Settle the exit Deferred on whichever of `exit` / `close` fires first. `exit` fires when the direct child process exits regardless of inherited-fd lifetime, which is what callers actually care about. Closes #20902
1 parent 0cf99cf commit 805ce1a

2 files changed

Lines changed: 48 additions & 5 deletions

File tree

packages/core/src/cross-spawn-spawner.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -267,17 +267,26 @@ export const make = Effect.gen(function* () {
267267
const signal = Deferred.makeUnsafe<readonly [code: number | null, signal: NodeJS.Signals | null]>()
268268
const proc = launch(command.command, command.args, opts)
269269
let end = false
270-
let exit: readonly [code: number | null, signal: NodeJS.Signals | null] | undefined
270+
const settle = (args: readonly [code: number | null, signal: NodeJS.Signals | null]) => {
271+
if (end) return
272+
end = true
273+
Deferred.doneUnsafe(signal, Exit.succeed(args))
274+
}
271275
proc.on("error", (err) => {
272276
resume(Effect.fail(toPlatformError("spawn", err, command)))
273277
})
278+
// Resolve on `exit` rather than `close`. The `close` event only fires
279+
// once all stdio descriptors are released, which can hang indefinitely
280+
// when the spawned process forks a background grandchild that inherits
281+
// those descriptors (e.g. `command &`, daemonized servers, gradle, etc).
282+
// The `exit` event fires when the direct child exits regardless of
283+
// inherited-fd lifetime, which is what we actually care about here.
284+
// See https://github.com/anomalyco/opencode/issues/20902.
274285
proc.on("exit", (...args) => {
275-
exit = args
286+
settle(args)
276287
})
277288
proc.on("close", (...args) => {
278-
if (end) return
279-
end = true
280-
Deferred.doneUnsafe(signal, Exit.succeed(exit ?? args))
289+
settle(args)
281290
})
282291
proc.on("spawn", () => {
283292
resume(Effect.succeed([proc, signal]))

packages/opencode/test/tool/shell.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,3 +1228,37 @@ describe("tool.shell truncation", () => {
12281228
),
12291229
)
12301230
})
1231+
1232+
// Regression test for https://github.com/anomalyco/opencode/issues/20902 —
1233+
// running a command that spawns a long-lived background grandchild used to
1234+
// hang the shell tool until the 2-minute timeout, because the grandchild
1235+
// kept the inherited stdout/stderr file descriptors open.
1236+
describe("tool.shell background children", () => {
1237+
if (process.platform === "win32") return
1238+
it.live(
1239+
"returns promptly when command backgrounds a long-lived child",
1240+
() =>
1241+
runIn(
1242+
projectRoot,
1243+
Effect.gen(function* () {
1244+
const start = Date.now()
1245+
// `sleep 30 &` spawns a backgrounded sleep that inherits stdout/stderr.
1246+
// The redirect on disown keeps it alive past the parent shell exit.
1247+
// `disown` removes it from the shell's job table; `sleep 30 &` alone
1248+
// would also work on most shells.
1249+
const result = yield* run({
1250+
command: "sleep 30 & disown; echo backgrounded",
1251+
description: "Background child regression test",
1252+
timeout: 15000,
1253+
})
1254+
const elapsed = Date.now() - start
1255+
expect(result.metadata.exit).toBe(0)
1256+
expect(result.output).toContain("backgrounded")
1257+
// Before the fix this took the full timeout (~15s) or longer.
1258+
// Allow generous headroom for slow CI but well under the timeout.
1259+
expect(elapsed).toBeLessThan(10_000)
1260+
}),
1261+
),
1262+
20_000,
1263+
)
1264+
})

0 commit comments

Comments
 (0)