fix(web): reject invalid dashboard periods without exiting (#554)

* fix(web): reject invalid dashboard periods without exiting

* test(web): assert invalid periods return 400 without exiting; drop redundant /api/devices re-parse

- Add tests/web-dashboard.test.ts: boots the dashboard on an ephemeral port and asserts
  /api/usage and /api/devices answer 400 (not process.exit) for a bad period, and that
  the server keeps serving afterward. runWebDashboard now returns the http.Server so it
  can be exercised in-process; callers that ignore the return value are unaffected.
- /api/devices: resolve periodInfo once instead of validating then re-parsing it inside
  localGetUsage (pullDevices invokes localGetUsage with the same already-validated query).

---------

Co-authored-by: AgentSeal <hello@agentseal.org>

dash/src/App.tsx

@@ -1,5 +1,5 @@
 import { useEffect, useMemo, useState, type ReactNode } from 'react'
-import { keepPreviousData, useQuery, useQueryClient } from '@tanstack/react-query'
+import { useQuery, useQueryClient } from '@tanstack/react-query'
 
 import {
   approvePairing,
@@ -354,7 +354,6 @@ export function App() {
   const { data, isError, error, refetch } = useQuery({
     queryKey: ['devices', period, provider],
     queryFn: () => fetchDevices(period, provider),
-    placeholderData: keepPreviousData,
     // When devices are paired, re-pull periodically so a device that briefly
     // dropped (asleep/network blip) reappears on its own instead of staying
     // gone until you switch tabs.

src/cli-date.ts

@@ -31,20 +31,35 @@ export const PERIOD_LABELS: Record<Period, string> = {
 
 const VALID_PERIODS: ReadonlyArray<Period> = ['today', 'week', '30days', 'month', 'all']
 
-export function toPeriod(s: string): Period {
+export class UsageQueryError extends Error {
+  constructor(message: string) {
+    super(message)
+    this.name = 'UsageQueryError'
+  }
+}
+
+export function parsePeriodOrThrow(s: string): Period {
   if ((VALID_PERIODS as readonly string[]).includes(s)) return s as Period
-  // Fail loudly instead of silently coercing to 'week'. Previously a typo
-  // like `-p mounth` produced a quiet 7-day report and the user thought
-  // they were viewing the month.
-  process.stderr.write(
-    `codeburn: unknown period "${s}". Valid values: ${VALID_PERIODS.join(', ')}.\n`
-  )
-  process.exit(1)
+  throw new UsageQueryError(`Unknown period "${s}". Valid values: ${VALID_PERIODS.join(', ')}.`)
+}
+
+export function toPeriod(s: string): Period {
+  try {
+    return parsePeriodOrThrow(s)
+  } catch {
+    // Fail loudly instead of silently coercing to 'week'. Previously a typo
+    // like `-p mounth` produced a quiet 7-day report and the user thought
+    // they were viewing the month.
+    process.stderr.write(
+      `codeburn: unknown period "${s}". Valid values: ${VALID_PERIODS.join(', ')}.\n`
+    )
+    process.exit(1)
+  }
 }
 
 function parseLocalDate(s: string): Date {
   if (!ISO_DATE_RE.test(s)) {
-    throw new Error(`Invalid date format "${s}": expected YYYY-MM-DD`)
+    throw new UsageQueryError(`Invalid date format "${s}": expected YYYY-MM-DD`)
   }
   const [y, m, d] = s.split('-').map(Number) as [number, number, number]
   const date = new Date(y, m - 1, d)
@@ -53,7 +68,7 @@ function parseLocalDate(s: string): Date {
   // dated Feb 28 - Mar 2. Reject overflow so the user gets a loud error
   // instead of an off-by-N-days date range.
   if (date.getFullYear() !== y || date.getMonth() !== m - 1 || date.getDate() !== d) {
-    throw new Error(`Invalid date "${s}": ${m}/${d}/${y} is not a real calendar date`)
+    throw new UsageQueryError(`Invalid date "${s}": ${m}/${d}/${y} is not a real calendar date`)
   }
   return date
 }
@@ -118,7 +133,7 @@ export function parseDateRangeFlags(from: string | undefined, to: string | undef
   const end = endOfLocalDay(endDate)
 
   if (start > end) {
-    throw new Error(`--from must not be after --to (got ${from} > ${to})`)
+    throw new UsageQueryError(`--from must not be after --to (got ${from} > ${to})`)
   }
   return { start, end }
 }
@@ -198,3 +213,15 @@ export function parseDaysFlag(days: string | undefined): { days: Set<string>; ra
 export function formatDateRangeLabel(from: string | undefined, to: string | undefined): string {
   return `${from ?? 'all'} to ${to ?? 'today'}`
 }
+
+/** Resolve a usage query period for HTTP handlers without calling process.exit. */
+export function periodInfoFromQuery(
+  q: { period?: string; from?: string; to?: string },
+  defaultPeriod: string,
+): { range: DateRange; label: string } {
+  const customRange = parseDateRangeFlags(q.from, q.to)
+  if (customRange) {
+    return { range: customRange, label: formatDateRangeLabel(q.from, q.to) }
+  }
+  return getDateRange(parsePeriodOrThrow(q.period ?? defaultPeriod))
+}

src/sharing/share-run.ts

@@ -9,7 +9,7 @@ import { sanitizeForSharing } from './sanitize.js'
 import { getSharingDir, loadPeers, savePeers } from './store.js'
 import { loadPricing } from '../models.js'
 import { buildMenubarPayloadForRange } from '../usage-aggregator.js'
-import { getDateRange, parseDateRangeFlags, formatDateRangeLabel, toPeriod } from '../cli-date.js'
+import { periodInfoFromQuery } from '../cli-date.js'
 
 function lanAddress(): string | null {
   for (const list of Object.values(networkInterfaces())) {
@@ -32,10 +32,7 @@ export async function runShareServer(opts: { port: number; pair: boolean; always
   const peers = new PeerStore(await loadPeers(dir))
 
   const getUsage = async (q: UsageQuery): Promise<unknown> => {
-    const customRange = parseDateRangeFlags(q.from, q.to)
-    const periodInfo = customRange
-      ? { range: customRange, label: formatDateRangeLabel(q.from, q.to) }
-      : getDateRange(toPeriod(q.period ?? 'month'))
+    const periodInfo = periodInfoFromQuery(q, 'month')
     return sanitizeForSharing(await buildMenubarPayloadForRange(periodInfo, { provider: 'all', optimize: false }))
   }
 

src/sharing/share-server.ts

@@ -3,6 +3,7 @@ import type { IncomingMessage, ServerResponse } from 'http'
 import type { TLSSocket } from 'tls'
 import type { AddressInfo } from 'net'
 
+import { UsageQueryError } from '../cli-date.js'
 import { certFingerprint, pairingCode, PeerStore, PairingWindow } from './pairing.js'
 import type { Identity } from './identity.js'
 
@@ -82,7 +83,10 @@ export class ShareServer {
     } catch (err) {
       // Never leave a request hanging (a hung peer makes the caller time out
       // and drop this device); always answer, even on an internal error.
-      if (!res.headersSent) json(500, { error: err instanceof Error ? err.message : String(err) })
+      if (!res.headersSent) {
+        const message = err instanceof Error ? err.message : String(err)
+        json(err instanceof UsageQueryError ? 400 : 500, { error: message })
+      }
     }
   }
 

src/web-dashboard.ts

@@ -1,4 +1,4 @@
-import { createServer } from 'http'
+import { createServer, type Server } from 'http'
 import { exec } from 'child_process'
 import { readFile } from 'fs/promises'
 import { existsSync } from 'fs'
@@ -10,7 +10,7 @@ import { hostname } from 'os'
 
 import { loadPricing } from './models.js'
 import { buildMenubarPayloadForRange } from './usage-aggregator.js'
-import { getDateRange, parseDateRangeFlags, formatDateRangeLabel, toPeriod } from './cli-date.js'
+import { periodInfoFromQuery, UsageQueryError } from './cli-date.js'
 import { pullDevices, linkRemote } from './sharing/host.js'
 import { browse } from './sharing/discovery.js'
 import { loadOrCreateIdentity } from './sharing/identity.js'
@@ -31,6 +31,11 @@ function readBody(req: import('http').IncomingMessage): Promise<string> {
   })
 }
 
+function writeJsonError(res: import('http').ServerResponse, status: number, error: string): void {
+  res.writeHead(status, { 'content-type': 'application/json; charset=utf-8' })
+  res.end(JSON.stringify({ error }))
+}
+
 const HERE = dirname(fileURLToPath(import.meta.url))
 
 // Locate the built React dashboard (dist/dash). Works both when running from a
@@ -87,17 +92,14 @@ export async function runWebDashboard(opts: {
   exclude: string[]
   port: number
   open: boolean
-}): Promise<void> {
+}): Promise<Server> {
   await loadPricing()
   const dashDir = resolveDashDir()
 
   // Sharing this device serves the SANITIZED aggregate (no project names/paths
   // or per-session detail), unlike the local /api/usage which shows everything.
   const shareGetUsage = async (q: { period?: string; from?: string; to?: string }) => {
-    const customRange = parseDateRangeFlags(q.from, q.to)
-    const periodInfo = customRange
-      ? { range: customRange, label: formatDateRangeLabel(q.from, q.to) }
-      : getDateRange(toPeriod(q.period ?? opts.period))
+    const periodInfo = periodInfoFromQuery(q, opts.period)
     return sanitizeForSharing(await buildMenubarPayloadForRange(periodInfo, { provider: 'all', optimize: false }))
   }
   const share = new ShareController(shareGetUsage)
@@ -126,10 +128,14 @@ export async function runWebDashboard(opts: {
         const provider = url.searchParams.get('provider') ?? opts.provider
         const from = url.searchParams.get('from') ?? opts.from
         const to = url.searchParams.get('to') ?? opts.to
-        const customRange = parseDateRangeFlags(from, to)
-        const periodInfo = customRange
-          ? { range: customRange, label: formatDateRangeLabel(from, to) }
-          : getDateRange(toPeriod(period))
+        let periodInfo
+        try {
+          periodInfo = periodInfoFromQuery({ period, from, to }, opts.period)
+        } catch (err) {
+          if (!(err instanceof UsageQueryError)) throw err
+          writeJsonError(res, 400, err instanceof Error ? err.message : String(err))
+          return
+        }
         const payload = await buildMenubarPayloadForRange(periodInfo, {
           provider,
           project: opts.project,
@@ -148,13 +154,16 @@ export async function runWebDashboard(opts: {
         const provider = url.searchParams.get('provider') ?? opts.provider
         const from = url.searchParams.get('from') ?? opts.from
         const to = url.searchParams.get('to') ?? opts.to
-        const localGetUsage = async (q: { period?: string; from?: string; to?: string }) => {
-          const customRange = parseDateRangeFlags(q.from, q.to)
-          const periodInfo = customRange
-            ? { range: customRange, label: formatDateRangeLabel(q.from, q.to) }
-            : getDateRange(toPeriod(q.period ?? period))
-          return buildMenubarPayloadForRange(periodInfo, { provider, project: opts.project, exclude: opts.exclude, optimize: false })
+        let periodInfo
+        try {
+          periodInfo = periodInfoFromQuery({ period, from, to }, opts.period)
+        } catch (err) {
+          if (!(err instanceof UsageQueryError)) throw err
+          writeJsonError(res, 400, err instanceof Error ? err.message : String(err))
+          return
         }
+        const localGetUsage = async () =>
+          buildMenubarPayloadForRange(periodInfo, { provider, project: opts.project, exclude: opts.exclude, optimize: false })
         const results = await pullDevices(localGetUsage, { period, from, to }, hostname(), {})
         res.writeHead(200, { 'content-type': 'application/json; charset=utf-8', 'cache-control': 'no-store' })
         res.end(JSON.stringify({ devices: results }))
@@ -302,6 +311,8 @@ export async function runWebDashboard(opts: {
     void share.stop().finally(() => process.exit(0))
   })
 
+  return server
+
   await new Promise<never>(() => {
     /* run until interrupted */
   })

tests/cli-date.test.ts

@@ -3,6 +3,8 @@ import {
   getDateRange,
   PERIODS,
   PERIOD_LABELS,
+  parsePeriodOrThrow,
+  periodInfoFromQuery,
   toPeriod,
   type Period,
 } from '../src/cli-date.js'
@@ -104,6 +106,42 @@ describe('PERIODS / PERIOD_LABELS', () => {
   })
 })
 
+describe('parsePeriodOrThrow', () => {
+  it('round-trips known periods', () => {
+    const known: Period[] = ['today', 'week', '30days', 'month', 'all']
+    for (const p of known) {
+      expect(parsePeriodOrThrow(p)).toBe(p)
+    }
+  })
+
+  it('throws on unknown input without calling process.exit', () => {
+    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('exit') })
+    try {
+      expect(() => parsePeriodOrThrow('garbage')).toThrow(/Unknown period "garbage"/)
+      expect(exitSpy).not.toHaveBeenCalled()
+    } finally {
+      exitSpy.mockRestore()
+    }
+  })
+})
+
+describe('periodInfoFromQuery', () => {
+  it('resolves a named period', () => {
+    const info = periodInfoFromQuery({ period: 'week' }, 'month')
+    expect(info.label).toBe('Last 7 Days')
+  })
+
+  it('throws for an invalid period without calling process.exit', () => {
+    const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('exit') })
+    try {
+      expect(() => periodInfoFromQuery({ period: 'garbage' }, 'month')).toThrow(/Unknown period "garbage"/)
+      expect(exitSpy).not.toHaveBeenCalled()
+    } finally {
+      exitSpy.mockRestore()
+    }
+  })
+})
+
 describe('toPeriod', () => {
   it('round-trips known periods', () => {
     const known: Period[] = ['today', 'week', '30days', 'month', 'all']

tests/sharing/transport.test.ts

@@ -3,6 +3,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'
 import { generateIdentity, type Identity } from '../../src/sharing/identity.js'
 import { PeerStore } from '../../src/sharing/pairing.js'
 import { ShareServer } from '../../src/sharing/share-server.js'
+import { getDateRange, parsePeriodOrThrow } from '../../src/cli-date.js'
 import { hello, pair, fetchUsage } from '../../src/sharing/client.js'
 
 describe('device sharing transport (loopback mutual TLS)', () => {
@@ -52,6 +53,80 @@ describe('device sharing transport (loopback mutual TLS)', () => {
     expect((ur.json as { current: { cost: number } }).current.cost).toBe(42)
   })
 
+  it('returns bad request when getUsage rejects an invalid period', async () => {
+    const badServer = new ShareServer({
+      identity: serverId,
+      peers,
+      getUsage: async (q) => {
+        getDateRange(parsePeriodOrThrow(q.period ?? 'month'))
+        return { current: { cost: 0 } }
+      },
+    })
+    const badPort = await badServer.listen(0, '127.0.0.1')
+    try {
+      const pin = badServer.openPairing()
+      const pr = await pair({ ...ep(), port: badPort }, pin, 'Mac Studio')
+      const token = (pr.json as { token: string }).token
+      const ur = await fetchUsage(
+        { ...ep(), port: badPort, expectedFingerprint: serverId.fingerprint },
+        token,
+        { period: 'garbage' },
+      )
+      expect(ur.status).toBe(400)
+      expect((ur.json as { error: string }).error).toMatch(/Unknown period "garbage"/)
+    } finally {
+      await badServer.close()
+    }
+  })
+
+  it('keeps unexpected getUsage failures as internal errors', async () => {
+    const badServer = new ShareServer({
+      identity: serverId,
+      peers,
+      getUsage: async () => {
+        throw new Error('database temporarily unavailable')
+      },
+    })
+    const badPort = await badServer.listen(0, '127.0.0.1')
+    try {
+      const pin = badServer.openPairing()
+      const pr = await pair({ ...ep(), port: badPort }, pin, 'Mac Studio')
+      const token = (pr.json as { token: string }).token
+      const ur = await fetchUsage(
+        { ...ep(), port: badPort, expectedFingerprint: serverId.fingerprint },
+        token,
+      )
+      expect(ur.status).toBe(500)
+      expect((ur.json as { error: string }).error).toMatch(/database temporarily unavailable/)
+    } finally {
+      await badServer.close()
+    }
+  })
+
+  it('does not classify plain string-matched errors as usage validation errors', async () => {
+    const badServer = new ShareServer({
+      identity: serverId,
+      peers,
+      getUsage: async () => {
+        throw new Error('Unknown period "garbage". Valid values: today, week, 30days, month, all.')
+      },
+    })
+    const badPort = await badServer.listen(0, '127.0.0.1')
+    try {
+      const pin = badServer.openPairing()
+      const pr = await pair({ ...ep(), port: badPort }, pin, 'Mac Studio')
+      const token = (pr.json as { token: string }).token
+      const ur = await fetchUsage(
+        { ...ep(), port: badPort, expectedFingerprint: serverId.fingerprint },
+        token,
+      )
+      expect(ur.status).toBe(500)
+      expect((ur.json as { error: string }).error).toMatch(/Unknown period "garbage"/)
+    } finally {
+      await badServer.close()
+    }
+  })
+
   it('rejects a wrong PIN', async () => {
     server.openPairing()
     const pr = await pair(ep(), '000000', 'x')

tests/web-dashboard.test.ts

@@ -0,0 +1,59 @@
+import { mkdtemp, rm } from 'fs/promises'
+import { tmpdir } from 'os'
+import { join } from 'path'
+import type { AddressInfo } from 'net'
+import type { Server } from 'http'
+
+import { afterAll, beforeAll, describe, expect, it } from 'vitest'
+
+import { runWebDashboard } from '../src/web-dashboard.js'
+
+// Regression guard for the original bug: a bad `period` query used to hit
+// process.exit(1) and kill the long-running dashboard server. The handlers must
+// now answer 400 and keep serving.
+describe('web dashboard server: invalid query returns 400 without exiting', () => {
+  let server: Server
+  let base: string
+  let homeDir: string
+  let cacheDir: string
+  const prevHome = process.env['HOME']
+  const prevCache = process.env['CODEBURN_CACHE_DIR']
+
+  beforeAll(async () => {
+    homeDir = await mkdtemp(join(tmpdir(), 'codeburn-web-home-'))
+    cacheDir = await mkdtemp(join(tmpdir(), 'codeburn-web-cache-'))
+    process.env['HOME'] = homeDir
+    process.env['CODEBURN_CACHE_DIR'] = cacheDir
+    server = await runWebDashboard({
+      period: 'today', provider: 'all', project: [], exclude: [], port: 0, open: false,
+    })
+    base = `http://127.0.0.1:${(server.address() as AddressInfo).port}`
+  })
+
+  afterAll(async () => {
+    await new Promise<void>((resolve) => server.close(() => resolve()))
+    if (prevHome === undefined) delete process.env['HOME']
+    else process.env['HOME'] = prevHome
+    if (prevCache === undefined) delete process.env['CODEBURN_CACHE_DIR']
+    else process.env['CODEBURN_CACHE_DIR'] = prevCache
+    await rm(homeDir, { recursive: true, force: true })
+    await rm(cacheDir, { recursive: true, force: true })
+  })
+
+  it('answers 400 for an invalid /api/usage period and keeps serving', async () => {
+    const bad = await fetch(`${base}/api/usage?period=garbage`)
+    expect(bad.status).toBe(400)
+    expect((await bad.json() as { error: string }).error).toMatch(/Unknown period "garbage"/)
+
+    // The bug was process.exit; if it regressed, this test process would die.
+    // A successful follow-up request proves the server survived the bad one.
+    const ok = await fetch(`${base}/api/usage?period=today`)
+    expect(ok.status).toBe(200)
+  })
+
+  it('answers 400 for an invalid /api/devices period', async () => {
+    const bad = await fetch(`${base}/api/devices?period=garbage`)
+    expect(bad.status).toBe(400)
+    expect((await bad.json() as { error: string }).error).toMatch(/Unknown period "garbage"/)
+  })
+})