From 64b6a2ec7287c8c32206ce2f8c60aee4c4c60739 Mon Sep 17 00:00:00 2001 From: Dhruv Sharma Date: Mon, 13 Apr 2026 19:23:29 +0530 Subject: [PATCH] fix(codex): address cursor bugbot comments on readJsonlPrefixLines Use a single StringDecoder across reads so multi-byte UTF-8 sequences that straddle the 8KB chunk boundary buffer correctly instead of producing U+FFFD replacement characters that break JSON.parse. Also fix the test mock: makeFakeFileHandle now advances an internal cursor and returns bytesRead: 0 at EOF. The prior mock copied from offset 0 every call, which would infinite-loop readJsonlPrefixLines for any line larger than the 8192-byte chunk size. Add a regression test using 3,000 CJK characters (9,000 bytes of payload) to exercise the chunk boundary path. Co-Authored-By: Claude Opus 4.6 --- .../plugins/agent-codex/src/index.test.ts | 47 +++++++++++++++++-- packages/plugins/agent-codex/src/index.ts | 8 +++- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index 59c620fad..7ab9faa8b 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -135,15 +135,23 @@ function mockTmuxWithProcess(processName: string, found = true) { } /** - * Create a mock file handle for `open()` that returns `content` from `read()`. - * Used by sessionFileMatchesCwd which reads the first few complete JSONL lines. + * Create a mock file handle for `open()` that streams `content` across + * successive `read()` calls. Tracks an internal cursor so sequential reads + * advance through the buffer and eventually return `bytesRead: 0` at EOF. + * Without position tracking, readJsonlPrefixLines would loop forever on + * lines larger than its chunk size. */ function makeFakeFileHandle(content: string) { const buf = Buffer.from(content, "utf-8"); + let cursor = 0; return { read: vi.fn().mockImplementation((buffer: Buffer, offset: number, length: number, _position: number) => { - const bytesToCopy = Math.min(length, buf.length); - buf.copy(buffer, offset, 0, bytesToCopy); + if (cursor >= buf.length) { + return Promise.resolve({ bytesRead: 0, buffer }); + } + const bytesToCopy = Math.min(length, buf.length - cursor); + buf.copy(buffer, offset, cursor, cursor + bytesToCopy); + cursor += bytesToCopy; return Promise.resolve({ bytesRead: bytesToCopy, buffer }); }), close: vi.fn().mockResolvedValue(undefined), @@ -851,6 +859,37 @@ describe("getActivityState", () => { expect(result?.state).toBe("ready"); }); + it("handles multi-byte UTF-8 characters straddling an 8KB chunk boundary", async () => { + mockTmuxWithProcess("codex"); + // Each 日 is 3 bytes. Padding with enough CJK chars to push the json + // payload past the 8192-byte chunk size, guaranteeing a multi-byte + // character will straddle a read boundary. Without StringDecoder, + // the split character decodes to U+FFFD and JSON.parse fails. + const padding = "日".repeat(3_000); // 9000 bytes of padding + const content = + `${JSON.stringify({ + type: "session_meta", + payload: { + cwd: "/workspace/test", + id: "thread-utf8", + base_instructions: padding, + }, + })}\n`; + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + mockStat.mockResolvedValue({ mtimeMs: Date.now(), mtime: new Date() }); + mockReadLastJsonlEntry.mockResolvedValue({ + lastType: "event_msg", + modifiedAt: new Date(), + }); + + const session = makeSession({ runtimeHandle: makeTmuxHandle(), workspacePath: "/workspace/test" }); + const result = await agent.getActivityState(session); + // If UTF-8 boundary handling is broken, JSON.parse fails, cwd never + // matches, no session file is selected, and state falls through to null. + expect(result?.state).toBe("ready"); + }); + it("returns exited when process handle has dead PID", async () => { const killSpy = vi.spyOn(process, "kill").mockImplementation(() => { throw new Error("ESRCH"); diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index 22a0e7d7a..c20829340 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -28,6 +28,7 @@ import { createReadStream } from "node:fs"; import { readdir, stat, lstat, open } from "node:fs/promises"; import { homedir } from "node:os"; import { basename, join } from "node:path"; +import { StringDecoder } from "node:string_decoder"; import { createInterface } from "node:readline"; import { promisify } from "node:util"; @@ -145,6 +146,10 @@ async function readJsonlPrefixLines(filePath: string, maxLines: number): Promise const handle = await open(filePath, "r"); const lines: string[] = []; let partialLine = ""; + // Reuse a single decoder across reads so multi-byte UTF-8 sequences that + // straddle a chunk boundary (e.g. CJK characters in base_instructions) get + // buffered correctly instead of producing U+FFFD replacement characters. + const decoder = new StringDecoder("utf8"); try { while (lines.length < maxLines) { @@ -152,12 +157,13 @@ async function readJsonlPrefixLines(filePath: string, maxLines: number): Promise const { bytesRead } = await handle.read(buffer, 0, buffer.length, null); if (bytesRead === 0) { + partialLine += decoder.end(); const finalLine = partialLine.trim(); if (finalLine) lines.push(finalLine); break; } - partialLine += buffer.subarray(0, bytesRead).toString("utf-8"); + partialLine += decoder.write(buffer.subarray(0, bytesRead)); let newlineIndex = partialLine.indexOf("\n"); while (newlineIndex !== -1 && lines.length < maxLines) {