From 3cc9df27d7aab54213fb6f356d3c45ae5be2eba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 18 Aug 2026 05:08:48 +0000 Subject: [PATCH] a failed usage write must not discard the reading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the panel rendered "Not signed in" over an account that was signed in fine. cause: `snapshotClaudeUsage` had the read and the write in ONE try block, so on a machine that had not run `db:push` the upsert threw on a missing `ai_provider_usage`, the catch returned null, and a perfectly good reading went in the bin. the route then merged null into an empty stored list and answered with no accounts at all. which makes the claim in docs/ai-usage-panel.md — "degrades to no tabs rather than a 500" — false as written. the stored half is an ENHANCEMENT: it buys tabs for accounts that cannot be measured, and an enhancement that fails must cost only itself. so the write gets its own try and the reading is returned either way. observed on this machine rather than reasoned about: to_regclass came back null, and the regression test fails against the old shape (verified by reverting it, not by assuming). Co-Authored-By: Claude Opus 5 --- .../api/chat/usage-store-degradation.test.ts | 48 +++++++++++++++ src/servers/api/chat/usage-store.ts | 61 ++++++++++++------- 2 files changed, 87 insertions(+), 22 deletions(-) create mode 100644 src/servers/api/chat/usage-store-degradation.test.ts diff --git a/src/servers/api/chat/usage-store-degradation.test.ts b/src/servers/api/chat/usage-store-degradation.test.ts new file mode 100644 index 00000000..e3e58a3b --- /dev/null +++ b/src/servers/api/chat/usage-store-degradation.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, test, mock } from 'bun:test'; + +/** + * A store failure must not lose the reading. + * + * These were one `try` block until 2026-08-18, so an install that had not run `db:push` had no + * `ai_provider_usage` table, the upsert threw, and the panel rendered "Not signed in" over an account that + * was signed in perfectly well. Observed on this machine, not reasoned about. + */ +const READING = { + account: { email: 'owner@example.com', plan: 'Max 20x' }, + limits: [{ label: 'Session', percent: 21, resetsAt: '2026-08-18T08:00:00Z' }], + days: [{ date: '2026-08-18', tokens: 100 }], + models: [{ name: 'Opus 5', total: 100, input: 1, output: 1, cacheRead: 1, cacheWrite: 1 }], + fetchedAt: '2026-08-18T05:00:00.000Z', +}; + +mock.module('./claude-usage', () => ({ + readClaudeUsage: async () => READING, + readAccountEmail: async () => READING.account.email, +})); + +mock.module('officerdb', () => ({ + recordProviderUsage: async () => { + throw new Error('relation "ai_provider_usage" does not exist'); + }, + listProviderUsage: async () => { + throw new Error('relation "ai_provider_usage" does not exist'); + }, +})); + +const { snapshotClaudeUsage, listStoredUsage } = await import('./usage-store'); + +describe('a missing ai_provider_usage table', () => { + test('does not stop the signed-in account from being reported', () => { + return snapshotClaudeUsage(1).then((account) => { + expect(account).not.toBeNull(); + expect(account!.email).toBe('owner@example.com'); + expect(account!.plan).toBe('Max 20x'); + expect(account!.limits).toHaveLength(1); + expect(account!.current).toBe(true); + }); + }); + + test('degrades the stored list to empty rather than throwing', () => { + return listStoredUsage(1, 'owner@example.com').then((rows) => expect(rows).toEqual([])); + }); +}); diff --git a/src/servers/api/chat/usage-store.ts b/src/servers/api/chat/usage-store.ts index a265f8b0..cb7c9edb 100644 --- a/src/servers/api/chat/usage-store.ts +++ b/src/servers/api/chat/usage-store.ts @@ -54,20 +54,38 @@ export type UsageAccount = { * row on). */ export async function snapshotClaudeUsage(userId: number): Promise { + let usage: Awaited>; try { - const usage = await readClaudeUsage(); - const email = usage.account.email; - // No account means no key. A reading that cannot be attributed is worse than no reading: it would land - // under whichever account happened to be signed in last. - if (!email) return null; + usage = await readClaudeUsage(); + } catch (err) { + logger.warn('Could not read Claude usage', { userId, error: String(err) }); + return null; + } - const snapshot: UsageSnapshot = { - limits: usage.limits, - days: usage.days, - models: usage.models, - ...(usage.limitsProblem ? { limitsProblem: usage.limitsProblem } : {}), - }; + const email = usage.account.email; + // No account means no key. A reading that cannot be attributed is worse than no reading: it would land + // under whichever account happened to be signed in last. + if (!email) return null; + const snapshot: UsageSnapshot = { + limits: usage.limits, + days: usage.days, + models: usage.models, + ...(usage.limitsProblem ? { limitsProblem: usage.limitsProblem } : {}), + }; + + /* + The write is separate from the read, and its failure is not the read's failure. + + These were one `try` until 2026-08-18, and the consequence was immediate and total: a machine that had + not run `db:push` had no `ai_provider_usage` table, so the upsert threw, so a perfectly good reading was + discarded and the panel rendered "Not signed in" over an account that was signed in fine. The stored + half is an ENHANCEMENT — it buys tabs for accounts that cannot be measured — and an enhancement that + fails must cost only itself. + + Observed rather than reasoned about: the table was missing on this machine and the panel went blank. + */ + try { await recordProviderUsage({ userId, aiProvider: CLAUDE_PROVIDER, @@ -75,19 +93,18 @@ export async function snapshotClaudeUsage(userId: number): Promise