Skip to content

Commit 715c19d

Browse files
only21milclaude
andcommitted
fix(codex): warn when a Codex notification fails to decode
The client dropped a server notification the generated schema rejected and logged nothing, so a Codex build that drifted from the schema left missing tool rows or unsettled turns with no trace. Log a warning with the method and schema diagnostics, once per distinct failure, without the payload. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 5ffe045 commit 715c19d

2 files changed

Lines changed: 78 additions & 1 deletion

File tree

‎packages/effect-codex-app-server/src/client.test.ts‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
1+
import * as Deferred from "effect/Deferred";
12
import * as Exit from "effect/Exit";
23
import * as Layer from "effect/Layer";
4+
import * as Logger from "effect/Logger";
35
import * as Path from "effect/Path";
46
import * as Effect from "effect/Effect";
7+
import * as Queue from "effect/Queue";
58
import * as Ref from "effect/Ref";
9+
import * as References from "effect/References";
610
import * as Scope from "effect/Scope";
711
import { ChildProcess, ChildProcessSpawner } from "effect/process";
812

913
import * as NodeServices from "@effect/platform-node/NodeServices";
10-
import { assert, it } from "@effect/vitest";
14+
import { assert, expect, it } from "@effect/vitest";
1115

1216
import * as CodexClient from "./client.ts";
17+
import { makeInMemoryStdio } from "./_internal/stdio.ts";
1318

1419
const mockPeerPath = Effect.map(Effect.service(Path.Path), (path) =>
1520
path.join(import.meta.dirname, "../test/fixtures/codex-app-server-mock-peer.ts"),
@@ -155,4 +160,52 @@ it.layer(NodeServices.layer)("effect-codex-app-server client", (it) => {
155160
assert.equal(initialized.userAgent, "mock-codex-app-server");
156161
}),
157162
);
163+
164+
it.effect("warns once about a notification that fails to decode and keeps dispatching", () =>
165+
Effect.gen(function* () {
166+
const secret = "codex-notification-secret-sentinel";
167+
const { stdio, input } = yield* makeInMemoryStdio();
168+
const warnings: Array<unknown> = [];
169+
const client = yield* CodexClient.make(stdio).pipe(
170+
Effect.provide(
171+
Logger.layer([
172+
Logger.make(({ logLevel, fiber }) => {
173+
if (logLevel === "Warn")
174+
warnings.push(fiber.getRef(References.CurrentLogAnnotations));
175+
}),
176+
]),
177+
),
178+
);
179+
const delivered = yield* Deferred.make<unknown>();
180+
yield* client.handleServerNotification("item/agentMessage/delta", (payload) =>
181+
Deferred.succeed(delivered, payload).pipe(Effect.asVoid),
182+
);
183+
184+
const send = (params: unknown) =>
185+
Queue.offer(
186+
input,
187+
new TextEncoder().encode(
188+
`${JSON.stringify({ method: "item/agentMessage/delta", params })}\n`,
189+
),
190+
);
191+
const malformed = { itemId: "item-1", threadId: "thread-1", turnId: secret };
192+
yield* send(malformed);
193+
yield* send(malformed);
194+
yield* send({ delta: "ok", itemId: "item-1", threadId: "thread-1", turnId: "turn-1" });
195+
196+
assert.deepEqual(yield* Deferred.await(delivered), {
197+
delta: "ok",
198+
itemId: "item-1",
199+
threadId: "thread-1",
200+
turnId: "turn-1",
201+
});
202+
assert.lengthOf(warnings, 1);
203+
expect(warnings[0]).toMatchObject({
204+
operation: "decode-notification-payload",
205+
method: "item/agentMessage/delta",
206+
issueKinds: expect.arrayContaining(["MissingKey"]),
207+
});
208+
assert.notInclude(JSON.stringify(warnings), secret);
209+
}),
210+
);
158211
});

‎packages/effect-codex-app-server/src/client.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,29 @@ export const make = Effect.fn("effect-codex-app-server/CodexAppServerClient.make
150150
>
151151
| undefined => CodexRpc.CLIENT_NOTIFICATION_PARAMS[method] as never;
152152

153+
// A notification the schema rejects has no reply to fail, so it is dropped.
154+
// A Codex build that drifted from the generated schema repeats the bad shape
155+
// on every frame, so each distinct failure is warned about once per client.
156+
const warnedDecodeFailures = new Set<string>();
157+
const warnUndecodableNotification = (error: CodexError.CodexAppServerProtocolParseError) => {
158+
const key = `${error.method}\n${error.issueKinds?.join(",")}`;
159+
if (warnedDecodeFailures.has(key)) {
160+
return Effect.void;
161+
}
162+
warnedDecodeFailures.add(key);
163+
return Effect.logWarning("Dropped a Codex App Server notification that failed to decode").pipe(
164+
Effect.annotateLogs({
165+
operation: error.operation,
166+
...(error.method === undefined ? {} : { method: error.method }),
167+
...(error.issueCount === undefined ? {} : { issueCount: error.issueCount }),
168+
...(error.issueKinds === undefined ? {} : { issueKinds: error.issueKinds }),
169+
...(error.maximumPathDepth === undefined
170+
? {}
171+
: { maximumPathDepth: error.maximumPathDepth }),
172+
}),
173+
);
174+
};
175+
153176
const dispatchNotification = (
154177
notification: CodexProtocol.CodexAppServerIncomingNotification,
155178
): Effect.Effect<void, never> => {
@@ -163,6 +186,7 @@ export const make = Effect.fn("effect-codex-app-server/CodexAppServerClient.make
163186

164187
if (schema) {
165188
return decodeNotificationPayload(notification.method, schema, notification.params).pipe(
189+
Effect.tapError(warnUndecodableNotification),
166190
Effect.flatMap((decoded) =>
167191
Effect.forEach(handlers, (handler) => handler(decoded), { discard: true }),
168192
),

0 commit comments

Comments
 (0)