Merge pull request #650 from getagentseal/fix/isolate-provider-discovery

Isolate a throwing provider so one bad file can't blank every scan

src/providers/index.ts

@@ -226,14 +226,40 @@ export async function getAllProviders(): Promise<Provider[]> {
 
 export const providers = coreProviders
 
-export async function discoverAllSessions(providerFilter?: string): Promise<SessionSource[]> {
-  const allProviders = await getAllProviders()
+// Isolate one provider's discovery. A provider that throws (a crafted/corrupt
+// file reaching a string op, an unexpected on-disk shape) must never take down
+// the whole scan and blank every other provider's usage. Warn once per
+// provider per run, then skip it. Mirrors the parse-failure isolation already
+// used per-file in parser.ts.
+const warnedDiscoveryFailures = new Set<string>()
+export async function safeDiscoverSessions(provider: Provider): Promise<SessionSource[]> {
+  try {
+    return await provider.discoverSessions()
+  } catch (err) {
+    if (!warnedDiscoveryFailures.has(provider.name)) {
+      warnedDiscoveryFailures.add(provider.name)
+      const msg = err instanceof Error ? err.message : String(err)
+      process.stderr.write(
+        `codeburn: skipped ${provider.name} discovery after an error: ${msg}\n`
+      )
+    }
+    return []
+  }
+}
+
+export async function discoverAllSessions(
+  providerFilter?: string,
+  // Injectable for tests so the isolation loop itself is exercised, not just
+  // the helper. Defaults to the real registry.
+  providerList?: Provider[],
+): Promise<SessionSource[]> {
+  const allProviders = providerList ?? await getAllProviders()
   const filtered = providerFilter && providerFilter !== 'all'
     ? allProviders.filter(p => p.name === providerFilter)
     : allProviders
   const all: SessionSource[] = []
   for (const provider of filtered) {
-    const sessions = await provider.discoverSessions()
+    const sessions = await safeDiscoverSessions(provider)
     all.push(...sessions)
   }
   return all

src/usage-aggregator.ts

@@ -3,7 +3,7 @@ import { CATEGORY_LABELS, type ProjectSummary, type TaskCategory, type DateRange
 import { type PeriodData, type ProviderCost, type BreakdownArrays, type MenubarPayload, buildMenubarPayload } from './menubar-json.js'
 import { parseAllSessions, filterProjectsByName, filterProjectsByDays } from './parser.js'
 import { findUnpricedModels, getLocalModelSavingsConfigHash, getPriceOverridesConfigHash, getShortModelName } from './models.js'
-import { getAllProviders } from './providers/index.js'
+import { getAllProviders, safeDiscoverSessions } from './providers/index.js'
 import { aggregateProjectsIntoDays, buildPeriodDataFromDays } from './day-aggregator.js'
 import { aggregateModelEfficiency } from './model-efficiency.js'
 import { aggregateModels } from './models-report.js'
@@ -200,7 +200,7 @@ export async function buildMenubarPayloadForRange(periodInfo: PeriodInfo, opts:
     }
     for (const p of allProviders) {
       if (providers.some(pc => pc.name === p.displayName)) continue
-      const sources = await p.discoverSessions()
+      const sources = await safeDiscoverSessions(p)
       if (sources.length > 0) providers.push({ name: p.displayName, cost: 0 })
     }
   } else {

tests/provider-registry.test.ts

@@ -1,5 +1,16 @@
-import { describe, it, expect } from 'vitest'
-import { providers, getAllProviders, getProvider } from '../src/providers/index.js'
+import { describe, it, expect, vi } from 'vitest'
+import { providers, getAllProviders, getProvider, safeDiscoverSessions, discoverAllSessions } from '../src/providers/index.js'
+import type { Provider } from '../src/providers/types.js'
+
+function fakeProvider(name: string, discover: Provider['discoverSessions']): Provider {
+  return {
+    name,
+    displayName: name,
+    modelDisplayName: (m: string) => m,
+    toolDisplayName: (t: string) => t,
+    discoverSessions: discover,
+  } as unknown as Provider
+}
 
 describe('provider registry', () => {
   it('has core providers registered synchronously', () => {
@@ -117,4 +128,44 @@ describe('provider registry', () => {
     expect(cursor.modelDisplayName('grok-code-fast-1')).toBe('Grok Code Fast')
     expect(cursor.modelDisplayName('unknown-model')).toBe('unknown-model')
   })
+
+  describe('provider-discovery isolation', () => {
+    it('safeDiscoverSessions returns [] and warns once instead of propagating', async () => {
+      const warn = vi.spyOn(process.stderr, 'write').mockReturnValue(true)
+      const boom = fakeProvider('boom-helper', async () => { throw new Error('crafted file blew up') })
+      try {
+        await expect(safeDiscoverSessions(boom)).resolves.toEqual([])
+        expect(warn.mock.calls.length).toBeGreaterThanOrEqual(1)
+        expect(String(warn.mock.calls[0]![0])).toContain('boom-helper')
+        // Deduped on repeat within the same run: no additional warning.
+        const afterFirst = warn.mock.calls.length
+        await safeDiscoverSessions(boom)
+        expect(warn.mock.calls.length).toBe(afterFirst)
+      } finally {
+        warn.mockRestore()
+      }
+    })
+
+    it('discoverAllSessions drops a throwing provider but keeps the healthy ones', async () => {
+      const warn = vi.spyOn(process.stderr, 'write').mockReturnValue(true)
+      const boom = fakeProvider('boom-loop', async () => { throw new Error('kaboom') })
+      const ok1 = fakeProvider('ok1', async () => [{ path: '/a.jsonl', project: 'p1', provider: 'ok1' }])
+      const ok2 = fakeProvider('ok2', async () => [{ path: '/b.jsonl', project: 'p2', provider: 'ok2' }])
+      try {
+        // A throwing provider in the middle must not abort the loop.
+        const sources = await discoverAllSessions('all', [ok1, boom, ok2])
+        expect(sources.map(s => s.path)).toEqual(['/a.jsonl', '/b.jsonl'])
+        expect(warn.mock.calls.some(c => String(c[0]).includes('boom-loop'))).toBe(true)
+      } finally {
+        warn.mockRestore()
+      }
+    })
+
+    it('discoverAllSessions honors the provider filter', async () => {
+      const ok1 = fakeProvider('keep', async () => [{ path: '/keep.jsonl', project: 'k', provider: 'keep' }])
+      const ok2 = fakeProvider('drop', async () => [{ path: '/drop.jsonl', project: 'd', provider: 'drop' }])
+      const sources = await discoverAllSessions('keep', [ok1, ok2])
+      expect(sources.map(s => s.path)).toEqual(['/keep.jsonl'])
+    })
+  })
 })