From 0cc4d9c418d6f5b83b907c1d93f36954d05f97b4 Mon Sep 17 00:00:00 2001 From: chilung Date: Wed, 16 Sep 2026 08:33:04 +0000 Subject: [PATCH 1/2] perf(usage): guard redundant directory creation and permission checks on append --- src/usage/log.ts | 13 ++++++++++++- tests/usage/usage-log.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/usage/log.ts b/src/usage/log.ts index aadeb1648b..2ddfa45a7c 100644 --- a/src/usage/log.ts +++ b/src/usage/log.ts @@ -852,18 +852,27 @@ function normalizeUsageEntry(entry: PersistedUsageEntry): PersistedUsageEntry { }; } +let ensuredUsageLogDir: string | null = null; +let ensuredUsageLogFile: string | null = null; + function ensureUsageLogDir(): void { const dir = getConfigDir(); + if (ensuredUsageLogDir === dir) return; recordOwnedConfigPath(dir, usageLogPath()); mkdirSync(dir, { recursive: true, mode: 0o700 }); try { chmodSync(dir, 0o700); } catch { /* best-effort on platforms that ignore chmod */ } + ensuredUsageLogDir = dir; } export function appendUsageEntry(entry: PersistedUsageEntry): void { ensureUsageLogDir(); const path = usageLogPath(); + const fileAlreadyEnsured = ensuredUsageLogFile === path; appendFileSync(path, `${JSON.stringify(normalizeUsageEntry(entry))}\n`, { encoding: "utf-8", mode: 0o600 }); - try { chmodSync(path, 0o600); } catch { /* best-effort on platforms that ignore chmod */ } + if (!fileAlreadyEnsured) { + try { chmodSync(path, 0o600); } catch { /* best-effort on platforms that ignore chmod */ } + ensuredUsageLogFile = path; + } } export type UsageLogRevision = { @@ -1057,6 +1066,8 @@ export function resetUsageReadCacheForTests(): void { managementUsageReadInflight?.abort.abort(); managementUsageReadInflight = null; retainedUsageSnapshot = null; + ensuredUsageLogDir = null; + ensuredUsageLogFile = null; } function readExactly(fd: number, length: number, position: number): Buffer | null { diff --git a/tests/usage/usage-log.test.ts b/tests/usage/usage-log.test.ts index 9cd97855a6..21973e6a8f 100644 --- a/tests/usage/usage-log.test.ts +++ b/tests/usage/usage-log.test.ts @@ -1012,4 +1012,28 @@ describe("usage log", () => { expect(readRecentUsageEntries(1)).toEqual([]); }, STORE_BUDGET_MS); + + test("appendUsageEntry avoids redundant mkdirSync and chmodSync on consecutive calls", async () => { + const nodeFs = await import("node:fs"); + const mkdirSpy = spyOn(nodeFs, "mkdirSync"); + const chmodSpy = spyOn(nodeFs, "chmodSync"); + + for (let i = 0; i < 5; i++) { + appendUsageEntry({ + requestId: `ocx-perf-${i}`, + timestamp: Date.now(), + provider: "openai", + model: "gpt-4o", + status: 200, + durationMs: 10, + usageStatus: "unreported", + }); + } + + expect(mkdirSpy.mock.calls.length).toBe(1); + expect(chmodSpy.mock.calls.length).toBeLessThanOrEqual(2); + + mkdirSpy.mockRestore(); + chmodSpy.mockRestore(); + }); }); From a04af5bda26dea930011558809c293b424592956 Mon Sep 17 00:00:00 2001 From: chilung Date: Wed, 16 Sep 2026 09:04:20 +0000 Subject: [PATCH 2/2] fix(usage): recover cached log directory on ENOENT and harden test cleanup Co-authored-by: chilung --- src/usage/log.ts | 26 +++++++++++---- tests/usage/usage-log.test.ts | 63 ++++++++++++++++++++++++++--------- 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/src/usage/log.ts b/src/usage/log.ts index 2ddfa45a7c..ef2857918e 100644 --- a/src/usage/log.ts +++ b/src/usage/log.ts @@ -865,13 +865,27 @@ function ensureUsageLogDir(): void { } export function appendUsageEntry(entry: PersistedUsageEntry): void { - ensureUsageLogDir(); + const line = `${JSON.stringify(normalizeUsageEntry(entry))}\n`; const path = usageLogPath(); - const fileAlreadyEnsured = ensuredUsageLogFile === path; - appendFileSync(path, `${JSON.stringify(normalizeUsageEntry(entry))}\n`, { encoding: "utf-8", mode: 0o600 }); - if (!fileAlreadyEnsured) { - try { chmodSync(path, 0o600); } catch { /* best-effort on platforms that ignore chmod */ } - ensuredUsageLogFile = path; + const doAppend = (): void => { + ensureUsageLogDir(); + const fileAlreadyEnsured = ensuredUsageLogFile === path; + appendFileSync(path, line, { encoding: "utf-8", mode: 0o600 }); + if (!fileAlreadyEnsured) { + try { chmodSync(path, 0o600); } catch { /* best-effort on platforms that ignore chmod */ } + ensuredUsageLogFile = path; + } + }; + try { + doAppend(); + } catch (error: any) { + if (error?.code === "ENOENT") { + ensuredUsageLogDir = null; + ensuredUsageLogFile = null; + doAppend(); + return; + } + throw error; } } diff --git a/tests/usage/usage-log.test.ts b/tests/usage/usage-log.test.ts index 21973e6a8f..58ba3fcc98 100644 --- a/tests/usage/usage-log.test.ts +++ b/tests/usage/usage-log.test.ts @@ -1013,27 +1013,60 @@ describe("usage log", () => { expect(readRecentUsageEntries(1)).toEqual([]); }, STORE_BUDGET_MS); - test("appendUsageEntry avoids redundant mkdirSync and chmodSync on consecutive calls", async () => { + test("appendUsageEntry avoids redundant mkdirSync and chmodSync on consecutive calls", async () => { const nodeFs = await import("node:fs"); const mkdirSpy = spyOn(nodeFs, "mkdirSync"); const chmodSpy = spyOn(nodeFs, "chmodSync"); - for (let i = 0; i < 5; i++) { - appendUsageEntry({ - requestId: `ocx-perf-${i}`, - timestamp: Date.now(), - provider: "openai", - model: "gpt-4o", - status: 200, - durationMs: 10, - usageStatus: "unreported", - }); + try { + for (let i = 0; i < 5; i++) { + appendUsageEntry({ + requestId: `ocx-perf-${i}`, + timestamp: Date.now(), + provider: "openai", + model: "gpt-4o", + status: 200, + durationMs: 10, + usageStatus: "unreported", + }); + } + + expect(mkdirSpy.mock.calls.length).toBe(1); + expect(chmodSpy.mock.calls.length).toBeLessThanOrEqual(2); + } finally { + mkdirSpy.mockRestore(); + chmodSpy.mockRestore(); } + }); + + test("appendUsageEntry recovers cleanly on ENOENT if usage directory is deleted between calls", () => { + const entry1: PersistedUsageEntry = { + requestId: "ocx-enoent-1", + timestamp: Date.now(), + provider: "openai", + model: "gpt-4o", + status: 200, + durationMs: 10, + usageStatus: "unreported", + }; + appendUsageEntry(entry1); - expect(mkdirSpy.mock.calls.length).toBe(1); - expect(chmodSpy.mock.calls.length).toBeLessThanOrEqual(2); + // Simulate directory deletion by log rotation / cleanup while process is running + rmSync(testDir, { recursive: true, force: true }); + expect(existsSync(testDir)).toBe(false); - mkdirSpy.mockRestore(); - chmodSpy.mockRestore(); + const entry2: PersistedUsageEntry = { + requestId: "ocx-enoent-2", + timestamp: Date.now(), + provider: "openai", + model: "gpt-4o", + status: 200, + durationMs: 12, + usageStatus: "unreported", + }; + expect(() => appendUsageEntry(entry2)).not.toThrow(); + const readBack = readRecentUsageEntries(10); + expect(readBack.length).toBe(1); + expect(readBack[0].requestId).toBe("ocx-enoent-2"); }); });