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 <noreply@anthropic.com>
This commit is contained in:
parent
df9f3c8b5d
commit
64b6a2ec72
|
|
@ -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");
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue