From 58ccf84f02773e3613dccc0a1cc129f21020c65d Mon Sep 17 00:00:00 2001 From: Resham Joshi <65915470+iamtoruk@users.noreply.github.com> Date: Sun, 17 May 2026 13:54:38 -0700 Subject: [PATCH] =?UTF-8?q?Fix=20menubar=20per-provider=20performance=20(2?= =?UTF-8?q?4s=20=E2=86=92=202s)=20and=20session=20cache=20safety=20(#344)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-provider menubar calls now use loadDailyCache() instead of hydrateCache(), splitting history into cache-based and fallback paths. Fixes session cache wipe when scanProjectDirs/parseProviderSources receive empty dirs. Strips _dirty flag before session cache serialization to prevent unnecessary 132MB rewrites. Removes dead keychain code from both credential stores. --- .../Data/ClaudeCredentialStore.swift | 63 --------- .../Data/CodexCredentialStore.swift | 61 --------- src/main.ts | 126 ++++++++++++------ src/parser.ts | 29 ++-- src/session-cache.ts | 1 + 5 files changed, 102 insertions(+), 178 deletions(-) diff --git a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift index 286c1952..56e1603d 100644 --- a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift @@ -37,10 +37,7 @@ enum ClaudeCredentialStore { private static let maxCredentialBytes = 64 * 1024 /// Legacy local cache file. New writes use the macOS Keychain; this path is - /// read once for migration and then removed. private static let cacheFilename = "claude-credentials.v1.json" - private static let ourKeychainService = "org.agentseal.codeburn.menubar.claude.oauth.v1" - private static let ourKeychainAccount = "default" private static let lock = NSLock() private nonisolated(unsafe) static var memoryCache: CachedRecord? @@ -279,13 +276,6 @@ enum ClaudeCredentialStore { } private static func readOurCache() throws -> CredentialRecord? { - // Migrate: if credentials exist in keychain from a previous build, move to file. - if let keychainRecord = try? readOurKeychainCache() { - try? writeOurFileCache(record: keychainRecord) - deleteOurKeychainCache() - return keychainRecord - } - let url = cacheFileURL() guard FileManager.default.fileExists(atPath: url.path) else { return nil } let data = try SafeFile.read(from: url.path, maxBytes: maxCredentialBytes) @@ -304,63 +294,10 @@ enum ClaudeCredentialStore { try data.write(to: url, options: [.atomic, .completeFileProtection]) } - private static func readOurKeychainCache() throws -> CredentialRecord? { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: ourKeychainService, - kSecAttrAccount as String: ourKeychainAccount, - kSecMatchLimit as String: kSecMatchLimitOne, - kSecReturnData as String: true, - ] - var result: CFTypeRef? - let status = SecItemCopyMatching(query as CFDictionary, &result) - if status == errSecItemNotFound { return nil } - guard status == errSecSuccess, let data = result as? Data else { - throw StoreError.keychainReadFailed(status) - } - return try? JSONDecoder().decode(CredentialRecord.self, from: data) - } - - private static func writeOurKeychainCache(record: CredentialRecord) throws { - let url = cacheFileURL() - let data = try JSONEncoder().encode(record) - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: ourKeychainService, - kSecAttrAccount as String: ourKeychainAccount, - ] - let attributes: [String: Any] = [ - kSecValueData as String: data, - kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly, - ] - let status = SecItemUpdate(query as CFDictionary, attributes as CFDictionary) - if status == errSecItemNotFound { - var add = query - add.merge(attributes) { _, new in new } - let addStatus = SecItemAdd(add as CFDictionary, nil) - guard addStatus == errSecSuccess else { - throw StoreError.keychainWriteFailed(addStatus) - } - } else if status != errSecSuccess { - throw StoreError.keychainWriteFailed(status) - } - try? FileManager.default.removeItem(at: url) - } - private static func deleteOurCache() { - deleteOurKeychainCache() try? FileManager.default.removeItem(at: cacheFileURL()) } - private static func deleteOurKeychainCache() { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: ourKeychainService, - kSecAttrAccount as String: ourKeychainAccount, - ] - SecItemDelete(query as CFDictionary) - } - private static func cacheInMemory(_ record: CredentialRecord) { lock.withLock { memoryCache = CachedRecord(record: record, cachedAt: Date()) } } diff --git a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift index e5ddb0eb..ba9a4c14 100644 --- a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift @@ -18,8 +18,6 @@ enum CodexCredentialStore { private static let maxCredentialBytes = 64 * 1024 private static let cacheFilename = "codex-credentials.v1.json" - private static let ourKeychainService = "org.agentseal.codeburn.menubar.codex.oauth.v1" - private static let ourKeychainAccount = "default" private static let lock = NSLock() private nonisolated(unsafe) static var memoryCache: CachedRecord? @@ -201,12 +199,6 @@ enum CodexCredentialStore { } private static func readOurCache() throws -> CredentialRecord? { - if let keychainRecord = try? readOurKeychainCache() { - try? writeOurFileCache(record: keychainRecord) - deleteOurKeychainCache() - return keychainRecord - } - let url = cacheFileURL() guard FileManager.default.fileExists(atPath: url.path) else { return nil } let data = try SafeFile.read(from: url.path, maxBytes: maxCredentialBytes) @@ -225,63 +217,10 @@ enum CodexCredentialStore { try data.write(to: url, options: [.atomic, .completeFileProtection]) } - private static func readOurKeychainCache() throws -> CredentialRecord? { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: ourKeychainService, - kSecAttrAccount as String: ourKeychainAccount, - kSecMatchLimit as String: kSecMatchLimitOne, - kSecReturnData as String: true, - ] - var result: CFTypeRef? - let status = SecItemCopyMatching(query as CFDictionary, &result) - if status == errSecItemNotFound { return nil } - guard status == errSecSuccess, let data = result as? Data else { - throw StoreError.fileWriteFailed("keychain read failed with status \(status)") - } - return try? JSONDecoder().decode(CredentialRecord.self, from: data) - } - - private static func writeOurKeychainCache(record: CredentialRecord) throws { - let url = cacheFileURL() - let data = try JSONEncoder().encode(record) - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: ourKeychainService, - kSecAttrAccount as String: ourKeychainAccount, - ] - let attributes: [String: Any] = [ - kSecValueData as String: data, - kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly, - ] - let status = SecItemUpdate(query as CFDictionary, attributes as CFDictionary) - if status == errSecItemNotFound { - var add = query - add.merge(attributes) { _, new in new } - let addStatus = SecItemAdd(add as CFDictionary, nil) - guard addStatus == errSecSuccess else { - throw StoreError.fileWriteFailed("keychain write failed with status \(addStatus)") - } - } else if status != errSecSuccess { - throw StoreError.fileWriteFailed("keychain update failed with status \(status)") - } - try? FileManager.default.removeItem(at: url) - } - private static func deleteOurCache() { - deleteOurKeychainCache() try? FileManager.default.removeItem(at: cacheFileURL()) } - private static func deleteOurKeychainCache() { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: ourKeychainService, - kSecAttrAccount as String: ourKeychainAccount, - ] - SecItemDelete(query as CFDictionary) - } - private static func cacheInMemory(_ record: CredentialRecord) { lock.withLock { memoryCache = CachedRecord(record: record, cachedAt: Date()) } } diff --git a/src/main.ts b/src/main.ts index 485aa4cc..68dfb1de 100644 --- a/src/main.ts +++ b/src/main.ts @@ -7,7 +7,7 @@ import { convertCost } from './currency.js' import { renderStatusBar } from './format.js' import { type PeriodData, type ProviderCost } from './menubar-json.js' import { buildMenubarPayload } from './menubar-json.js' -import { getDaysInRange, ensureCacheHydrated, emptyCache, BACKFILL_DAYS, toDateString } from './daily-cache.js' +import { getDaysInRange, ensureCacheHydrated, loadDailyCache, emptyCache, BACKFILL_DAYS, toDateString, type DailyCache } from './daily-cache.js' import { aggregateProjectsIntoDays, buildPeriodDataFromDays, dateKey } from './day-aggregator.js' import { CATEGORY_LABELS, type DateRange, type ProjectSummary, type TaskCategory } from './types.js' import { aggregateModelEfficiency } from './model-efficiency.js' @@ -444,7 +444,6 @@ program const rangeEndStr = toDateString(periodInfo.range.end) const isAllProviders = pf === 'all' - const cache = await hydrateCache() let todayAllProjects: ProjectSummary[] | null = null let todayAllDays: ReturnType | null = null @@ -462,17 +461,15 @@ program return todayAllDays } - // CURRENT PERIOD DATA - // - .all provider: assemble from cache + today (fast) - // - specific provider: parse the period range with provider filter (correct, but slower) let currentData: PeriodData let scanProjects: ProjectSummary[] let scanRange: DateRange + let cache: DailyCache + let todayProviderData: PeriodData | null = null + let usedPerProviderCachePath = false if (isAllProviders) { - // Parse today's all-provider sessions once; historical data comes from cache to avoid - // double-counting. Reusing the same parsed object is important for the menubar path: - // large active sessions can OOM if this command retains multiple near-identical scans. + cache = await hydrateCache() const todayProjects = await getTodayAllProjects() const todayDays = await getTodayAllDays() const historicalDays = getDaysInRange(cache, rangeStartStr, yesterdayStr) @@ -480,26 +477,35 @@ program const allDays = [...historicalDays, ...todayInRange].sort((a, b) => a.date.localeCompare(b.date)) currentData = buildPeriodDataFromDays(allDays, periodInfo.label) scanProjects = todayProjects - scanRange = periodInfo.range - } else { - // Per-provider: parse only today (fast), use cache for historical days. - // The cache stores per-provider cost+calls per day, so we extract those - // and combine with today's fully-parsed provider data. - const todayProviderProjects = fp(await parseAllSessions(todayRange, pf)) - const todayData = buildPeriodData(periodInfo.label, todayProviderProjects) - const historicalDays = getDaysInRange(cache, rangeStartStr, yesterdayStr) - let histCost = 0, histCalls = 0 - for (const d of historicalDays) { - const prov = d.providers[pf] - if (prov) { histCost += prov.cost; histCalls += prov.calls } - } - currentData = { - ...todayData, - cost: todayData.cost + histCost, - calls: todayData.calls + histCalls, - } - scanProjects = todayProviderProjects scanRange = todayRange + } else { + cache = await loadDailyCache() + const cacheIsCurrent = cache.lastComputedDate !== null + && cache.lastComputedDate >= yesterdayStr + if (cacheIsCurrent && rangeStartStr < todayStr) { + const todayProviderProjects = fp(await parseAllSessions(todayRange, pf)) + todayProviderData = buildPeriodData(periodInfo.label, todayProviderProjects) + const historicalDays = getDaysInRange(cache, rangeStartStr, yesterdayStr) + let histCost = 0, histCalls = 0 + for (const d of historicalDays) { + const prov = d.providers[pf] + if (prov) { histCost += prov.cost; histCalls += prov.calls } + } + currentData = { + ...todayProviderData, + cost: todayProviderData.cost + histCost, + calls: todayProviderData.calls + histCalls, + } + scanProjects = todayProviderProjects + scanRange = todayRange + usedPerProviderCachePath = true + } else { + const fullProjects = fp(await parseAllSessions(periodInfo.range, pf)) + todayProviderData = buildPeriodData(periodInfo.label, fullProjects) + currentData = todayProviderData + scanProjects = fullProjects + scanRange = periodInfo.range + } } // PROVIDERS @@ -538,9 +544,12 @@ program // in the cache, so the filtered view shows zero tokens (heatmap/trend still works on cost). const historyStartStr = toDateString(new Date(now.getFullYear(), now.getMonth(), now.getDate() - BACKFILL_DAYS)) const allCacheDays = getDaysInRange(cache, historyStartStr, yesterdayStr) - const fullHistory = [...allCacheDays, ...(await getTodayAllDays()).filter(d => d.date === todayStr)] - const dailyHistory = fullHistory.map(d => { - if (isAllProviders) { + + let dailyHistory + if (isAllProviders) { + const todayDays = (await getTodayAllDays()).filter(d => d.date === todayStr) + const fullHistory = [...allCacheDays, ...todayDays] + dailyHistory = fullHistory.map(d => { const topModels = Object.entries(d.models) .filter(([name]) => name !== '') .sort(([, a], [, b]) => b.cost - a.cost) @@ -562,19 +571,52 @@ program cacheWriteTokens: d.cacheWriteTokens, topModels, } + }) + } else if (usedPerProviderCachePath) { + const historyFromCache = allCacheDays.map(d => { + const prov = d.providers[pf] ?? { calls: 0, cost: 0 } + return { + date: d.date, + cost: prov.cost, + calls: prov.calls, + inputTokens: 0, + outputTokens: 0, + cacheReadTokens: 0, + cacheWriteTokens: 0, + topModels: [] as { name: string; cost: number; calls: number; inputTokens: number; outputTokens: number }[], + } + }) + const todayCost = todayProviderData!.cost + const todayCalls = todayProviderData!.calls + if (todayCost > 0 || todayCalls > 0) { + historyFromCache.push({ + date: todayStr, + cost: todayCost, + calls: todayCalls, + inputTokens: 0, + outputTokens: 0, + cacheReadTokens: 0, + cacheWriteTokens: 0, + topModels: [], + }) } - const prov = d.providers[pf] ?? { calls: 0, cost: 0 } - return { - date: d.date, - cost: prov.cost, - calls: prov.calls, - inputTokens: 0, - outputTokens: 0, - cacheReadTokens: 0, - cacheWriteTokens: 0, - topModels: [], - } - }) + dailyHistory = historyFromCache + } else { + const fallbackDays = aggregateProjectsIntoDays(scanProjects) + dailyHistory = fallbackDays.map(d => { + const prov = d.providers[pf] ?? { calls: 0, cost: 0 } + return { + date: d.date, + cost: prov.cost, + calls: prov.calls, + inputTokens: 0, + outputTokens: 0, + cacheReadTokens: 0, + cacheWriteTokens: 0, + topModels: [] as { name: string; cost: number; calls: number; inputTokens: number; outputTokens: number }[], + } + }) + } const optimize = opts.optimize === false ? null : await scanAndDetect(scanProjects, scanRange) console.log(JSON.stringify(buildMenubarPayload(currentData, providers, optimize, dailyHistory))) diff --git a/src/parser.ts b/src/parser.ts index 51c70b0e..162d9c54 100644 --- a/src/parser.ts +++ b/src/parser.ts @@ -1373,9 +1373,7 @@ async function scanProjectDirs( } } - // Parse changed files, update cache for (const { filePath, info } of changedFiles) { - // Clear stale entry before parse — if parse fails, file is excluded delete section.files[filePath] const tracker = { lastCompleteLineOffset: 0 } @@ -1390,16 +1388,18 @@ async function scanProjectDirs( mcpInventory: extractMcpInventory(entries), turns: turns.map(parsedTurnToCachedTurn), } + ;(diskCache as { _dirty?: boolean })._dirty = true } - // Remove deleted files from cache - for (const cachedPath of Object.keys(section.files)) { - if (!allDiscoveredFiles.has(cachedPath)) { - delete section.files[cachedPath] + if (dirs.length > 0) { + for (const cachedPath of Object.keys(section.files)) { + if (!allDiscoveredFiles.has(cachedPath)) { + delete section.files[cachedPath] + ;(diskCache as { _dirty?: boolean })._dirty = true + } } } - // Query-time: derive ProjectSummary[] from all cached turns const projectMap = new Map() const allFiles = [ @@ -1716,6 +1716,7 @@ async function parseProviderSources( } section.files[source.path] = { fingerprint: fp, mcpInventory: [], turns } didParse = true + ;(diskCache as { _dirty?: boolean })._dirty = true } catch (err) { if (isSqliteBusyError(err)) { warnProviderReadFailureOnce(providerName, err) @@ -1732,10 +1733,12 @@ async function parseProviderSources( } } - // Remove deleted files from cache - for (const cachedPath of Object.keys(section.files)) { - if (!allDiscoveredFiles.has(cachedPath)) { - delete section.files[cachedPath] + if (sources.length > 0) { + for (const cachedPath of Object.keys(section.files)) { + if (!allDiscoveredFiles.has(cachedPath)) { + delete section.files[cachedPath] + ;(diskCache as { _dirty?: boolean })._dirty = true + } } } @@ -1919,7 +1922,9 @@ export async function parseAllSessions(dateRange?: DateRange, providerFilter?: s otherProjects.push(...projects) } - try { await saveCache(diskCache) } catch {} + if ((diskCache as { _dirty?: boolean })._dirty) { + try { await saveCache(diskCache) } catch {} + } const mergedMap = new Map() for (const p of [...claudeProjects, ...otherProjects]) { diff --git a/src/session-cache.ts b/src/session-cache.ts index 41c91b24..76568188 100644 --- a/src/session-cache.ts +++ b/src/session-cache.ts @@ -209,6 +209,7 @@ export async function saveCache(cache: SessionCache): Promise { const finalPath = getCachePath() const tempPath = `${finalPath}.${randomBytes(8).toString('hex')}.tmp` + delete (cache as { _dirty?: boolean })._dirty const payload = JSON.stringify(cache) const handle = await open(tempPath, 'w', 0o600)