Skip to content
Open
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
19 changes: 14 additions & 5 deletions packages/core/src/cross-spawn-spawner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,17 +267,26 @@ export const make = Effect.gen(function* () {
const signal = Deferred.makeUnsafe<readonly [code: number | null, signal: NodeJS.Signals | null]>()
const proc = launch(command.command, command.args, opts)
let end = false
let exit: readonly [code: number | null, signal: NodeJS.Signals | null] | undefined
const settle = (args: readonly [code: number | null, signal: NodeJS.Signals | null]) => {
if (end) return
end = true
Deferred.doneUnsafe(signal, Exit.succeed(args))
}
proc.on("error", (err) => {
resume(Effect.fail(toPlatformError("spawn", err, command)))
})
// Resolve on `exit` rather than `close`. The `close` event only fires
// once all stdio descriptors are released, which can hang indefinitely
// when the spawned process forks a background grandchild that inherits
// those descriptors (e.g. `command &`, daemonized servers, gradle, etc).
// The `exit` event fires when the direct child exits regardless of
// inherited-fd lifetime, which is what we actually care about here.
// See https://github.com/anomalyco/opencode/issues/20902.
proc.on("exit", (...args) => {
exit = args
settle(args)
})
proc.on("close", (...args) => {
if (end) return
end = true
Deferred.doneUnsafe(signal, Exit.succeed(exit ?? args))
settle(args)
})
proc.on("spawn", () => {
resume(Effect.succeed([proc, signal]))
Expand Down
34 changes: 34 additions & 0 deletions packages/opencode/test/tool/shell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1228,3 +1228,37 @@ describe("tool.shell truncation", () => {
),
)
})

// Regression test for https://github.com/anomalyco/opencode/issues/20902 —
// running a command that spawns a long-lived background grandchild used to
// hang the shell tool until the 2-minute timeout, because the grandchild
// kept the inherited stdout/stderr file descriptors open.
describe("tool.shell background children", () => {
if (process.platform === "win32") return
it.live(
"returns promptly when command backgrounds a long-lived child",
() =>
runIn(
projectRoot,
Effect.gen(function* () {
const start = Date.now()
// `sleep 30 &` spawns a backgrounded sleep that inherits stdout/stderr.
// The redirect on disown keeps it alive past the parent shell exit.
// `disown` removes it from the shell's job table; `sleep 30 &` alone
// would also work on most shells.
const result = yield* run({
command: "sleep 30 & disown; echo backgrounded",
description: "Background child regression test",
timeout: 15000,
})
const elapsed = Date.now() - start
expect(result.metadata.exit).toBe(0)
expect(result.output).toContain("backgrounded")
// Before the fix this took the full timeout (~15s) or longer.
// Allow generous headroom for slow CI but well under the timeout.
expect(elapsed).toBeLessThan(10_000)
}),
),
20_000,
)
})
Loading