From ad406cf10f9296d2867f2e418fa4081fece8c7bb Mon Sep 17 00:00:00 2001 From: Aditya Vikram Singh <247195684+avs-io@users.noreply.github.com> Date: Wed, 19 Aug 2026 13:48:50 +0530 Subject: [PATCH 01/15] fix(tui): coalesce resize bursts without dropping updates Hold Ink stdout columns/rows frozen during a SIGWINCH burst and emit one settled resize, then rerender. Do not intercept writes, so a mid-burst state update still paints even when net size is unchanged. Fixes #977. --- src/dashboard.tsx | 153 ++++++++++++++++++++++++--- tests/dashboard-resize.test.ts | 184 +++++++++++++++++++++++++++++++++ 2 files changed, 322 insertions(+), 15 deletions(-) create mode 100644 tests/dashboard-resize.test.ts diff --git a/src/dashboard.tsx b/src/dashboard.tsx index 7eeeaf35..35c6eeef 100644 --- a/src/dashboard.tsx +++ b/src/dashboard.tsx @@ -1,7 +1,8 @@ import { homedir } from 'os' +import { EventEmitter } from 'node:events' import React, { Fragment, useState, useCallback, useEffect, useLayoutEffect, useMemo, useRef } from 'react' -import { render, Box, Text, measureElement, useInput, useApp, useWindowSize, type DOMElement } from 'ink' +import { render, Box, Text, measureElement, useInput, useApp, useWindowSize, type DOMElement, type Instance, type RenderOptions } from 'ink' import { CATEGORY_LABELS, type DateRange, type ProjectSummary, type TaskCategory } from './types.js' import { formatCost, formatTokens, markEstimated, carriedCostNote } from './format.js' import { aggregateModelEfficiency } from './model-efficiency.js' @@ -31,6 +32,138 @@ export type DailyActivityRow = { export const DAILY_ACTIVITY_PAGE_SIZE = 10 export const INTERACTIVE_RENDER_OPTIONS = { alternateScreen: true } as const +export const RESIZE_DEBOUNCE_MS = 150 + +export type TerminalSize = { columns: number; rows: number } +export type DebouncedResizeStream = NodeJS.WriteStream & { + dispose(): void + onSettledResize(listener: (size: TerminalSize) => void): () => void +} + +function normalizeTerminalDimension(value: number | undefined, fallback: number): number { + return Number.isFinite(value) && value! > 0 ? Math.floor(value!) : fallback +} + +function terminalSizeOf(source: NodeJS.WriteStream): TerminalSize { + return { + columns: normalizeTerminalDimension(source.columns, 80), + rows: normalizeTerminalDimension(source.rows, 24), + } +} + +const RESIZE_LISTENER_METHODS = new Set([ + 'addListener', 'on', 'once', 'prependListener', 'prependOnceListener', 'off', 'removeListener', +]) + +export function createDebouncedResizeStream(source: NodeJS.WriteStream, delayMs: number): DebouncedResizeStream { + let resizeTimer: ReturnType | undefined + let disposed = false + let { columns, rows } = terminalSizeOf(source) + const resizeEvents = new EventEmitter() + const settledResizeListeners = new Set<(size: TerminalSize) => void>() + + const resize = () => { + if (disposed) return + if (resizeTimer) clearTimeout(resizeTimer) + resizeTimer = setTimeout(() => { + resizeTimer = undefined + if (disposed) return + const next = terminalSizeOf(source) + const changed = next.columns !== columns || next.rows !== rows + columns = next.columns + rows = next.rows + if (!changed) return + // Rerender first so the settled view owns the first paint at the new + // size; then notify Ink/useWindowSize. Writes are never intercepted, so + // a mid-burst state update still reaches the terminal even when net + // size is unchanged. + for (const listener of [...settledResizeListeners]) listener(next) + resizeEvents.emit('resize') + }, delayMs) + } + source.on('resize', resize) + + const dispose = () => { + if (disposed) return + disposed = true + source.off('resize', resize) + if (resizeTimer) clearTimeout(resizeTimer) + resizeTimer = undefined + resizeEvents.removeAllListeners() + settledResizeListeners.clear() + } + + const stream = new Proxy(source as DebouncedResizeStream, { + get(target, property) { + if (property === 'dispose') return dispose + if (property === 'onSettledResize') { + return (listener: (size: TerminalSize) => void) => { + if (disposed) return () => {} + settledResizeListeners.add(listener) + return () => settledResizeListeners.delete(listener) + } + } + if (property === 'columns') return columns + if (property === 'rows') return rows + if (RESIZE_LISTENER_METHODS.has(property)) { + return (event: string | symbol, ...args: unknown[]) => { + if (event === 'resize') { + if (!disposed) { + Reflect.apply(Reflect.get(resizeEvents, property) as (...args: unknown[]) => unknown, resizeEvents, [event, ...args]) + } + return stream + } + Reflect.apply(Reflect.get(target, property, target) as (...args: unknown[]) => unknown, target, [event, ...args]) + return stream + } + } + const value = Reflect.get(target, property, target) + return typeof value === 'function' ? value.bind(target) : value + }, + }) + return stream +} + +function DisposeOnUnmount({ dispose, children }: { dispose: () => void; children: React.ReactNode }) { + useLayoutEffect(() => dispose, [dispose]) + return children +} + +export type DebouncedInteractiveInstance = Instance & { + dispose(): void + stdout: DebouncedResizeStream +} + +export function renderDebouncedInteractive( + source: NodeJS.WriteStream, + view: (size: TerminalSize) => React.ReactElement, + options: Omit = INTERACTIVE_RENDER_OPTIONS, +): DebouncedInteractiveInstance { + const stdout = createDebouncedResizeStream(source, RESIZE_DEBOUNCE_MS) + let size = { columns: stdout.columns, rows: stdout.rows } + let unsubscribe = () => {} + let disposed = false + const dispose = () => { + if (disposed) return + disposed = true + unsubscribe() + stdout.dispose() + } + const dashboard = () => {view(size)} + let app: Instance + try { + app = render(dashboard(), { ...options, stdout }) + } catch (error) { + dispose() + throw error + } + unsubscribe = stdout.onSettledResize(nextSize => { + if (disposed) return + size = nextSize + app.rerender(dashboard()) + }) + return Object.assign(app, { dispose, stdout }) +} export function getDailyActivityPageSize(columnCount: 1 | 2 | 3, projectRows: number, activityRows: number, dayMode = false): number { if (dayMode) return 1 @@ -1738,23 +1871,13 @@ export async function renderDashboard(period: Period = 'week', provider: string const label = initialDay ? formatDayRangeLabel(initialDay) : customRangeLabel patchStdoutForWindows() if (isTTY) { - let windowColumns = process.stdout.columns - const dashboard = () => ( - - ) - const app = render( - dashboard(), - INTERACTIVE_RENDER_OPTIONS, - ) - const resize = () => { - windowColumns = process.stdout.columns - app.rerender(dashboard()) - } - process.stdout.prependListener('resize', resize) + const app = renderDebouncedInteractive(process.stdout, ({ columns }) => ( + + )) try { await app.waitUntilExit() } finally { - process.stdout.off('resize', resize) + app.dispose() } } else { const { unmount } = render(, { patchConsole: false }) diff --git a/tests/dashboard-resize.test.ts b/tests/dashboard-resize.test.ts new file mode 100644 index 00000000..a1eb80a6 --- /dev/null +++ b/tests/dashboard-resize.test.ts @@ -0,0 +1,184 @@ +import { readFileSync } from 'node:fs' +import { PassThrough } from 'node:stream' + +import React, { useEffect } from 'react' +import { Text } from 'ink' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { RESIZE_DEBOUNCE_MS, createDebouncedResizeStream, renderDebouncedInteractive } from '../src/dashboard.js' +import { stripSyncUpdateEscapes } from '../src/ink-win.js' + +function makeTerminal(columns = 100, rows = 24): PassThrough & NodeJS.WriteStream { + const terminal = new PassThrough() as PassThrough & NodeJS.WriteStream + terminal.isTTY = true + terminal.columns = columns + terminal.rows = rows + return terminal +} + +function paintedFrames(writes: string[]): string[] { + return writes + .map(chunk => stripSyncUpdateEscapes(chunk)) + .flatMap(chunk => chunk.match(/FRAME:[^\r\n]*/g) ?? []) +} + +describe('interactive dashboard resize stream', () => { + afterEach(() => vi.useRealTimers()) + + it('does not intercept writes or parse synchronized-update frames', () => { + const source = readFileSync(new URL('../src/dashboard.tsx', import.meta.url), 'utf8') + expect(source).not.toContain('suppressingFrame') + expect(source).not.toContain('capturingResizeWrites') + expect(source).not.toContain('finalFramePreamble') + expect(source).not.toContain('indexOf(BSU)') + expect(source).not.toContain('indexOf(ESU)') + expect(source).not.toContain('process.stdout.prependListener') + }) + + it('publishes one settled paint after a resize burst', async () => { + vi.useFakeTimers() + const terminal = makeTerminal() + const writes: string[] = [] + terminal.on('data', chunk => writes.push(String(chunk))) + const app = renderDebouncedInteractive(terminal, size => ( + React.createElement(Text, null, `FRAME:${size.columns}x${size.rows}`) + ), { + interactive: true, + patchConsole: false, + alternateScreen: true, + }) + await vi.advanceTimersByTimeAsync(100) + writes.length = 0 + + terminal.columns = 99 + terminal.emit('resize') + await vi.advanceTimersByTimeAsync(50) + terminal.columns = 98 + terminal.emit('resize') + await vi.advanceTimersByTimeAsync(50) + terminal.columns = 97 + terminal.rows = 30 + terminal.emit('resize') + + await vi.advanceTimersByTimeAsync(RESIZE_DEBOUNCE_MS + 100) + + const frames = paintedFrames(writes) + expect(frames.filter(frame => frame !== 'FRAME:97x30'), 'a resize burst must not paint intermediate sizes').toEqual([]) + expect(frames).toContain('FRAME:97x30') + + app.unmount() + app.dispose() + await vi.runAllTimersAsync() + await app.waitUntilExit() + }) + + it('paints a mid-burst state update when the burst nets to no size change', async () => { + vi.useFakeTimers() + const terminal = makeTerminal() + const writes: string[] = [] + terminal.on('data', chunk => writes.push(String(chunk))) + let updateVisibleState = () => {} + const StatefulProbe = ({ size }: { size: { columns: number; rows: number } }) => { + const [revision, setRevision] = React.useState(0) + updateVisibleState = () => setRevision(value => value + 1) + return React.createElement(Text, null, `FRAME:revision=${revision}:size=${size.columns}x${size.rows}`) + } + const app = renderDebouncedInteractive(terminal, size => React.createElement(StatefulProbe, { size }), { + interactive: true, + patchConsole: false, + alternateScreen: true, + }) + await vi.advanceTimersByTimeAsync(100) + writes.length = 0 + + terminal.columns = 80 + terminal.emit('resize') + updateVisibleState() + terminal.columns = 100 + terminal.emit('resize') + + await vi.advanceTimersByTimeAsync(RESIZE_DEBOUNCE_MS + 100) + + expect(paintedFrames(writes).some(frame => frame.includes('revision=1')), 'a mid-burst state update must reach the terminal even when net size is unchanged').toBe(true) + + app.unmount() + app.dispose() + await vi.runAllTimersAsync() + await app.waitUntilExit() + }) + + it('paints a state update after a spurious identical-dimension SIGWINCH', async () => { + vi.useFakeTimers() + const terminal = makeTerminal() + const writes: string[] = [] + terminal.on('data', chunk => writes.push(String(chunk))) + let updateVisibleState = () => {} + const StatefulProbe = ({ size }: { size: { columns: number; rows: number } }) => { + const [revision, setRevision] = React.useState(0) + updateVisibleState = () => setRevision(value => value + 1) + return React.createElement(Text, null, `FRAME:revision=${revision}:size=${size.columns}x${size.rows}`) + } + const app = renderDebouncedInteractive(terminal, size => React.createElement(StatefulProbe, { size }), { + interactive: true, + patchConsole: false, + alternateScreen: true, + }) + await vi.advanceTimersByTimeAsync(100) + writes.length = 0 + + terminal.emit('resize') + updateVisibleState() + + await vi.advanceTimersByTimeAsync(RESIZE_DEBOUNCE_MS + 100) + + expect(paintedFrames(writes).some(frame => frame.includes('revision=1')), 'a state update must still paint after a no-op SIGWINCH').toBe(true) + + app.unmount() + app.dispose() + await vi.runAllTimersAsync() + await app.waitUntilExit() + }) + + it('removes the source relay and cancels pending resize delivery on dispose', async () => { + vi.useFakeTimers() + const terminal = makeTerminal() + const renderedSizes: Array<{ columns: number; rows: number }> = [] + const Probe = ({ size }: { size: { columns: number; rows: number } }) => { + useEffect(() => { + renderedSizes.push(size) + }, [size]) + return React.createElement(Text, null, `FRAME:${size.columns}x${size.rows}`) + } + const app = renderDebouncedInteractive(terminal, size => React.createElement(Probe, { size }), { + interactive: true, + patchConsole: false, + }) + await vi.advanceTimersByTimeAsync(100) + renderedSizes.length = 0 + + terminal.columns = 90 + terminal.emit('resize') + app.unmount() + app.dispose() + await vi.runAllTimersAsync() + await app.waitUntilExit() + + await vi.advanceTimersByTimeAsync(RESIZE_DEBOUNCE_MS) + expect(renderedSizes).toEqual([]) + expect(terminal.listenerCount('resize')).toBe(0) + }) + + it('disposes a stream that never rendered', () => { + const terminal = makeTerminal() + const stdout = createDebouncedResizeStream(terminal, RESIZE_DEBOUNCE_MS) + expect(terminal.listenerCount('resize')).toBe(1) + stdout.dispose() + expect(terminal.listenerCount('resize')).toBe(0) + + const resize = vi.fn() + stdout.on('resize', resize) + terminal.emit('resize') + expect(resize).not.toHaveBeenCalled() + expect(terminal.listenerCount('resize')).toBe(0) + }) +}) From cdaa5b7ed37456063bdb5cf8e48deecffd752b5f Mon Sep 17 00:00:00 2001 From: Aditya Vikram Singh <247195684+avs-io@users.noreply.github.com> Date: Wed, 19 Aug 2026 13:39:51 +0530 Subject: [PATCH 02/15] fix(menubar): migrate Claude/Codex caches to namespaced Keychain Stop writing OAuth caches as Application Support JSON. Persist CodeBurn-owned items in Keychain, secure-read and migrate leftover 0644 files only after read-back verification, and keep Claude from storing a refresh token. --- mac/Sources/CodeBurnMenubar/AppStore.swift | 12 +- .../Data/ClaudeCredentialStore.swift | 241 +++++++++++-- .../Data/ClaudeSubscriptionService.swift | 2 +- .../Data/CodexCredentialStore.swift | 167 +++++++-- .../Data/CodexSubscriptionService.swift | 2 +- .../Security/KeychainCredentialCache.swift | 186 ++++++++++ .../CodeBurnMenubar/Security/SafeFile.swift | 66 ++++ .../CodeBurnMenubar/Views/SettingsView.swift | 2 +- .../CredentialKeychainCacheRedTests.swift | 149 ++++++++ .../CredentialKeychainContinuityTests.swift | 341 ++++++++++++++++++ 10 files changed, 1112 insertions(+), 56 deletions(-) create mode 100644 mac/Sources/CodeBurnMenubar/Security/KeychainCredentialCache.swift create mode 100644 mac/Tests/CodeBurnMenubarTests/CredentialKeychainCacheRedTests.swift create mode 100644 mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift diff --git a/mac/Sources/CodeBurnMenubar/AppStore.swift b/mac/Sources/CodeBurnMenubar/AppStore.swift index 005a7531..33ac8d12 100644 --- a/mac/Sources/CodeBurnMenubar/AppStore.swift +++ b/mac/Sources/CodeBurnMenubar/AppStore.swift @@ -1070,7 +1070,11 @@ final class AppStore { // result instead of re-populating the cleared state. claudeRefreshGen &+= 1 subscription = nil - subscriptionError = nil + if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.isSuccess { + subscriptionError = "Could not fully remove the local Claude credential cache." + } else { + subscriptionError = nil + } subscriptionLoadState = .notBootstrapped capacityEstimates = [:] Task.detached { await SubscriptionSnapshotStore.clearAll() } @@ -1134,7 +1138,11 @@ final class AppStore { CodexSubscriptionService.disconnect() codexRefreshGen &+= 1 codexUsage = nil - codexError = nil + if let result = CodexCredentialStore.lastCacheDeleteResult, !result.isSuccess { + codexError = "Could not fully remove the local Codex credential cache." + } else { + codexError = nil + } codexLoadState = .notBootstrapped NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil) } diff --git a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift index 002e861e..d9e691e6 100644 --- a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift @@ -16,9 +16,9 @@ import Security /// the refresh endpoint. If the CLI hasn't rotated yet we report a /// transient staleness (`sourceTokenStale`) and recover on its next use. /// -/// 3. **In-memory + file cache** so back-to-back reads in the same refresh +/// 3. **In-memory + Keychain cache** so back-to-back reads in the same refresh /// cycle don't re-hit the source, and we keep serving the last good token -/// across launches. +/// across launches without a plaintext Application Support file. enum ClaudeCredentialStore { private static let bootstrapCompletedKey = "codeburn.claude.bootstrapCompleted" private static let inMemoryTTL: TimeInterval = 5 * 60 @@ -28,12 +28,45 @@ enum ClaudeCredentialStore { private static let credentialsRelativePath = ".claude/.credentials.json" private static let maxCredentialBytes = 64 * 1024 - /// Legacy local cache file. New writes use the macOS Keychain; this path is + /// Legacy local cache file under Application Support. Migration to the + /// CodeBurn-namespaced Keychain item is staged behind an injectable seam. private static let cacheFilename = "claude-credentials.v1.json" + static let ourKeychainService = CodeBurnKeychainIdentity.claudeService + static let ourKeychainAccount = CodeBurnKeychainIdentity.account + private static let lock = NSLock() private nonisolated(unsafe) static var memoryCache: CachedRecord? + // MARK: - Injectable seams (tests + staged Keychain migration) + + /// Override Application Support root. Nil uses the real user domain. + nonisolated(unsafe) static var applicationSupportDirectoryOverride: URL? + /// Override home for Claude CLI credential discovery. Nil uses the real home. + nonisolated(unsafe) static var homeDirectoryOverride: URL? + /// Override defaults used for bootstrap flags. + nonisolated(unsafe) static var userDefaultsOverride: UserDefaults? + /// Keychain backend. Production uses Live; tests inject InMemory. + nonisolated(unsafe) static var keychainCache: any KeychainCredentialCaching = LiveKeychainCredentialCache() + + static func resetTestSeams() { + applicationSupportDirectoryOverride = nil + homeDirectoryOverride = nil + userDefaultsOverride = nil + keychainCache = LiveKeychainCredentialCache() + lastCacheDeleteResult = nil + lastLegacyCleanupFailed = false + lock.withLock { memoryCache = nil } + } + + private static var defaults: UserDefaults { + userDefaultsOverride ?? .standard + } + + private static var homeDirectory: URL { + homeDirectoryOverride ?? FileManager.default.homeDirectoryForCurrentUser + } + struct CachedRecord { let record: CredentialRecord let cachedAt: Date @@ -88,18 +121,37 @@ enum ClaudeCredentialStore { /// True once the user has explicitly connected (clicked Connect in the Plan /// tab AND we successfully read their credentials). Persists across launches. static var isBootstrapCompleted: Bool { - get { UserDefaults.standard.bool(forKey: bootstrapCompletedKey) } - set { UserDefaults.standard.set(newValue, forKey: bootstrapCompletedKey) } + get { defaults.bool(forKey: bootstrapCompletedKey) } + set { defaults.set(newValue, forKey: bootstrapCompletedKey) } } /// Reset bootstrap state. Used when the user explicitly wants to disconnect - /// or when the refresh token has been revoked terminally. - static func resetBootstrap() { + /// or when the refresh token has been revoked terminally. Deletion failures + /// are recorded on `lastCacheDeleteResult` — callers must not claim the + /// local copy is gone when `isSuccess` is false. + @discardableResult + static func resetBootstrap() -> CacheDeleteResult { lock.withLock { memoryCache = nil } - deleteOurCache() + let result = deleteOurCache() + lastCacheDeleteResult = result isBootstrapCompleted = false + return result } + /// Outcome of deleting CodeBurn-owned Claude cache material. + struct CacheDeleteResult: Equatable { + var keychainDeletedOrAbsent: Bool + var legacyDeletedOrAbsent: Bool + var isSuccess: Bool { keychainDeletedOrAbsent && legacyDeletedOrAbsent } + } + + /// Last disconnect/cleanup result. Nil until the first delete attempt. + nonisolated(unsafe) static var lastCacheDeleteResult: CacheDeleteResult? + + /// Last legacy-unlink failure after a verified Keychain write (cleanup retry signal). + nonisolated(unsafe) static var lastLegacyCleanupFailed = false + + // MARK: - Public API /// User-initiated entry point. Reads from Claude's source (PROMPTS for the @@ -190,7 +242,7 @@ enum ClaudeCredentialStore { } private static func readClaudeFile() throws -> CredentialRecord? { - let url = FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent(credentialsRelativePath) + let url = homeDirectory.appendingPathComponent(credentialsRelativePath) guard FileManager.default.fileExists(atPath: url.path) else { return nil } let data = try SafeFile.read(from: url.path, maxBytes: maxCredentialBytes) return try parseClaudeBlob(data: sanitizeClaudeBlob(data)) @@ -305,37 +357,174 @@ enum ClaudeCredentialStore { } } - // MARK: - Local cache file (no keychain involvement) + // MARK: - Local cache (injectable Application Support + Keychain seam) - private static func cacheFileURL() -> URL { - let support = FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask).first - ?? FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent("Library/Application Support") + static func cacheFileURL() -> URL { + let support = applicationSupportDirectoryOverride + ?? FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask).first + ?? homeDirectory.appendingPathComponent("Library/Application Support") return support .appendingPathComponent("CodeBurn", isDirectory: true) .appendingPathComponent(cacheFilename) } + /// Production-used write path. Persists to the CodeBurn-namespaced Keychain + /// item. Claude never stores a refresh token in this cache. After a verified + /// Keychain read-back, any legacy JSON is unlinked (not "securely erased"). + static func writeOurCache(record: CredentialRecord) throws { + let persisted = encodePersisted(record) + let data = try JSONEncoder().encode(persisted) + try keychainCache.upsert( + service: ourKeychainService, + account: ourKeychainAccount, + data: data + ) + try verifyKeychainMatches(persisted) + tryUnlinkLegacyAfterVerifiedKeychain() + } + + /// Cache shape stored in Keychain — intentionally omits refreshToken. + struct PersistedCacheRecord: Codable, Equatable { + let accessToken: String + let expiresAt: Date? + let rateLimitTier: String? + } + + private static func encodePersisted(_ record: CredentialRecord) -> PersistedCacheRecord { + PersistedCacheRecord( + accessToken: record.accessToken, + expiresAt: record.expiresAt, + rateLimitTier: record.rateLimitTier + ) + } + + private static func decodePersisted(_ data: Data) -> CredentialRecord? { + if let persisted = try? JSONDecoder().decode(PersistedCacheRecord.self, from: data) { + return CredentialRecord( + accessToken: persisted.accessToken, + refreshToken: nil, + expiresAt: persisted.expiresAt, + rateLimitTier: persisted.rateLimitTier + ) + } + // Historical blobs may still include refreshToken; drop it on read. + if let legacy = try? JSONDecoder().decode(CredentialRecord.self, from: data) { + return CredentialRecord( + accessToken: legacy.accessToken, + refreshToken: nil, + expiresAt: legacy.expiresAt, + rateLimitTier: legacy.rateLimitTier + ) + } + return nil + } + + private static func verifyKeychainMatches(_ expected: PersistedCacheRecord) throws { + guard let data = try keychainCache.read(service: ourKeychainService, account: ourKeychainAccount), + let roundTrip = decodePersisted(data), + encodePersisted(roundTrip) == expected + else { + throw StoreError.keychainWriteFailed(-1) + } + } + private static func readOurCache() throws -> CredentialRecord? { + if let data = try keychainCache.read(service: ourKeychainService, account: ourKeychainAccount), + let record = decodePersisted(data) { + // Rewrite historical Claude blobs once without refreshToken. + let sanitized = encodePersisted(record) + if let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any], + object.keys.contains("refreshToken") { + try? writeOurCache(record: record) + } else { + tryUnlinkLegacyAfterVerifiedKeychain() + _ = sanitized + } + return record + } + + return try migrateLegacyFileIfPresent() + } + + /// Secure-read legacy JSON, upsert Keychain, verify read-back, then unlink. + private static func migrateLegacyFileIfPresent() throws -> CredentialRecord? { let url = cacheFileURL() guard FileManager.default.fileExists(atPath: url.path) else { return nil } - let data = try SafeFile.read(from: url.path, maxBytes: maxCredentialBytes) - guard let record = try? JSONDecoder().decode(CredentialRecord.self, from: data) else { return nil } - return record + + let data: Data + do { + data = try SafeFile.readAfterSecuringPermissions( + from: url.path, + maxBytes: maxCredentialBytes + ) + } catch { + // Symlink / ownership / chmod failures: leave the file alone. + return nil + } + + guard let decoded = try? JSONDecoder().decode(CredentialRecord.self, from: data) else { + // Invalid data stays in place at 0600; do not delete. + return nil + } + let migrated = CredentialRecord( + accessToken: decoded.accessToken, + refreshToken: nil, + expiresAt: decoded.expiresAt, + rateLimitTier: decoded.rateLimitTier + ) + + do { + try writeOurCache(record: migrated) + return migrated + } catch { + // Keychain write/read-back failure: leave repaired 0600 legacy file. + lastLegacyCleanupFailed = false + return migrated + } } - private static func writeOurCache(record: CredentialRecord) throws { - try writeOurFileCache(record: record) - } - - private static func writeOurFileCache(record: CredentialRecord) throws { + private static func tryUnlinkLegacyAfterVerifiedKeychain() { let url = cacheFileURL() - try FileManager.default.createDirectory(at: url.deletingLastPathComponent(), withIntermediateDirectories: true) - let data = try JSONEncoder().encode(record) - try data.write(to: url, options: [.atomic, .completeFileProtection]) + guard FileManager.default.fileExists(atPath: url.path) else { + lastLegacyCleanupFailed = false + return + } + do { + try FileManager.default.removeItem(at: url) + lastLegacyCleanupFailed = false + } catch { + // Retain valid Keychain item + 0600 file; surface for later retry. + lastLegacyCleanupFailed = true + } } - private static func deleteOurCache() { - try? FileManager.default.removeItem(at: cacheFileURL()) + @discardableResult + private static func deleteOurCache() -> CacheDeleteResult { + var keychainOK = true + do { + try keychainCache.delete(service: ourKeychainService, account: ourKeychainAccount) + } catch { + keychainOK = false + } + + var legacyOK = true + let url = cacheFileURL() + if FileManager.default.fileExists(atPath: url.path) { + do { + try FileManager.default.removeItem(at: url) + } catch { + legacyOK = false + } + } + return CacheDeleteResult( + keychainDeletedOrAbsent: keychainOK, + legacyDeletedOrAbsent: legacyOK + ) + } + + /// Clears only the in-memory TTL cache (simulates process restart in tests). + static func clearMemoryCacheForTesting() { + lock.withLock { memoryCache = nil } } private static func cacheInMemory(_ record: CredentialRecord) { diff --git a/mac/Sources/CodeBurnMenubar/Data/ClaudeSubscriptionService.swift b/mac/Sources/CodeBurnMenubar/Data/ClaudeSubscriptionService.swift index d2876d1b..d452b049 100644 --- a/mac/Sources/CodeBurnMenubar/Data/ClaudeSubscriptionService.swift +++ b/mac/Sources/CodeBurnMenubar/Data/ClaudeSubscriptionService.swift @@ -100,7 +100,7 @@ enum ClaudeSubscriptionService { /// Reset everything — used on user-initiated disconnect. static func disconnect() { - ClaudeCredentialStore.resetBootstrap() + _ = ClaudeCredentialStore.resetBootstrap() clearUsageBlock() } diff --git a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift index 9f7ae18f..96bf71fd 100644 --- a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift @@ -5,7 +5,7 @@ import Security /// ClaudeCredentialStore but reads from ~/.codex/auth.json — Codex CLI /// already stores its tokens as plaintext JSON in the home directory, so /// no keychain prompt is involved on bootstrap. After the user clicks -/// Connect we cache a copy under ~/Library/Application Support/CodeBurn so +/// Connect we cache a CodeBurn-owned copy in the macOS Keychain so /// we keep using rotated tokens after refresh. enum CodexCredentialStore { private static let bootstrapCompletedKey = "codeburn.codex.bootstrapCompleted" @@ -23,9 +23,37 @@ enum CodexCredentialStore { private static let cacheFilename = "codex-credentials.v1.json" + static let ourKeychainService = CodeBurnKeychainIdentity.codexService + static let ourKeychainAccount = CodeBurnKeychainIdentity.account + private static let lock = NSLock() private nonisolated(unsafe) static var memoryCache: CachedRecord? + // MARK: - Injectable seams (tests + staged Keychain migration) + + nonisolated(unsafe) static var applicationSupportDirectoryOverride: URL? + nonisolated(unsafe) static var homeDirectoryOverride: URL? + nonisolated(unsafe) static var userDefaultsOverride: UserDefaults? + nonisolated(unsafe) static var keychainCache: any KeychainCredentialCaching = LiveKeychainCredentialCache() + + static func resetTestSeams() { + applicationSupportDirectoryOverride = nil + homeDirectoryOverride = nil + userDefaultsOverride = nil + keychainCache = LiveKeychainCredentialCache() + lastCacheDeleteResult = nil + lastLegacyCleanupFailed = false + lock.withLock { memoryCache = nil } + } + + private static var defaults: UserDefaults { + userDefaultsOverride ?? .standard + } + + private static var homeDirectory: URL { + homeDirectoryOverride ?? FileManager.default.homeDirectoryForCurrentUser + } + struct CachedRecord { let record: CredentialRecord let cachedAt: Date @@ -97,16 +125,28 @@ enum CodexCredentialStore { // MARK: - Bootstrap state static var isBootstrapCompleted: Bool { - get { UserDefaults.standard.bool(forKey: bootstrapCompletedKey) } - set { UserDefaults.standard.set(newValue, forKey: bootstrapCompletedKey) } + get { defaults.bool(forKey: bootstrapCompletedKey) } + set { defaults.set(newValue, forKey: bootstrapCompletedKey) } } - static func resetBootstrap() { + static func resetBootstrap() -> CacheDeleteResult { lock.withLock { memoryCache = nil } - deleteOurCache() + let result = deleteOurCache() + lastCacheDeleteResult = result isBootstrapCompleted = false + return result } + struct CacheDeleteResult: Equatable { + var keychainDeletedOrAbsent: Bool + var legacyDeletedOrAbsent: Bool + var isSuccess: Bool { keychainDeletedOrAbsent && legacyDeletedOrAbsent } + } + + nonisolated(unsafe) static var lastCacheDeleteResult: CacheDeleteResult? + nonisolated(unsafe) static var lastLegacyCleanupFailed = false + + // MARK: - Public API @discardableResult @@ -170,7 +210,7 @@ enum CodexCredentialStore { // MARK: - Bootstrap source: ~/.codex/auth.json private static func readCodexAuth() throws -> CredentialRecord { - let url = FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent(codexAuthPath) + let url = homeDirectory.appendingPathComponent(codexAuthPath) guard FileManager.default.fileExists(atPath: url.path) else { throw StoreError.bootstrapNoSource } @@ -231,7 +271,7 @@ enum CodexCredentialStore { /// key (OPENAI_API_KEY, auth_mode, ...) and only rewrites the tokens dict and /// last_refresh. Keeps the CLI and the menubar on the same rotated grant. private static func writeBackToCodexAuth(record: CredentialRecord) { - let url = FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent(codexAuthPath) + let url = homeDirectory.appendingPathComponent(codexAuthPath) var json: [String: Any] = [:] if let data = try? SafeFile.read(from: url.path, maxBytes: maxCredentialBytes), let existing = try? JSONSerialization.jsonObject(with: data) as? [String: Any] { @@ -250,40 +290,117 @@ enum CodexCredentialStore { guard let out = try? JSONSerialization.data(withJSONObject: json, options: [.prettyPrinted, .sortedKeys]) else { return } - try? out.write(to: url, options: .atomic) + try? SafeFile.write(out, to: url.path, mode: 0o600) } - // MARK: - Local cache file + // MARK: - Local cache (injectable Application Support + Keychain seam) - private static func cacheFileURL() -> URL { - let support = FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask).first - ?? FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent("Library/Application Support") + static func cacheFileURL() -> URL { + let support = applicationSupportDirectoryOverride + ?? FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask).first + ?? homeDirectory.appendingPathComponent("Library/Application Support") return support .appendingPathComponent("CodeBurn", isDirectory: true) .appendingPathComponent(cacheFilename) } + /// Production-used write path. Persists rotation fields to the CodeBurn + /// Keychain item. After verified read-back, unlinks any legacy JSON. + static func writeOurCache(record: CredentialRecord) throws { + let data = try JSONEncoder().encode(record) + try keychainCache.upsert( + service: ourKeychainService, + account: ourKeychainAccount, + data: data + ) + guard let readBack = try keychainCache.read(service: ourKeychainService, account: ourKeychainAccount), + let roundTrip = try? JSONDecoder().decode(CredentialRecord.self, from: readBack), + roundTrip.accessToken == record.accessToken, + roundTrip.refreshToken == record.refreshToken, + roundTrip.idToken == record.idToken, + roundTrip.accountId == record.accountId + else { + throw StoreError.fileWriteFailed("keychain read-back mismatch") + } + tryUnlinkLegacyAfterVerifiedKeychain() + } + private static func readOurCache() throws -> CredentialRecord? { + if let data = try keychainCache.read(service: ourKeychainService, account: ourKeychainAccount), + let record = try? JSONDecoder().decode(CredentialRecord.self, from: data) { + tryUnlinkLegacyAfterVerifiedKeychain() + return record + } + return try migrateLegacyFileIfPresent() + } + + private static func migrateLegacyFileIfPresent() throws -> CredentialRecord? { let url = cacheFileURL() guard FileManager.default.fileExists(atPath: url.path) else { return nil } - let data = try SafeFile.read(from: url.path, maxBytes: maxCredentialBytes) - guard let record = try? JSONDecoder().decode(CredentialRecord.self, from: data) else { return nil } - return record + + let data: Data + do { + data = try SafeFile.readAfterSecuringPermissions( + from: url.path, + maxBytes: maxCredentialBytes + ) + } catch { + return nil + } + + guard let decoded = try? JSONDecoder().decode(CredentialRecord.self, from: data) else { + return nil + } + + do { + try writeOurCache(record: decoded) + return decoded + } catch { + lastLegacyCleanupFailed = false + return decoded + } } - private static func writeOurCache(record: CredentialRecord) throws { - try writeOurFileCache(record: record) - } - - private static func writeOurFileCache(record: CredentialRecord) throws { + private static func tryUnlinkLegacyAfterVerifiedKeychain() { let url = cacheFileURL() - try FileManager.default.createDirectory(at: url.deletingLastPathComponent(), withIntermediateDirectories: true) - let data = try JSONEncoder().encode(record) - try data.write(to: url, options: [.atomic, .completeFileProtection]) + guard FileManager.default.fileExists(atPath: url.path) else { + lastLegacyCleanupFailed = false + return + } + do { + try FileManager.default.removeItem(at: url) + lastLegacyCleanupFailed = false + } catch { + lastLegacyCleanupFailed = true + } } - private static func deleteOurCache() { - try? FileManager.default.removeItem(at: cacheFileURL()) + @discardableResult + private static func deleteOurCache() -> CacheDeleteResult { + var keychainOK = true + do { + try keychainCache.delete(service: ourKeychainService, account: ourKeychainAccount) + } catch { + keychainOK = false + } + + var legacyOK = true + let url = cacheFileURL() + if FileManager.default.fileExists(atPath: url.path) { + do { + try FileManager.default.removeItem(at: url) + } catch { + legacyOK = false + } + } + return CacheDeleteResult( + keychainDeletedOrAbsent: keychainOK, + legacyDeletedOrAbsent: legacyOK + ) + } + + static func clearMemoryCacheForTesting() { + lock.withLock { memoryCache = nil } } private static func cacheInMemory(_ record: CredentialRecord) { diff --git a/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift b/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift index d25637c1..7fd5968b 100644 --- a/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift +++ b/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift @@ -79,7 +79,7 @@ enum CodexSubscriptionService { } static func disconnect() { - CodexCredentialStore.resetBootstrap() + _ = CodexCredentialStore.resetBootstrap() clearUsageBlock() } diff --git a/mac/Sources/CodeBurnMenubar/Security/KeychainCredentialCache.swift b/mac/Sources/CodeBurnMenubar/Security/KeychainCredentialCache.swift new file mode 100644 index 00000000..8ea079fa --- /dev/null +++ b/mac/Sources/CodeBurnMenubar/Security/KeychainCredentialCache.swift @@ -0,0 +1,186 @@ +import Foundation +import Security + +/// Serializes credential-store test harnesses that mutate process-wide seams. +enum CredentialStoreTestIsolation { + static let lock = NSLock() +} + +/// Narrow CodeBurn-owned Keychain cache over exact service/account pairs. +/// +/// Production uses `LiveKeychainCredentialCache`. Tests inject +/// `InMemoryKeychainCredentialCache` so the suite never touches the login +/// Keychain. Errors carry only operation, service, and OSStatus — never blob data. +protocol KeychainCredentialCaching: Sendable { + func read(service: String, account: String) throws -> Data? + func upsert(service: String, account: String, data: Data) throws + func delete(service: String, account: String) throws +} + +enum KeychainCredentialCacheError: Error, LocalizedError, Equatable { + case readFailed(service: String, status: OSStatus) + case writeFailed(service: String, status: OSStatus) + case deleteFailed(service: String, status: OSStatus) + + var errorDescription: String? { + switch self { + case let .readFailed(service, status): + return "Keychain read failed for \(service) (status \(status))." + case let .writeFailed(service, status): + return "Keychain write failed for \(service) (status \(status))." + case let .deleteFailed(service, status): + return "Keychain delete failed for \(service) (status \(status))." + } + } +} + +/// Published CodeBurn Keychain identities. Keep these exact — Electron contracts +/// on the Codex pair, and historical items use the same names. +enum CodeBurnKeychainIdentity { + static let claudeService = "org.agentseal.codeburn.menubar.claude.oauth.v1" + static let codexService = "org.agentseal.codeburn.menubar.codex.oauth.v1" + static let account = "default" +} + +struct LiveKeychainCredentialCache: KeychainCredentialCaching { + func read(service: String, account: String) throws -> Data? { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service, + kSecAttrAccount as String: account, + 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 KeychainCredentialCacheError.readFailed(service: service, status: status) + } + return data + } + + func upsert(service: String, account: String, data: Data) throws { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service, + kSecAttrAccount as String: account, + ] + let attributes: [String: Any] = [ + kSecValueData as String: data, + ] + let updateStatus = SecItemUpdate(query as CFDictionary, attributes as CFDictionary) + if updateStatus == errSecSuccess { return } + if updateStatus == errSecItemNotFound { + var add = query + add[kSecValueData as String] = data + let addStatus = SecItemAdd(add as CFDictionary, nil) + if addStatus == errSecSuccess { return } + if addStatus == errSecDuplicateItem { + let retry = SecItemUpdate(query as CFDictionary, attributes as CFDictionary) + guard retry == errSecSuccess else { + throw KeychainCredentialCacheError.writeFailed(service: service, status: retry) + } + return + } + throw KeychainCredentialCacheError.writeFailed(service: service, status: addStatus) + } + throw KeychainCredentialCacheError.writeFailed(service: service, status: updateStatus) + } + + func delete(service: String, account: String) throws { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service, + kSecAttrAccount as String: account, + ] + let status = SecItemDelete(query as CFDictionary) + if status == errSecSuccess || status == errSecItemNotFound { return } + throw KeychainCredentialCacheError.deleteFailed(service: service, status: status) + } +} + +/// Process-local fake for tests. Never writes to the system Keychain. +final class InMemoryKeychainCredentialCache: KeychainCredentialCaching, @unchecked Sendable { + private let lock = NSLock() + private var items: [String: Data] = [:] + private(set) var upsertCount = 0 + private(set) var readCount = 0 + private(set) var deleteCount = 0 + + private func key(_ service: String, _ account: String) -> String { + "\(service)\u{1f}\(account)" + } + + func read(service: String, account: String) throws -> Data? { + lock.lock(); defer { lock.unlock() } + readCount += 1 + return items[key(service, account)] + } + + func upsert(service: String, account: String, data: Data) throws { + lock.lock(); defer { lock.unlock() } + upsertCount += 1 + items[key(service, account)] = data + } + + func delete(service: String, account: String) throws { + lock.lock(); defer { lock.unlock() } + deleteCount += 1 + items.removeValue(forKey: key(service, account)) + } + + func storedJSONObject(service: String, account: String) -> [String: Any]? { + lock.lock(); defer { lock.unlock() } + guard let data = items[key(service, account)] else { return nil } + return (try? JSONSerialization.jsonObject(with: data)) as? [String: Any] + } + + func storedKeys(service: String, account: String) -> [String]? { + storedJSONObject(service: service, account: account).map { Array($0.keys).sorted() } + } + + /// Snapshot for simulated process restart: keep Keychain bytes, drop nothing else. + func cloneStorage() -> InMemoryKeychainCredentialCache { + lock.lock(); defer { lock.unlock() } + let copy = InMemoryKeychainCredentialCache() + copy.items = items + return copy + } +} + +/// Test double that wraps another backend and can force upsert/read/delete failures. +final class ControllableKeychainCredentialCache: KeychainCredentialCaching, @unchecked Sendable { + private let inner: any KeychainCredentialCaching + var failUpsert = false + var failRead = false + var failDelete = false + var upsertStatus: OSStatus = -1 + var readStatus: OSStatus = -1 + var deleteStatus: OSStatus = -1 + + init(inner: any KeychainCredentialCaching) { + self.inner = inner + } + + func read(service: String, account: String) throws -> Data? { + if failRead { + throw KeychainCredentialCacheError.readFailed(service: service, status: readStatus) + } + return try inner.read(service: service, account: account) + } + + func upsert(service: String, account: String, data: Data) throws { + if failUpsert { + throw KeychainCredentialCacheError.writeFailed(service: service, status: upsertStatus) + } + try inner.upsert(service: service, account: account, data: data) + } + + func delete(service: String, account: String) throws { + if failDelete { + throw KeychainCredentialCacheError.deleteFailed(service: service, status: deleteStatus) + } + try inner.delete(service: service, account: account) + } +} diff --git a/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift b/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift index 3b3dea29..080682d0 100644 --- a/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift +++ b/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift @@ -101,6 +101,72 @@ enum SafeFile { return data } + enum SecureReadError: Swift.Error, Equatable { + case notRegularFile(String) + case wrongOwner(String) + case chmodFailed(String, Int32) + case modeVerifyFailed(String, mode_t) + } + + /// Legacy credential migration path: open with `O_NOFOLLOW`, refuse non-regular / + /// non-owned files, `fchmod(0600)` and verify mode, then read bounded bytes from + /// the same descriptor. Permissions are repaired before any secret byte is read. + static func readAfterSecuringPermissions( + from path: String, + maxBytes: Int = defaultReadLimit, + expectedOwner: uid_t = geteuid() + ) throws -> Data { + var linkInfo = stat() + guard lstat(path, &linkInfo) == 0 else { + throw Error.readFailed(path, errno) + } + if (linkInfo.st_mode & S_IFMT) == S_IFLNK { + throw Error.symlinkDetected(path) + } + guard (linkInfo.st_mode & S_IFMT) == S_IFREG else { + throw SecureReadError.notRegularFile(path) + } + guard linkInfo.st_uid == expectedOwner else { + throw SecureReadError.wrongOwner(path) + } + + let fd = Darwin.open(path, O_RDONLY | O_NOFOLLOW) + guard fd >= 0 else { + throw Error.readFailed(path, errno) + } + defer { Darwin.close(fd) } + + if fchmod(fd, 0o600) != 0 { + throw SecureReadError.chmodFailed(path, errno) + } + var verified = stat() + guard fstat(fd, &verified) == 0 else { + throw Error.readFailed(path, errno) + } + let mode = verified.st_mode & 0o777 + guard mode == 0o600 else { + throw SecureReadError.modeVerifyFailed(path, mode) + } + + let size = Int(verified.st_size) + if size > maxBytes { + throw Error.sizeLimitExceeded(path, size) + } + + var data = Data(count: max(size, 0)) + let readBytes: Int = data.withUnsafeMutableBytes { buffer -> Int in + guard let base = buffer.baseAddress else { return 0 } + return Darwin.read(fd, base, buffer.count) + } + guard readBytes >= 0 else { + throw Error.readFailed(path, errno) + } + if readBytes < data.count { + data = data.prefix(readBytes) + } + return data + } + /// Runs `body` while holding an exclusive POSIX advisory lock on `path`. The lock file is /// created if missing (with 0o600 permissions) and released on scope exit, so other /// codeburn processes (the CLI running in a terminal, say) block on the same file instead diff --git a/mac/Sources/CodeBurnMenubar/Views/SettingsView.swift b/mac/Sources/CodeBurnMenubar/Views/SettingsView.swift index e8c1f218..3ee25a50 100644 --- a/mac/Sources/CodeBurnMenubar/Views/SettingsView.swift +++ b/mac/Sources/CodeBurnMenubar/Views/SettingsView.swift @@ -508,7 +508,7 @@ private struct CodexSettingsTab: View { CodexConnectionRow() } Section { - Text("Codex live-quota tracking reads `~/.codex/auth.json` once on Connect, then keeps a local copy under Application Support so subsequent quota fetches don't re-read the original. Only ChatGPT-mode auth (Plus / Pro / Team / Business / Edu / Enterprise) is supported. API-key users are billed per request and have a different reporting surface. Credit-metered workspaces report no rate-limit windows, so their monthly credit allowance is shown instead.") + Text("Codex live-quota tracking reads `~/.codex/auth.json` once on Connect, then keeps a CodeBurn-owned copy in the macOS Keychain so subsequent quota fetches don't re-read the original. Only ChatGPT-mode auth (Plus / Pro / Team / Business / Edu / Enterprise) is supported. API-key users are billed per request and have a different reporting surface. Credit-metered workspaces report no rate-limit windows, so their monthly credit allowance is shown instead.") .font(.system(size: 11)) .foregroundStyle(.secondary) } header: { diff --git a/mac/Tests/CodeBurnMenubarTests/CredentialKeychainCacheRedTests.swift b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainCacheRedTests.swift new file mode 100644 index 00000000..51646d9b --- /dev/null +++ b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainCacheRedTests.swift @@ -0,0 +1,149 @@ +import Foundation +import Testing +@testable import CodeBurnMenubar + +/// Red receipt for 0B: current plaintext writers must fail these assertions. +/// Disposable sentinels only — never log or expect raw secret values in receipts. +@Suite("Credential Keychain cache red", .serialized) +struct CredentialKeychainCacheRedTests { + private let accessSentinel = "cb-red-access-sentinel" + private let refreshSentinel = "cb-red-refresh-sentinel" + private let idSentinel = "cb-red-id-sentinel" + private let accountSentinel = "cb-red-account-sentinel" + + private func withIsolatedSeams( + _ body: (URL, InMemoryKeychainCredentialCache) throws -> Void + ) throws { + CredentialStoreTestIsolation.lock.lock() + defer { CredentialStoreTestIsolation.lock.unlock() } + + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("codeburn-0b-red-\(UUID().uuidString)", isDirectory: true) + let support = root.appendingPathComponent("Application Support", isDirectory: true) + try FileManager.default.createDirectory(at: support, withIntermediateDirectories: true) + let suiteName = "codeburn.0b.red.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defaults.removePersistentDomain(forName: suiteName) + + let fakeKeychain = InMemoryKeychainCredentialCache() + ClaudeCredentialStore.resetTestSeams() + CodexCredentialStore.resetTestSeams() + ClaudeCredentialStore.applicationSupportDirectoryOverride = support + CodexCredentialStore.applicationSupportDirectoryOverride = support + ClaudeCredentialStore.userDefaultsOverride = defaults + CodexCredentialStore.userDefaultsOverride = defaults + ClaudeCredentialStore.keychainCache = fakeKeychain + CodexCredentialStore.keychainCache = fakeKeychain + + defer { + ClaudeCredentialStore.resetTestSeams() + CodexCredentialStore.resetTestSeams() + defaults.removePersistentDomain(forName: suiteName) + try? FileManager.default.removeItem(at: root) + } + + try body(support, fakeKeychain) + } + + private func posixMode(at url: URL) -> mode_t? { + var info = stat() + guard lstat(url.path, &info) == 0 else { return nil } + return info.st_mode & 0o777 + } + + private func jsonKeys(at url: URL) throws -> [String] { + let data = try Data(contentsOf: url) + let object = try #require(JSONSerialization.jsonObject(with: data) as? [String: Any]) + return object.keys.sorted() + } + + @Test("Claude writeOurCache leaves no JSON and stores Keychain payload without refreshToken") + func claudeWriteUsesKeychainWithoutRefreshToken() throws { + try withIsolatedSeams { support, fakeKeychain in + let record = ClaudeCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + ) + + try ClaudeCredentialStore.writeOurCache(record: record) + + let legacyURL = ClaudeCredentialStore.cacheFileURL() + let legacyExists = FileManager.default.fileExists(atPath: legacyURL.path) + let mode = legacyExists ? posixMode(at: legacyURL) : nil + let fileKeys = legacyExists ? try jsonKeys(at: legacyURL) : [] + let keychainKeys = fakeKeychain.storedKeys( + service: ClaudeCredentialStore.ourKeychainService, + account: ClaudeCredentialStore.ourKeychainAccount + ) + let keychainObject = fakeKeychain.storedJSONObject( + service: ClaudeCredentialStore.ourKeychainService, + account: ClaudeCredentialStore.ourKeychainAccount + ) + let hasRefreshKey = keychainObject?.keys.contains("refreshToken") == true + let refreshValueMatches = (keychainObject?["refreshToken"] as? String) == refreshSentinel + + // Intended green behavior (must fail against current plaintext writer): + #expect(!legacyExists, "legacy Claude JSON must not be created under Application Support") + #expect(fakeKeychain.upsertCount >= 1, "fake Keychain must receive a Claude upsert") + #expect(keychainKeys != nil, "Claude Keychain payload must exist") + #expect(!(keychainKeys?.contains("refreshToken") ?? false), "Claude Keychain keys must omit refreshToken") + #expect(!hasRefreshKey && !refreshValueMatches, "Claude Keychain must not persist refreshToken") + + // Red diagnostic (keys + mode only; never fixture values): + if legacyExists { + Issue.record( + Comment(rawValue: "RED Claude legacy present mode=\(String(mode.map { String($0, radix: 8) } ?? "nil")) keys=\(fileKeys.joined(separator: ","))") + ) + } + if fakeKeychain.upsertCount == 0 { + Issue.record(Comment(rawValue: "RED Claude Keychain upsertCount=0")) + } + _ = support + } + } + + @Test("Codex writeOurCache leaves no JSON and stores Keychain rotation fields") + func codexWriteUsesKeychainWithRotationFields() throws { + try withIsolatedSeams { support, fakeKeychain in + let record = CodexCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + idToken: idSentinel, + accountId: accountSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_100), + lastRefresh: Date(timeIntervalSince1970: 1_700_000_000) + ) + + try CodexCredentialStore.writeOurCache(record: record) + + let legacyURL = CodexCredentialStore.cacheFileURL() + let legacyExists = FileManager.default.fileExists(atPath: legacyURL.path) + let mode = legacyExists ? posixMode(at: legacyURL) : nil + let fileKeys = legacyExists ? try jsonKeys(at: legacyURL) : [] + let keychainKeys = fakeKeychain.storedKeys( + service: CodexCredentialStore.ourKeychainService, + account: CodexCredentialStore.ourKeychainAccount + ) + + let required = ["accessToken", "refreshToken", "idToken", "accountId", "lastRefresh"] + let missingRequired = required.filter { !(keychainKeys?.contains($0) ?? false) } + + #expect(!legacyExists, "legacy Codex JSON must not be created under Application Support") + #expect(fakeKeychain.upsertCount >= 1, "fake Keychain must receive a Codex upsert") + #expect(keychainKeys != nil, "Codex Keychain payload must exist") + #expect(missingRequired.isEmpty, "Codex Keychain must retain rotation fields") + + if legacyExists { + Issue.record( + Comment(rawValue: "RED Codex legacy present mode=\(String(mode.map { String($0, radix: 8) } ?? "nil")) keys=\(fileKeys.joined(separator: ","))") + ) + } + if fakeKeychain.upsertCount == 0 { + Issue.record(Comment(rawValue: "RED Codex Keychain upsertCount=0")) + } + _ = support + } + } +} diff --git a/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift new file mode 100644 index 00000000..00ffc36f --- /dev/null +++ b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift @@ -0,0 +1,341 @@ +import Foundation +import Testing +@testable import CodeBurnMenubar + +/// Implementation-continuity tests for option-3 evidence bar. +/// Uses only InMemory/Controllable Keychain backends and temp Application Support — +/// never the operator login Keychain or live credential files. +@Suite("Credential Keychain implementation continuity", .serialized) +struct CredentialKeychainContinuityTests { + private let accessSentinel = "cb-cont-access-sentinel" + private let refreshSentinel = "cb-cont-refresh-sentinel" + private let idSentinel = "cb-cont-id-sentinel" + private let accountSentinel = "cb-cont-account-sentinel" + + private struct Harness { + let root: URL + let support: URL + let defaults: UserDefaults + let suiteName: String + let fakeKeychain: InMemoryKeychainCredentialCache + } + + private func withHarness( + _ body: (Harness) throws -> Void + ) throws { + CredentialStoreTestIsolation.lock.lock() + defer { CredentialStoreTestIsolation.lock.unlock() } + + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("codeburn-0b-cont-\(UUID().uuidString)", isDirectory: true) + let support = root.appendingPathComponent("Application Support", isDirectory: true) + try FileManager.default.createDirectory(at: support, withIntermediateDirectories: true) + let suiteName = "codeburn.0b.cont.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defaults.removePersistentDomain(forName: suiteName) + let fake = InMemoryKeychainCredentialCache() + + ClaudeCredentialStore.resetTestSeams() + CodexCredentialStore.resetTestSeams() + ClaudeCredentialStore.applicationSupportDirectoryOverride = support + CodexCredentialStore.applicationSupportDirectoryOverride = support + ClaudeCredentialStore.homeDirectoryOverride = root + CodexCredentialStore.homeDirectoryOverride = root + ClaudeCredentialStore.userDefaultsOverride = defaults + CodexCredentialStore.userDefaultsOverride = defaults + ClaudeCredentialStore.keychainCache = fake + CodexCredentialStore.keychainCache = fake + + defer { + ClaudeCredentialStore.resetTestSeams() + CodexCredentialStore.resetTestSeams() + defaults.removePersistentDomain(forName: suiteName) + try? FileManager.default.removeItem(at: root) + } + + try body(Harness(root: root, support: support, defaults: defaults, suiteName: suiteName, fakeKeychain: fake)) + } + + private func posixMode(at url: URL) -> mode_t? { + var info = stat() + guard lstat(url.path, &info) == 0 else { return nil } + return info.st_mode & 0o777 + } + + private func writeLegacyClaude0644(record: ClaudeCredentialStore.CredentialRecord) throws { + let url = ClaudeCredentialStore.cacheFileURL() + try FileManager.default.createDirectory(at: url.deletingLastPathComponent(), withIntermediateDirectories: true) + let data = try JSONEncoder().encode(record) + try data.write(to: url) + try FileManager.default.setAttributes([.posixPermissions: 0o644], ofItemAtPath: url.path) + } + + private func writeLegacyCodex0644(record: CodexCredentialStore.CredentialRecord) throws { + let url = CodexCredentialStore.cacheFileURL() + try FileManager.default.createDirectory(at: url.deletingLastPathComponent(), withIntermediateDirectories: true) + let data = try JSONEncoder().encode(record) + try data.write(to: url) + try FileManager.default.setAttributes([.posixPermissions: 0o644], ofItemAtPath: url.path) + } + + // MARK: - Continuity lifecycle + + @Test("Claude write → simulated restart → read → update → delete") + func claudeWriteRestartReadUpdateDelete() throws { + try withHarness { harness in + let initial = ClaudeCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + ) + try ClaudeCredentialStore.writeOurCache(record: initial) + ClaudeCredentialStore.isBootstrapCompleted = true + + #expect(!FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) + let keys = harness.fakeKeychain.storedKeys( + service: ClaudeCredentialStore.ourKeychainService, + account: ClaudeCredentialStore.ourKeychainAccount + ) + #expect(keys?.contains("refreshToken") != true) + #expect(keys?.contains("accessToken") == true) + + // Simulated process restart: drop memory, keep Keychain bytes. + ClaudeCredentialStore.clearMemoryCacheForTesting() + let afterRestart = try #require(try ClaudeCredentialStore.currentRecord()) + #expect(afterRestart.accessToken == accessSentinel) + #expect(afterRestart.refreshToken == nil) + + let updated = ClaudeCredentialStore.CredentialRecord( + accessToken: accessSentinel + "-rotated", + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_100), + rateLimitTier: "default" + ) + try ClaudeCredentialStore.writeOurCache(record: updated) + ClaudeCredentialStore.clearMemoryCacheForTesting() + let afterUpdate = try #require(try ClaudeCredentialStore.currentRecord()) + #expect(afterUpdate.accessToken == accessSentinel + "-rotated") + + let deleteResult = ClaudeCredentialStore.resetBootstrap() + #expect(deleteResult.isSuccess) + let afterDelete = try harness.fakeKeychain.read( + service: ClaudeCredentialStore.ourKeychainService, + account: ClaudeCredentialStore.ourKeychainAccount + ) + #expect(afterDelete == nil) + let afterDisconnect = try ClaudeCredentialStore.currentRecord() + #expect(afterDisconnect == nil) + } + } + + @Test("Codex write → simulated restart → read → update → delete") + func codexWriteRestartReadUpdateDelete() throws { + try withHarness { harness in + let initial = CodexCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + idToken: idSentinel, + accountId: accountSentinel, + expiresAt: nil, + lastRefresh: Date(timeIntervalSince1970: 1_700_000_000) + ) + try CodexCredentialStore.writeOurCache(record: initial) + CodexCredentialStore.isBootstrapCompleted = true + + #expect(!FileManager.default.fileExists(atPath: CodexCredentialStore.cacheFileURL().path)) + let keys = harness.fakeKeychain.storedKeys( + service: CodexCredentialStore.ourKeychainService, + account: CodexCredentialStore.ourKeychainAccount + ) + for required in ["accessToken", "refreshToken", "idToken", "accountId", "lastRefresh"] { + #expect(keys?.contains(required) == true) + } + + CodexCredentialStore.clearMemoryCacheForTesting() + // Without ~/.codex/auth.json, currentRecord falls through to Keychain cache. + let afterRestart = try #require(try CodexCredentialStore.currentRecord()) + #expect(afterRestart.accessToken == accessSentinel) + #expect(afterRestart.refreshToken == refreshSentinel) + + let updated = CodexCredentialStore.CredentialRecord( + accessToken: accessSentinel + "-rotated", + refreshToken: refreshSentinel + "-rotated", + idToken: idSentinel, + accountId: accountSentinel, + expiresAt: nil, + lastRefresh: Date(timeIntervalSince1970: 1_700_000_800) + ) + try CodexCredentialStore.writeOurCache(record: updated) + CodexCredentialStore.clearMemoryCacheForTesting() + let afterUpdate = try #require(try CodexCredentialStore.currentRecord()) + #expect(afterUpdate.accessToken == accessSentinel + "-rotated") + #expect(afterUpdate.refreshToken == refreshSentinel + "-rotated") + + let deleteResult = CodexCredentialStore.resetBootstrap() + #expect(deleteResult.isSuccess) + let afterDisconnect = try CodexCredentialStore.currentRecord() + #expect(afterDisconnect == nil) + } + } + + // MARK: - Migration + + @Test("successful Claude legacy 0644 migration unlinks JSON and omits refreshToken") + func claudeSuccessfulLegacyMigration() throws { + try withHarness { harness in + let legacy = ClaudeCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + ) + try writeLegacyClaude0644(record: legacy) + #expect(posixMode(at: ClaudeCredentialStore.cacheFileURL()) == 0o644) + + ClaudeCredentialStore.isBootstrapCompleted = true + let migrated = try #require(try ClaudeCredentialStore.currentRecord()) + #expect(migrated.accessToken == accessSentinel) + #expect(migrated.refreshToken == nil) + #expect(!FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) + #expect(harness.fakeKeychain.upsertCount >= 1) + let keys = harness.fakeKeychain.storedKeys( + service: ClaudeCredentialStore.ourKeychainService, + account: ClaudeCredentialStore.ourKeychainAccount + ) + #expect(keys?.contains("refreshToken") != true) + #expect(ClaudeCredentialStore.lastLegacyCleanupFailed == false) + } + } + + @Test("failed Claude Keychain upsert leaves secured legacy file") + func claudeFailedMigrationKeepsLegacy() throws { + try withHarness { harness in + let controllable = ControllableKeychainCredentialCache(inner: harness.fakeKeychain) + controllable.failUpsert = true + ClaudeCredentialStore.keychainCache = controllable + + let legacy = ClaudeCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + ) + try writeLegacyClaude0644(record: legacy) + ClaudeCredentialStore.isBootstrapCompleted = true + + let record = try #require(try ClaudeCredentialStore.currentRecord()) + #expect(record.accessToken == accessSentinel) + #expect(FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) + #expect(posixMode(at: ClaudeCredentialStore.cacheFileURL()) == 0o600) + #expect(harness.fakeKeychain.upsertCount == 0) + } + } + + @Test("successful Codex legacy migration unlinks JSON and keeps rotation fields") + func codexSuccessfulLegacyMigration() throws { + try withHarness { harness in + let legacy = CodexCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + idToken: idSentinel, + accountId: accountSentinel, + expiresAt: nil, + lastRefresh: Date(timeIntervalSince1970: 1_700_000_000) + ) + try writeLegacyCodex0644(record: legacy) + CodexCredentialStore.isBootstrapCompleted = true + + let migrated = try #require(try CodexCredentialStore.currentRecord()) + #expect(migrated.refreshToken == refreshSentinel) + #expect(!FileManager.default.fileExists(atPath: CodexCredentialStore.cacheFileURL().path)) + let keys = harness.fakeKeychain.storedKeys( + service: CodexCredentialStore.ourKeychainService, + account: CodexCredentialStore.ourKeychainAccount + ) + #expect(keys?.contains("refreshToken") == true) + #expect(keys?.contains("lastRefresh") == true) + } + } + + @Test("symlink legacy Claude file is refused and left in place") + func claudeSymlinkLegacyRefused() throws { + try withHarness { _ in + let codeburnDir = ClaudeCredentialStore.cacheFileURL().deletingLastPathComponent() + try FileManager.default.createDirectory(at: codeburnDir, withIntermediateDirectories: true) + let target = codeburnDir.appendingPathComponent("not-a-cred.txt") + try Data("x".utf8).write(to: target) + let link = ClaudeCredentialStore.cacheFileURL() + try FileManager.default.createSymbolicLink(at: link, withDestinationURL: target) + + ClaudeCredentialStore.isBootstrapCompleted = true + let afterSymlink = try ClaudeCredentialStore.currentRecord() + #expect(afterSymlink == nil) + #expect(FileManager.default.fileExists(atPath: link.path)) + } + } + + // MARK: - Disconnect / reinstall + + @Test("disconnect not-found is success; delete failure is observable") + func disconnectIdempotentAndPartialFailure() throws { + try withHarness { harness in + let empty = ClaudeCredentialStore.resetBootstrap() + #expect(empty.isSuccess) + + try ClaudeCredentialStore.writeOurCache(record: .init( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: nil, + rateLimitTier: nil + )) + let controllable = ControllableKeychainCredentialCache(inner: harness.fakeKeychain) + controllable.failDelete = true + ClaudeCredentialStore.keychainCache = controllable + + let failed = ClaudeCredentialStore.resetBootstrap() + #expect(!failed.isSuccess) + #expect(failed.keychainDeletedOrAbsent == false) + #expect(ClaudeCredentialStore.lastCacheDeleteResult?.isSuccess == false) + } + } + + @Test("reinstall with empty Keychain and no legacy clears bootstrap on read") + func reinstallMissingCacheClearsBootstrap() throws { + try withHarness { _ in + ClaudeCredentialStore.isBootstrapCompleted = true + let missing = try ClaudeCredentialStore.currentRecord() + #expect(missing == nil) + #expect(ClaudeCredentialStore.isBootstrapCompleted == false) + } + } + + @Test("corrupt Keychain plus valid legacy repairs the CodeBurn item") + func corruptKeychainRepairedFromLegacy() throws { + try withHarness { harness in + try harness.fakeKeychain.upsert( + service: ClaudeCredentialStore.ourKeychainService, + account: ClaudeCredentialStore.ourKeychainAccount, + data: Data("%not-json%".utf8) + ) + let legacy = ClaudeCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + ) + try writeLegacyClaude0644(record: legacy) + ClaudeCredentialStore.isBootstrapCompleted = true + + let repaired = try #require(try ClaudeCredentialStore.currentRecord()) + #expect(repaired.accessToken == accessSentinel) + #expect(repaired.refreshToken == nil) + #expect(!FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) + let object = harness.fakeKeychain.storedJSONObject( + service: ClaudeCredentialStore.ourKeychainService, + account: ClaudeCredentialStore.ourKeychainAccount + ) + #expect(object?.keys.contains("refreshToken") != true) + } + } +} From 252ea92d3b8a8fc3057fe835e74ff72d4f445fd3 Mon Sep 17 00:00:00 2001 From: Aditya Vikram Singh <247195684+avs-io@users.noreply.github.com> Date: Wed, 19 Aug 2026 20:02:44 +0530 Subject: [PATCH 03/15] fix(menubar): keep Keychain failure paths from leaving plaintext A valid Keychain item plus a leftover JSON used to skip chmod, so a failed unlink could leave 0644 secrets on disk. Failed Disconnect also cleared bootstrap and hid the retry. Repair leftover files to 0600, keep bootstrap when Keychain delete fails, revalidate the opened fd, and loop the secure read. --- mac/Sources/CodeBurnMenubar/AppStore.swift | 12 +++++++ .../Data/ClaudeCredentialStore.swift | 18 ++++++++-- .../Data/CodexCredentialStore.swift | 13 +++++-- .../CodeBurnMenubar/Security/SafeFile.swift | 35 ++++++++++++++----- .../CredentialKeychainContinuityTests.swift | 25 +++++++++++++ 5 files changed, 89 insertions(+), 14 deletions(-) diff --git a/mac/Sources/CodeBurnMenubar/AppStore.swift b/mac/Sources/CodeBurnMenubar/AppStore.swift index 33ac8d12..98bd91f8 100644 --- a/mac/Sources/CodeBurnMenubar/AppStore.swift +++ b/mac/Sources/CodeBurnMenubar/AppStore.swift @@ -1069,6 +1069,13 @@ final class AppStore { // resumes after this point detects the disconnect and discards its // result instead of re-populating the cleared state. claudeRefreshGen &+= 1 + if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.keychainDeletedOrAbsent { + // Keychain item still present — keep Connect/Disconnect on the + // connected path so the user can retry delete. + subscriptionError = "Could not fully remove the local Claude credential cache. Disconnect again to retry." + NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil) + return + } subscription = nil if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.isSuccess { subscriptionError = "Could not fully remove the local Claude credential cache." @@ -1137,6 +1144,11 @@ final class AppStore { func disconnectCodex() { CodexSubscriptionService.disconnect() codexRefreshGen &+= 1 + if let result = CodexCredentialStore.lastCacheDeleteResult, !result.keychainDeletedOrAbsent { + codexError = "Could not fully remove the local Codex credential cache. Disconnect again to retry." + NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil) + return + } codexUsage = nil if let result = CodexCredentialStore.lastCacheDeleteResult, !result.isSuccess { codexError = "Could not fully remove the local Codex credential cache." diff --git a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift index d9e691e6..438dc397 100644 --- a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift @@ -56,6 +56,7 @@ enum ClaudeCredentialStore { keychainCache = LiveKeychainCredentialCache() lastCacheDeleteResult = nil lastLegacyCleanupFailed = false + unlinkLegacyOverride = nil lock.withLock { memoryCache = nil } } @@ -134,7 +135,11 @@ enum ClaudeCredentialStore { lock.withLock { memoryCache = nil } let result = deleteOurCache() lastCacheDeleteResult = result - isBootstrapCompleted = false + // A failed Keychain delete must not pretend the provider is disconnected. + // Clearing the flag hides Disconnect and orphans the remaining item. + if result.keychainDeletedOrAbsent { + isBootstrapCompleted = false + } return result } @@ -150,6 +155,8 @@ enum ClaudeCredentialStore { /// Last legacy-unlink failure after a verified Keychain write (cleanup retry signal). nonisolated(unsafe) static var lastLegacyCleanupFailed = false + /// Test seam: force legacy unlink to throw so we can assert the 0600 repair. + nonisolated(unsafe) static var unlinkLegacyOverride: ((URL) throws -> Void)? // MARK: - Public API @@ -490,11 +497,16 @@ enum ClaudeCredentialStore { return } do { - try FileManager.default.removeItem(at: url) + if let unlinkLegacyOverride { + try unlinkLegacyOverride(url) + } else { + try FileManager.default.removeItem(at: url) + } lastLegacyCleanupFailed = false } catch { - // Retain valid Keychain item + 0600 file; surface for later retry. lastLegacyCleanupFailed = true + // Verified Keychain item exists — leftover plaintext must not stay 0644. + try? FileManager.default.setAttributes([.posixPermissions: 0o600], ofItemAtPath: url.path) } } diff --git a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift index 96bf71fd..8ab257e3 100644 --- a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift @@ -43,6 +43,7 @@ enum CodexCredentialStore { keychainCache = LiveKeychainCredentialCache() lastCacheDeleteResult = nil lastLegacyCleanupFailed = false + unlinkLegacyOverride = nil lock.withLock { memoryCache = nil } } @@ -133,7 +134,9 @@ enum CodexCredentialStore { lock.withLock { memoryCache = nil } let result = deleteOurCache() lastCacheDeleteResult = result - isBootstrapCompleted = false + if result.keychainDeletedOrAbsent { + isBootstrapCompleted = false + } return result } @@ -145,6 +148,7 @@ enum CodexCredentialStore { nonisolated(unsafe) static var lastCacheDeleteResult: CacheDeleteResult? nonisolated(unsafe) static var lastLegacyCleanupFailed = false + nonisolated(unsafe) static var unlinkLegacyOverride: ((URL) throws -> Void)? // MARK: - Public API @@ -368,10 +372,15 @@ enum CodexCredentialStore { return } do { - try FileManager.default.removeItem(at: url) + if let unlinkLegacyOverride { + try unlinkLegacyOverride(url) + } else { + try FileManager.default.removeItem(at: url) + } lastLegacyCleanupFailed = false } catch { lastLegacyCleanupFailed = true + try? FileManager.default.setAttributes([.posixPermissions: 0o600], ofItemAtPath: url.path) } } diff --git a/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift b/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift index 080682d0..6a300a7f 100644 --- a/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift +++ b/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift @@ -136,6 +136,17 @@ enum SafeFile { } defer { Darwin.close(fd) } + var opened = stat() + guard fstat(fd, &opened) == 0 else { + throw Error.readFailed(path, errno) + } + guard (opened.st_mode & S_IFMT) == S_IFREG else { + throw SecureReadError.notRegularFile(path) + } + guard opened.st_uid == expectedOwner else { + throw SecureReadError.wrongOwner(path) + } + if fchmod(fd, 0o600) != 0 { throw SecureReadError.chmodFailed(path, errno) } @@ -153,16 +164,22 @@ enum SafeFile { throw Error.sizeLimitExceeded(path, size) } - var data = Data(count: max(size, 0)) - let readBytes: Int = data.withUnsafeMutableBytes { buffer -> Int in - guard let base = buffer.baseAddress else { return 0 } - return Darwin.read(fd, base, buffer.count) + var data = Data() + data.reserveCapacity(max(size, 0)) + var chunk = [UInt8](repeating: 0, count: 4096) + while data.count < maxBytes { + let n = chunk.withUnsafeMutableBytes { buffer -> Int in + guard let base = buffer.baseAddress else { return 0 } + return Darwin.read(fd, base, buffer.count) + } + guard n >= 0 else { + throw Error.readFailed(path, errno) + } + if n == 0 { break } + data.append(contentsOf: chunk.prefix(n)) } - guard readBytes >= 0 else { - throw Error.readFailed(path, errno) - } - if readBytes < data.count { - data = data.prefix(readBytes) + if data.count > maxBytes { + throw Error.sizeLimitExceeded(path, data.count) } return data } diff --git a/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift index 00ffc36f..1b75a9e1 100644 --- a/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift +++ b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift @@ -289,6 +289,7 @@ struct CredentialKeychainContinuityTests { expiresAt: nil, rateLimitTier: nil )) + ClaudeCredentialStore.isBootstrapCompleted = true let controllable = ControllableKeychainCredentialCache(inner: harness.fakeKeychain) controllable.failDelete = true ClaudeCredentialStore.keychainCache = controllable @@ -297,6 +298,30 @@ struct CredentialKeychainContinuityTests { #expect(!failed.isSuccess) #expect(failed.keychainDeletedOrAbsent == false) #expect(ClaudeCredentialStore.lastCacheDeleteResult?.isSuccess == false) + #expect(ClaudeCredentialStore.isBootstrapCompleted == true) + } + } + + @Test("failed unlink after verified Keychain repairs leftover JSON to 0600") + func failedUnlinkRepairsLegacyMode() throws { + try withHarness { _ in + let record = ClaudeCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + ) + try ClaudeCredentialStore.writeOurCache(record: record) + try writeLegacyClaude0644(record: record) + ClaudeCredentialStore.isBootstrapCompleted = true + ClaudeCredentialStore.unlinkLegacyOverride = { _ in + throw POSIXError(.EPERM) + } + ClaudeCredentialStore.clearMemoryCacheForTesting() + _ = try ClaudeCredentialStore.currentRecord() + #expect(ClaudeCredentialStore.lastLegacyCleanupFailed == true) + #expect(FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) + #expect(posixMode(at: ClaudeCredentialStore.cacheFileURL()) == 0o600) } } From d3f86f5d16250d42c8c800bcb3393e334930da53 Mon Sep 17 00:00:00 2001 From: Aditya Vikram Singh <247195684+avs-io@users.noreply.github.com> Date: Wed, 19 Aug 2026 20:44:18 +0530 Subject: [PATCH 04/15] fix(models): alias bare MiMo 2.5 ids to the LiteLLM Xiaomi rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hermes and token-plan sessions store mimo-v2.5-pro. The snapshot row is xiaomi/mimo-v2.5-pro. Same class as the existing mimo-v2-flash alias. No invented rate. Looking up the display name on the stripped leaf before following a pricing alias, so cline-pass/mimo-v2.5-pro cannot recurse strip → alias → last-segment forever. --- src/models.ts | 16 ++++++++++++---- tests/models.test.ts | 2 ++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/models.ts b/src/models.ts index c0e6746f..1b910469 100644 --- a/src/models.ts +++ b/src/models.ts @@ -297,6 +297,10 @@ const BUILTIN_ALIASES: Record = { 'k3-agent': 'kimi-k3', 'k2d6-agent': 'kimi-k2p6', 'mimo-v2-flash': 'xiaomi/mimo-v2-flash', + // Hermes / Xiaomi token-plan sessions store the bare id. LiteLLM's row is + // namespaced. Same class as mimo-v2-flash above — do not invent a rate. + 'mimo-v2.5-pro': 'xiaomi/mimo-v2.5-pro', + 'mimo-v2.5': 'xiaomi/mimo-v2.5', 'kat-coder-pro-v1': 'kwaipilot/kat-coder-pro', // Cursor emits dot-version tier-last names plus tier/reasoning suffixes // that LiteLLM does not index (`-high`, `-low`, `-medium`, `-thinking`, @@ -983,13 +987,17 @@ function deriveClaudeShortName(canonical: string): string | undefined { export function getShortModelName(model: string): string { if (autoModelNames[model]) return autoModelNames[model] - const canonical = resolveAlias(getCanonicalName(model)) + const stripped = getCanonicalName(model) + // Pricing aliases may re-namespace a leaf (mimo-v2.5-pro → xiaomi/…). + // Display names live on the leaf. Look that up before following the alias + // or we recurse forever: strip → alias → last-segment → strip. + for (const [key, name] of SORTED_SHORT_NAMES) { + if (stripped === key || stripped.startsWith(key + '-')) return name + } + const canonical = resolveAlias(stripped) const claude = deriveClaudeShortName(canonical) if (claude) return claude for (const [key, name] of SORTED_SHORT_NAMES) { - // Match on a version boundary, not a bare prefix: an unlisted future minor - // (e.g. gpt-5.6) must NOT collapse into the base "gpt-5" entry — it should - // fall through to its raw id rather than show a wrong name/tier. if (canonical === key || canonical.startsWith(key + '-')) return name } // getCanonicalName only strips the leading provider prefix, so a raw diff --git a/tests/models.test.ts b/tests/models.test.ts index 3e2b1655..080dec4a 100644 --- a/tests/models.test.ts +++ b/tests/models.test.ts @@ -737,6 +737,8 @@ describe('provider pricing suffix variants', () => { describe('observed provider model aliases', () => { const cases: Array<[string, string]> = [ ['MiMo-V2-Flash', 'xiaomi/mimo-v2-flash'], + ['mimo-v2.5-pro', 'xiaomi/mimo-v2.5-pro'], + ['MiMo-v2.5-Pro', 'xiaomi/mimo-v2.5-pro'], ['KAT-Coder-Pro-V1', 'kwaipilot/kat-coder-pro'], // Kimi Code wires report bare `k3` in llm.request.model; it must price // through the kimi-k3 table entry, not fall through to $0. From 98d109d425f265b8f6f223d39a67b79a0be6302c Mon Sep 17 00:00:00 2001 From: Aditya Vikram Singh <247195684+avs-io@users.noreply.github.com> Date: Wed, 19 Aug 2026 20:56:07 +0530 Subject: [PATCH 05/15] fix(menubar): fail closed on leftover JSON and partial Disconnect Extra High MERGE AFTER FIX on 252ea92. Pathname chmod was unverified. Disconnect hid retry when only the legacy file survived. Secure read stopped at exactly maxBytes. Tighten leftovers via opened-fd fchmod+fstat. Keep bootstrap unless both Keychain and legacy deletes succeed. Read maxBytes+1 so growth past the limit is rejected. --- mac/Sources/CodeBurnMenubar/AppStore.swift | 8 ++-- .../Data/ClaudeCredentialStore.swift | 21 +++++++-- .../Data/CodexCredentialStore.swift | 20 ++++++-- .../CodeBurnMenubar/Security/SafeFile.swift | 41 ++++++++++++++++- .../CredentialKeychainContinuityTests.swift | 46 +++++++++++++++++++ 5 files changed, 123 insertions(+), 13 deletions(-) diff --git a/mac/Sources/CodeBurnMenubar/AppStore.swift b/mac/Sources/CodeBurnMenubar/AppStore.swift index 98bd91f8..2ca365f0 100644 --- a/mac/Sources/CodeBurnMenubar/AppStore.swift +++ b/mac/Sources/CodeBurnMenubar/AppStore.swift @@ -1069,9 +1069,9 @@ final class AppStore { // resumes after this point detects the disconnect and discards its // result instead of re-populating the cleared state. claudeRefreshGen &+= 1 - if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.keychainDeletedOrAbsent { - // Keychain item still present — keep Connect/Disconnect on the - // connected path so the user can retry delete. + if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.isSuccess { + // Any leftover Keychain item or plaintext must keep Disconnect + // so the user can retry delete. subscriptionError = "Could not fully remove the local Claude credential cache. Disconnect again to retry." NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil) return @@ -1144,7 +1144,7 @@ final class AppStore { func disconnectCodex() { CodexSubscriptionService.disconnect() codexRefreshGen &+= 1 - if let result = CodexCredentialStore.lastCacheDeleteResult, !result.keychainDeletedOrAbsent { + if let result = CodexCredentialStore.lastCacheDeleteResult, !result.isSuccess { codexError = "Could not fully remove the local Codex credential cache. Disconnect again to retry." NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil) return diff --git a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift index 438dc397..5df6bbc8 100644 --- a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift @@ -57,6 +57,7 @@ enum ClaudeCredentialStore { lastCacheDeleteResult = nil lastLegacyCleanupFailed = false unlinkLegacyOverride = nil + tightenLegacyOverride = nil lock.withLock { memoryCache = nil } } @@ -137,7 +138,7 @@ enum ClaudeCredentialStore { lastCacheDeleteResult = result // A failed Keychain delete must not pretend the provider is disconnected. // Clearing the flag hides Disconnect and orphans the remaining item. - if result.keychainDeletedOrAbsent { + if result.isSuccess { isBootstrapCompleted = false } return result @@ -157,6 +158,7 @@ enum ClaudeCredentialStore { nonisolated(unsafe) static var lastLegacyCleanupFailed = false /// Test seam: force legacy unlink to throw so we can assert the 0600 repair. nonisolated(unsafe) static var unlinkLegacyOverride: ((URL) throws -> Void)? + nonisolated(unsafe) static var tightenLegacyOverride: ((URL) throws -> Void)? // MARK: - Public API @@ -505,8 +507,15 @@ enum ClaudeCredentialStore { lastLegacyCleanupFailed = false } catch { lastLegacyCleanupFailed = true - // Verified Keychain item exists — leftover plaintext must not stay 0644. - try? FileManager.default.setAttributes([.posixPermissions: 0o600], ofItemAtPath: url.path) + do { + if let tightenLegacyOverride { + try tightenLegacyOverride(url) + } else { + try SafeFile.tightenToOwnerReadWrite(at: url.path) + } + } catch { + // Still leftover, and 0600 is unproven. Retry signal stays set. + } } } @@ -523,7 +532,11 @@ enum ClaudeCredentialStore { let url = cacheFileURL() if FileManager.default.fileExists(atPath: url.path) { do { - try FileManager.default.removeItem(at: url) + if let unlinkLegacyOverride { + try unlinkLegacyOverride(url) + } else { + try FileManager.default.removeItem(at: url) + } } catch { legacyOK = false } diff --git a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift index 8ab257e3..00385b05 100644 --- a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift @@ -44,6 +44,7 @@ enum CodexCredentialStore { lastCacheDeleteResult = nil lastLegacyCleanupFailed = false unlinkLegacyOverride = nil + tightenLegacyOverride = nil lock.withLock { memoryCache = nil } } @@ -134,7 +135,7 @@ enum CodexCredentialStore { lock.withLock { memoryCache = nil } let result = deleteOurCache() lastCacheDeleteResult = result - if result.keychainDeletedOrAbsent { + if result.isSuccess { isBootstrapCompleted = false } return result @@ -149,6 +150,7 @@ enum CodexCredentialStore { nonisolated(unsafe) static var lastCacheDeleteResult: CacheDeleteResult? nonisolated(unsafe) static var lastLegacyCleanupFailed = false nonisolated(unsafe) static var unlinkLegacyOverride: ((URL) throws -> Void)? + nonisolated(unsafe) static var tightenLegacyOverride: ((URL) throws -> Void)? // MARK: - Public API @@ -380,7 +382,15 @@ enum CodexCredentialStore { lastLegacyCleanupFailed = false } catch { lastLegacyCleanupFailed = true - try? FileManager.default.setAttributes([.posixPermissions: 0o600], ofItemAtPath: url.path) + do { + if let tightenLegacyOverride { + try tightenLegacyOverride(url) + } else { + try SafeFile.tightenToOwnerReadWrite(at: url.path) + } + } catch { + // Still leftover, and 0600 is unproven. Retry signal stays set. + } } } @@ -397,7 +407,11 @@ enum CodexCredentialStore { let url = cacheFileURL() if FileManager.default.fileExists(atPath: url.path) { do { - try FileManager.default.removeItem(at: url) + if let unlinkLegacyOverride { + try unlinkLegacyOverride(url) + } else { + try FileManager.default.removeItem(at: url) + } } catch { legacyOK = false } diff --git a/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift b/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift index 6a300a7f..9ff37fe4 100644 --- a/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift +++ b/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift @@ -21,6 +21,42 @@ enum SafeFile { /// from exhausting memory in the Swift process. static let defaultReadLimit = 8 * 1024 * 1024 + /// Open the existing regular file with O_NOFOLLOW, fchmod 0600, and + /// fstat-verify the mode. Used when leftover credential JSON cannot be + /// unlinked after a verified Keychain write. + static func tightenToOwnerReadWrite(at path: String) throws { + var linkInfo = stat() + guard lstat(path, &linkInfo) == 0 else { + throw Error.readFailed(path, errno) + } + if (linkInfo.st_mode & S_IFMT) == S_IFLNK { + throw Error.symlinkDetected(path) + } + let fd = Darwin.open(path, O_RDONLY | O_NOFOLLOW) + guard fd >= 0 else { + throw Error.readFailed(path, errno) + } + defer { Darwin.close(fd) } + var opened = stat() + guard fstat(fd, &opened) == 0 else { + throw Error.readFailed(path, errno) + } + guard (opened.st_mode & S_IFMT) == S_IFREG else { + throw SecureReadError.notRegularFile(path) + } + if fchmod(fd, 0o600) != 0 { + throw SecureReadError.chmodFailed(path, errno) + } + var verified = stat() + guard fstat(fd, &verified) == 0 else { + throw Error.readFailed(path, errno) + } + let mode = verified.st_mode & 0o777 + guard mode == 0o600 else { + throw SecureReadError.modeVerifyFailed(path, mode) + } + } + /// Refuses to follow symlinks and writes atomically via a tmp file + rename. `mode` is the /// final file permission (0o600 by default so cache files stay user-private). static func write(_ data: Data, to path: String, mode: mode_t = 0o600) throws { @@ -167,10 +203,11 @@ enum SafeFile { var data = Data() data.reserveCapacity(max(size, 0)) var chunk = [UInt8](repeating: 0, count: 4096) - while data.count < maxBytes { + let limit = maxBytes + 1 + while data.count < limit { let n = chunk.withUnsafeMutableBytes { buffer -> Int in guard let base = buffer.baseAddress else { return 0 } - return Darwin.read(fd, base, buffer.count) + return Darwin.read(fd, base, min(buffer.count, limit - data.count)) } guard n >= 0 else { throw Error.readFailed(path, errno) diff --git a/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift index 1b75a9e1..b4b8ebb6 100644 --- a/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift +++ b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift @@ -325,6 +325,52 @@ struct CredentialKeychainContinuityTests { } } + @Test("failed tighten after failed unlink keeps leftover and retry signal") + func failedTightenLeavesRetrySignal() throws { + try withHarness { _ in + let record = ClaudeCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + ) + try ClaudeCredentialStore.writeOurCache(record: record) + try writeLegacyClaude0644(record: record) + ClaudeCredentialStore.isBootstrapCompleted = true + ClaudeCredentialStore.unlinkLegacyOverride = { _ in throw POSIXError(.EPERM) } + ClaudeCredentialStore.tightenLegacyOverride = { _ in throw POSIXError(.EPERM) } + ClaudeCredentialStore.clearMemoryCacheForTesting() + _ = try ClaudeCredentialStore.currentRecord() + #expect(ClaudeCredentialStore.lastLegacyCleanupFailed == true) + #expect(FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) + } + } + + @Test("legacy-only disconnect failure keeps bootstrap so retry stays") + func legacyOnlyDisconnectKeepsBootstrap() throws { + try withHarness { _ in + try ClaudeCredentialStore.writeOurCache(record: .init( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: nil, + rateLimitTier: nil + )) + try writeLegacyClaude0644(record: .init( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: nil, + rateLimitTier: nil + )) + ClaudeCredentialStore.isBootstrapCompleted = true + ClaudeCredentialStore.unlinkLegacyOverride = { _ in throw POSIXError(.EPERM) } + let failed = ClaudeCredentialStore.resetBootstrap() + #expect(failed.keychainDeletedOrAbsent == true) + #expect(failed.legacyDeletedOrAbsent == false) + #expect(failed.isSuccess == false) + #expect(ClaudeCredentialStore.isBootstrapCompleted == true) + } + } + @Test("reinstall with empty Keychain and no legacy clears bootstrap on read") func reinstallMissingCacheClearsBootstrap() throws { try withHarness { _ in From d5ced78595d6196ef48d5af089c65f2f9ef8c30b Mon Sep 17 00:00:00 2001 From: Aditya Vikram Singh <247195684+avs-io@users.noreply.github.com> Date: Wed, 19 Aug 2026 21:10:33 +0530 Subject: [PATCH 06/15] fix(models): cycle-safe short names; keep user-alias display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extra High MERGE AFTER FIX on d3f86f5. mimo-v2.5 aliased to xiaomi/mimo-v2.5 then last-segment recursed forever. Looking up SHORT_NAMES before resolveAlias also froze user remaps of known ids (gpt-4o still displayed as GPT-4o). Follow user aliases first. Break strip→alias→leaf cycles. Do not invent a Kimi rate. Do not paper over this with a mimo-v2.5 SHORT_NAMES row. --- src/models.ts | 60 +++++++++++++++++++++++++++++--------------- tests/models.test.ts | 16 ++++++++++++ 2 files changed, 56 insertions(+), 20 deletions(-) diff --git a/src/models.ts b/src/models.ts index 1b910469..907c2327 100644 --- a/src/models.ts +++ b/src/models.ts @@ -985,32 +985,52 @@ function deriveClaudeShortName(canonical: string): string | undefined { return `${CLAUDE_FAMILY[family]} ${major}${minor ? `.${minor}` : ''}` } -export function getShortModelName(model: string): string { - if (autoModelNames[model]) return autoModelNames[model] - const stripped = getCanonicalName(model) - // Pricing aliases may re-namespace a leaf (mimo-v2.5-pro → xiaomi/…). - // Display names live on the leaf. Look that up before following the alias - // or we recurse forever: strip → alias → last-segment → strip. - for (const [key, name] of SORTED_SHORT_NAMES) { - if (stripped === key || stripped.startsWith(key + '-')) return name - } - const canonical = resolveAlias(stripped) - const claude = deriveClaudeShortName(canonical) +function lookupShortName(id: string): string | undefined { + const claude = deriveClaudeShortName(id) if (claude) return claude for (const [key, name] of SORTED_SHORT_NAMES) { - if (canonical === key || canonical.startsWith(key + '-')) return name + if (id === key || id.startsWith(key + '-')) return name } - // getCanonicalName only strips the leading provider prefix, so a raw - // path-style id (e.g. accounts/fireworks/models/glm-5p2) still has slashes - // here. Take the last path segment and re-resolve it: the segment may itself - // be a known model slug (Fireworks fleet ids), earning a friendly name; a - // genuinely unmapped slug resolves to itself, preserving the raw-segment - // fallback for everything else. + return undefined +} + +export function getShortModelName(model: string, seen: Set = new Set()): string { + if (autoModelNames[model]) return autoModelNames[model] + if (seen.has(model)) { + const leaf = model.includes('/') ? model.slice(model.lastIndexOf('/') + 1) : model + return lookupShortName(leaf) ?? leaf + } + seen.add(model) + + // User aliases win over built-in display names. A remap of gpt-4o must + // show the target, not "GPT-4o". + if (Object.hasOwn(userAliases, model)) { + return getShortModelName(userAliases[model]!, seen) + } + + const stripped = getCanonicalName(model) + if (stripped !== model) { + if (Object.hasOwn(userAliases, stripped)) { + return getShortModelName(userAliases[stripped]!, seen) + } + const knownStripped = lookupShortName(stripped) + if (knownStripped && !Object.hasOwn(BUILTIN_ALIASES, stripped) && !Object.hasOwn(BUILTIN_ALIASES, stripped.toLowerCase())) { + return knownStripped + } + } + + const canonical = resolveAlias(stripped) + const known = lookupShortName(canonical) + if (known) return known + if (canonical.includes('/')) { const segment = canonical.slice(canonical.lastIndexOf('/') + 1) - return segment ? getShortModelName(segment) : canonical + if (!segment || seen.has(segment) || segment === stripped) { + return lookupShortName(segment) ?? segment + } + return getShortModelName(segment, seen) } - return canonical + return lookupShortName(canonical) ?? canonical } // Pricing is process-global state assembled at CLI startup from the cached diff --git a/tests/models.test.ts b/tests/models.test.ts index 080dec4a..2c81f74f 100644 --- a/tests/models.test.ts +++ b/tests/models.test.ts @@ -310,6 +310,13 @@ describe('user aliases via setModelAliases', () => { expect(getModelCosts('anthropic--claude-4.6-opus')).toEqual(getModelCosts('claude-sonnet-4-5')) }) + it('user alias whose source already has a short name displays the target', () => { + setModelAliases({ 'gpt-4o': 'claude-opus-4-6' }) + expect(getModelCosts('gpt-4o')).toEqual(getModelCosts('claude-opus-4-6')) + expect(getShortModelName('gpt-4o')).toBe('Opus 4.6') + setModelAliases({}) + }) + it('resetting aliases restores builtins', () => { setModelAliases({ 'anthropic--claude-4.6-opus': 'claude-sonnet-4-5' }) setModelAliases({}) @@ -739,6 +746,8 @@ describe('observed provider model aliases', () => { ['MiMo-V2-Flash', 'xiaomi/mimo-v2-flash'], ['mimo-v2.5-pro', 'xiaomi/mimo-v2.5-pro'], ['MiMo-v2.5-Pro', 'xiaomi/mimo-v2.5-pro'], + ['mimo-v2.5', 'xiaomi/mimo-v2.5'], + ['MiMo-v2.5', 'xiaomi/mimo-v2.5'], ['KAT-Coder-Pro-V1', 'kwaipilot/kat-coder-pro'], // Kimi Code wires report bare `k3` in llm.request.model; it must price // through the kimi-k3 table entry, not fall through to $0. @@ -760,6 +769,13 @@ describe('observed provider model aliases', () => { expect(getShortModelName('k3')).toBe('Kimi K3') }) + it('does not recurse on vendor-requalified MiMo aliases', () => { + expect(getShortModelName('mimo-v2.5')).toBe('mimo-v2.5') + expect(getShortModelName('MiMo-v2.5')).toBe('mimo-v2.5') + expect(getShortModelName('cline-pass/mimo-v2.5')).toBe('mimo-v2.5') + expect(getShortModelName('cline-pass/mimo-v2.5-pro')).toBe('MiMo v2.5 Pro') + }) + it('does not map dated Qwen3 Max to a reseller price without provider context', () => { expect(getModelCosts('qwen3-max-2026-01-23')).toBeNull() expect(calculateCost('qwen3-max-2026-01-23', 1_000_000, 1_000_000, 0, 0, 0)).toBe(0) From 827a41241cbf77ec8501ae19f9e3ec6b8d33148f Mon Sep 17 00:00:00 2001 From: Aditya Vikram Singh <247195684+avs-io@users.noreply.github.com> Date: Wed, 19 Aug 2026 21:27:13 +0530 Subject: [PATCH 07/15] fix(models): keep getShortModelName unary for Array.map CI typecheck failed: sessions-report maps getShortModelName, and the Extra High cycle Set was a second parameter. Array.map fed the index as `seen`. Cycle tracking stays on an internal helper. Display and alias behavior unchanged. No second Extra High. --- src/models.ts | 13 +++++++++---- tests/models.test.ts | 8 ++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/models.ts b/src/models.ts index 907c2327..d429ab47 100644 --- a/src/models.ts +++ b/src/models.ts @@ -994,7 +994,12 @@ function lookupShortName(id: string): string | undefined { return undefined } -export function getShortModelName(model: string, seen: Set = new Set()): string { +// Public API stays unary so Array.map/forEach cannot feed index as cycle state. +export function getShortModelName(model: string): string { + return shortModelName(model, new Set()) +} + +function shortModelName(model: string, seen: Set): string { if (autoModelNames[model]) return autoModelNames[model] if (seen.has(model)) { const leaf = model.includes('/') ? model.slice(model.lastIndexOf('/') + 1) : model @@ -1005,13 +1010,13 @@ export function getShortModelName(model: string, seen: Set = new Set()): // User aliases win over built-in display names. A remap of gpt-4o must // show the target, not "GPT-4o". if (Object.hasOwn(userAliases, model)) { - return getShortModelName(userAliases[model]!, seen) + return shortModelName(userAliases[model]!, seen) } const stripped = getCanonicalName(model) if (stripped !== model) { if (Object.hasOwn(userAliases, stripped)) { - return getShortModelName(userAliases[stripped]!, seen) + return shortModelName(userAliases[stripped]!, seen) } const knownStripped = lookupShortName(stripped) if (knownStripped && !Object.hasOwn(BUILTIN_ALIASES, stripped) && !Object.hasOwn(BUILTIN_ALIASES, stripped.toLowerCase())) { @@ -1028,7 +1033,7 @@ export function getShortModelName(model: string, seen: Set = new Set()): if (!segment || seen.has(segment) || segment === stripped) { return lookupShortName(segment) ?? segment } - return getShortModelName(segment, seen) + return shortModelName(segment, seen) } return lookupShortName(canonical) ?? canonical } diff --git a/tests/models.test.ts b/tests/models.test.ts index 2c81f74f..44461252 100644 --- a/tests/models.test.ts +++ b/tests/models.test.ts @@ -776,6 +776,14 @@ describe('observed provider model aliases', () => { expect(getShortModelName('cline-pass/mimo-v2.5-pro')).toBe('MiMo v2.5 Pro') }) + it('stays unary so Array.map cannot feed the index as cycle state', () => { + expect(['mimo-v2.5', 'gpt-4o', 'cline-pass/mimo-v2.5-pro'].map(getShortModelName)).toEqual([ + 'mimo-v2.5', + 'GPT-4o', + 'MiMo v2.5 Pro', + ]) + }) + it('does not map dated Qwen3 Max to a reseller price without provider context', () => { expect(getModelCosts('qwen3-max-2026-01-23')).toBeNull() expect(calculateCost('qwen3-max-2026-01-23', 1_000_000, 1_000_000, 0, 0, 0)).toBe(0) From ef040a4c4d1a0fc11758a6b7a24a1726492bd7a6 Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Wed, 19 Aug 2026 11:26:22 -0700 Subject: [PATCH 08/15] test(models): pin the shipped MiMo v2 Flash crash; name the base 2.5 row The `mimo-v2-flash -> xiaomi/mimo-v2-flash` alias shipped before this branch and already cycled through display-name resolution, so getShortModelName threw RangeError on every real MiMo v2 Flash session. The new cycle-safe resolver fixes it, but nothing pinned the ids that actually crashed in production: cover the four spellings found in a real session cache, including the unnamespaced `mimo/mimo-v2-flash`. Add the base `mimo-v2.5` display name so the row reads next to "MiMo v2.5 Pro" instead of showing a raw slug; SORTED_SHORT_NAMES is longest-first, so the Pro tier still wins its own entry. --- CHANGELOG.md | 1 + src/models.ts | 1 + tests/models.test.ts | 26 ++++++++++++++++++++++---- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bee3bad..37e4ed57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ - **The resident `codeburn serve` child.** The first real panel request is also the cache warm-up, so startup never runs an artificial warm-up query beside a duplicate one-shot child; each served command carries its own read-only option allowlist, and anything outside it falls back to a normal spawn; the child exits when its stdin closes, so it can never outlive the app. Requests whose response exceeds the 16 MiB frame limit still replace the child, but that deliberate kill no longer spends the resident's unexpected-death budget. (#972) ### Fixed +- **MiMo sessions price from the LiteLLM Xiaomi rows, and MiMo v2 Flash no longer crashes the display path.** Hermes / Xiaomi token-plan sessions store the bare id (`mimo-v2.5-pro`, `mimo-v2.5`) while LiteLLM namespaces its row (`xiaomi/…`), so those models reported $0. They now alias to the existing snapshot rows — no invented rate, and `kimi-k3` still has none — which means a session Hermes left costless is priced from the shared tables and carries the estimated marker, exactly as `mimo-v2-flash` already did. The same change fixes a **pre-existing** crash that this alias did not introduce: the shipped `mimo-v2-flash -> xiaomi/mimo-v2-flash` alias already cycled through display-name resolution — strip the namespace, alias it back, take the leaf, repeat — so `getShortModelName` blew the stack on any real MiMo v2 Flash session and took every surface that names a model down with it, the `models` table included. Display-name resolution is now cycle-safe, and the base `mimo-v2.5` row is named rather than shown as a raw slug. - **A date-ranged run no longer republishes the month shards it never read.** A scoped load leaves an out-of-range month on disk, so the files it holds have no visible cache entry and the reconcile re-parses them — re-deriving the entry the shard already stores. That re-parse marked the unloaded month dirty, and the save merged and republished it under a fresh nonce name on every single run, byte-identical content and all, so a repeated `codeburn status --format json` churned old months (on a real corpus: claude/2026-03, cursor/2026-02 and warp/2026-03 renamed every run) and left the retired shards for the sweeper. A merge into an unloaded month that neither adds, changes nor removes an entry now keeps the published shard, so unchanged months keep their names and their bytes. (#1032) - **`models` and `audit` no longer show two identical `Grok 4.5` rows.** `grok-4.5-build` — the Grok Build harness's variant id — fell into the `grok-4.5` display entry by prefix, and since rows bucket by model id, not display name, the two came out as visually identical rows with different numbers. The variant now shows as `Grok 4.5 (build)`. Display only: no id is rewritten and no cost moves. (#1029) - **An upgrade no longer loses history for days whose transcripts have only PARTLY aged out.** The never-lose contract carried a cached (day, provider) slice forward only when the re-derivation found NOTHING for it, but transcripts expire per FILE rather than per day: on a day whose sources are mostly gone, a handful of turns from surviving later files still bucket onto it, so the fresh slice came back non-empty but truncated and REPLACED the full cached one. On a real cache upgrading from the last shipped daily-cache version, 2026-07-16 fell from $1,685.17 / 12,530 calls to $385.44 / 560 calls, and 13 days lost $2,765.75, 19,209 calls and 520 sessions in total. A fresh slice now replaces a settled baseline slice only when it carries at least as many CALLS - the same or more evidence; fewer calls means the source set demonstrably lost data, and the baseline is kept whole. The comparison is on calls alone: cost and tokens are re-priced accounting on the same evidence, which is exactly what a legitimate re-derivation changes (the Grok accounting fix keeps its per-day calls and is unaffected), and session counts drift down by a few on days whose sources are entirely intact. Days inside a 7-day settle window stay authoritative - their session files are still on disk, so a shrink there is a real change rather than expiry. The trade-off is deliberate and matches the direction this cache has always chosen: a future fix that legitimately REDUCES calls on a settled day keeps the older, higher value until that day is re-derived at an equal or greater call count. The timezone-change re-derive gets the exact form of the same rule - what the fresh parse can no longer explain under the old bucketing is added on top of the fresh slice instead of being dropped - and the cross-file adoption union is unchanged, where the newer schema still wins per (day, provider). diff --git a/src/models.ts b/src/models.ts index d429ab47..616185e4 100644 --- a/src/models.ts +++ b/src/models.ts @@ -960,6 +960,7 @@ const SHORT_NAMES: Record = { // table, the same way it handles `accounts/fireworks/models/`. 'qwen3.7-max': 'Qwen 3.7 Max', 'mimo-v2.5-pro': 'MiMo v2.5 Pro', + 'mimo-v2.5': 'MiMo v2.5', // Both spellings occur in the wild: OpenRouter gap-filled keys are lowercase // slugs while sessions report the capitalized name (see the case-insensitive // pricing index above). SHORT_NAMES matching is case-sensitive, so map both. diff --git a/tests/models.test.ts b/tests/models.test.ts index 44461252..dbaf0183 100644 --- a/tests/models.test.ts +++ b/tests/models.test.ts @@ -770,15 +770,33 @@ describe('observed provider model aliases', () => { }) it('does not recurse on vendor-requalified MiMo aliases', () => { - expect(getShortModelName('mimo-v2.5')).toBe('mimo-v2.5') - expect(getShortModelName('MiMo-v2.5')).toBe('mimo-v2.5') - expect(getShortModelName('cline-pass/mimo-v2.5')).toBe('mimo-v2.5') + expect(getShortModelName('mimo-v2.5')).toBe('MiMo v2.5') + expect(getShortModelName('MiMo-v2.5')).toBe('MiMo v2.5') + expect(getShortModelName('cline-pass/mimo-v2.5')).toBe('MiMo v2.5') expect(getShortModelName('cline-pass/mimo-v2.5-pro')).toBe('MiMo v2.5 Pro') }) + // The `mimo-v2-flash -> xiaomi/mimo-v2-flash` alias shipped before this + // change and already cycled: strip the namespace, alias it back, take the + // leaf, repeat. Every display surface (overview's model table included) + // threw RangeError on a real MiMo v2 Flash session. Pin the shipped ids. + it('resolves the already-shipped MiMo v2 Flash alias without blowing the stack', () => { + for (const id of ['mimo-v2-flash', 'MiMo-V2-Flash', 'cline-pass/mimo-v2-flash', 'mimo/mimo-v2-flash']) { + expect(() => getShortModelName(id)).not.toThrow() + expect(getShortModelName(id)).toBe('mimo-v2-flash') + expect(getModelCosts(id)).toEqual(getModelCosts('xiaomi/mimo-v2-flash')) + } + }) + + it('names the base MiMo 2.5 row without swallowing the Pro tier', () => { + expect(getShortModelName('mimo-v2.5')).toBe('MiMo v2.5') + expect(getShortModelName('mimo-v2.5-pro')).toBe('MiMo v2.5 Pro') + expect(getModelCosts('mimo-v2.5')).not.toEqual(getModelCosts('mimo-v2.5-pro')) + }) + it('stays unary so Array.map cannot feed the index as cycle state', () => { expect(['mimo-v2.5', 'gpt-4o', 'cline-pass/mimo-v2.5-pro'].map(getShortModelName)).toEqual([ - 'mimo-v2.5', + 'MiMo v2.5', 'GPT-4o', 'MiMo v2.5 Pro', ]) From 6bd86dbc366bcead7906f12adfabbefb4600dccb Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Wed, 19 Aug 2026 11:29:14 -0700 Subject: [PATCH 09/15] ci: build, test and package the macOS menubar on every mac/ change release-menubar.yml only packages; the 170+ Swift tests gated nothing. --- .github/workflows/mac-menubar-ci.yml | 34 ++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 .github/workflows/mac-menubar-ci.yml diff --git a/.github/workflows/mac-menubar-ci.yml b/.github/workflows/mac-menubar-ci.yml new file mode 100644 index 00000000..f7d9cc1c --- /dev/null +++ b/.github/workflows/mac-menubar-ci.yml @@ -0,0 +1,34 @@ +name: macOS Menubar CI + +# The macOS menubar (mac/) ships from release-menubar.yml, which only packages the app. +# Its Swift test suite (170+ tests covering the credential stores, Keychain cache, +# serve connection, quota parsing) gated nothing until this workflow. Runs on every +# PR touching mac/** so a red test fails the PR, the same way tests.yml does for the CLI. +on: + push: + branches: [main] + paths: + - .github/workflows/mac-menubar-ci.yml + - mac/** + pull_request: + paths: + - .github/workflows/mac-menubar-ci.yml + - mac/** + +permissions: + contents: read + +jobs: + test: + runs-on: macos-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@v6 + - name: Swift toolchain + run: swift --version + - name: Build + run: swift build --package-path mac + - name: Test + run: swift test --package-path mac + - name: Package (same script the release uses) + run: mac/Scripts/package-app.sh ci-smoke From bb5b71dff1eb8be24c3b29769fe29fb922ffdc8e Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Wed, 19 Aug 2026 11:33:13 -0700 Subject: [PATCH 10/15] feat(models): name the MiMo v2 Flash row It was the only MiMo row still rendering as its raw slug next to "MiMo v2.5" and "MiMo v2.5 Pro". SORTED_SHORT_NAMES is longest-first, so the two v2.5 entries keep their own labels. --- CHANGELOG.md | 2 +- src/models.ts | 1 + tests/models.test.ts | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37e4ed57..0479fe82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,7 @@ - **The resident `codeburn serve` child.** The first real panel request is also the cache warm-up, so startup never runs an artificial warm-up query beside a duplicate one-shot child; each served command carries its own read-only option allowlist, and anything outside it falls back to a normal spawn; the child exits when its stdin closes, so it can never outlive the app. Requests whose response exceeds the 16 MiB frame limit still replace the child, but that deliberate kill no longer spends the resident's unexpected-death budget. (#972) ### Fixed -- **MiMo sessions price from the LiteLLM Xiaomi rows, and MiMo v2 Flash no longer crashes the display path.** Hermes / Xiaomi token-plan sessions store the bare id (`mimo-v2.5-pro`, `mimo-v2.5`) while LiteLLM namespaces its row (`xiaomi/…`), so those models reported $0. They now alias to the existing snapshot rows — no invented rate, and `kimi-k3` still has none — which means a session Hermes left costless is priced from the shared tables and carries the estimated marker, exactly as `mimo-v2-flash` already did. The same change fixes a **pre-existing** crash that this alias did not introduce: the shipped `mimo-v2-flash -> xiaomi/mimo-v2-flash` alias already cycled through display-name resolution — strip the namespace, alias it back, take the leaf, repeat — so `getShortModelName` blew the stack on any real MiMo v2 Flash session and took every surface that names a model down with it, the `models` table included. Display-name resolution is now cycle-safe, and the base `mimo-v2.5` row is named rather than shown as a raw slug. +- **MiMo sessions price from the LiteLLM Xiaomi rows, and MiMo v2 Flash no longer crashes the display path.** Hermes / Xiaomi token-plan sessions store the bare id (`mimo-v2.5-pro`, `mimo-v2.5`) while LiteLLM namespaces its row (`xiaomi/…`), so those models reported $0. They now alias to the existing snapshot rows — no invented rate, and `kimi-k3` still has none — which means a session Hermes left costless is priced from the shared tables and carries the estimated marker, exactly as `mimo-v2-flash` already did. The same change fixes a **pre-existing** crash that this alias did not introduce: the shipped `mimo-v2-flash -> xiaomi/mimo-v2-flash` alias already cycled through display-name resolution — strip the namespace, alias it back, take the leaf, repeat — so `getShortModelName` blew the stack on any real MiMo v2 Flash session and took every surface that names a model down with it, the `models` table included. Display-name resolution is now cycle-safe, and the `mimo-v2-flash` and `mimo-v2.5` rows are named rather than shown as raw slugs. - **A date-ranged run no longer republishes the month shards it never read.** A scoped load leaves an out-of-range month on disk, so the files it holds have no visible cache entry and the reconcile re-parses them — re-deriving the entry the shard already stores. That re-parse marked the unloaded month dirty, and the save merged and republished it under a fresh nonce name on every single run, byte-identical content and all, so a repeated `codeburn status --format json` churned old months (on a real corpus: claude/2026-03, cursor/2026-02 and warp/2026-03 renamed every run) and left the retired shards for the sweeper. A merge into an unloaded month that neither adds, changes nor removes an entry now keeps the published shard, so unchanged months keep their names and their bytes. (#1032) - **`models` and `audit` no longer show two identical `Grok 4.5` rows.** `grok-4.5-build` — the Grok Build harness's variant id — fell into the `grok-4.5` display entry by prefix, and since rows bucket by model id, not display name, the two came out as visually identical rows with different numbers. The variant now shows as `Grok 4.5 (build)`. Display only: no id is rewritten and no cost moves. (#1029) - **An upgrade no longer loses history for days whose transcripts have only PARTLY aged out.** The never-lose contract carried a cached (day, provider) slice forward only when the re-derivation found NOTHING for it, but transcripts expire per FILE rather than per day: on a day whose sources are mostly gone, a handful of turns from surviving later files still bucket onto it, so the fresh slice came back non-empty but truncated and REPLACED the full cached one. On a real cache upgrading from the last shipped daily-cache version, 2026-07-16 fell from $1,685.17 / 12,530 calls to $385.44 / 560 calls, and 13 days lost $2,765.75, 19,209 calls and 520 sessions in total. A fresh slice now replaces a settled baseline slice only when it carries at least as many CALLS - the same or more evidence; fewer calls means the source set demonstrably lost data, and the baseline is kept whole. The comparison is on calls alone: cost and tokens are re-priced accounting on the same evidence, which is exactly what a legitimate re-derivation changes (the Grok accounting fix keeps its per-day calls and is unaffected), and session counts drift down by a few on days whose sources are entirely intact. Days inside a 7-day settle window stay authoritative - their session files are still on disk, so a shrink there is a real change rather than expiry. The trade-off is deliberate and matches the direction this cache has always chosen: a future fix that legitimately REDUCES calls on a settled day keeps the older, higher value until that day is re-derived at an equal or greater call count. The timezone-change re-derive gets the exact form of the same rule - what the fresh parse can no longer explain under the old bucketing is added on top of the fresh slice instead of being dropped - and the cross-file adoption union is unchanged, where the newer schema still wins per (day, provider). diff --git a/src/models.ts b/src/models.ts index 616185e4..4413d084 100644 --- a/src/models.ts +++ b/src/models.ts @@ -961,6 +961,7 @@ const SHORT_NAMES: Record = { 'qwen3.7-max': 'Qwen 3.7 Max', 'mimo-v2.5-pro': 'MiMo v2.5 Pro', 'mimo-v2.5': 'MiMo v2.5', + 'mimo-v2-flash': 'MiMo v2 Flash', // Both spellings occur in the wild: OpenRouter gap-filled keys are lowercase // slugs while sessions report the capitalized name (see the case-insensitive // pricing index above). SHORT_NAMES matching is case-sensitive, so map both. diff --git a/tests/models.test.ts b/tests/models.test.ts index dbaf0183..6ec6d9d4 100644 --- a/tests/models.test.ts +++ b/tests/models.test.ts @@ -783,7 +783,7 @@ describe('observed provider model aliases', () => { it('resolves the already-shipped MiMo v2 Flash alias without blowing the stack', () => { for (const id of ['mimo-v2-flash', 'MiMo-V2-Flash', 'cline-pass/mimo-v2-flash', 'mimo/mimo-v2-flash']) { expect(() => getShortModelName(id)).not.toThrow() - expect(getShortModelName(id)).toBe('mimo-v2-flash') + expect(getShortModelName(id)).toBe('MiMo v2 Flash') expect(getModelCosts(id)).toEqual(getModelCosts('xiaomi/mimo-v2-flash')) } }) From e213e192b45308b3e917f9dd1aff09b63908fa5d Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Wed, 19 Aug 2026 11:43:22 -0700 Subject: [PATCH 11/15] fix(menubar): never let a Keychain read raise UI on the refresh timer Cache reads run on the background quota timer, so they must not be able to put a panel on screen. Measured on macOS 15 against a throwaway keychain: with the keychain locked, SecItemCopyMatching blocks on an unlock panel even when the query carries kSecUseAuthenticationUI: ...Fail or a non-interactive LAContext. Both of those govern the data-protection keychain; unlocking a file-based keychain is something securityd drives itself. The only reliable suppression is not issuing the read, so check lock state first and report .unavailable instead. .unavailable is separate from readFailed on purpose: a locked keychain means "cannot look right now", not "the item is gone", and callers must not turn it into a disconnect. It also carries a readable errorDescription so a -25308 reaching the UI reads as "Keychain unavailable" rather than a struct dump. SecKeychainGetStatus is soft-deprecated with no replacement that reports file-keychain lock state; annotating the warning away only moves it to the call site, so it is left visible with a comment. Adds the first test that touches a real Keychain, against a throwaway service name no build reads, skipped when the host has no usable Keychain. --- .../Security/KeychainCredentialCache.swift | 64 +++++++++++- .../LiveKeychainCredentialCacheTests.swift | 97 +++++++++++++++++++ 2 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 mac/Tests/CodeBurnMenubarTests/LiveKeychainCredentialCacheTests.swift diff --git a/mac/Sources/CodeBurnMenubar/Security/KeychainCredentialCache.swift b/mac/Sources/CodeBurnMenubar/Security/KeychainCredentialCache.swift index 8ea079fa..2b8f5d3c 100644 --- a/mac/Sources/CodeBurnMenubar/Security/KeychainCredentialCache.swift +++ b/mac/Sources/CodeBurnMenubar/Security/KeychainCredentialCache.swift @@ -1,4 +1,5 @@ import Foundation +import LocalAuthentication import Security /// Serializes credential-store test harnesses that mutate process-wide seams. @@ -21,6 +22,9 @@ enum KeychainCredentialCacheError: Error, LocalizedError, Equatable { case readFailed(service: String, status: OSStatus) case writeFailed(service: String, status: OSStatus) case deleteFailed(service: String, status: OSStatus) + /// Keychain is locked or consent was refused. Transient — callers keep the + /// last known token and must not treat it as "the item is gone". + case unavailable(service: String, status: OSStatus) var errorDescription: String? { switch self { @@ -30,12 +34,30 @@ enum KeychainCredentialCacheError: Error, LocalizedError, Equatable { return "Keychain write failed for \(service) (status \(status))." case let .deleteFailed(service, status): return "Keychain delete failed for \(service) (status \(status))." + case .unavailable: + return "Keychain unavailable — unlock your login keychain to refresh quota." } } + + /// Statuses that mean "we were not allowed to look right now", as opposed to + /// "the item does not exist". `errSecInteractionNotAllowed` (-25308) is what + /// a locked keychain returns once UI is suppressed. + static func isUnavailable(_ status: OSStatus) -> Bool { + status == errSecInteractionNotAllowed + || status == errSecAuthFailed + || status == errSecUserCanceled + || status == errSecInteractionRequired + } } /// Published CodeBurn Keychain identities. Keep these exact — Electron contracts -/// on the Codex pair, and historical items use the same names. +/// on the Codex pair (`app/electron/quota/codex.ts`), and installs going back to +/// May 2026 already hold items under these names. +/// +/// Deliberately NOT derived from `CFBundleIdentifier`: the Electron app hardcodes +/// the same strings, so a per-bundle suffix would break that contract. The +/// tradeoff is that a dev/beta build sharing this source shares the item — patch +/// these constants when running a second build alongside the release. enum CodeBurnKeychainIdentity { static let claudeService = "org.agentseal.codeburn.menubar.claude.oauth.v1" static let codexService = "org.agentseal.codeburn.menubar.codex.oauth.v1" @@ -43,17 +65,53 @@ enum CodeBurnKeychainIdentity { } struct LiveKeychainCredentialCache: KeychainCredentialCaching { + /// True when the default (login) keychain exists and is currently locked. + /// Returns false when the state cannot be determined, so an unexpected + /// failure degrades to "just try the read" rather than a hard outage. + /// + /// `SecKeychain*` is soft-deprecated with no replacement that reports + /// file-keychain lock state — `kSecUseDataProtectionKeychain` would move our + /// item to a different store and orphan every existing install. The + /// This is the one intentional deprecation warning in the file; annotating it + /// away only moves the warning to the call site, so it is left visible. + private func isDefaultKeychainLocked() -> Bool { + var status: SecKeychainStatus = 0 + guard SecKeychainGetStatus(nil, &status) == errSecSuccess else { return false } + return (status & SecKeychainStatus(kSecUnlockStateStatus)) == 0 + } + func read(service: String, account: String) throws -> Data? { + // Reads happen on the background refresh timer, so they must never be + // able to raise UI. Measured on macOS 15 against a locked test keychain: + // NEITHER `kSecUseAuthenticationUI: …Fail` NOR + // `LAContext.interactionNotAllowed` suppresses the unlock panel for a + // file-based keychain — both govern the data-protection keychain, while + // unlocking is a keychain-level operation securityd drives itself. The + // only thing that reliably avoids the panel is not issuing the read at + // all, so check lock state first. Same class of bug as the + // partition-list re-prompt in #490. + if isDefaultKeychainLocked() { + throw KeychainCredentialCacheError.unavailable( + service: service, status: errSecInteractionNotAllowed) + } + // Still pass a non-interactive context: it is the supported way to keep + // a data-protection-backed item from raising biometric/passcode UI. + let context = LAContext() + context.interactionNotAllowed = true let query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, kSecAttrAccount as String: account, kSecMatchLimit as String: kSecMatchLimitOne, kSecReturnData as String: true, + kSecUseAuthenticationContext as String: context, ] var result: CFTypeRef? let status = SecItemCopyMatching(query as CFDictionary, &result) if status == errSecItemNotFound { return nil } + if KeychainCredentialCacheError.isUnavailable(status) { + throw KeychainCredentialCacheError.unavailable(service: service, status: status) + } guard status == errSecSuccess, let data = result as? Data else { throw KeychainCredentialCacheError.readFailed(service: service, status: status) } @@ -165,6 +223,10 @@ final class ControllableKeychainCredentialCache: KeychainCredentialCaching, @unc func read(service: String, account: String) throws -> Data? { if failRead { + // Mirror the live adapter's mapping so tests exercise the same branch. + if KeychainCredentialCacheError.isUnavailable(readStatus) { + throw KeychainCredentialCacheError.unavailable(service: service, status: readStatus) + } throw KeychainCredentialCacheError.readFailed(service: service, status: readStatus) } return try inner.read(service: service, account: account) diff --git a/mac/Tests/CodeBurnMenubarTests/LiveKeychainCredentialCacheTests.swift b/mac/Tests/CodeBurnMenubarTests/LiveKeychainCredentialCacheTests.swift new file mode 100644 index 00000000..3eaf0eeb --- /dev/null +++ b/mac/Tests/CodeBurnMenubarTests/LiveKeychainCredentialCacheTests.swift @@ -0,0 +1,97 @@ +import Foundation +import Security +import Testing +@testable import CodeBurnMenubar + +/// The ONLY test that touches a real Keychain. Everything else in the suite runs +/// against `InMemoryKeychainCredentialCache`. +/// +/// It writes to a throwaway service name (`…menubar.selftest.oauth.v1`) that no +/// build ever reads, never touches the Claude/Codex production items, and deletes +/// what it created. If the login Keychain is locked or unavailable — headless CI, +/// SSH session, no login Keychain — the whole suite is SKIPPED rather than failed; +/// look for "live Keychain unavailable" in the output to tell a skip from a pass. +@Suite("Live Keychain adapter", .serialized) +struct LiveKeychainCredentialCacheTests { + private static let service = "org.agentseal.codeburn.menubar.selftest.oauth.v1" + private static let account = "selftest" + + /// True when this machine can round-trip a generic password right now. + private static let isAvailable: Bool = { + let live = LiveKeychainCredentialCache() + do { + try live.upsert(service: service, account: account, data: Data("probe".utf8)) + _ = try live.read(service: service, account: account) + try live.delete(service: service, account: account) + return true + } catch { + try? live.delete(service: service, account: account) + return false + } + }() + + @Test("live adapter round-trips write → read → update → delete") + func liveRoundTrip() throws { + guard Self.isAvailable else { + print("SKIP: live Keychain unavailable on this host") + return + } + let live = LiveKeychainCredentialCache() + defer { try? live.delete(service: Self.service, account: Self.account) } + + #expect(try live.read(service: Self.service, account: Self.account) == nil) + + try live.upsert(service: Self.service, account: Self.account, data: Data(#"{"v":1}"#.utf8)) + let first = try #require(try live.read(service: Self.service, account: Self.account)) + #expect(String(data: first, encoding: .utf8) == #"{"v":1}"#) + + // upsert must update in place, not duplicate. + try live.upsert(service: Self.service, account: Self.account, data: Data(#"{"v":2}"#.utf8)) + let second = try #require(try live.read(service: Self.service, account: Self.account)) + #expect(String(data: second, encoding: .utf8) == #"{"v":2}"#) + + try live.delete(service: Self.service, account: Self.account) + #expect(try live.read(service: Self.service, account: Self.account) == nil) + // Deleting an absent item is success, so disconnect stays idempotent. + #expect(throws: Never.self) { + try live.delete(service: Self.service, account: Self.account) + } + } + + /// Documents the measured behaviour that motivates the pre-flight lock check. + /// The locked-keychain case itself is deliberately NOT exercised at runtime: + /// reproducing it requires a keychain operation that raises a password panel + /// on the tester's screen. Measured once by hand on macOS 15 against a + /// throwaway keychain: with the keychain locked, `SecItemCopyMatching` blocks + /// on an unlock panel even when the query carries + /// `kSecUseAuthenticationUI: …Fail` or a non-interactive `LAContext` — both + /// govern the data-protection keychain, not file-keychain unlocking. Skipping + /// the read while locked is therefore the only reliable suppression. + @Test("unavailable statuses are classified as transient, not as a missing item") + func unavailableClassification() { + #expect(KeychainCredentialCacheError.isUnavailable(errSecInteractionNotAllowed)) + #expect(KeychainCredentialCacheError.isUnavailable(errSecAuthFailed)) + #expect(KeychainCredentialCacheError.isUnavailable(errSecUserCanceled)) + #expect(KeychainCredentialCacheError.isUnavailable(errSecInteractionRequired)) + // errSecItemNotFound is a real miss and must never be treated as transient. + #expect(!KeychainCredentialCacheError.isUnavailable(errSecItemNotFound)) + #expect(!KeychainCredentialCacheError.isUnavailable(errSecDecode)) + } + + @Test("live reads never block on an interactive prompt") + func liveReadIsNonInteractive() throws { + guard Self.isAvailable else { + print("SKIP: live Keychain unavailable on this host") + return + } + let live = LiveKeychainCredentialCache() + defer { try? live.delete(service: Self.service, account: Self.account) } + try live.upsert(service: Self.service, account: Self.account, data: Data("x".utf8)) + + // The read carries a non-interactive LAContext, so it either returns or + // fails fast. A prompt would park this call until a human dismissed it. + let start = Date() + _ = try? live.read(service: Self.service, account: Self.account) + #expect(Date().timeIntervalSince(start) < 5) + } +} From 0fd8419bfdea243a021803e2cb8beb286f599627 Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Wed, 19 Aug 2026 11:43:32 -0700 Subject: [PATCH 12/15] fix(menubar): keep the newer credential copy and drop the sticky retry flag Three fixes in the store read path. A locked keychain no longer reads as a disconnect. currentRecord() treated any failure from readOurCache() as fatal, and a nil as "the item vanished", which cleared isBootstrapCompleted. .unavailable now falls back to the last known record and leaves the flag set. Recency. A Keychain hit always won and the legacy file was then unlinked, even when the file was newer. This service name has been in use since May 2026, so an upgrading install can hold a months-old item beside a file the pre-migration build wrote today; the older token won and the newer copy was deleted. Both stores now compare first (expiresAt for Claude, lastRefresh for Codex) and adopt the later one before anything is removed. Codex matters most here: serving a spent rotating refresh token ends in a terminal invalid_grant. lastLegacyCleanupFailed is gone. It was set on every cleanup path and read only by tests, never surfaced. The retry it was meant to signal already happens, because the unlink is attempted on every successful read. Also serializes migrate + unlink under the existing SafeFile.withExclusiveLock so two menubar instances cannot race on the same legacy file, and drops a leftover no-op local. --- .../Data/ClaudeCredentialStore.swift | 114 ++++++++----- .../Data/CodexCredentialStore.swift | 111 ++++++++----- .../CredentialKeychainContinuityTests.swift | 153 +++++++++++++++++- 3 files changed, 288 insertions(+), 90 deletions(-) diff --git a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift index 5df6bbc8..a5a7e8f7 100644 --- a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift @@ -55,7 +55,6 @@ enum ClaudeCredentialStore { userDefaultsOverride = nil keychainCache = LiveKeychainCredentialCache() lastCacheDeleteResult = nil - lastLegacyCleanupFailed = false unlinkLegacyOverride = nil tightenLegacyOverride = nil lock.withLock { memoryCache = nil } @@ -154,8 +153,6 @@ enum ClaudeCredentialStore { /// Last disconnect/cleanup result. Nil until the first delete attempt. nonisolated(unsafe) static var lastCacheDeleteResult: CacheDeleteResult? - /// Last legacy-unlink failure after a verified Keychain write (cleanup retry signal). - nonisolated(unsafe) static var lastLegacyCleanupFailed = false /// Test seam: force legacy unlink to throw so we can assert the 0600 repair. nonisolated(unsafe) static var unlinkLegacyOverride: ((URL) throws -> Void)? nonisolated(unsafe) static var tightenLegacyOverride: ((URL) throws -> Void)? @@ -185,7 +182,19 @@ enum ClaudeCredentialStore { if let cached = lock.withLock({ memoryCache }), cached.isFresh { return cached.record } - if let stored = try readOurCache() { + let fetched: CredentialRecord? + do { + fetched = try readOurCache() + } catch let err as KeychainCredentialCacheError { + // A locked/denied keychain means "can't look right now", not "the + // item is gone". Serve the last known token and leave the bootstrap + // flag alone so we don't silently disconnect the user. + if case .unavailable = err { + return lock.withLock { memoryCache }?.record + } + throw err + } + if let stored = fetched { cacheInMemory(stored) return stored } @@ -437,17 +446,37 @@ enum ClaudeCredentialStore { } } + /// Serializes migrate + unlink across processes so two menubar instances (or the + /// CLI) cannot race on the same legacy file. Only `SafeFile.Error` from acquiring + /// the lock is tolerated — `readOurCacheLocked` never lets one escape, because + /// `readLegacyFile` swallows its own read failures. private static func readOurCache() throws -> CredentialRecord? { + do { + return try SafeFile.withExclusiveLock(at: cacheFileURL().path + ".lock") { + try readOurCacheLocked() + } + } catch is SafeFile.Error { + return try readOurCacheLocked() + } + } + + private static func readOurCacheLocked() throws -> CredentialRecord? { if let data = try keychainCache.read(service: ourKeychainService, account: ourKeychainAccount), let record = decodePersisted(data) { + // The Keychain item can predate the legacy file by a long way — this + // service name has been in use since May 2026, so an upgrading install + // can hold a months-old item beside a file the old build wrote today. + // Adopt whichever expires later before unlinking anything. + if let fresher = legacyRecordIfNewer(than: record) { + try? writeOurCache(record: fresher) + return fresher + } // Rewrite historical Claude blobs once without refreshToken. - let sanitized = encodePersisted(record) if let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any], object.keys.contains("refreshToken") { try? writeOurCache(record: record) } else { tryUnlinkLegacyAfterVerifiedKeychain() - _ = sanitized } return record } @@ -455,66 +484,63 @@ enum ClaudeCredentialStore { return try migrateLegacyFileIfPresent() } - /// Secure-read legacy JSON, upsert Keychain, verify read-back, then unlink. - private static func migrateLegacyFileIfPresent() throws -> CredentialRecord? { + /// Reads the legacy file only to compare recency. Returns it when it expires + /// strictly later than `record`; nil when absent, unreadable, or not newer. + private static func legacyRecordIfNewer(than record: CredentialRecord) -> CredentialRecord? { + guard let legacy = readLegacyFile() else { return nil } + guard let legacyExpiry = legacy.expiresAt else { return nil } + guard let currentExpiry = record.expiresAt else { return legacy } + return legacyExpiry > currentExpiry ? legacy : nil + } + + /// Secure-read + decode the legacy JSON, dropping any refreshToken it holds. + /// Returns nil on symlink / ownership / chmod / decode failure, leaving the + /// file in place. + private static func readLegacyFile() -> CredentialRecord? { let url = cacheFileURL() guard FileManager.default.fileExists(atPath: url.path) else { return nil } - - let data: Data - do { - data = try SafeFile.readAfterSecuringPermissions( - from: url.path, - maxBytes: maxCredentialBytes - ) - } catch { - // Symlink / ownership / chmod failures: leave the file alone. - return nil - } - + guard let data = try? SafeFile.readAfterSecuringPermissions( + from: url.path, + maxBytes: maxCredentialBytes + ) else { return nil } guard let decoded = try? JSONDecoder().decode(CredentialRecord.self, from: data) else { // Invalid data stays in place at 0600; do not delete. return nil } - let migrated = CredentialRecord( + return CredentialRecord( accessToken: decoded.accessToken, refreshToken: nil, expiresAt: decoded.expiresAt, rateLimitTier: decoded.rateLimitTier ) - - do { - try writeOurCache(record: migrated) - return migrated - } catch { - // Keychain write/read-back failure: leave repaired 0600 legacy file. - lastLegacyCleanupFailed = false - return migrated - } } + /// Secure-read legacy JSON, upsert Keychain, verify read-back, then unlink. + private static func migrateLegacyFileIfPresent() throws -> CredentialRecord? { + guard let migrated = readLegacyFile() else { return nil } + // Keychain write/read-back failure leaves the repaired 0600 legacy file + // in place; the next read retries the migration. + try? writeOurCache(record: migrated) + return migrated + } + + /// Unlink the redundant legacy JSON. Called on every successful cache read, + /// so a failure here is retried on the next read without a sticky flag. private static func tryUnlinkLegacyAfterVerifiedKeychain() { let url = cacheFileURL() - guard FileManager.default.fileExists(atPath: url.path) else { - lastLegacyCleanupFailed = false - return - } + guard FileManager.default.fileExists(atPath: url.path) else { return } do { if let unlinkLegacyOverride { try unlinkLegacyOverride(url) } else { try FileManager.default.removeItem(at: url) } - lastLegacyCleanupFailed = false } catch { - lastLegacyCleanupFailed = true - do { - if let tightenLegacyOverride { - try tightenLegacyOverride(url) - } else { - try SafeFile.tightenToOwnerReadWrite(at: url.path) - } - } catch { - // Still leftover, and 0600 is unproven. Retry signal stays set. + // Could not remove it — at least make sure it is not world-readable. + if let tightenLegacyOverride { + try? tightenLegacyOverride(url) + } else { + try? SafeFile.tightenToOwnerReadWrite(at: url.path) } } } diff --git a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift index 00385b05..2d500cfa 100644 --- a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift @@ -42,7 +42,6 @@ enum CodexCredentialStore { userDefaultsOverride = nil keychainCache = LiveKeychainCredentialCache() lastCacheDeleteResult = nil - lastLegacyCleanupFailed = false unlinkLegacyOverride = nil tightenLegacyOverride = nil lock.withLock { memoryCache = nil } @@ -148,7 +147,6 @@ enum CodexCredentialStore { } nonisolated(unsafe) static var lastCacheDeleteResult: CacheDeleteResult? - nonisolated(unsafe) static var lastLegacyCleanupFailed = false nonisolated(unsafe) static var unlinkLegacyOverride: ((URL) throws -> Void)? nonisolated(unsafe) static var tightenLegacyOverride: ((URL) throws -> Void)? @@ -178,7 +176,18 @@ enum CodexCredentialStore { if let cached = lock.withLock({ memoryCache }), cached.isFresh { return cached.record } - if let stored = try readOurCache() { + let fetched: CredentialRecord? + do { + fetched = try readOurCache() + } catch let err as KeychainCredentialCacheError { + // Locked/denied keychain: serve the last known token rather than + // reporting the grant as missing. + if case .unavailable = err { + return lock.withLock { memoryCache }?.record + } + throw err + } + if let stored = fetched { cacheInMemory(stored) return stored } @@ -331,65 +340,83 @@ enum CodexCredentialStore { tryUnlinkLegacyAfterVerifiedKeychain() } + /// Serializes migrate + unlink across processes so two menubar instances (or the + /// CLI) cannot race on the same legacy file. Only `SafeFile.Error` from acquiring + /// the lock is tolerated — `readOurCacheLocked` never lets one escape, because + /// `readLegacyFile` swallows its own read failures. private static func readOurCache() throws -> CredentialRecord? { + do { + return try SafeFile.withExclusiveLock(at: cacheFileURL().path + ".lock") { + try readOurCacheLocked() + } + } catch is SafeFile.Error { + return try readOurCacheLocked() + } + } + + private static func readOurCacheLocked() throws -> CredentialRecord? { if let data = try keychainCache.read(service: ourKeychainService, account: ourKeychainAccount), let record = try? JSONDecoder().decode(CredentialRecord.self, from: data) { + // Only reached when auth.json is unreadable, but the Keychain item can + // still be older than the legacy file — and serving a spent rotating + // refresh token here ends in a terminal invalid_grant. Prefer the + // later `lastRefresh` before unlinking anything. + if let fresher = legacyRecordIfNewer(than: record) { + try? writeOurCache(record: fresher) + return fresher + } tryUnlinkLegacyAfterVerifiedKeychain() return record } return try migrateLegacyFileIfPresent() } - private static func migrateLegacyFileIfPresent() throws -> CredentialRecord? { - let url = cacheFileURL() - guard FileManager.default.fileExists(atPath: url.path) else { return nil } - - let data: Data - do { - data = try SafeFile.readAfterSecuringPermissions( - from: url.path, - maxBytes: maxCredentialBytes - ) - } catch { - return nil - } - - guard let decoded = try? JSONDecoder().decode(CredentialRecord.self, from: data) else { - return nil - } - - do { - try writeOurCache(record: decoded) - return decoded - } catch { - lastLegacyCleanupFailed = false - return decoded - } + /// Returns the legacy file's record when it refreshed strictly later than + /// `record`; nil when absent, unreadable, or not newer. + private static func legacyRecordIfNewer(than record: CredentialRecord) -> CredentialRecord? { + guard let legacy = readLegacyFile() else { return nil } + guard let legacyRefresh = legacy.lastRefresh else { return nil } + guard let currentRefresh = record.lastRefresh else { return legacy } + return legacyRefresh > currentRefresh ? legacy : nil } + /// Secure-read + decode the legacy JSON. Returns nil on symlink / ownership / + /// chmod / decode failure, leaving the file in place. + private static func readLegacyFile() -> CredentialRecord? { + let url = cacheFileURL() + guard FileManager.default.fileExists(atPath: url.path) else { return nil } + guard let data = try? SafeFile.readAfterSecuringPermissions( + from: url.path, + maxBytes: maxCredentialBytes + ) else { return nil } + return try? JSONDecoder().decode(CredentialRecord.self, from: data) + } + + private static func migrateLegacyFileIfPresent() throws -> CredentialRecord? { + guard let decoded = readLegacyFile() else { return nil } + // Keychain write/read-back failure leaves the repaired 0600 legacy file + // in place; the next read retries the migration. + try? writeOurCache(record: decoded) + return decoded + } + + /// Unlink the redundant legacy JSON. Called on every successful cache read, + /// so a failure here is retried on the next read without a sticky flag. private static func tryUnlinkLegacyAfterVerifiedKeychain() { let url = cacheFileURL() - guard FileManager.default.fileExists(atPath: url.path) else { - lastLegacyCleanupFailed = false - return - } + guard FileManager.default.fileExists(atPath: url.path) else { return } do { if let unlinkLegacyOverride { try unlinkLegacyOverride(url) } else { try FileManager.default.removeItem(at: url) } - lastLegacyCleanupFailed = false } catch { - lastLegacyCleanupFailed = true - do { - if let tightenLegacyOverride { - try tightenLegacyOverride(url) - } else { - try SafeFile.tightenToOwnerReadWrite(at: url.path) - } - } catch { - // Still leftover, and 0600 is unproven. Retry signal stays set. + // Could not remove it — at least make sure it is not world-readable. + if let tightenLegacyOverride { + try? tightenLegacyOverride(url) + } else { + try? SafeFile.tightenToOwnerReadWrite(at: url.path) } } } diff --git a/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift index b4b8ebb6..13385a94 100644 --- a/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift +++ b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift @@ -204,7 +204,6 @@ struct CredentialKeychainContinuityTests { account: ClaudeCredentialStore.ourKeychainAccount ) #expect(keys?.contains("refreshToken") != true) - #expect(ClaudeCredentialStore.lastLegacyCleanupFailed == false) } } @@ -319,13 +318,18 @@ struct CredentialKeychainContinuityTests { } ClaudeCredentialStore.clearMemoryCacheForTesting() _ = try ClaudeCredentialStore.currentRecord() - #expect(ClaudeCredentialStore.lastLegacyCleanupFailed == true) #expect(FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) #expect(posixMode(at: ClaudeCredentialStore.cacheFileURL()) == 0o600) + + // No sticky flag: the next read retries the unlink on its own. + ClaudeCredentialStore.unlinkLegacyOverride = nil + ClaudeCredentialStore.clearMemoryCacheForTesting() + _ = try ClaudeCredentialStore.currentRecord() + #expect(!FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) } } - @Test("failed tighten after failed unlink keeps leftover and retry signal") + @Test("failed tighten after failed unlink keeps leftover in place") func failedTightenLeavesRetrySignal() throws { try withHarness { _ in let record = ClaudeCredentialStore.CredentialRecord( @@ -341,7 +345,6 @@ struct CredentialKeychainContinuityTests { ClaudeCredentialStore.tightenLegacyOverride = { _ in throw POSIXError(.EPERM) } ClaudeCredentialStore.clearMemoryCacheForTesting() _ = try ClaudeCredentialStore.currentRecord() - #expect(ClaudeCredentialStore.lastLegacyCleanupFailed == true) #expect(FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) } } @@ -381,6 +384,148 @@ struct CredentialKeychainContinuityTests { } } + // MARK: - Locked / denied Keychain + + @Test("unavailable Keychain read is a miss, not a disconnect") + func unavailableKeychainKeepsBootstrap() throws { + try withHarness { harness in + try ClaudeCredentialStore.writeOurCache(record: .init( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + )) + ClaudeCredentialStore.isBootstrapCompleted = true + + let controllable = ControllableKeychainCredentialCache(inner: harness.fakeKeychain) + controllable.failRead = true + controllable.readStatus = errSecInteractionNotAllowed // -25308 + ClaudeCredentialStore.keychainCache = controllable + ClaudeCredentialStore.clearMemoryCacheForTesting() + + // Must not throw and must not clear bootstrap: a locked keychain is + // "can't look right now", not "the user disconnected". + #expect(try ClaudeCredentialStore.currentRecord() == nil) + #expect(ClaudeCredentialStore.isBootstrapCompleted == true) + } + } + + @Test("a genuine read failure still surfaces as an error") + func nonUnavailableReadStillThrows() throws { + try withHarness { harness in + ClaudeCredentialStore.isBootstrapCompleted = true + let controllable = ControllableKeychainCredentialCache(inner: harness.fakeKeychain) + controllable.failRead = true + controllable.readStatus = errSecDecode + ClaudeCredentialStore.keychainCache = controllable + ClaudeCredentialStore.clearMemoryCacheForTesting() + + #expect(throws: KeychainCredentialCacheError.self) { + _ = try ClaudeCredentialStore.currentRecord() + } + } + } + + @Test("Keychain errors render a readable message, not a struct dump") + func keychainErrorMessageIsReadable() { + let unavailable = KeychainCredentialCacheError.unavailable( + service: ClaudeCredentialStore.ourKeychainService, + status: errSecInteractionNotAllowed + ) + let text = unavailable.localizedDescription + #expect(text.contains("Keychain unavailable")) + // AppStore renders errors via localizedDescription; a struct dump would + // read "unavailable(service:" and leak the raw item name. + #expect(!text.contains("unavailable(service:")) + #expect(!text.contains(ClaudeCredentialStore.ourKeychainService)) + } + + // MARK: - Recency between Keychain item and legacy file + + @Test("newer legacy file beats an older Keychain item and is then unlinked") + func newerLegacyFileWins() throws { + try withHarness { harness in + // Keychain item from an old build: already expired. + try ClaudeCredentialStore.writeOurCache(record: .init( + accessToken: "cb-stale-keychain-token", + refreshToken: nil, + expiresAt: Date(timeIntervalSince1970: 1_700_000_000), + rateLimitTier: "default" + )) + // Legacy file written far later by the pre-migration build. + try writeLegacyClaude0644(record: .init( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + )) + ClaudeCredentialStore.isBootstrapCompleted = true + ClaudeCredentialStore.clearMemoryCacheForTesting() + + let record = try #require(try ClaudeCredentialStore.currentRecord()) + #expect(record.accessToken == accessSentinel) + #expect(record.refreshToken == nil) + #expect(!FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) + let keys = harness.fakeKeychain.storedKeys( + service: ClaudeCredentialStore.ourKeychainService, + account: ClaudeCredentialStore.ourKeychainAccount + ) + #expect(keys?.contains("refreshToken") != true) + } + } + + @Test("older legacy file loses to a newer Keychain item and is unlinked") + func olderLegacyFileLoses() throws { + try withHarness { _ in + try ClaudeCredentialStore.writeOurCache(record: .init( + accessToken: accessSentinel, + refreshToken: nil, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + )) + try writeLegacyClaude0644(record: .init( + accessToken: "cb-stale-file-token", + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_700_000_000), + rateLimitTier: "default" + )) + ClaudeCredentialStore.isBootstrapCompleted = true + ClaudeCredentialStore.clearMemoryCacheForTesting() + + let record = try #require(try ClaudeCredentialStore.currentRecord()) + #expect(record.accessToken == accessSentinel) + #expect(!FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) + } + } + + @Test("Codex prefers the legacy file with the later lastRefresh") + func codexNewerLegacyFileWins() throws { + try withHarness { _ in + try CodexCredentialStore.writeOurCache(record: .init( + accessToken: "cb-stale-codex-access", + refreshToken: "cb-stale-codex-refresh", + idToken: nil, + accountId: accountSentinel, + expiresAt: nil, + lastRefresh: Date(timeIntervalSince1970: 1_700_000_000) + )) + try writeLegacyCodex0644(record: .init( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + idToken: idSentinel, + accountId: accountSentinel, + expiresAt: nil, + lastRefresh: Date(timeIntervalSince1970: 1_800_000_000) + )) + CodexCredentialStore.isBootstrapCompleted = true + CodexCredentialStore.clearMemoryCacheForTesting() + + let record = try #require(try CodexCredentialStore.currentRecord()) + #expect(record.refreshToken == refreshSentinel) + #expect(!FileManager.default.fileExists(atPath: CodexCredentialStore.cacheFileURL().path)) + } + } + @Test("corrupt Keychain plus valid legacy repairs the CodeBurn item") func corruptKeychainRepairedFromLegacy() throws { try withHarness { harness in From 0f7bfb3eb2b433874c0cf3e5f8937d3edcd18315 Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Wed, 19 Aug 2026 11:43:44 -0700 Subject: [PATCH 13/15] fix(menubar): make a failed disconnect leave a consistent state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit disconnect() cleared the usage block before anyone knew whether the delete had worked, and AppStore then returned early on failure — so a failed disconnect cleared some state, left the rest, and still posted subscriptionDisconnected. It also carried a second !isSuccess branch that the early return had already made unreachable. Both services now return the delete result and only clear the usage block on success, so a failure changes nothing at all: the provider stays connected, Disconnect stays available, and the banner asks for a retry. That matches the success path's ordering instead of half-applying it. Errors reaching the generic catches now render localizedDescription rather than String(describing:), so a Keychain failure shows its message instead of an enum dump with the raw item name in it. --- mac/Sources/CodeBurnMenubar/AppStore.swift | 39 ++++++++----------- .../Data/ClaudeSubscriptionService.swift | 11 ++++-- .../Data/CodexSubscriptionService.swift | 11 ++++-- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/mac/Sources/CodeBurnMenubar/AppStore.swift b/mac/Sources/CodeBurnMenubar/AppStore.swift index 2ca365f0..f8ff5bab 100644 --- a/mac/Sources/CodeBurnMenubar/AppStore.swift +++ b/mac/Sources/CodeBurnMenubar/AppStore.swift @@ -1053,7 +1053,7 @@ final class AppStore { return false } catch { guard gen == claudeRefreshGen else { return false } - subscriptionError = sanitizeForUI(String(describing: error)) + subscriptionError = sanitizeForUI(error.localizedDescription) subscriptionLoadState = .failed return false } @@ -1064,24 +1064,20 @@ final class AppStore { /// account or tier) starts clean. capacityEstimates and the snapshot store /// would otherwise contaminate "Based on last cycle" projections. func disconnectSubscription() { - ClaudeSubscriptionService.disconnect() + let result = ClaudeSubscriptionService.disconnect() // Bump the generation token so any in-flight refreshSubscription that // resumes after this point detects the disconnect and discards its // result instead of re-populating the cleared state. claudeRefreshGen &+= 1 - if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.isSuccess { - // Any leftover Keychain item or plaintext must keep Disconnect - // so the user can retry delete. + guard result.isSuccess else { + // Nothing was removed, so nothing is disconnected. Leave the + // connected state exactly as it was — the bootstrap flag is still + // set, Disconnect stays available, and the banner says to retry. subscriptionError = "Could not fully remove the local Claude credential cache. Disconnect again to retry." - NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil) return } subscription = nil - if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.isSuccess { - subscriptionError = "Could not fully remove the local Claude credential cache." - } else { - subscriptionError = nil - } + subscriptionError = nil subscriptionLoadState = .notBootstrapped capacityEstimates = [:] Task.detached { await SubscriptionSnapshotStore.clearAll() } @@ -1102,7 +1098,7 @@ final class AppStore { } catch let err as CodexSubscriptionService.FetchError { applyCodexFetchError(err) } catch { - codexError = sanitizeForUI(String(describing: error)) + codexError = sanitizeForUI(error.localizedDescription) codexLoadState = .failed } } @@ -1135,26 +1131,23 @@ final class AppStore { return false } catch { guard gen == codexRefreshGen else { return false } - codexError = sanitizeForUI(String(describing: error)) + codexError = sanitizeForUI(error.localizedDescription) codexLoadState = .failed return false } } func disconnectCodex() { - CodexSubscriptionService.disconnect() + let result = CodexSubscriptionService.disconnect() codexRefreshGen &+= 1 - if let result = CodexCredentialStore.lastCacheDeleteResult, !result.isSuccess { + guard result.isSuccess else { + // Nothing removed means nothing disconnected; keep state intact so + // Disconnect stays available for a retry. codexError = "Could not fully remove the local Codex credential cache. Disconnect again to retry." - NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil) return } codexUsage = nil - if let result = CodexCredentialStore.lastCacheDeleteResult, !result.isSuccess { - codexError = "Could not fully remove the local Codex credential cache." - } else { - codexError = nil - } + codexError = nil codexLoadState = .notBootstrapped NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil) } @@ -1196,7 +1189,7 @@ final class AppStore { applyKimiFetchError(err) } catch { guard gen == kimiRefreshGen else { return } - kimiError = sanitizeForUI(String(describing: error)) + kimiError = sanitizeForUI(error.localizedDescription) kimiLoadState = .failed } } @@ -1230,7 +1223,7 @@ final class AppStore { return false } catch { guard gen == kimiRefreshGen else { return false } - kimiError = sanitizeForUI(String(describing: error)) + kimiError = sanitizeForUI(error.localizedDescription) kimiLoadState = .failed return false } diff --git a/mac/Sources/CodeBurnMenubar/Data/ClaudeSubscriptionService.swift b/mac/Sources/CodeBurnMenubar/Data/ClaudeSubscriptionService.swift index d452b049..8a12b411 100644 --- a/mac/Sources/CodeBurnMenubar/Data/ClaudeSubscriptionService.swift +++ b/mac/Sources/CodeBurnMenubar/Data/ClaudeSubscriptionService.swift @@ -99,9 +99,14 @@ enum ClaudeSubscriptionService { } /// Reset everything — used on user-initiated disconnect. - static func disconnect() { - _ = ClaudeCredentialStore.resetBootstrap() - clearUsageBlock() + /// Returns the delete outcome so callers only tear down UI state once the + /// credential material is actually gone. A failed delete leaves the usage + /// block intact too, so a retry starts from the same state. + @discardableResult + static func disconnect() -> ClaudeCredentialStore.CacheDeleteResult { + let result = ClaudeCredentialStore.resetBootstrap() + if result.isSuccess { clearUsageBlock() } + return result } // MARK: - Internal diff --git a/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift b/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift index 7fd5968b..275f7848 100644 --- a/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift +++ b/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift @@ -78,9 +78,14 @@ enum CodexSubscriptionService { } } - static func disconnect() { - _ = CodexCredentialStore.resetBootstrap() - clearUsageBlock() + /// Returns the delete outcome so callers only tear down UI state once the + /// credential material is actually gone. A failed delete leaves the usage + /// block intact too, so a retry starts from the same state. + @discardableResult + static func disconnect() -> CodexCredentialStore.CacheDeleteResult { + let result = CodexCredentialStore.resetBootstrap() + if result.isSuccess { clearUsageBlock() } + return result } private static func fetchWithToken(_ token: String, allowOne401Recovery: Bool) async throws -> CodexUsage { From 0dff0b66d50bcfe687f9fc4a752d5cee95ad8c0b Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Wed, 19 Aug 2026 11:43:44 -0700 Subject: [PATCH 14/15] docs(menubar): state what the Keychain move actually guarantees The Codex settings copy implied the cached credential was app-private. It is a normal login-Keychain item: reachable by programs running as you, with no per-app ACL. The real win is that it is no longer a world-readable 0644 file, so say that instead. Also documents why readAfterSecuringPermissions repairs permissions before validating content (validating first would read the secret while it is still world-readable, which is the window the function exists to close), and why the Keychain service names are deliberately not derived from CFBundleIdentifier (the Electron app hardcodes the same strings). Adds the #1037 changelog entry. --- CHANGELOG.md | 1 + mac/Sources/CodeBurnMenubar/Security/SafeFile.swift | 6 ++++++ mac/Sources/CodeBurnMenubar/Views/SettingsView.swift | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bee3bad..6a2a7fb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ - **One rule for every cache file.** `CODEBURN_CACHE_DIR` when set, otherwise `~/.cache/codeburn`. `XDG_CACHE_HOME` is no longer consulted; the sync ledger, the only file that ever honored it, is merged into the canonical location on first read and the legacy copy is retired, so nothing is re-uploaded after the move. (#972) ### Fixed (Desktop & Menubar) +- **The menubar's copies of your Claude and Codex credentials move out of Application Support and into the login Keychain.** Connecting a provider used to leave the copied OAuth material in `~/Library/Application Support/CodeBurn/*-credentials.v1.json`, written world-readable (0644) because macOS ignores `.completeFileProtection` outside iOS. The copy now lives in a CodeBurn-owned login-Keychain item, and the first read after upgrading migrates the old file: it is reopened with `O_NOFOLLOW`, refused if it is a symlink or not owned by you, repaired to 0600 before a single secret byte is read, written to the Keychain, read back and compared, and only then unlinked — a failed or unverified write leaves the (now 0600) file in place so a retry can still find it, and the next read retries the cleanup. Where both a Keychain item and an old file exist, the one that expires later wins before anything is removed, so an item left behind by a much older build cannot displace a fresher token. Claude's entry no longer stores a refresh token at all — the CLI owns that grant and the menubar never spends it — and any refresh token in a historical blob is dropped on read. Disconnect only reports success once the material is actually gone; if the delete fails it says so and leaves the provider connected so you can retry. Keychain reads are non-interactive and are skipped outright while the login Keychain is locked, so a background quota refresh can never raise an unlock panel. (#1037) - **First launch no longer asks to control System Events.** The macOS menubar registered its login item by driving System Events over AppleScript, which made macOS put up an Automation consent dialog the first time the app ran. It now registers itself through `SMAppService.mainApp`, an in-process call that needs no Automation grant; there is no AppleScript fallback, so a failure logs and leaves the login item unset rather than bringing the prompt back. The same `codeburn.loginItemRegistered` guard still limits this to the first launch, so a login item you removed by hand stays removed. (#1026) - **The resident `codeburn serve` child.** The first real panel request is also the cache warm-up, so startup never runs an artificial warm-up query beside a duplicate one-shot child; each served command carries its own read-only option allowlist, and anything outside it falls back to a normal spawn; the child exits when its stdin closes, so it can never outlive the app. Requests whose response exceeds the 16 MiB frame limit still replace the child, but that deliberate kill no longer spends the resident's unexpected-death budget. (#972) diff --git a/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift b/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift index 9ff37fe4..3746f539 100644 --- a/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift +++ b/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift @@ -147,6 +147,12 @@ enum SafeFile { /// Legacy credential migration path: open with `O_NOFOLLOW`, refuse non-regular / /// non-owned files, `fchmod(0600)` and verify mode, then read bounded bytes from /// the same descriptor. Permissions are repaired before any secret byte is read. + /// + /// The chmod deliberately precedes any content check: validating JSON first would + /// mean reading the secret while it is still world-readable, which is the exact + /// window this function exists to close. The cost is that a non-credential file + /// sitting at the caller's exact cache path also gets tightened to 0600 — bounded + /// to our own Application Support directory, and already symlink- and owner-checked. static func readAfterSecuringPermissions( from path: String, maxBytes: Int = defaultReadLimit, diff --git a/mac/Sources/CodeBurnMenubar/Views/SettingsView.swift b/mac/Sources/CodeBurnMenubar/Views/SettingsView.swift index 3ee25a50..d63bcffa 100644 --- a/mac/Sources/CodeBurnMenubar/Views/SettingsView.swift +++ b/mac/Sources/CodeBurnMenubar/Views/SettingsView.swift @@ -508,7 +508,7 @@ private struct CodexSettingsTab: View { CodexConnectionRow() } Section { - Text("Codex live-quota tracking reads `~/.codex/auth.json` once on Connect, then keeps a CodeBurn-owned copy in the macOS Keychain so subsequent quota fetches don't re-read the original. Only ChatGPT-mode auth (Plus / Pro / Team / Business / Edu / Enterprise) is supported. API-key users are billed per request and have a different reporting surface. Credit-metered workspaces report no rate-limit windows, so their monthly credit allowance is shown instead.") + Text("Codex live-quota tracking reads `~/.codex/auth.json` once on Connect, then keeps a CodeBurn-owned copy in your login Keychain instead of a world-readable file, so subsequent quota fetches don't re-read the original. The item is reachable by programs running as you, the same as any login-Keychain entry. Only ChatGPT-mode auth (Plus / Pro / Team / Business / Edu / Enterprise) is supported. API-key users are billed per request and have a different reporting surface. Credit-metered workspaces report no rate-limit windows, so their monthly credit allowance is shown instead.") .font(.system(size: 11)) .foregroundStyle(.secondary) } header: { From dba9a4582539767eae8c95b886200055086018da Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Wed, 19 Aug 2026 11:58:39 -0700 Subject: [PATCH 15/15] test(mac): stop the concurrent-timeout test starving the pool it waits on The test blocked on a DispatchSemaphore with a 15s deadline, commented as running "on a real thread, not the cooperative pool". Swift Testing invokes synchronous test bodies from a task on the cooperative pool, so the wait parked one of activeProcessorCount workers on the very task group it was waiting for. With 16 cores locally there is slack; on the 3-core macos-latest runner, alongside the rest of the parallel suite, the group made no progress at all and the wait expired. Await the group directly instead, which also lets the compiler reject the blocking wait (unavailable from async contexts), and bound the test with .timeLimit rather than a hand-rolled wall clock. Assert each child came back with a signal status, so the test now proves the timeout killed every hung process instead of only that the group returned. Reproduced by parking all but 3 cooperative threads for the run: 3/3 failures at 15.0s before, 3/3 passes after. 10x full suite under CPU load: 160/160. --- .../DataClientProcessTests.swift | 58 +++++++++++-------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/mac/Tests/CodeBurnMenubarTests/DataClientProcessTests.swift b/mac/Tests/CodeBurnMenubarTests/DataClientProcessTests.swift index 93d4ed6e..24939dea 100644 --- a/mac/Tests/CodeBurnMenubarTests/DataClientProcessTests.swift +++ b/mac/Tests/CodeBurnMenubarTests/DataClientProcessTests.swift @@ -89,38 +89,46 @@ struct DataClientProcessTests { /// Concurrency + timeout smoke test: launch more hung subprocesses than /// there are cooperative threads, all at once, with a short timeout, and - /// assert every call returns once the timeout kills its sleep. + /// assert every call returns because the timeout killed its sleep. /// /// NOTE: this does NOT reproduce the production permanent deadlock (16/16 - /// cooperative threads parked in waitUntilExit). In a short-lived unit-test - /// process libdispatch spins up replacement threads for blocked workers, so - /// even the old blocking-on-the-pool code completes here. The real deadlock - /// built up over ~2 days under the @MainActor refresh loop and is confirmed - /// by the live `sample`, not by this test. Kept as a guard that the - /// off-pool wait + timeout path stays correct under concurrency. - @Test("concurrent timed-out processes all complete") - func concurrentTimedOutProcessesAllComplete() { + /// cooperative threads parked in waitUntilExit). The real deadlock built up + /// over ~2 days under the @MainActor refresh loop and is confirmed by the + /// live `sample`, not by this test. Kept as a guard that the off-pool wait + /// + timeout path stays correct under concurrency. + /// + /// The body must stay `async` and await the group directly. It used to + /// block on a `DispatchSemaphore` with a 15s deadline, on the claim that a + /// test body runs on a real thread. It does not: Swift Testing invokes even + /// synchronous test bodies from a task on the cooperative pool, so the wait + /// parked one of the pool's `activeProcessorCount` workers on the very work + /// it was waiting for. A 16-core dev box has slack, a 3-core CI runner does + /// not, and the wait expired with the group making no progress at all. + /// Keeping it `async` also lets the compiler reject the blocking wait, + /// which is unavailable from asynchronous contexts. + @Test("concurrent timed-out processes all complete", .timeLimit(.minutes(1))) + func concurrentTimedOutProcessesAllComplete() async { let count = ProcessInfo.processInfo.activeProcessorCount * 2 + 4 - let done = DispatchSemaphore(value: 0) - - Task { - await withTaskGroup(of: Void.self) { group in - for _ in 0.. [Int32?] in + for _ in 0..