From da82e000a24bc798a9501b2cab40d212220b1b94 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 24 Oct 2026 23:16:01 +0000 Subject: [PATCH 1/3] test(codex): takeover tests for #12777 pending requests at stdout EOF Co-Authored-By: Claude Opus 5.5 --- .../src/protocol.test.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/effect-codex-app-server/src/protocol.test.ts b/packages/effect-codex-app-server/src/protocol.test.ts index 10e979c650108..c4578afe67626 100644 --- a/packages/effect-codex-app-server/src/protocol.test.ts +++ b/packages/effect-codex-app-server/src/protocol.test.ts @@ -644,4 +644,43 @@ it.layer(NodeServices.layer)("effect-codex-app-server protocol", (it) => { assert.notInclude(error.message, secret); }), ); + + it.effect("fails pending requests when input ends while the process is still running", () => + Effect.gen(function* () { + const { stdio, input, output } = yield* makeInMemoryStdio(); + const transport = yield* CodexProtocol.makeCodexAppServerPatchedProtocol({ + stdio, + terminationError: Effect.never, + }); + const pending = yield* transport.request("thread/read", {}).pipe(Effect.forkScoped); + yield* Queue.take(output); + + yield* Queue.end(input); + + const failure = yield* Fiber.join(pending).pipe( + Effect.match({ + onFailure: (error) => error, + onSuccess: () => assert.fail("Expected the pending request to fail"), + }), + ); + assert.instanceOf(failure, CodexError.CodexAppServerInputStreamEndedError); + }), + ); + + it.effect("reports a known process exit when input ends", () => + Effect.gen(function* () { + const { stdio, input } = yield* makeInMemoryStdio(); + const exitError = new CodexError.CodexAppServerProcessExitedError({ code: 1, pid: 123 }); + const termination = yield* Deferred.make(); + yield* CodexProtocol.makeCodexAppServerPatchedProtocol({ + stdio, + terminationError: Effect.succeed(exitError), + onTermination: (error) => Deferred.succeed(termination, error).pipe(Effect.asVoid), + }); + + yield* Queue.end(input); + + assert.strictEqual(yield* Deferred.await(termination), exitError); + }), + ); }); From 719fb8ef684d2462c97432fc6e6248225b122f1d Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 24 Oct 2026 23:19:13 +0000 Subject: [PATCH 2/3] fix(codex): pending requests fail when app-server stdout closes early When Codex app-server stdout reached EOF while the process was still running, the protocol waited on the process exit status before failing pending requests, so every in-flight request hung. Race the exit status against an immediate input-ended error: an exit status that is already known still wins, otherwise pending requests fail with CodexAppServerInputStreamEndedError. Based on #12777 by VendettaGhub. Co-Authored-By: Claude Opus 5.5 --- packages/effect-codex-app-server/src/protocol.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/effect-codex-app-server/src/protocol.ts b/packages/effect-codex-app-server/src/protocol.ts index bf67174033f4d..f51fc7c3ca74f 100644 --- a/packages/effect-codex-app-server/src/protocol.ts +++ b/packages/effect-codex-app-server/src/protocol.ts @@ -445,10 +445,14 @@ export const makeCodexAppServerPatchedProtocol = Effect.fn("makeCodexAppServerPa Effect.matchEffect({ onFailure: (error) => handleTermination(() => Effect.succeed(error)), onSuccess: () => - handleTermination( - () => - options.terminationError ?? + // stdout can close while the process is still alive, so waiting for + // its exit status would hang every pending request. raceFirst runs + // the exit status first, so one that is already known still wins. + handleTermination(() => + Effect.raceFirst( + options.terminationError ?? Effect.never, Effect.succeed(new CodexError.CodexAppServerInputStreamEndedError({})), + ), ), }), ), From 23cee434441629deae178c9ac9f8ec43c268547f Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 24 Oct 2026 23:27:40 +0000 Subject: [PATCH 3/3] fix(codex): wait briefly for the exit status when stdout closes Node ends a crashed child's stdout before it reports the exit, so racing the exit status against an immediate EOF error dropped the exit code and pid for real crashes. Wait up to 250ms for the exit status before failing pending requests with the input-stream-ended error. Co-Authored-By: Claude Opus 5.5 --- .../src/protocol.test.ts | 11 +++++++-- .../effect-codex-app-server/src/protocol.ts | 24 ++++++++++++------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/packages/effect-codex-app-server/src/protocol.test.ts b/packages/effect-codex-app-server/src/protocol.test.ts index c4578afe67626..f470d0b25bfae 100644 --- a/packages/effect-codex-app-server/src/protocol.test.ts +++ b/packages/effect-codex-app-server/src/protocol.test.ts @@ -5,6 +5,7 @@ import * as Queue from "effect/Queue"; import * as Schema from "effect/Schema"; import * as Stdio from "effect/Stdio"; import * as Stream from "effect/Stream"; +import * as TestClock from "effect/testing/TestClock"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; @@ -656,6 +657,9 @@ it.layer(NodeServices.layer)("effect-codex-app-server protocol", (it) => { yield* Queue.take(output); yield* Queue.end(input); + yield* TestClock.adjust("249 millis"); + assert.isUndefined(pending.pollUnsafe()); + yield* TestClock.adjust("1 millis"); const failure = yield* Fiber.join(pending).pipe( Effect.match({ @@ -667,18 +671,21 @@ it.layer(NodeServices.layer)("effect-codex-app-server protocol", (it) => { }), ); - it.effect("reports a known process exit when input ends", () => + it.effect("reports a process exit that follows input end", () => Effect.gen(function* () { const { stdio, input } = yield* makeInMemoryStdio(); const exitError = new CodexError.CodexAppServerProcessExitedError({ code: 1, pid: 123 }); + const exited = yield* Deferred.make(); const termination = yield* Deferred.make(); yield* CodexProtocol.makeCodexAppServerPatchedProtocol({ stdio, - terminationError: Effect.succeed(exitError), + terminationError: Deferred.await(exited), onTermination: (error) => Deferred.succeed(termination, error).pipe(Effect.asVoid), }); yield* Queue.end(input); + yield* TestClock.adjust("100 millis"); + yield* Deferred.succeed(exited, exitError); assert.strictEqual(yield* Deferred.await(termination), exitError); }), diff --git a/packages/effect-codex-app-server/src/protocol.ts b/packages/effect-codex-app-server/src/protocol.ts index f51fc7c3ca74f..b0ff5436e9e77 100644 --- a/packages/effect-codex-app-server/src/protocol.ts +++ b/packages/effect-codex-app-server/src/protocol.ts @@ -18,6 +18,7 @@ const isCodexAppServerError = Schema.is(CodexError.CodexAppServerError); const MAX_BUFFERED_RAW_MESSAGES = 32; // Bound JSONL lines before joining and parsing; the LF delimiter is excluded. const MAX_INCOMING_MESSAGE_BYTES = 128 * 1024 * 1024; +const EXIT_STATUS_GRACE = "250 millis"; export interface CodexAppServerProtocolLogEvent { readonly direction: "incoming" | "outgoing"; @@ -445,15 +446,20 @@ export const makeCodexAppServerPatchedProtocol = Effect.fn("makeCodexAppServerPa Effect.matchEffect({ onFailure: (error) => handleTermination(() => Effect.succeed(error)), onSuccess: () => - // stdout can close while the process is still alive, so waiting for - // its exit status would hang every pending request. raceFirst runs - // the exit status first, so one that is already known still wins. - handleTermination(() => - Effect.raceFirst( - options.terminationError ?? Effect.never, - Effect.succeed(new CodexError.CodexAppServerInputStreamEndedError({})), - ), - ), + handleTermination(() => { + const ended = new CodexError.CodexAppServerInputStreamEndedError({}); + // Node ends stdout before it reports the exit of a crashed process, so + // wait briefly for the exit status. stdout can also close while the + // process keeps running, so the wait must not hang pending requests. + return options.terminationError === undefined + ? Effect.succeed(ended) + : options.terminationError.pipe( + Effect.timeoutOrElse({ + duration: EXIT_STATUS_GRACE, + orElse: () => Effect.succeed(ended), + }), + ); + }), }), ), }),