Compare commits

...

4 Commits

Author SHA1 Message Date
Hona 82565bf336 fix(opencode): handle shell capture failures 2026-07-21 01:11:33 +00:00
Hona 9e883bfebf fix(opencode): describe retained shell pipes 2026-07-21 00:43:32 +00:00
Hona 421100d322 fix(opencode): clarify incomplete shell output 2026-07-21 00:42:02 +00:00
Hona cb1cef6747 fix(opencode): bound shell output after exit 2026-07-21 00:40:22 +00:00
4 changed files with 107 additions and 5 deletions
+2 -3
View File
@@ -268,17 +268,16 @@ export const make = Effect.gen(function* () {
Effect.callback<readonly [NodeChildProcess.ChildProcess, ExitSignal], PlatformError.PlatformError>((resume) => { Effect.callback<readonly [NodeChildProcess.ChildProcess, ExitSignal], PlatformError.PlatformError>((resume) => {
const signal = Deferred.makeUnsafe<readonly [code: number | null, signal: NodeJS.Signals | null]>() const signal = Deferred.makeUnsafe<readonly [code: number | null, signal: NodeJS.Signals | null]>()
const proc = launch(command.command, command.args, opts) const proc = launch(command.command, command.args, opts)
let end = false
let exit: readonly [code: number | null, signal: NodeJS.Signals | null] | undefined let exit: readonly [code: number | null, signal: NodeJS.Signals | null] | undefined
proc.on("error", (err) => { proc.on("error", (err) => {
resume(Effect.fail(toPlatformError("spawn", err, command))) resume(Effect.fail(toPlatformError("spawn", err, command)))
}) })
proc.on("exit", (...args) => { proc.on("exit", (...args) => {
exit = args exit = args
Deferred.doneUnsafe(signal, Exit.succeed(args))
}) })
proc.on("close", (...args) => { proc.on("close", (...args) => {
if (end) return // cross-spawn can suppress `exit` for a Windows ENOENT and only emit `close`.
end = true
Deferred.doneUnsafe(signal, Exit.succeed(exit ?? args)) Deferred.doneUnsafe(signal, Exit.succeed(exit ?? args))
}) })
proc.on("spawn", () => { proc.on("spawn", () => {
@@ -285,6 +285,46 @@ describe("cross-spawn spawner", () => {
expect(running).toBe(false) expect(running).toBe(false)
}), }),
) )
fx.effect(
"resolves exit before inherited output pipes close",
Effect.gen(function* () {
const tmp = yield* Effect.acquireRelease(
Effect.promise(() => tmpdir()),
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
)
const file = path.join(tmp.path, "child.pid")
const child = "setInterval(() => {}, 10_000)"
const handle = yield* ChildProcess.make(process.execPath, [
"-e",
[
'const { spawn } = require("node:child_process")',
'const { writeFileSync } = require("node:fs")',
`const child = spawn(process.execPath, ["-e", ${JSON.stringify(child)}], { detached: true, stdio: "inherit" })`,
"child.unref()",
`writeFileSync(${JSON.stringify(file)}, String(child.pid))`,
].join("\n"),
])
yield* Effect.addFinalizer(() =>
Effect.promise(async () => {
const pid = Number(await fs.readFile(file, "utf8").catch(() => ""))
if (!pid) return
try {
process.kill(pid, "SIGKILL")
} catch {}
}),
)
const code = yield* handle.exitCode.pipe(
Effect.timeoutOrElse({
duration: "2 seconds",
orElse: () => Effect.die("exitCode waited for inherited output pipes"),
}),
)
expect(code).toBe(ChildProcessSpawner.ExitCode(0))
expect(yield* handle.isRunning).toBe(false)
}),
)
}) })
describe("error handling", () => { describe("error handling", () => {
+18 -2
View File
@@ -1,4 +1,4 @@
import { Effect, Stream } from "effect" import { Effect, Exit, Fiber, Stream } from "effect"
import os from "os" import os from "os"
import { createWriteStream } from "node:fs" import { createWriteStream } from "node:fs"
import * as Tool from "./tool" import * as Tool from "./tool"
@@ -25,6 +25,7 @@ import { BashArity } from "@/permission/arity"
export { Parameters } from "./shell/prompt" export { Parameters } from "./shell/prompt"
const MAX_METADATA_LENGTH = 30_000 const MAX_METADATA_LENGTH = 30_000
const POST_EXIT_OUTPUT_GRACE_MS = 500
const CWD = new Set(["cd", "chdir", "popd", "pushd", "push-location", "set-location"]) const CWD = new Set(["cd", "chdir", "popd", "pushd", "push-location", "set-location"])
const FILES = new Set([ const FILES = new Set([
...CWD, ...CWD,
@@ -446,6 +447,7 @@ export const ShellTool = Tool.define(
let cut = false let cut = false
let expired = false let expired = false
let aborted = false let aborted = false
let incomplete = false
const closeSink = Effect.fnUntraced(function* () { const closeSink = Effect.fnUntraced(function* () {
const stream = sink const stream = sink
@@ -483,7 +485,7 @@ export const ShellTool = Tool.define(
yield* Effect.addFinalizer(closeSink) yield* Effect.addFinalizer(closeSink)
const handle = yield* spawner.spawn(cmd(input.shell, input.command, input.cwd, input.env)) 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) => { Stream.runForEach(Stream.decodeText(handle.all), (chunk) => {
const size = Buffer.byteLength(chunk, "utf-8") const size = Buffer.byteLength(chunk, "utf-8")
list.push({ text: chunk, size }) list.push({ text: chunk, size })
@@ -554,6 +556,15 @@ export const ShellTool = Tool.define(
yield* handle.kill({ forceKillAfter: "3 seconds" }).pipe(Effect.orDie) yield* handle.kill({ forceKillAfter: "3 seconds" }).pipe(Effect.orDie)
} }
const outputComplete = yield* Fiber.await(output).pipe(
Effect.map(Exit.isSuccess),
Effect.timeoutOrElse({
duration: `${POST_EXIT_OUTPUT_GRACE_MS} millis`,
orElse: () => Effect.succeed(false),
}),
)
if (!outputComplete) incomplete = true
return exit.kind === "exit" ? exit.code : null return exit.kind === "exit" ? exit.code : null
}), }),
).pipe(Effect.orDie) ).pipe(Effect.orDie)
@@ -565,6 +576,10 @@ export const ShellTool = Tool.define(
) )
} }
if (aborted) meta.push("User aborted the command") if (aborted) meta.push("User aborted the command")
if (incomplete)
meta.push(
`shell process exited, but stdout/stderr did not reach EOF within ${POST_EXIT_OUTPUT_GRACE_MS} ms; a descendant process may still hold inherited pipe handles open`,
)
const raw = list.map((item) => item.text).join("") const raw = list.map((item) => item.text).join("")
const end = tail(raw, limits.maxLines, limits.maxBytes) const end = tail(raw, limits.maxLines, limits.maxBytes)
if (end.cut) cut = true if (end.cut) cut = true
@@ -588,6 +603,7 @@ export const ShellTool = Tool.define(
output: last || preview(output), output: last || preview(output),
exit: code, exit: code,
truncated: cut, truncated: cut,
...(incomplete ? { outputIncomplete: true } : {}),
...(cut && file ? { outputPath: file } : {}), ...(cut && file ? { outputPath: file } : {}),
}, },
output, output,
+47
View File
@@ -152,6 +152,15 @@ const withShell = <A, E, R>(item: { label: string; shell: string }, self: Effect
}), }),
) )
const kill = (file: string) =>
Effect.promise(async () => {
const pid = Number(await Bun.file(file).text().catch(() => ""))
if (!pid) return
try {
process.kill(pid, "SIGKILL")
} catch {}
})
const each = ( const each = (
name: string, name: string,
fn: (item: { label: string; shell: string }) => Effect.Effect<void, unknown, ShellTestServices>, fn: (item: { label: string; shell: string }) => Effect.Effect<void, unknown, ShellTestServices>,
@@ -1077,6 +1086,44 @@ describe("tool.shell abort", () => {
15_000, 15_000,
) )
it.live(
"stops waiting when a descendant retains the output pipe",
() =>
Effect.gen(function* () {
const tmp = yield* tmpdirScoped()
const file = path.join(tmp, "parent.js")
const pid = path.join(tmp, "child.pid")
const child = 'setInterval(() => process.stdout.write("tick\\n"), 20)'
yield* Effect.promise(() =>
Bun.write(
file,
[
'const { spawn } = require("node:child_process")',
'const { writeFileSync } = require("node:fs")',
`const child = spawn(process.execPath, ["-e", ${JSON.stringify(child)}], { detached: true, stdio: "inherit" })`,
"child.unref()",
`writeFileSync(${JSON.stringify(pid)}, String(child.pid))`,
'console.log("parent done")',
].join("\n"),
),
)
const command = `${bin} ${quote(file.replaceAll("\\", "/"))}`
const started = Date.now()
const result = yield* runIn(
tmp,
run({ command: PS.has(sh()) ? `& ${command}` : command, timeout: 5_000 }),
).pipe(Effect.ensuring(kill(pid)))
expect(Date.now() - started).toBeLessThan(3_000)
expect(result.output).toContain("parent done")
expect(result.output).toContain(
"shell process exited, but stdout/stderr did not reach EOF within 500 ms; a descendant process may still hold inherited pipe handles open",
)
expect(result.metadata.outputIncomplete).toBe(true)
}),
15_000,
)
if (process.platform !== "win32") { if (process.platform !== "win32") {
it.live("captures stderr in output", () => it.live("captures stderr in output", () =>
runIn( runIn(