diff --git a/packages/core/src/cross-spawn-spawner.ts b/packages/core/src/cross-spawn-spawner.ts index ad8d4126d454..e8d2dffb410f 100644 --- a/packages/core/src/cross-spawn-spawner.ts +++ b/packages/core/src/cross-spawn-spawner.ts @@ -92,7 +92,7 @@ const toPlatformError = ( }) } -type ExitSignal = Deferred.Deferred +type ProcessSignal = Deferred.Deferred export const make = Effect.gen(function* () { const fs = yield* FileSystem.FileSystem @@ -263,21 +263,28 @@ export const make = Effect.gen(function* () { } const spawn = (command: ChildProcess.StandardCommand, opts: NodeChildProcess.SpawnOptions) => - Effect.callback((resume) => { + Effect.callback((resume) => { const signal = Deferred.makeUnsafe() const proc = launch(command.command, command.args, opts) - let end = false + let exited = false + let closed = false let exit: readonly [code: number | null, signal: NodeJS.Signals | null] | undefined + const done = (args: readonly [code: number | null, signal: NodeJS.Signals | null]) => { + if (exited) return + exited = true + exit = args + Deferred.doneUnsafe(signal, Exit.succeed(args)) + } proc.on("error", (err) => { resume(Effect.fail(toPlatformError("spawn", err, command))) }) proc.on("exit", (...args) => { - exit = args + done(args) }) proc.on("close", (...args) => { - if (end) return - end = true - Deferred.doneUnsafe(signal, Exit.succeed(exit ?? args)) + if (closed) return + closed = true + done(exit ?? args) }) proc.on("spawn", () => { resume(Effect.succeed([proc, signal])) diff --git a/packages/core/test/effect/cross-spawn-spawner.test.ts b/packages/core/test/effect/cross-spawn-spawner.test.ts index 8a2fab493063..176d9c8bc24a 100644 --- a/packages/core/test/effect/cross-spawn-spawner.test.ts +++ b/packages/core/test/effect/cross-spawn-spawner.test.ts @@ -88,6 +88,45 @@ describe("cross-spawn spawner", () => { }), ) + fx.effect( + "returns exit code before inherited stdout pipe closes", + Effect.gen(function* () { + if (process.platform === "win32") return + + const tmp = yield* Effect.acquireRelease( + Effect.promise(() => tmpdir()), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ) + const pidfile = path.join(tmp.path, "child.pid") + yield* Effect.addFinalizer(() => + Effect.promise(async () => { + const pid = Number(await fs.readFile(pidfile, "utf8").catch(() => "0")) + if (!pid) return + try { + process.kill(pid, "SIGKILL") + } catch {} + }), + ) + + const handle = yield* js( + [ + 'const { spawn } = require("node:child_process")', + 'const fs = require("node:fs")', + `const child = spawn(process.execPath, ["-e", "setTimeout(() => {}, 10000)"], { detached: true, stdio: ["ignore", "inherit", "ignore"] })`, + `fs.writeFileSync(${JSON.stringify(pidfile)}, String(child.pid))`, + "child.unref()", + ].join("\n"), + ) + const code = yield* handle.exitCode.pipe( + Effect.timeoutOrElse({ + duration: "1 second", + orElse: () => Effect.fail(new Error("timed out waiting for process exit code")), + }), + ) + expect(code).toBe(ChildProcessSpawner.ExitCode(0)) + }), + ) + fx.effect( "returns non-zero exit code", Effect.gen(function* () { diff --git a/packages/opencode/src/tool/shell.ts b/packages/opencode/src/tool/shell.ts index b6a95b5c0970..98a0a922989b 100644 --- a/packages/opencode/src/tool/shell.ts +++ b/packages/opencode/src/tool/shell.ts @@ -1,4 +1,4 @@ -import { Effect, Stream } from "effect" +import { Effect, Fiber, Stream } from "effect" import os from "os" import { createWriteStream } from "node:fs" import * as Tool from "./tool" @@ -481,7 +481,7 @@ export const ShellTool = Tool.define( yield* Effect.addFinalizer(closeSink) const handle = yield* spawner.spawn(cmd(input.shell, input.command, input.cwd, input.env)) - yield* Effect.forkScoped( + const output = yield* Effect.forkScoped( Stream.runForEach(Stream.decodeText(handle.all), (chunk) => { const size = Buffer.byteLength(chunk, "utf-8") list.push({ text: chunk, size }) @@ -554,6 +554,14 @@ export const ShellTool = Tool.define( yield* handle.kill({ forceKillAfter: "3 seconds" }).pipe(Effect.orDie) } + yield* Fiber.join(output).pipe( + Effect.timeoutOrElse({ + duration: "500 millis", + orElse: () => Effect.void, + }), + Effect.ignore, + ) + return exit.kind === "exit" ? exit.code : null }), ).pipe(Effect.orDie) diff --git a/packages/opencode/test/tool/shell.test.ts b/packages/opencode/test/tool/shell.test.ts index ddaa5c2ec7b1..345c8d95db07 100644 --- a/packages/opencode/test/tool/shell.test.ts +++ b/packages/opencode/test/tool/shell.test.ts @@ -1026,6 +1026,50 @@ describe("tool.shell permissions", () => { }) describe("tool.shell abort", () => { + if (process.platform !== "win32") { + it.live( + "returns when command exits before inherited stdout closes", + () => + runIn( + projectRoot, + Effect.gen(function* () { + const tmp = yield* tmpdirScoped() + const pidfile = path.join(tmp, "child.pid") + yield* Effect.addFinalizer(() => + Effect.promise(async () => { + const pid = Number( + await Bun.file(pidfile) + .text() + .catch(() => "0"), + ) + if (!pid) return + try { + process.kill(pid, "SIGKILL") + } catch {} + }), + ) + + const result = yield* run({ + command: `${bin} -e ${squote( + [ + 'const cp = require("node:child_process")', + 'const fs = require("node:fs")', + `const child = cp.spawn(process.execPath, ["-e", "setTimeout(() => {}, 10000)"], { detached: true, stdio: ["ignore", "inherit", "ignore"] })`, + `fs.writeFileSync(${JSON.stringify(pidfile)}, String(child.pid))`, + "child.unref()", + 'console.log("parent done")', + ].join(";"), + )}`, + description: "Spawn detached child inheriting stdout", + }) + expect(result.metadata.exit).toBe(0) + expect(result.output).toContain("parent done") + }), + ), + 5_000, + ) + } + it.live( "preserves output when aborted", () =>