From e2d35bd8202608694c40001e9998fe9f29e8756b Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 31 Oct 2026 09:40:43 +0000 Subject: [PATCH 1/2] test(provider-acp): reproduce #28067 wait_for_exit hanging on inherited pipes Adds a failing test that reproduces pingdotgg/t3code#28067: a Devin shell command that exits while a background child still holds its stdout/stderr never gets an exit status, so terminal/wait_for_exit does not answer. Co-Authored-By: Claude Opus 5.5 --- .../src/server/clientTerminals.test.ts | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/packages/provider-acp/src/server/clientTerminals.test.ts b/packages/provider-acp/src/server/clientTerminals.test.ts index 791bbc22a6a03..1a476c9582642 100644 --- a/packages/provider-acp/src/server/clientTerminals.test.ts +++ b/packages/provider-acp/src/server/clientTerminals.test.ts @@ -96,6 +96,68 @@ describe("AcpClientTerminals", () => { }).pipe(Effect.provide(NodeServices.layer), Effect.scoped), ); + it.live( + "answers wait_for_exit when a command exits while a background child holds its output", + () => + Effect.gen(function* () { + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const spawned = yield* Deferred.make(); + const orphanStillHoldsOutput = yield* Deferred.make(); + let stdout = ""; + const terminals = yield* makeAcpClientTerminals({ + defaultCwd: process.cwd(), + shellCommands: true, + }).pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, { + ...spawner, + spawn: (command) => + spawner.spawn(command).pipe( + Effect.tap((child) => Deferred.succeed(spawned, child)), + Effect.map((child) => ({ + ...child, + stdout: child.stdout.pipe( + Stream.tap((chunk) => { + stdout += Buffer.from(chunk).toString("utf8"); + return stdout.includes("orphan-still-holds-output") + ? Deferred.succeed(orphanStillHoldsOutput, undefined) + : Effect.void; + }), + ), + })), + ), + }), + ); + yield* Effect.addFinalizer(() => terminals.disposeAll); + // Reap only the orphan this test started, by the PID the shell printed. + yield* Effect.addFinalizer(() => + Effect.sync(() => { + const orphanPid = Number(stdout.split("\n")[0]); + if (Number.isInteger(orphanPid) && orphanPid > 0) { + try { + process.kill(orphanPid, "SIGKILL"); + } catch {} + } + }), + ); + const terminal = yield* terminals.create({ + sessionId: "devin", + command: + '"$T3_TEST_NODE" -e \'setTimeout(() => console.log("orphan-still-holds-output"), 2000)\' & echo "$!"', + env: [{ name: "T3_TEST_NODE", value: process.execPath }], + }); + const child = yield* Deferred.await(spawned); + + const outcome = yield* Effect.raceFirst( + terminals.waitForExit({ sessionId: "devin", ...terminal }), + Deferred.await(orphanStillHoldsOutput).pipe( + Effect.as("no answer while the background child holds stdout/stderr"), + ), + ); + expect(yield* child.isRunning).toBe(false); + expect(outcome).toEqual({ exitCode: 0, signal: null }); + }).pipe(Effect.provide(NodeServices.layer), Effect.scoped), + ); + it.live("truncates buffered output from the beginning at the byte limit", () => withTerminals((terminals) => Effect.gen(function* () { From af1a6f51453150ccf09b2250f165aa7ade99b94e Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 31 Oct 2026 09:48:22 +0000 Subject: [PATCH 2/2] fix(provider-acp): answer wait_for_exit when a background child holds the terminal's output A terminal's exit status was published only once both stdout and stderr hit EOF. When the command exited but a background child inherited its pipes, EOF never came, so terminal/wait_for_exit, release and disposeAll hung and the agent turn stalled. Publish the exit status on process exit after a bounded 250 ms drain of the output pumps. The pumps keep reading afterwards, so a background child never sees a broken pipe. Fixes pingdotgg/t3code#28067 Co-Authored-By: Claude Opus 5.5 --- packages/provider-acp/src/server/clientTerminals.test.ts | 2 ++ packages/provider-acp/src/server/clientTerminals.ts | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/packages/provider-acp/src/server/clientTerminals.test.ts b/packages/provider-acp/src/server/clientTerminals.test.ts index 1a476c9582642..7d5c763f84db6 100644 --- a/packages/provider-acp/src/server/clientTerminals.test.ts +++ b/packages/provider-acp/src/server/clientTerminals.test.ts @@ -155,6 +155,8 @@ describe("AcpClientTerminals", () => { ); expect(yield* child.isRunning).toBe(false); expect(outcome).toEqual({ exitCode: 0, signal: null }); + // The background child keeps a reader, so it never sees a broken pipe. + yield* Deferred.await(orphanStillHoldsOutput); }).pipe(Effect.provide(NodeServices.layer), Effect.scoped), ); diff --git a/packages/provider-acp/src/server/clientTerminals.ts b/packages/provider-acp/src/server/clientTerminals.ts index 2c3278d3c144d..c709200b28fb4 100644 --- a/packages/provider-acp/src/server/clientTerminals.ts +++ b/packages/provider-acp/src/server/clientTerminals.ts @@ -18,6 +18,7 @@ const MAX_RETAINED_TERMINALS = 32; const MAX_RETAINED_OUTPUT_BYTES = 16 * 1024 * 1024; const DEFAULT_OUTPUT_BYTE_LIMIT = 4 * 1024 * 1024; const MAX_OUTPUT_BYTE_LIMIT = 8 * 1024 * 1024; +const OUTPUT_DRAIN_AFTER_EXIT = "250 millis"; const terminalRequestError = (message: string, cause?: unknown) => EffectAcpErrors.AcpRequestError.internalError( @@ -297,6 +298,9 @@ export const makeAcpClientTerminals = Effect.fn("makeAcpClientTerminals")(functi Effect.all([Fiber.join(stdoutPump), Fiber.join(stderrPump)], { discard: true, }).pipe( + // Node can report exit before the last output is read, but a background child + // may hold the pipes open forever. The pumps keep reading after this deadline. + Effect.timeoutOption(OUTPUT_DRAIN_AFTER_EXIT), Effect.andThen( Effect.sync(() => { record.exitStatus = status;