From 020001d9b28583301ed1d4d3f8b9a8c1b6d6f19f Mon Sep 17 00:00:00 2001 From: jiarung Date: Fri, 13 Mar 2026 14:25:13 +0000 Subject: [PATCH] fix(usage-log): propagate non-ENOENT read errors to prevent silent data loss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit readJsonArray previously caught all errors and returned [], so a malformed token-usage.json (e.g. from an interrupted writeFile) caused the next recordTokenUsage call to overwrite the file with only the new entry, permanently erasing all prior records. Fix: only suppress ENOENT (file not yet created). Any other error (SyntaxError, EACCES, …) is re-thrown so appendRecord aborts and the existing file is left intact. The write-queue slot still absorbs the rejection via .catch() so future writes are not stalled; callers that need to observe the failure (e.g. attempt.ts) can attach their own .catch() handler. --- src/agents/usage-log.test.ts | 20 ++++++++++++++++++++ src/agents/usage-log.ts | 11 +++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/agents/usage-log.test.ts b/src/agents/usage-log.test.ts index ed2efd755df..b61b3154703 100644 --- a/src/agents/usage-log.test.ts +++ b/src/agents/usage-log.test.ts @@ -120,6 +120,26 @@ describe("recordTokenUsage", () => { expect(records[0].outputTokens).toBe(50); }); + it("does not overwrite a malformed token-usage.json — preserves corrupted file", async () => { + // Simulate an interrupted write that left partial JSON + await fs.mkdir(path.join(tmpDir, "memory"), { recursive: true }); + await fs.writeFile(usageFile, '{"broken":true', "utf-8"); + + // recordTokenUsage must reject (caller is responsible for handling, e.g. + // attempt.ts uses .catch()) and must NOT overwrite the existing file. + await expect( + recordTokenUsage({ + workspaceDir: tmpDir, + label: "llm_output", + usage: { input: 100, output: 50, total: 150 }, + }), + ).rejects.toThrow(SyntaxError); + + // File must still contain the original corrupted content, not a new array. + const content = await fs.readFile(usageFile, "utf-8"); + expect(content).toBe('{"broken":true'); + }); + it("serialises concurrent writes — no record is lost", async () => { const N = 20; await Promise.all( diff --git a/src/agents/usage-log.ts b/src/agents/usage-log.ts index 802e3899f2c..2d071978510 100644 --- a/src/agents/usage-log.ts +++ b/src/agents/usage-log.ts @@ -27,8 +27,15 @@ async function readJsonArray(file: string): Promise { const raw = await fs.readFile(file, "utf-8"); const parsed = JSON.parse(raw); return Array.isArray(parsed) ? (parsed as TokenUsageRecord[]) : []; - } catch { - return []; + } catch (err) { + // File does not exist yet — start with an empty array. + if ((err as NodeJS.ErrnoException).code === "ENOENT") { + return []; + } + // Any other error (malformed JSON, permission denied, partial write, …) + // must propagate so appendRecord aborts and the existing file is not + // silently overwritten with only the new entry. + throw err; } }