From e6fc7d0c67a5376260ef19d5af1247f24b366e2a Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:19:02 -0700 Subject: [PATCH] Extract usage cache writer into reusable durable snapshot class Consolidates serialized durable-write and generation-veto logic from three usage stores into UsageCacheSnapshotWriter. Eliminates duplication, centralizes multi-MB JSON serialization on the main thread via write-queue serialization, and vetoes superseded snapshots to avoid wasted rewrites. --- src/main/claude-usage/store.ts | 46 +----- src/main/codex-usage/store.ts | 46 +----- src/main/daemon/history-manager.test.ts | 34 +++-- src/main/opencode-usage/store.test.ts | 183 +++++++++++++++++++++++- src/main/opencode-usage/store.ts | 46 +----- src/main/usage-cache-snapshot-writer.ts | 56 ++++++++ 6 files changed, 277 insertions(+), 134 deletions(-) create mode 100644 src/main/usage-cache-snapshot-writer.ts diff --git a/src/main/claude-usage/store.ts b/src/main/claude-usage/store.ts index a02f39e801b..49c64f8548d 100644 --- a/src/main/claude-usage/store.ts +++ b/src/main/claude-usage/store.ts @@ -1,13 +1,8 @@ /* eslint-disable max-lines -- Why: this store is the single main-process owner for Claude usage persistence, scan gating, and query semantics. Keeping those policy decisions together avoids split-brain range/scope logic across multiple files. */ import { app } from 'electron' import { existsSync, readFileSync } from 'node:fs' -import { mkdir } from 'node:fs/promises' -import { dirname, join } from 'node:path' -import { - durableWriteTempPath, - removeStaleDurableWriteTempFiles, - writeFileDurableIfCurrent -} from '../durable-file-write' +import { join } from 'node:path' +import { UsageCacheSnapshotWriter } from '../usage-cache-snapshot-writer' import type { ClaudeUsageBreakdownKind, ClaudeUsageBreakdownRow, @@ -337,17 +332,13 @@ export class ClaudeUsageStore { private state: ClaudeUsagePersistedState private readonly store: Store private scanPromise: Promise | null = null - // Why: multi-MB usage JSON must not block the Electron main thread; generation vetoes superseded renames. - private writeGeneration = 0 - // Why serialize: two overlapping async writes can both veto themselves, leaving nothing on disk. - private pendingWrite: Promise = Promise.resolve() + // Why: the 20 MB usage JSON must not block the Electron main thread; the writer serializes writes + // and vetoes superseded renames. + private readonly writer = new UsageCacheSnapshotWriter('[claude-usage]', getClaudeUsageFile) constructor(store: Store) { this.store = store this.state = this.load() - // Why: a crash between write and rename orphans a 20 MB temp file; reclaim once per launch. - // Seeding the write queue with it also orders the sweep ahead of the first write of this launch. - this.pendingWrite = removeStaleDurableWriteTempFiles(getClaudeUsageFile()) } private load(): ClaudeUsagePersistedState { @@ -389,33 +380,8 @@ export class ClaudeUsageStore { } private writeToDisk(): Promise { - const generation = ++this.writeGeneration - const write = this.pendingWrite.then(() => this.commitToDisk(generation)) - // Why: keep the queue usable after a failure — the awaiting caller still sees the rejection. - this.pendingWrite = write.catch((error: unknown) => { - console.error('[claude-usage] Failed to persist usage cache:', error) - }) - return write - } - - private async commitToDisk(generation: number): Promise { - // Why: a newer snapshot is already queued behind this one, so writing 20 MB here would be wasted. - if (generation !== this.writeGeneration) { - return - } - // Why stringify here and not at queue time: it blocks the main process for a multi-MB cache, and - // superseded generations must not pay for it. Synchronous, so no mutation can tear the JSON. // Pretty-print preserved: humans inspect this analytics cache on disk. - const payload = JSON.stringify(this.state, null, 2) - const usageFile = getClaudeUsageFile() - await mkdir(dirname(usageFile), { recursive: true }).catch(() => {}) - // Why: atomic temp-file + durable rename so a crash cannot leave a truncated analytics file. - await writeFileDurableIfCurrent( - durableWriteTempPath(usageFile), - usageFile, - payload, - () => generation === this.writeGeneration - ) + return this.writer.write(() => JSON.stringify(this.state, null, 2)) } async setEnabled(enabled: boolean): Promise { diff --git a/src/main/codex-usage/store.ts b/src/main/codex-usage/store.ts index b32c6c7983a..0abab5e3ba5 100644 --- a/src/main/codex-usage/store.ts +++ b/src/main/codex-usage/store.ts @@ -1,13 +1,8 @@ /* eslint-disable max-lines -- Why: this store owns Codex analytics persistence, scan policy, and renderer query semantics. Keeping them together prevents the Codex range/scope rules from drifting away from the scanner’s event model. */ import { app } from 'electron' -import { dirname, join } from 'node:path' +import { join } from 'node:path' import { existsSync, readFileSync } from 'node:fs' -import { mkdir } from 'node:fs/promises' -import { - durableWriteTempPath, - removeStaleDurableWriteTempFiles, - writeFileDurableIfCurrent -} from '../durable-file-write' +import { UsageCacheSnapshotWriter } from '../usage-cache-snapshot-writer' import type { CodexUsageBreakdownKind, CodexUsageBreakdownRow, @@ -372,17 +367,13 @@ export class CodexUsageStore { private state: CodexUsagePersistedState private readonly store: Store private scanPromise: Promise | null = null - // Why: multi-MB usage JSON must not block the Electron main thread; generation vetoes superseded renames. - private writeGeneration = 0 - // Why serialize: two overlapping async writes can both veto themselves, leaving nothing on disk. - private pendingWrite: Promise = Promise.resolve() + // Why: the 60 MB usage JSON must not block the Electron main thread; the writer serializes writes + // and vetoes superseded renames. + private readonly writer = new UsageCacheSnapshotWriter('[codex-usage]', getCodexUsageFile) constructor(store: Store) { this.store = store this.state = this.load() - // Why: a crash between write and rename orphans a 60 MB temp file; reclaim once per launch. - // Seeding the write queue with it also orders the sweep ahead of the first write of this launch. - this.pendingWrite = removeStaleDurableWriteTempFiles(getCodexUsageFile()) } private load(): CodexUsagePersistedState { @@ -407,31 +398,8 @@ export class CodexUsageStore { } private writeToDisk(): Promise { - const generation = ++this.writeGeneration - const write = this.pendingWrite.then(() => this.commitToDisk(generation)) - // Why: keep the queue usable after a failure — the awaiting caller still sees the rejection. - this.pendingWrite = write.catch((error: unknown) => { - console.error('[codex-usage] Failed to persist usage cache:', error) - }) - return write - } - - private async commitToDisk(generation: number): Promise { - // Why: a newer snapshot is already queued behind this one, so writing 60 MB here would be wasted. - if (generation !== this.writeGeneration) { - return - } - // Why stringify here and not at queue time: it blocks the main process for a multi-MB cache, and - // superseded generations must not pay for it. Synchronous, so no mutation can tear the JSON. - const payload = JSON.stringify(this.state) - const usageFile = getCodexUsageFile() - await mkdir(dirname(usageFile), { recursive: true }).catch(() => {}) - await writeFileDurableIfCurrent( - durableWriteTempPath(usageFile), - usageFile, - payload, - () => generation === this.writeGeneration - ) + // Compact: this cache reaches 60 MB, and pretty-printing it costs main-thread time per scan. + return this.writer.write(() => JSON.stringify(this.state)) } async setEnabled(enabled: boolean): Promise { diff --git a/src/main/daemon/history-manager.test.ts b/src/main/daemon/history-manager.test.ts index 0e1c05613f6..cf516482d53 100644 --- a/src/main/daemon/history-manager.test.ts +++ b/src/main/daemon/history-manager.test.ts @@ -497,27 +497,35 @@ describe('HistoryManager', () => { writeFileSync(join(sessionDir, `chunk-${i}.log`), `payload-${i}`) } - // Why loop turns, not timer gaps: removeSession runs on the Electron main thread, and the - // regression is a blocked loop. Wall-clock gap bounds measure runner scheduling and flake; - // a turn count doesn't — a sync recursive rm yields exactly zero turns while it walks the - // tree, an async one yields one per libuv completion regardless of how fast the box is. - let loopTurns = 0 - let counting = true - const countTurn = (): void => { - if (counting) { - loopTurns++ - setImmediate(countTurn) + // Why a half-deleted tree and not a turn count: removeSession runs on the Electron main + // thread and the regression is a blocked loop, but any fixed turn threshold is a guess about + // Node/filesystem internals. Seeing the tree partially removed from a loop callback can only + // happen if the walk yields — a sync recursive rm goes from every entry to none with no turn + // in between, on any platform. + const initialEntries = readdirSync(sessionDir).length + let sawPartialTree = false + let sampling = true + const sampleTree = (): void => { + if (!sampling || sawPartialTree) { + return } + try { + const remaining = readdirSync(sessionDir).length + sawPartialTree = remaining > 0 && remaining < initialEntries + } catch { + // Tree already gone; nothing left to sample. + } + setImmediate(sampleTree) } - setImmediate(countTurn) + setImmediate(sampleTree) try { await mgr.removeSession(sessionId) } finally { - counting = false + sampling = false } expect(existsSync(sessionDir)).toBe(false) - expect(loopTurns).toBeGreaterThan(100) + expect(sawPartialTree).toBe(true) }) }) }) diff --git a/src/main/opencode-usage/store.test.ts b/src/main/opencode-usage/store.test.ts index aea35628f60..44b03eec4e0 100644 --- a/src/main/opencode-usage/store.test.ts +++ b/src/main/opencode-usage/store.test.ts @@ -1,12 +1,24 @@ +import { mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs' +import type * as FsPromises from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { OpenCodeUsageDailyAggregate, + OpenCodeUsagePersistedDatabase, OpenCodeUsagePersistedState, OpenCodeUsageSession } from './types' -const { getPathMock } = vi.hoisted(() => ({ - getPathMock: vi.fn(() => '/tmp/orca-test-userdata') +const { getPathMock, writeOpens, writeGate } = vi.hoisted(() => ({ + getPathMock: vi.fn(() => '/tmp/orca-test-userdata'), + // Why only mode 'w': the durable write also opens the directory read-only to fsync it, so counting + // every open would hide a regression back to multiple full-cache rewrites per scan. + writeOpens: { value: 0, inFlight: 0, maxConcurrent: 0 }, + writeGate: { + blocked: false, + waiters: [] as (() => void)[] + } })) vi.mock('electron', () => ({ @@ -15,7 +27,61 @@ vi.mock('electron', () => ({ } })) -import { OpenCodeUsageStore, normalizePersistedState } from './store' +vi.mock('node:fs/promises', async () => { + const actual = await vi.importActual('node:fs/promises') + return { + ...actual, + open: (async (...args: Parameters) => { + if (args[1] !== 'w') { + return actual.open(...args) + } + writeOpens.value += 1 + writeOpens.inFlight += 1 + writeOpens.maxConcurrent = Math.max(writeOpens.maxConcurrent, writeOpens.inFlight) + try { + if (writeGate.blocked) { + await new Promise((resolve) => writeGate.waiters.push(resolve)) + } + return await actual.open(...args) + } finally { + writeOpens.inFlight -= 1 + } + }) as typeof actual.open + } +}) + +vi.mock('./scanner', () => ({ + createWorktreeRefs: vi.fn(() => []), + scanOpenCodeUsageDatabases: vi.fn() +})) + +import { OpenCodeUsageStore, initOpenCodeUsagePath, normalizePersistedState } from './store' +import { scanOpenCodeUsageDatabases } from './scanner' + +type ScanResult = { + processedDatabases: OpenCodeUsagePersistedDatabase[] + sessions: OpenCodeUsageSession[] + dailyAggregates: OpenCodeUsageDailyAggregate[] +} + +function createDeferred(): { + promise: Promise + resolve: (value: T) => void +} { + let resolve!: (value: T) => void + const promise = new Promise((promiseResolve) => { + resolve = promiseResolve + }) + return { promise, resolve } +} + +function createEmptyScanResult(): ScanResult { + return { + processedDatabases: [], + sessions: [], + dailyAggregates: [] + } +} function getDefaultState(): OpenCodeUsagePersistedState { return { @@ -36,6 +102,7 @@ function getDefaultState(): OpenCodeUsagePersistedState { function createStoreWithState(state: Partial): OpenCodeUsageStore { const store = new OpenCodeUsageStore({ getRepos: () => [], + getAllWorktreeMeta: () => ({}), getWorktreeMeta: () => undefined } as never) @@ -140,13 +207,123 @@ function makeDaily( } describe('OpenCodeUsageStore', () => { + let tempUserData: string + beforeEach(() => { + tempUserData = mkdtempSync(join(tmpdir(), 'orca-opencode-usage-store-')) + getPathMock.mockReturnValue(tempUserData) + initOpenCodeUsagePath() + writeOpens.value = 0 + writeOpens.inFlight = 0 + writeOpens.maxConcurrent = 0 + writeGate.blocked = false + writeGate.waiters = [] + vi.mocked(scanOpenCodeUsageDatabases).mockReset() + vi.mocked(scanOpenCodeUsageDatabases).mockResolvedValue(createEmptyScanResult()) vi.useFakeTimers() vi.setSystemTime(new Date('2026-04-10T12:00:00.000-04:00')) }) afterEach(() => { vi.useRealTimers() + rmSync(tempUserData, { recursive: true, force: true }) + }) + + it('persists a successful refresh with one full-cache write', async () => { + const store = createStoreWithState({ + scanState: { + enabled: true, + lastScanStartedAt: null, + lastScanCompletedAt: null, + lastScanError: null + } + }) + + await store.refresh(true) + + // Why exactly one: a refresh that rewrites the whole multi-MB cache twice is the regression this guards. + expect(writeOpens.value).toBe(1) + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + const persistedJson = readFileSync(join(tempUserData, 'orca-opencode-usage.json'), 'utf-8') + expect(persistedJson).toContain('\n') + expect(JSON.parse(persistedJson).scanState).toMatchObject({ + enabled: true, + lastScanStartedAt: new Date('2026-04-10T12:00:00.000-04:00').getTime(), + lastScanCompletedAt: new Date('2026-04-10T12:00:00.000-04:00').getTime(), + lastScanError: null + }) + }) + + it('keeps scan start visible in memory while scan-start persistence is skipped', async () => { + const pendingScan = createDeferred() + vi.mocked(scanOpenCodeUsageDatabases).mockReturnValueOnce(pendingScan.promise) + const store = createStoreWithState({ + scanState: { + enabled: true, + lastScanStartedAt: null, + lastScanCompletedAt: null, + lastScanError: 'previous failure' + } + }) + + const refreshPromise = store.refresh(true) + await Promise.resolve() + + expect(store.getScanState()).toMatchObject({ + isScanning: true, + lastScanStartedAt: new Date('2026-04-10T12:00:00.000-04:00').getTime(), + lastScanError: null + }) + expect(writeOpens.value).toBe(0) + + pendingScan.resolve(createEmptyScanResult()) + await refreshPromise + + expect(store.getScanState().isScanning).toBe(false) + expect(writeOpens.value).toBe(1) + }) + + it('vetoes a stale concurrent async write so the newer snapshot wins without leaking tmp files', async () => { + const store = createStoreWithState({ + scanState: { + enabled: true, + lastScanStartedAt: null, + lastScanCompletedAt: null, + lastScanError: null + } + }) + const internals = store as unknown as { + writeToDisk: () => Promise + state: OpenCodeUsagePersistedState + } + + writeGate.blocked = true + const first = internals.writeToDisk() + await vi.waitFor(() => expect(writeGate.waiters.length).toBe(1)) + + internals.state.scanState.enabled = false + writeGate.blocked = false + const second = internals.writeToDisk() + writeGate.waiters.splice(0).forEach((resolve) => resolve()) + await Promise.all([first, second]) + + expect( + JSON.parse(readFileSync(join(tempUserData, 'orca-opencode-usage.json'), 'utf-8')).scanState + .enabled + ).toBe(false) + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + // Serialized, so the superseded write can be skipped safely rather than racing the newer one. + expect(writeOpens.maxConcurrent).toBe(1) + }) + + it('sweeps a usage temp file orphaned by a crash between write and rename', async () => { + const orphan = join(tempUserData, 'orca-opencode-usage.json.999.1.abc.tmp') + writeFileSync(orphan, '{}') + + createStoreWithState({}) + await vi.waitFor(() => + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + ) }) it('reports no data for Orca scope when only non-Orca OpenCode usage exists', async () => { diff --git a/src/main/opencode-usage/store.ts b/src/main/opencode-usage/store.ts index d8815e6e927..6bde514f318 100644 --- a/src/main/opencode-usage/store.ts +++ b/src/main/opencode-usage/store.ts @@ -1,13 +1,8 @@ /* eslint-disable max-lines -- Why: this store owns OpenCode analytics persistence, scan policy, and renderer query semantics. Keeping range/scope queries next to scan persistence prevents UI totals from drifting from the SQLite projection. */ import { app } from 'electron' -import { dirname, join } from 'node:path' +import { join } from 'node:path' import { existsSync, readFileSync } from 'node:fs' -import { mkdir } from 'node:fs/promises' -import { - durableWriteTempPath, - removeStaleDurableWriteTempFiles, - writeFileDurableIfCurrent -} from '../durable-file-write' +import { UsageCacheSnapshotWriter } from '../usage-cache-snapshot-writer' import type { OpenCodeUsageBreakdownKind, OpenCodeUsageBreakdownRow, @@ -158,17 +153,13 @@ export class OpenCodeUsageStore { private state: OpenCodeUsagePersistedState private readonly store: Store private scanPromise: Promise | null = null - // Why: multi-MB usage JSON must not block the Electron main thread; generation vetoes superseded renames. - private writeGeneration = 0 - // Why serialize: two overlapping async writes can both veto themselves, leaving nothing on disk. - private pendingWrite: Promise = Promise.resolve() + // Why: the multi-MB usage JSON must not block the Electron main thread; the writer serializes + // writes and vetoes superseded renames. + private readonly writer = new UsageCacheSnapshotWriter('[opencode-usage]', getOpenCodeUsageFile) constructor(store: Store) { this.store = store this.state = this.load() - // Why: a crash between write and rename orphans a multi-MB temp file; reclaim once per launch. - // Seeding the write queue with it also orders the sweep ahead of the first write of this launch. - this.pendingWrite = removeStaleDurableWriteTempFiles(getOpenCodeUsageFile()) } private load(): OpenCodeUsagePersistedState { @@ -193,31 +184,8 @@ export class OpenCodeUsageStore { } private writeToDisk(): Promise { - const generation = ++this.writeGeneration - const write = this.pendingWrite.then(() => this.commitToDisk(generation)) - // Why: keep the queue usable after a failure — the awaiting caller still sees the rejection. - this.pendingWrite = write.catch((error: unknown) => { - console.error('[opencode-usage] Failed to persist usage cache:', error) - }) - return write - } - - private async commitToDisk(generation: number): Promise { - // Why: a newer snapshot is already queued behind this one, so rewriting the cache here is wasted. - if (generation !== this.writeGeneration) { - return - } - // Why stringify here and not at queue time: it blocks the main process for a multi-MB cache, and - // superseded generations must not pay for it. Synchronous, so no mutation can tear the JSON. - const payload = JSON.stringify(this.state, null, 2) - const usageFile = getOpenCodeUsageFile() - await mkdir(dirname(usageFile), { recursive: true }).catch(() => {}) - await writeFileDurableIfCurrent( - durableWriteTempPath(usageFile), - usageFile, - payload, - () => generation === this.writeGeneration - ) + // Pretty-print preserved: humans inspect this analytics cache on disk. + return this.writer.write(() => JSON.stringify(this.state, null, 2)) } async setEnabled(enabled: boolean): Promise { diff --git a/src/main/usage-cache-snapshot-writer.ts b/src/main/usage-cache-snapshot-writer.ts new file mode 100644 index 00000000000..c8b5b426d46 --- /dev/null +++ b/src/main/usage-cache-snapshot-writer.ts @@ -0,0 +1,56 @@ +import { mkdir } from 'node:fs/promises' +import { dirname } from 'node:path' +import { + durableWriteTempPath, + removeStaleDurableWriteTempFiles, + writeFileDurableIfCurrent +} from './durable-file-write' + +/** + * Serialized durable writer for the per-provider usage caches. One per store: the multi-MB JSON must + * not block the Electron main thread, writes are queued so two overlapping renames can't both veto + * themselves, and a snapshot superseded before its turn is dropped instead of paying for a rewrite. + */ +export class UsageCacheSnapshotWriter { + private generation = 0 + private pending: Promise + + constructor( + private readonly logTag: string, + private readonly resolveFile: () => string + ) { + // Why: a crash between write and rename orphans a multi-MB temp file; reclaim once per launch. + // Seeding the write queue with it also orders the sweep ahead of the first write of this launch. + this.pending = removeStaleDurableWriteTempFiles(resolveFile()) + } + + /** `serialize` runs only if this snapshot is still the newest one when its turn comes. */ + write(serialize: () => string): Promise { + const generation = ++this.generation + const write = this.pending.then(() => this.commit(generation, serialize)) + // Why: keep the queue usable after a failure — the awaiting caller still sees the rejection. + this.pending = write.catch((error: unknown) => { + console.error(`${this.logTag} Failed to persist usage cache:`, error) + }) + return write + } + + private async commit(generation: number, serialize: () => string): Promise { + // Why: a newer snapshot is already queued behind this one, so rewriting the cache here is wasted. + if (generation !== this.generation) { + return + } + // Why serialize here and not at queue time: it blocks the main process for a multi-MB cache, and + // superseded generations must not pay for it. Synchronous, so no mutation can tear the JSON. + const payload = serialize() + const usageFile = this.resolveFile() + await mkdir(dirname(usageFile), { recursive: true }).catch(() => {}) + // Why: atomic temp-file + durable rename so a crash cannot leave a truncated analytics file. + await writeFileDurableIfCurrent( + durableWriteTempPath(usageFile), + usageFile, + payload, + () => generation === this.generation + ) + } +}