diff --git a/packages/core/test/effect/cross-spawn-spawner.test.ts b/packages/core/test/effect/cross-spawn-spawner.test.ts index a3c950cb854..8f6614a308c 100644 --- a/packages/core/test/effect/cross-spawn-spawner.test.ts +++ b/packages/core/test/effect/cross-spawn-spawner.test.ts @@ -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((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( diff --git a/packages/core/test/session-shell.test.ts b/packages/core/test/session-shell.test.ts index a0cb863c7ef..f2ec0a21724 100644 --- a/packages/core/test/session-shell.test.ts +++ b/packages/core/test/session-shell.test.ts @@ -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", diff --git a/packages/util/src/cross-spawn-spawner.ts b/packages/util/src/cross-spawn-spawner.ts index 3c37a7eceb3..8c643c2df2b 100644 --- a/packages/util/src/cross-spawn-spawner.ts +++ b/packages/util/src/cross-spawn-spawner.ts @@ -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(