diff --git a/packages/core/src/cross-spawn-spawner.ts b/packages/core/src/cross-spawn-spawner.ts index ad8d4126d454..d931d6f27dcd 100644 --- a/packages/core/src/cross-spawn-spawner.ts +++ b/packages/core/src/cross-spawn-spawner.ts @@ -267,17 +267,26 @@ export const make = Effect.gen(function* () { const signal = Deferred.makeUnsafe() 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])) diff --git a/packages/opencode/test/tool/shell.test.ts b/packages/opencode/test/tool/shell.test.ts index ddaa5c2ec7b1..9b4d3cd44f9a 100644 --- a/packages/opencode/test/tool/shell.test.ts +++ b/packages/opencode/test/tool/shell.test.ts @@ -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, + ) +})