Skip to content
Closed
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
21 changes: 14 additions & 7 deletions packages/core/src/cross-spawn-spawner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const toPlatformError = (
})
}

type ExitSignal = Deferred.Deferred<readonly [code: number | null, signal: NodeJS.Signals | null]>
type ProcessSignal = Deferred.Deferred<readonly [code: number | null, signal: NodeJS.Signals | null]>

export const make = Effect.gen(function* () {
const fs = yield* FileSystem.FileSystem
Expand Down Expand Up @@ -263,21 +263,28 @@ export const make = Effect.gen(function* () {
}

const spawn = (command: ChildProcess.StandardCommand, opts: NodeChildProcess.SpawnOptions) =>
Effect.callback<readonly [NodeChildProcess.ChildProcess, ExitSignal], PlatformError.PlatformError>((resume) => {
Effect.callback<readonly [NodeChildProcess.ChildProcess, ProcessSignal], PlatformError.PlatformError>((resume) => {
const signal = Deferred.makeUnsafe<readonly [code: number | null, signal: NodeJS.Signals | null]>()
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]))
Expand Down
39 changes: 39 additions & 0 deletions packages/core/test/effect/cross-spawn-spawner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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* () {
Expand Down
12 changes: 10 additions & 2 deletions packages/opencode/src/tool/shell.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -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)
Expand Down
44 changes: 44 additions & 0 deletions packages/opencode/test/tool/shell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
() =>
Expand Down
Loading