Skip to content

Commit 1acf761

Browse files
juliusmarmingeQblaauw
authored andcommitted
fix(codex): avoid quadratic app-server input buffering (pingdotgg#8605)
(cherry picked from commit 702a6ad)
1 parent 4a8265b commit 1acf761

2 files changed

Lines changed: 133 additions & 8 deletions

File tree

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,114 @@ it.layer(NodeServices.layer)("effect-codex-app-server protocol", (it) => {
290290
}),
291291
);
292292

293+
it.effect("routes a large notification fragmented across thousands of input chunks", () =>
294+
Effect.gen(function* () {
295+
const { stdio, input, output } = yield* makeInMemoryStdio();
296+
const notifications: Array<CodexProtocol.CodexAppServerIncomingNotification> = [];
297+
const transport = yield* CodexProtocol.makeCodexAppServerPatchedProtocol({
298+
stdio,
299+
onNotification: (notification) =>
300+
Effect.sync(() => {
301+
notifications.push(notification);
302+
}),
303+
});
304+
const response = yield* transport.request("thread/read", {}).pipe(Effect.forkScoped);
305+
yield* Queue.take(output);
306+
307+
const notification = {
308+
method: "turn/diff/updated",
309+
params: {
310+
threadId: "thread-1",
311+
turnId: "turn-1",
312+
diff: "x".repeat(4 * 1024 * 1024),
313+
},
314+
};
315+
const bytes = encoder.encode(
316+
`${encodeUnknownJsonString(notification)}\n${encodeUnknownJsonString({ id: 1, result: { ok: true } })}\n`,
317+
);
318+
for (let offset = 0; offset < bytes.length; offset += 1024) {
319+
yield* Queue.offer(input, bytes.subarray(offset, offset + 1024));
320+
}
321+
322+
assert.deepEqual(yield* Fiber.join(response), { ok: true });
323+
assert.deepEqual(notifications, [notification]);
324+
}),
325+
);
326+
327+
it.effect.each([1, 7, 1024])(
328+
"preserves JSONL framing and UTF-8 across %i-byte input chunks",
329+
(chunkSize) =>
330+
Effect.gen(function* () {
331+
const { stdio, input } = yield* makeInMemoryStdio();
332+
const notifications: Array<CodexProtocol.CodexAppServerIncomingNotification> = [];
333+
const rawLines: Array<unknown> = [];
334+
const termination = yield* Deferred.make<CodexError.CodexAppServerError>();
335+
yield* CodexProtocol.makeCodexAppServerPatchedProtocol({
336+
stdio,
337+
logIncoming: true,
338+
logger: (event) =>
339+
Effect.sync(() => {
340+
if (event.stage === "raw") {
341+
rawLines.push(event.payload);
342+
}
343+
}),
344+
onNotification: (notification) =>
345+
Effect.sync(() => {
346+
notifications.push(notification);
347+
}),
348+
onTermination: (error) => Deferred.succeed(termination, error).pipe(Effect.asVoid),
349+
});
350+
351+
const firstLine = '{"method":"x/first",\r"params":{"text":"hé🙂"}}';
352+
const secondLine = '{"method":"x/second","params":{"value":2}}';
353+
const finalLine = '{"method":"x/final","params":{"text":"最後"}}\r';
354+
const bytes = encoder.encode(`\n \t\r\n${firstLine}\r\n\n${secondLine}\n${finalLine}`);
355+
for (let offset = 0; offset < bytes.length; offset += chunkSize) {
356+
yield* Queue.offer(input, bytes.subarray(offset, offset + chunkSize));
357+
}
358+
yield* Queue.end(input);
359+
360+
assert.instanceOf(
361+
yield* Deferred.await(termination),
362+
CodexError.CodexAppServerInputStreamEndedError,
363+
);
364+
assert.deepEqual(notifications, [
365+
{ method: "x/first", params: { text: "hé🙂" } },
366+
{ method: "x/second", params: { value: 2 } },
367+
{ method: "x/final", params: { text: "最後" } },
368+
]);
369+
assert.deepEqual(rawLines, [firstLine, secondLine, finalLine]);
370+
}),
371+
);
372+
373+
it.effect("reports a malformed fragmented final line before input stream termination", () =>
374+
Effect.gen(function* () {
375+
const { stdio, input, output } = yield* makeInMemoryStdio();
376+
const termination = yield* Deferred.make<CodexError.CodexAppServerError>();
377+
const transport = yield* CodexProtocol.makeCodexAppServerPatchedProtocol({
378+
stdio,
379+
onTermination: (error) => Deferred.succeed(termination, error).pipe(Effect.asVoid),
380+
});
381+
const response = yield* transport.request("thread/read", {}).pipe(Effect.forkScoped);
382+
yield* Queue.take(output);
383+
384+
yield* Queue.offer(input, encoder.encode('{"id":1,'));
385+
yield* Queue.offer(input, encoder.encode('"result":'));
386+
yield* Queue.end(input);
387+
388+
const error = yield* Deferred.await(termination);
389+
assert.instanceOf(error, CodexError.CodexAppServerProtocolParseError);
390+
assert.equal(error.operation, "decode-wire-message");
391+
const responseError = yield* Fiber.join(response).pipe(
392+
Effect.match({
393+
onFailure: (failure) => failure,
394+
onSuccess: () => assert.fail("Expected the malformed response to fail the request"),
395+
}),
396+
);
397+
assert.strictEqual(responseError, error);
398+
}),
399+
);
400+
293401
it.effect("surfaces JSON encoding failures as protocol parse errors", () =>
294402
Effect.gen(function* () {
295403
const { stdio } = yield* makeInMemoryStdio();

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

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ export const makeCodexAppServerPatchedProtocol = Effect.fn("makeCodexAppServerPa
157157
const incomingRequests = yield* Queue.unbounded<CodexAppServerIncomingRequest>();
158158
const pending = yield* Ref.make(new Map<string, CodexAppServerPendingRequest>());
159159
const nextRequestId = yield* Ref.make(1);
160-
const remainder = yield* Ref.make("");
160+
const remainder: Array<string> = [];
161161
const terminationHandled = yield* Ref.make(false);
162162

163163
const logProtocol = (event: CodexAppServerProtocolLogEvent) => {
@@ -354,11 +354,24 @@ export const makeCodexAppServerPatchedProtocol = Effect.fn("makeCodexAppServerPa
354354
yield* options.stdio.stdin.pipe(
355355
Stream.decodeText(),
356356
Stream.runForEach((chunk) =>
357-
Ref.modify(remainder, (current) => {
358-
const combined = current + chunk;
359-
const lines = combined.split("\n");
360-
const nextRemainder = lines.pop() ?? "";
361-
return [lines.map((line) => line.replace(/\r$/, "")), nextRemainder] as const;
357+
Effect.sync(() => {
358+
const lines: Array<string> = [];
359+
let start = 0;
360+
for (
361+
let newline = chunk.indexOf("\n");
362+
newline !== -1;
363+
newline = chunk.indexOf("\n", start)
364+
) {
365+
remainder.push(chunk.slice(start, newline));
366+
lines.push(remainder.join("").replace(/\r$/, ""));
367+
remainder.length = 0;
368+
start = newline + 1;
369+
}
370+
// Keep unfinished lines in fragments so each chunk is scanned only once.
371+
if (start < chunk.length) {
372+
remainder.push(chunk.slice(start));
373+
}
374+
return lines;
362375
}).pipe(Effect.flatMap((lines) => Effect.forEach(lines, handleLine, { discard: true }))),
363376
),
364377
Effect.matchEffect({
@@ -367,8 +380,12 @@ export const makeCodexAppServerPatchedProtocol = Effect.fn("makeCodexAppServerPa
367380
Effect.succeed(normalizeIncomingError(error, "read-input-stream")),
368381
),
369382
onSuccess: () =>
370-
Ref.get(remainder).pipe(
371-
Effect.flatMap((line) => (line.trim().length === 0 ? Effect.void : handleLine(line))),
383+
Effect.sync(() => {
384+
const line = remainder.join("");
385+
remainder.length = 0;
386+
return line;
387+
}).pipe(
388+
Effect.flatMap(handleLine),
372389
Effect.matchEffect({
373390
onFailure: (error) => handleTermination(() => Effect.succeed(error)),
374391
onSuccess: () =>

0 commit comments

Comments
 (0)