mirror of
https://github.com/anomalyco/opencode.git
synced 2026-09-02 16:24:17 +00:00
fix(util): separate process exit from capture completion
Report exit and running state from the child exit signal, independently of buffered output. On scope release, discard abandoned capture and retain the existing pipe-close or capture-deadline wait before applying process-group cleanup policy. Cover unread output after confirmed process exit, successful descendant survival, and the approved policy that a parent exiting successfully before its invocation timeout retains success while the bounded capture grace finishes.
This commit is contained in:
parent
ef8b8c1b8d
commit
acaca2cc01
3 changed files with 87 additions and 11 deletions
|
|
@ -185,8 +185,9 @@ describe("cross-spawn spawner", () => {
|
|||
`captures ${output} when reading starts after process exit`,
|
||||
Effect.gen(function* () {
|
||||
const handle = yield* js('process.stdout.write("stdout\\n"); process.stderr.write("stderr\\n")')
|
||||
// Tiny output lets the child and its pipes close before any reader starts, with the scope still open.
|
||||
expect(yield* handle.exitCode).toBe(ChildProcessSpawner.ExitCode(0))
|
||||
// Let exit callbacks finish before attaching a reader; the handle scope remains open.
|
||||
yield* Effect.promise(() => new Promise<void>((resolve) => setImmediate(resolve)))
|
||||
expect((yield* decodeByteStream(handle[output])).split("\n").toSorted()).toEqual(
|
||||
output === "all" ? ["stderr", "stdout"] : [output],
|
||||
)
|
||||
|
|
@ -247,6 +248,16 @@ describe("cross-spawn spawner", () => {
|
|||
})
|
||||
|
||||
describe("process control", () => {
|
||||
fx.live(
|
||||
"reports exit without waiting for unread stdout",
|
||||
Effect.gen(function* () {
|
||||
const handle = yield* js("process.stdout.write(Buffer.alloc(1024 * 1024)); process.exit(0)")
|
||||
expect(yield* Effect.promise(() => gone(Number(handle.pid)))).toBe(true)
|
||||
expect(yield* handle.exitCode.pipe(Effect.timeout("500 millis"))).toBe(ChildProcessSpawner.ExitCode(0))
|
||||
expect(yield* handle.isRunning).toBe(false)
|
||||
}),
|
||||
)
|
||||
|
||||
fx.live(
|
||||
"releases a process with unread buffered stdout",
|
||||
Effect.gen(function* () {
|
||||
|
|
@ -264,6 +275,34 @@ describe("cross-spawn spawner", () => {
|
|||
}).pipe(Effect.timeout("3 seconds")),
|
||||
)
|
||||
|
||||
fx.live(
|
||||
"preserves successful descendants when an exit-only scope closes",
|
||||
Effect.gen(function* () {
|
||||
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.tryPromise(async () => process.kill(Number(await fs.readFile(pidFile, "utf8")), "SIGKILL")).pipe(
|
||||
Effect.ignore,
|
||||
),
|
||||
)
|
||||
yield* Effect.scoped(
|
||||
Effect.gen(function* () {
|
||||
// This fixture's child shares the process group and holds stdio after the parent exits on stdin EOF.
|
||||
const handle = yield* ChildProcess.make(
|
||||
"node",
|
||||
[path.join(import.meta.dir, "../fixture/held-stdio.cjs"), "mcp", pidFile],
|
||||
{ stdin: "ignore", forceKillAfter: 100 },
|
||||
)
|
||||
expect(yield* handle.exitCode).toBe(ChildProcessSpawner.ExitCode(0))
|
||||
}),
|
||||
)
|
||||
expect(alive(Number(yield* Effect.promise(() => fs.readFile(pidFile, "utf8"))))).toBe(true)
|
||||
}).pipe(Effect.timeout("3 seconds")),
|
||||
)
|
||||
|
||||
for (const mode of ["exit", "SIGKILL"] as const) {
|
||||
const test = mode === "SIGKILL" && process.platform === "win32" ? fx.live.skip : fx.live
|
||||
test(
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { describe, expect } from "bun:test"
|
|||
import fs from "fs/promises"
|
||||
import path from "path"
|
||||
import { Cause, Context, Deferred, Effect, Exit, Fiber, Layer, Option, Schedule, Stream } from "effect"
|
||||
import { TestClock } from "effect/testing"
|
||||
import { Bus } from "@opencode-ai/core/bus"
|
||||
import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder"
|
||||
import { Location } from "@opencode-ai/core/location"
|
||||
|
|
@ -272,6 +273,45 @@ describe("Session.shell", () => {
|
|||
)
|
||||
}
|
||||
|
||||
it.effect("keeps success when the invocation timeout expires during post-exit capture", () =>
|
||||
Effect.gen(function* () {
|
||||
const fixture = yield* setup
|
||||
const pidFile = path.join(fixture.tmp.path, "child.pid")
|
||||
yield* Effect.addFinalizer(() =>
|
||||
Effect.tryPromise(async () => process.kill(Number(await fs.readFile(pidFile, "utf8")), "SIGKILL")).pipe(
|
||||
Effect.ignore,
|
||||
),
|
||||
)
|
||||
const info = yield* fixture.shell.create({
|
||||
command: `node "${path.join(import.meta.dir, "fixture/held-stdio.cjs")}" exit "${pidFile}"`,
|
||||
timeout: 500,
|
||||
})
|
||||
const completion = yield* fixture.shell.wait(info.id).pipe(Effect.forkScoped)
|
||||
// Wait for the real process without advancing its invocation timeout or capture deadline.
|
||||
yield* fixture.shell
|
||||
.get(info.id)
|
||||
.pipe(
|
||||
Effect.repeat({ until: (info) => info.status === "exited", schedule: Schedule.spaced("10 millis") }),
|
||||
Effect.timeout("3 seconds"),
|
||||
TestClock.withLive,
|
||||
)
|
||||
yield* TestClock.adjust("500 millis")
|
||||
expect(yield* fixture.shell.get(info.id)).toMatchObject({ status: "exited", exit: 0 })
|
||||
expect(completion.pollUnsafe()).toBeUndefined()
|
||||
|
||||
yield* TestClock.adjust("500 millis")
|
||||
expect(yield* Fiber.join(completion).pipe(Effect.timeout("3 seconds"), TestClock.withLive)).toMatchObject({
|
||||
status: "exited",
|
||||
exit: 0,
|
||||
})
|
||||
const result = yield* fixture.shell.result(info)
|
||||
expect(result.capture?.output).toContain("foreground-out")
|
||||
expect(result.capture?.output).toContain("foreground-err")
|
||||
const pid = Number(yield* Effect.promise(() => fs.readFile(pidFile, "utf8")))
|
||||
expect(() => process.kill(pid, 0)).not.toThrow()
|
||||
}),
|
||||
)
|
||||
|
||||
for (const outcome of [
|
||||
{
|
||||
status: "killed",
|
||||
|
|
|
|||
|
|
@ -442,8 +442,11 @@ const makeCrossSpawnSpawner = Effect.gen(function* () {
|
|||
}),
|
||||
Effect.fnUntraced(
|
||||
function* ([proc, closed, exited, stopOutput]) {
|
||||
const done = (yield* Deferred.isDone(closed)) || (yield* Deferred.isDone(stopOutput))
|
||||
if (done) {
|
||||
discard(proc.stdout)
|
||||
discard(proc.stderr)
|
||||
if (yield* Deferred.isDone(exited)) {
|
||||
// Reporting exit must not shorten the inherited-pipe grace period on scope release.
|
||||
yield* Effect.raceFirst(Deferred.await(closed), Deferred.await(stopOutput))
|
||||
const [code] = yield* Deferred.await(exited)
|
||||
if (process.platform === "win32") return
|
||||
if (code === 0 || Predicate.isNull(code)) return
|
||||
|
|
@ -466,10 +469,6 @@ const makeCrossSpawnSpawner = Effect.gen(function* () {
|
|||
),
|
||||
)
|
||||
|
||||
const completion = Effect.raceFirst(
|
||||
Deferred.await(closed),
|
||||
Deferred.await(stopOutput).pipe(Effect.andThen(Deferred.await(exited))),
|
||||
)
|
||||
const fd = yield* setupFds(command, proc, extra)
|
||||
const out = yield* setupOutput(command, proc, sout, serr, stopOutput)
|
||||
let ref = true
|
||||
|
|
@ -481,10 +480,8 @@ const makeCrossSpawnSpawner = Effect.gen(function* () {
|
|||
all: out.all,
|
||||
getInputFd: fd.getInputFd,
|
||||
getOutputFd: fd.getOutputFd,
|
||||
isRunning: Effect.gen(function* () {
|
||||
return !(yield* Deferred.isDone(closed)) && !(yield* Deferred.isDone(stopOutput))
|
||||
}),
|
||||
exitCode: Effect.flatMap(completion, ([code, signal]) => {
|
||||
isRunning: Effect.map(Deferred.isDone(exited), (done) => !done),
|
||||
exitCode: Effect.flatMap(Deferred.await(exited), ([code, signal]) => {
|
||||
if (Predicate.isNotNull(code)) return Effect.succeed(ExitCode(code))
|
||||
return Effect.fail(
|
||||
toPlatformError(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue