Compare commits
4 commits
dev
...
shell-exit
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
82565bf336 | ||
|
|
9e883bfebf | ||
|
|
421100d322 | ||
|
|
cb1cef6747 |
4 changed files with 107 additions and 5 deletions
|
|
@ -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", () => {
|
||||||
|
|
|
||||||
|
|
@ -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,
|
||||||
|
|
|
||||||
|
|
@ -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(
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue