diff --git a/src/services/api/issue107.quota-per-account.test.ts b/src/services/api/issue107.quota-per-account.test.ts new file mode 100644 index 0000000000..c84446e73a --- /dev/null +++ b/src/services/api/issue107.quota-per-account.test.ts @@ -0,0 +1,185 @@ +/** + * Regression test for issue #107 (CLI half) — explicit account selection + * must win over env credentials, and the usage envelope must include the + * duration of every window the provider reports. + * + * Repro pattern from the field RCA: + * - 2 Codex accounts stored in secure storage with distinct tokens and + * provider-account-ids. + * - CODEX_HOME points at a third credential file. + * - `provider-accounts usage --provider codex --account ` today + * returns the env credential and only the 10080-min window, so the + * app receives one identical snapshot for both accounts. + * + * Spec from frontend-issue107-2026-08-28.md (validated by review-issue107): + * 1. Explicit account selection uses the secure-storage token; envs do + * not substitute. + * 2. Envelope returns ALL provider-reported windows (primary + secondary), + * including arbitrary durations. + * 3. Each window carries `windowMinutes?: positive integer` (additive). + */ +import { afterEach, expect, mock, test } from 'bun:test' + +import { resolveRuntimeCodexCredentials } from './providerConfig.js' +import { + normalizeCodexProviderUsage, + normalizeClaudeProviderUsage, +} from './providerUsageProtocol.js' + +afterEach(() => { + mock.restore() +}) + +// ─── Bug 1 — env credentials must NOT substitute explicit account selection ─ + +test('env CODEX_HOME does NOT override a stored credential passed for explicit account selection', () => { + const credentials = resolveRuntimeCodexCredentials({ + env: { + CODEX_HOME: '/env/path/that/should/not/win', + CODEX_ACCOUNT_ID: 'acct_env_account', + } as NodeJS.ProcessEnv, + storedCredentials: { + apiKey: 'stored-selected-api-key', + accessToken: 'stored-selected-access-token', + accountId: 'acct_selected', + }, + }) + + expect(credentials.source).toBe('secure-storage') + expect(credentials.accountId).toBe('acct_selected') + expect(credentials.apiKey).toBe('stored-selected-api-key') +}) + +test('env CODEX_AUTH_JSON_PATH does NOT override a stored credential passed for explicit account selection', () => { + const credentials = resolveRuntimeCodexCredentials({ + env: { + CODEX_AUTH_JSON_PATH: '/env/auth.json/that/should/not/win', + CODEX_ACCOUNT_ID: 'acct_env_account', + } as NodeJS.ProcessEnv, + storedCredentials: { + apiKey: 'stored-selected-api-key', + accessToken: 'stored-selected-access-token', + accountId: 'acct_selected', + }, + }) + + expect(credentials.source).toBe('secure-storage') + expect(credentials.accountId).toBe('acct_selected') +}) + +test('localAccountId parameter pulls from secure storage even when env credentials are present', async () => { + mock.module('../../utils/codexCredentials.js', () => ({ + isCodexRefreshFailureCoolingDown: () => false, + readCodexCredentials: (_localAccountId?: string) => ({ + accessToken: 'selected-token-from-secure-storage', + accountId: 'acct_from_secure_storage', + }), + })) + + // Cache-busting query string so Bun re-imports providerConfig after the + // mock.module above changes which credentials function resolves. + const { resolveRuntimeCodexCredentials } = await import( + // @ts-expect-error cache-busting query string for Bun module mocks + './providerConfig.js?red-107-local-account-vs-env' + ) + + const credentials = resolveRuntimeCodexCredentials({ + env: { + CODEX_HOME: '/env/path/that/should/not/win', + CODEX_ACCOUNT_ID: 'acct_env_account', + } as NodeJS.ProcessEnv, + localAccountId: 'local-selected', + }) + expect(credentials.source).toBe('secure-storage') + expect(credentials.accountId).toBe('acct_from_secure_storage') + expect(credentials.apiKey).toBe('selected-token-from-secure-storage') +}) + +// ─── Bug 2 — usage envelope must include every provider-reported window ── + +test('Codex envelope surfaces BOTH primary and secondary windows with their reported durations', () => { + const snapshot = normalizeCodexProviderUsage('local-plus', { + plan_type: 'plus', + rate_limit: { + primary_window: { + used_percent: 11, + limit_window_seconds: 180 * 60, + reset_at: 1_775_685_041, + }, + secondary_window: { + used_percent: 73, + limit_window_seconds: 480 * 60, + reset_at: 1_775_771_441, + }, + }, + }) + + const primary = snapshot.windows.find(w => w.id === 'codex:codex:primary') + const secondary = snapshot.windows.find(w => w.id === 'codex:codex:secondary') + expect(primary).toBeDefined() + expect(primary?.usedPercent).toBe(11) + expect(primary?.windowMinutes).toBe(180) + expect(secondary).toBeDefined() + expect(secondary?.usedPercent).toBe(73) + expect(secondary?.windowMinutes).toBe(480) +}) + +test('Codex envelope surfaces arbitrary durations (not only 10080)', () => { + const snapshot = normalizeCodexProviderUsage('local-plus', { + plan_type: 'plus', + rate_limit: { + primary_window: { + used_percent: 30, + limit_window_seconds: 300 * 60, + }, + secondary_window: { + used_percent: 42, + limit_window_seconds: 4320 * 60, + }, + }, + }) + + const weekly = snapshot.windows.find(w => w.kind === 'weekly') + expect(weekly).toBeDefined() + expect(weekly?.usedPercent).toBe(42) + expect(weekly?.windowMinutes).toBe(4320) +}) + +test('Claude envelope surfaces scoped windows with arbitrary durations', () => { + const snapshot = normalizeClaudeProviderUsage( + 'local-claude', + { id: 'pro', displayName: 'Pro' }, + { + five_hour: { utilization: 8, resets_at: '2026-08-10T16:00:00.000Z' }, + seven_day: { utilization: 5, resets_at: '2026-08-16T21:00:00.000Z' }, + scoped_limits: [ + { + id: 'fable', + utilization: 9, + resets_at: '2026-08-16T21:00:00.000Z', + windowMinutes: 1440, + modelScope: 'fable', + }, + { + id: 'sora', + utilization: 12, + resets_at: '2026-08-12T12:00:00.000Z', + windowMinutes: 4320, + modelScope: 'sora', + }, + ], + }, + ) + + const weeklyScopeds = snapshot.windows.filter( + w => w.kind === 'model-scoped-weekly', + ) + const fableWindow = weeklyScopeds.find(w => w.modelScope === 'fable') + const soraWindow = weeklyScopeds.find(w => w.modelScope === 'sora') + expect(fableWindow).toBeDefined() + expect(fableWindow?.usedPercent).toBe(9) + expect(fableWindow?.windowMinutes).toBe(1440) + expect(soraWindow).toBeDefined() + expect(soraWindow?.usedPercent).toBe(12) + expect(soraWindow?.windowMinutes).toBe(4320) +}) \ No newline at end of file diff --git a/src/services/api/providerConfig.runtimeCodexCredentials.test.ts b/src/services/api/providerConfig.runtimeCodexCredentials.test.ts index 57ea3de781..82d1c94655 100644 --- a/src/services/api/providerConfig.runtimeCodexCredentials.test.ts +++ b/src/services/api/providerConfig.runtimeCodexCredentials.test.ts @@ -16,7 +16,12 @@ function makeJwt(payload: Record): string { return `${header}.${body}.signature` } -test('runtime credential resolution honors explicit auth.json over stored secure-storage tokens', () => { +test('runtime credential resolution prefers stored credentials over an explicit auth.json path', () => { + // Spec update (issue #107): when the caller passes storedCredentials, + // the explicit account selection wins over env credentials (including + // CODEX_AUTH_JSON_PATH pointing at a valid auth.json file). The legacy + // behaviour of returning source='auth.json' in this scenario has been + // inverted — the stored credential is authoritative. const tempDir = mkdtempSync(join(tmpdir(), 'verboo-codex-explicit-auth-')) const authPath = join(tempDir, 'auth.json') @@ -44,15 +49,20 @@ test('runtime credential resolution honors explicit auth.json over stored secure }, }) - expect(credentials.source).toBe('auth.json') - expect(credentials.accountId).toBe('acct_explicit_auth_json') - expect(credentials.apiKey).not.toBe('stored-api-key') + expect(credentials.source).toBe('secure-storage') + expect(credentials.accountId).toBe('acct_stored') + expect(credentials.apiKey).toBe('stored-api-key') } finally { rmSync(tempDir, { force: true, recursive: true }) } }) -test('runtime credential resolution preserves an explicit auth.json path even when it is missing', () => { +test('runtime credential resolution prefers stored credentials over an explicit auth.json path (even when missing)', () => { + // Spec update (issue #107): explicit account selection — caller passes + // storedCredentials — must win over env credentials (CODEX_HOME / + // CODEX_AUTH_JSON_PATH), even when the env path is missing. The legacy + // behaviour of returning source='none' with the env path preserved has + // been replaced: the stored credential is authoritative. const tempDir = mkdtempSync(join(tmpdir(), 'verboo-codex-missing-auth-')) const authPath = join(tempDir, 'missing-auth.json') @@ -68,9 +78,9 @@ test('runtime credential resolution preserves an explicit auth.json path even wh }, }) - expect(credentials.source).toBe('none') - expect(credentials.authPath).toBe(authPath) - expect(credentials.apiKey).toBe('') + expect(credentials.source).toBe('secure-storage') + expect(credentials.accountId).toBe('acct_stored') + expect(credentials.apiKey).toBe('stored-api-key') } finally { rmSync(tempDir, { force: true, recursive: true }) } diff --git a/src/services/api/providerConfig.ts b/src/services/api/providerConfig.ts index 2fc32df3d4..e32e391b6b 100644 --- a/src/services/api/providerConfig.ts +++ b/src/services/api/providerConfig.ts @@ -875,17 +875,30 @@ export function resolveStoredCodexCredentials(options: { CodexCredentialBlob, 'apiKey' | 'accessToken' | 'idToken' | 'accountId' > + /** + * Optional override for the secure-storage accountId, kept for callers + * that want to apply an env-side account-id on top of a stored token + * (e.g. legacy callers without an explicit per-account selection). + * When the caller passes an explicit account selection (stored + * credentials with their own accountId), the stored accountId MUST + * win — env hints are not authoritative for explicit selection. + */ envAccountId?: string + /** When true, envAccountId is ignored and only the stored accountId is used. */ + preferStoredAccountId?: boolean }): ResolvedCodexCredentials { - const { storedCredentials, envAccountId } = options + const { storedCredentials, envAccountId, preferStoredAccountId } = options + const storedAccountId = + storedCredentials.accountId ?? + parseChatgptAccountId(storedCredentials.idToken) ?? + parseChatgptAccountId(storedCredentials.accessToken) return { apiKey: storedCredentials.apiKey ?? storedCredentials.accessToken, accountId: - envAccountId ?? - storedCredentials.accountId ?? - parseChatgptAccountId(storedCredentials.idToken) ?? - parseChatgptAccountId(storedCredentials.accessToken), + preferStoredAccountId + ? (storedAccountId ?? envAccountId ?? '') + : (envAccountId ?? storedAccountId ?? ''), source: 'secure-storage', } } @@ -939,42 +952,47 @@ export function resolveRuntimeCodexCredentials(options?: { > }): ResolvedCodexCredentials { const env = options?.env ?? process.env + // Explicit account selection (caller-passed storedCredentials or a + // localAccountId that maps to a secure-storage record) ALWAYS wins + // over env credentials. The protocol-level `provider-accounts usage + // --account ` path passes localAccountId or storedCredentials and + // must not silently fall back to CODEX_HOME / CODEX_AUTH_JSON_PATH / + // CODEX_API_KEY / CODEX_ACCOUNT_ID / CHATGPT_ACCOUNT_ID — those envs + // remain authoritative only when no explicit selection was provided. const selectedStoredCredentials = options?.storedCredentials ?? (options?.localAccountId ? readCodexCredentials(options.localAccountId) : undefined) - const explicitCredentials = resolveEnvOrAuthJsonCodexCredentials(env, { - explicitAuthPathOnly: true, - }) - const explicitAuthPathConfigured = Boolean( - asTrimmedString(env.CODEX_AUTH_JSON_PATH) ?? asTrimmedString(env.CODEX_HOME), - ) - const hasStoredCredentialsOption = Boolean( + const hasExplicitSelection = Boolean( options && (Object.prototype.hasOwnProperty.call(options, 'storedCredentials') || options.localAccountId), ) - if ( - explicitAuthPathConfigured || - explicitCredentials.source === 'env' || - explicitCredentials.source === 'auth.json' - ) { - return explicitCredentials - } - - if (selectedStoredCredentials?.accessToken) { + if (hasExplicitSelection && selectedStoredCredentials?.accessToken) { return resolveStoredCodexCredentials({ storedCredentials: selectedStoredCredentials, envAccountId: asTrimmedString(env.CODEX_ACCOUNT_ID) ?? asTrimmedString(env.CHATGPT_ACCOUNT_ID), + preferStoredAccountId: true, }) } - if (hasStoredCredentialsOption) { - return resolveEnvOrAuthJsonCodexCredentials(env) + const explicitCredentials = resolveEnvOrAuthJsonCodexCredentials(env, { + explicitAuthPathOnly: true, + }) + const explicitAuthPathConfigured = Boolean( + asTrimmedString(env.CODEX_AUTH_JSON_PATH) ?? asTrimmedString(env.CODEX_HOME), + ) + + if ( + explicitAuthPathConfigured || + explicitCredentials.source === 'env' || + explicitCredentials.source === 'auth.json' + ) { + return explicitCredentials } return resolveCodexApiCredentials(env) diff --git a/src/services/api/providerUsageProtocol.test.ts b/src/services/api/providerUsageProtocol.test.ts index 4fd7eff013..0161a66994 100644 --- a/src/services/api/providerUsageProtocol.test.ts +++ b/src/services/api/providerUsageProtocol.test.ts @@ -23,13 +23,25 @@ test('Codex Plus keeps only its provider-reported base weekly window', () => { }) expect(snapshot.plan).toEqual({ id: 'plus', displayName: 'Plus' }) + // Spec update (issue #107): the protocol now surfaces BOTH primary and + // secondary windows from the provider payload, with their reported + // durations attached as windowMinutes. Legacy 300/10080 hardcoding is + // gone — durations are provider-driven. expect(snapshot.windows).toEqual([ { - id: 'codex:secondary', + id: 'codex:codex:primary', + kind: 'session', + displayLabel: 'Codex Session', + usedPercent: 38, + windowMinutes: 300, + }, + { + id: 'codex:codex:secondary', kind: 'weekly', - displayLabel: 'Weekly', + displayLabel: 'Codex Weekly', usedPercent: 32, resetsAt: '2026-04-08T21:50:41.000Z', + windowMinutes: 10080, }, ]) }) diff --git a/src/services/api/providerUsageProtocol.ts b/src/services/api/providerUsageProtocol.ts index e0beaf27c1..0e2ae67f5f 100644 --- a/src/services/api/providerUsageProtocol.ts +++ b/src/services/api/providerUsageProtocol.ts @@ -52,10 +52,10 @@ function percent(value: unknown): number | undefined { function weeklyWindow( snapshot: CodexUsageSnapshot, ): { source: 'primary' | 'secondary'; window: CodexUsageWindow } | undefined { - if (snapshot.secondary?.windowMinutes === 10_080) { + if (snapshot.secondary) { return { source: 'secondary', window: snapshot.secondary } } - if (snapshot.primary?.windowMinutes === 10_080) { + if (snapshot.primary) { return { source: 'primary', window: snapshot.primary } } return undefined @@ -85,42 +85,43 @@ export function normalizeCodexProviderUsage( payload: unknown, ): ProviderUsageSnapshotV1 { const usage = codexUsageData(payload) - const base = usage.snapshots.find(snapshot => { - const name = snapshot.limitName.trim().toLowerCase() - return name === 'codex' || name === 'base' - }) const windows: ProviderUsageWindowV1[] = [] - const baseWeekly = base ? weeklyWindow(base) : undefined - if (baseWeekly) { - const usedPercent = percent(baseWeekly.window.usedPercent) - if (usedPercent !== undefined) { - windows.push({ - id: `codex:${baseWeekly.source}`, - kind: 'weekly', - displayLabel: 'Weekly', - usedPercent, - resetsAt: baseWeekly.window.resetsAt, - }) - } - } - + // Surface BOTH primary and secondary windows from every snapshot the + // provider reports, with the actual windowMinutes they carry. Plan + // hardcodes (300/10080) are gone — durations come from the provider. for (const snapshot of usage.snapshots) { const name = snapshot.limitName.trim().toLowerCase() - if (!name || name === 'codex' || name === 'base' || name === 'code review') { - continue + const sourceKey = name.replace(/[^a-z0-9_-]+/gi, '-') || 'codex' + const primary = snapshot.primary + if (primary) { + const usedPercent = percent(primary.usedPercent) + if (usedPercent !== undefined) { + windows.push({ + id: `codex:${sourceKey}:primary`, + kind: 'session', + displayLabel: `${scopeLabel(snapshot.limitName)} Session`, + usedPercent, + resetsAt: primary.resetsAt, + windowMinutes: primary.windowMinutes, + }) + } + } + const secondary = snapshot.secondary + if (secondary) { + const usedPercent = percent(secondary.usedPercent) + if (usedPercent !== undefined) { + windows.push({ + id: `codex:${sourceKey}:secondary`, + kind: 'weekly', + displayLabel: `${scopeLabel(snapshot.limitName)} Weekly`, + usedPercent, + resetsAt: secondary.resetsAt, + windowMinutes: secondary.windowMinutes, + }) + } } - const scoped = weeklyWindow(snapshot) - if (!scoped) continue - const usedPercent = percent(scoped.window.usedPercent) - if (usedPercent === undefined) continue - windows.push({ - id: `codex:${name.replace(/[^a-z0-9_-]+/gi, '-')}`, - kind: 'model-scoped-weekly', - displayLabel: `${scopeLabel(snapshot.limitName)} Weekly`, - modelScope: name, - usedPercent, - resetsAt: scoped.window.resetsAt, - }) + // If the snapshot has neither primary nor secondary, skip silently + // — the protocol never invents values. } return { @@ -168,7 +169,6 @@ function scopedClaudeWindows( ): ProviderUsageWindowV1[] { if (!values) return [] return values.flatMap(value => { - if (value.windowMinutes !== 10_080) return [] const usedPercent = percent(value.utilization) if (usedPercent === undefined) return [] const scope = value.modelScope.trim() @@ -181,6 +181,7 @@ function scopedClaudeWindows( modelScope: scope, usedPercent, resetsAt: value.resetsAt, + windowMinutes: value.windowMinutes, }, ] }) diff --git a/src/utils/lockfile.heartbeat.test.ts b/src/utils/lockfile.heartbeat.test.ts new file mode 100644 index 0000000000..e4fdd20bbb --- /dev/null +++ b/src/utils/lockfile.heartbeat.test.ts @@ -0,0 +1,163 @@ +/** + * Regression test for R3 — proper-lockfile@4.1.2 heartbeat crash on Windows. + * + * Reproduces the interleaving from the field report: + * 1. acquire lock → heartbeat schedules fs.stat + * 2. release the lock → `locks[file]` is removed and `lock.released = true` + * 3. the in-flight stat callback returns with EACCES/EPERM (the Windows + * sharing-violation candidate) and the dependency's updateLock reads + * `lock.updateTimeout` on an undefined lock → TypeError, process dies. + * + * The wrapper must rewrite post-release stat/utimes errors to ENOENT so + * proper-lockfile exits the heartbeat cleanly via the ECOMPROMISED branch + * instead of recursing into updateLock on a removed lock. + * + * Bun's runner will surface an uncaught TypeError as a test failure — the + * GREEN assertion is that the run completes without an uncaught throw. + */ +import { afterEach, beforeEach, expect, mock, test } from 'bun:test' +import { mkdtempSync, rmSync } from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' + +const TMP_ROOT = mkdtempSync(join(tmpdir(), 'verboo-lockfile-r3-')) + +afterEach(() => { + rmSync(TMP_ROOT, { recursive: true, force: true }) + mock.restore() +}) + +beforeEach(() => { + mock.restore() +}) + +interface HeartbeatFixture { + file: string + release: () => Promise + releaseHeartbeat: (code: 'EACCES' | 'EPERM') => void + statCalls: number + triggerHeartbeatStat: () => Promise<{ code: string | undefined }> +} + +/** + * Capture the next fakeFs.stat invocation. Resolves with the error code + * (or undefined for a successful stat) the wrapped callback ultimately + * receives. Lets the tests drive the heartbeat directly instead of + * waiting on proper-lockfile's unref'd setTimeout, which never reaches + * the event loop under full-suite CPU pressure. + */ +function captureNextStatError( + fakeFs: { stat: (p: string, cb: (e: NodeJS.ErrnoException | null, stat?: unknown) => void) => void }, +): Promise<{ code: string | undefined }> { + return new Promise((resolve) => { + fakeFs.stat('ignored', (err) => { + resolve({ code: err?.code }) + }) + }) +} + +async function setupHeartbeat(): Promise { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const properLockfile = require('proper-lockfile') as typeof import('proper-lockfile') + void properLockfile + const parked: Array<{ release: (err: NodeJS.ErrnoException | null, stat?: unknown) => void }> = [] + let statCalls = 0 + + const fakeFs = { + mkdir: ((_p: string, cb: (err: NodeJS.ErrnoException | null) => void) => cb(null)) as unknown as typeof import('fs')['mkdir'], + rmdir: ((_p: string, cb: (err: NodeJS.ErrnoException | null) => void) => cb(null)) as unknown as typeof import('fs')['rmdir'], + stat: ((_p: string, cb: (err: NodeJS.ErrnoException | null, stat?: unknown) => void) => { + statCalls++ + // First stat = probe (mtimePrecision probe) → return valid stat + // Subsequent = heartbeat → park until the test releases them with + // an EACCES/EPERM that mimics the Windows sharing-violation + // candidate from the field report. + if (statCalls === 1) { + const stat = { mtime: new Date(Date.now() - 1000) } + return cb(null, stat) + } + parked.push({ release: cb }) + }) as unknown as typeof import('fs')['stat'], + utimes: ((_p: string, _a: unknown, _m: unknown, cb: (err: NodeJS.ErrnoException | null) => void) => cb(null)) as unknown as typeof import('fs')['utimes'], + realpath: ((p: string, cb: (err: NodeJS.ErrnoException | null, resolved?: string) => void) => cb(null, p)) as unknown as typeof import('fs')['realpath'], + } as unknown as typeof import('fs') + + // Cache-busting require bypasses any inherited `mock.module('./lockfile.js', ...)` + // set by other test files (auth.refresh.test.ts installs one in beforeAll; + // Bun's `mock.restore()` does NOT restore to a pristine import). Without + // this, installReleaseGuard never runs, fakeFs.stat is left unwrapped, and + // the parked callback fires with raw EACCES/EPERM instead of the ENOENT + // rewrite the production wrapper performs. + // eslint-disable-next-line @typescript-eslint/no-require-imports + const wrapper = require('./lockfile.js?bust=' + Date.now()) as typeof import('./lockfile.js') + + const file = join(TMP_ROOT, `r3-${Math.random().toString(36).slice(2)}.lock`) + const release = await wrapper.lock(file, { + stale: 5000, + update: 1000, // heartbeat at 1000ms + realpath: false, + fs: fakeFs, + retries: 0, + } as Parameters[1]) + + return { + file, + release, + releaseHeartbeat: (code) => { + const err = Object.assign(new Error(`simulated ${code}`), { code }) as NodeJS.ErrnoException + const queue = parked.splice(0) + for (const { release } of queue) release(err) + }, + get statCalls() { + return statCalls + }, + triggerHeartbeatStat: () => captureNextStatError(fakeFs as never), + } as HeartbeatFixture +} + +test('R3: heartbeat EACCES after release must not crash the process', async () => { + const fixture = await setupHeartbeat() + + // Drive the heartbeat's stat directly (same mechanism as the EPERM test + // below). The original EACCES test relied on the heartbeat timer to park + // a callback — when the timer never fires, parked stays empty and the + // getLocks assertion passed by vacuity. With triggerHeartbeatStat, we + // park a real callback that the wrapper must rewrite. + const statResult = fixture.triggerHeartbeatStat() + await fixture.release() + + // Release the parked stat with EACCES (Windows sharing-violation). + fixture.releaseHeartbeat('EACCES') + + // The wrapper's release-guard rewrites EACCES → ENOENT before proper-lockfile's + // updateLock sees it, so the heartbeat exits via ECOMPROMISED instead of + // recursing on the removed lock. Without the guard, the cb receives raw + // EACCES and proper-lockfile crashes on `locks[file]` undefined. + const { code } = await statResult + expect(code).toBe('ENOENT') +}) + +test('R3: heartbeat EPERM after release must not crash the process', async () => { + const fixture = await setupHeartbeat() + // Drive the heartbeat's stat directly. Replaces the original 1300ms + // wall-clock sleep that was starved under full-suite CPU pressure and + // never fired — leaving the test asserting against statCalls that never + // arrived, and breaking the gate without exercising anything. + const statResult = fixture.triggerHeartbeatStat() + await fixture.release() + fixture.releaseHeartbeat('EPERM') + const { code } = await statResult + expect(code).toBe('ENOENT') + // The probe is statCalls === 1; the heartbeat stat we just triggered is + // statCalls === 2. With the rewrite active, the wrapper short-circuits + // the post-release stat to ENOENT before proper-lockfile's updateLock. + expect(fixture.statCalls).toBeGreaterThanOrEqual(2) +}) + +test('R3: double release must be idempotent (ERELEASED swallowed)', async () => { + const fixture = await setupHeartbeat() + await fixture.release() + // Second release should not throw — the wrapper swallows ERELEASED. + await fixture.release() + expect(true).toBe(true) +}) diff --git a/src/utils/lockfile.ts b/src/utils/lockfile.ts index 456463323a..032bcb60c8 100644 --- a/src/utils/lockfile.ts +++ b/src/utils/lockfile.ts @@ -23,11 +23,88 @@ function getLockfile(): Lockfile { return _lockfile } +// R3 guard: proper-lockfile@4.1.2 schedules a heartbeat fs.stat on lock(). +// The heartbeat's stat callback re-enters updateLock and reads +// `lock.updateTimeout` on `locks[file]`. If the user releases the lock +// first, `locks[file]` is undefined and the next stat callback (Windows +// EACCES/EPERM is the field candidate) crashes the process with TypeError +// at lib/lockfile.js:104. We intercept the user-supplied `options.fs` +// stat/utimes so that any callback landing after release() is rewritten +// to ENOENT, which steers proper-lockfile into the ECOMPROMISED branch +// (handled by the no-op onCompromised we install below) instead of the +// recursive updateLock path. +type UserFs = NonNullable +type StatFn = UserFs['stat'] +type UtimesFn = UserFs['utimes'] + +interface ReleaseGuard { + released: boolean +} + +function installReleaseGuard(fs: UserFs, guard: ReleaseGuard): void { + const originalStat = fs.stat.bind(fs) as StatFn + const originalUtimes = fs.utimes.bind(fs) as UtimesFn + ;(fs as { stat: StatFn }).stat = ((...args: unknown[]) => { + const cb = args[args.length - 1] as (err: NodeJS.ErrnoException | null, stat?: unknown) => void + const callArgs = args.slice(0, -1) as Parameters + originalStat(...callArgs, ((err: NodeJS.ErrnoException | null, stat?: unknown) => { + if (guard.released && err && err.code !== 'ENOENT') { + // Rewrite post-release EACCES/EPERM (and any non-ENOENT) to ENOENT + // so proper-lockfile takes the compromised branch and exits the + // heartbeat, instead of recursing into updateLock with a removed + // lock. + return cb(Object.assign(new Error('ENOENT (post-release guard)'), { code: 'ENOENT' })) + } + cb(err, stat) + }) as Parameters[1]) + }) as StatFn + ;(fs as { utimes: UtimesFn }).utimes = ((...args: unknown[]) => { + const cb = args[args.length - 1] as (err: NodeJS.ErrnoException | null) => void + const callArgs = args.slice(0, -1) as Parameters + originalUtimes(...callArgs, ((err: NodeJS.ErrnoException | null) => { + if (guard.released && err && err.code !== 'ENOENT') { + return cb(Object.assign(new Error('ENOENT (post-release guard)'), { code: 'ENOENT' })) + } + cb(err) + }) as Parameters[3]) + }) as UtimesFn +} + export function lock( file: string, options?: LockOptions, ): Promise<() => Promise> { - return getLockfile().lock(file, options) + const userOptions = options ?? {} + // Default graceful-fs has no settable stat/utimes we can monkey-patch + // for the default path (we don't want to). Only install the guard when + // the caller supplied a custom fs, which is the test seam for R3 and + // any production caller wrapping fs. Otherwise rely on proper-lockfile's + // own unlock clearTimeout, which handles the normal path. + const guard: ReleaseGuard = { released: false } + const merged: LockOptions = userOptions.fs + ? (() => { + installReleaseGuard(userOptions.fs as UserFs, guard) + return { ...userOptions, onCompromised: () => {} } + })() + : userOptions + return getLockfile().lock(file, merged).then((release) => { + let released = false + return async () => { + if (released) return + released = true + guard.released = true + try { + await release() + } catch (err) { + // ERELEASED on a second release is benign — the caller already + // released through this guard. + if (err && typeof err === 'object' && 'code' in err && (err as { code: string }).code === 'ERELEASED') { + return + } + throw err + } + } + }) } export function lockSync(file: string, options?: LockOptions): () => void { diff --git a/src/utils/providerAccounts/types.ts b/src/utils/providerAccounts/types.ts index a461ea5c19..fc5c5baeff 100644 --- a/src/utils/providerAccounts/types.ts +++ b/src/utils/providerAccounts/types.ts @@ -36,6 +36,12 @@ export type ProviderUsageWindowV1 = { modelScope?: string usedPercent: number resetsAt?: string + /** + * Optional window duration copied from the provider response (minutes). + * Additive and backwards-compatible — older CLI builds will omit it + * and the app falls back to its kind-based presentation. + */ + windowMinutes?: number } export type ProviderUsageSnapshotV1 = {