diff --git a/eslint.config.mjs b/eslint.config.mjs index fc93951080..57ad41f70f 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -18,6 +18,37 @@ const config = [ 'react-hooks/immutability': 'off', }, }, + // P1.5 — Discourage bare `fetch('/api/...')`. + // The team must migrate to `apiFetch()` from `@/lib/api-client` so that + // 401 / 403 / 5xx / network failures are handled uniformly. + // Phase 1 (this PR): warn level, allow incremental migration. + // Phase 3 (final cleanup): upgrade to error and forbid merges that introduce + // new bare fetch('/api/...') sites. + // Selector rationale: + // - covers single-quoted, double-quoted, and template-literal forms + // - filters by /api prefix so cross-origin / external fetches stay untouched + // - exempts api-client.ts itself (the one allowed implementer) + { + files: ['src/**/*.{ts,tsx,js,jsx}'], + ignores: ['src/lib/api-client.ts'], + rules: { + 'no-restricted-syntax': [ + 'warn', + { + selector: + "CallExpression[callee.name='fetch'] > Literal[value=/^\\/api\\//]", + message: + "Use apiFetch() from '@/lib/api-client' instead of bare fetch('/api/...'). It handles 401 redirect, 403/5xx typed errors, and network failures uniformly. See PR-api-client.md.", + }, + { + selector: + "CallExpression[callee.name='fetch'] > TemplateLiteral.arguments:first-child[quasis.0.value.raw=/^\\/api\\//]", + message: + "Use apiFetch() from '@/lib/api-client' instead of bare fetch(`/api/...`). It handles 401 redirect, 403/5xx typed errors, and network failures uniformly.", + }, + ], + }, + }, ] export default config diff --git a/src/app/layout.tsx b/src/app/layout.tsx index e80a841622..70401fa04d 100644 --- a/src/app/layout.tsx +++ b/src/app/layout.tsx @@ -6,6 +6,7 @@ import { NextIntlClientProvider } from 'next-intl' import { getLocale, getMessages } from 'next-intl/server' import { THEME_IDS } from '@/lib/themes' import { ThemeBackground } from '@/components/ui/theme-background' +import { AuthExpiredListener } from '@/components/auth-expired-listener' import './globals.css' const inter = Inter({ @@ -114,6 +115,7 @@ export default async function RootLayout({ disableTransitionOnChange > +
{children}
diff --git a/src/components/auth-expired-listener.tsx b/src/components/auth-expired-listener.tsx new file mode 100644 index 0000000000..f376983084 --- /dev/null +++ b/src/components/auth-expired-listener.tsx @@ -0,0 +1,33 @@ +'use client' + +import { useEffect } from 'react' + +/** + * Listens for `mc:auth-expired` CustomEvent dispatched by `apiFetch()` when the + * server returns 401. The redirect to `/login?from=...` is already handled inside + * `apiFetch`; this listener exists so we have a single observability hook the + * team can extend (toast, telemetry, Sentry). + * + * Mounted once at the root layout. SSR-safe (effect runs only on the client). + * + * Why a separate component? + * - layout.tsx is a server component (uses `await headers()`); we cannot + * attach window listeners there directly. + * - Co-locating the listener with the api-client keeps the auth-failure + * contract in one place. + */ +export function AuthExpiredListener(): null { + useEffect(() => { + const onExpired = (e: Event) => { + const detail = (e as CustomEvent<{ path: string; status: number }>).detail + // No toast lib installed yet — log for now. Replace with sonner in next PR. + console.warn( + `[mc] session expired on ${detail?.path ?? 'unknown'} (status=${detail?.status ?? 401}), redirecting to /login` + ) + } + window.addEventListener('mc:auth-expired', onExpired) + return () => window.removeEventListener('mc:auth-expired', onExpired) + }, []) + + return null +} diff --git a/src/components/dashboard/dashboard.tsx b/src/components/dashboard/dashboard.tsx index f22816b611..bbddb93426 100644 --- a/src/components/dashboard/dashboard.tsx +++ b/src/components/dashboard/dashboard.tsx @@ -8,6 +8,7 @@ import { SignalPill, getLocalOsStatus, getProviderHealth, getMcHealth } from './ import { OnboardingChecklistWidget } from './widgets/onboarding-checklist-widget' import { EmptyStateLaunchpad } from './empty-state-launchpad' import { WidgetGrid } from './widget-grid' +import { apiFetch } from '@/lib/api-client' import type { DbStats, ClaudeStats, LogLike, DashboardData } from './widget-primitives' export function Dashboard() { @@ -55,61 +56,56 @@ export function Dashboard() { const requests: Promise[] = [] requests.push( - fetch('/api/status?action=dashboard') - .then(async (res) => { - if (!res.ok) return - const data = await res.json() + (async () => { + try { + const data = await apiFetch('/api/status?action=dashboard') if (data && !data.error) { setSystemStats(data) if (data.db) setDbStats(data.db) } - }) - .catch(() => {}) - .finally(() => setLoading(prev => ({ ...prev, system: false }))) + } catch {} + finally { setLoading(prev => ({ ...prev, system: false })) } + })() ) requests.push( - fetch('/api/sessions') - .then(async (res) => { - if (!res.ok) return - const data = await res.json() + (async () => { + try { + const data = await apiFetch('/api/sessions') if (data && !data.error) setSessions(data.sessions || data) - }) - .catch(() => {}) - .finally(() => setLoading(prev => ({ ...prev, sessions: false }))) + } catch {} + finally { setLoading(prev => ({ ...prev, sessions: false })) } + })() ) if (isLocal) { requests.push( - fetch('/api/claude/sessions') - .then(async (res) => { - if (!res.ok) return - const data = await res.json() + (async () => { + try { + const data = await apiFetch('/api/claude/sessions') if (data?.stats) setClaudeStats(data.stats) - }) - .catch(() => {}) - .finally(() => setLoading(prev => ({ ...prev, claude: false }))) + } catch {} + finally { setLoading(prev => ({ ...prev, claude: false })) } + })() ) requests.push( - fetch('/api/github?action=stats') - .then(async (res) => { - if (!res.ok) return - const data = await res.json() + (async () => { + try { + const data = await apiFetch('/api/github?action=stats') if (data && !data.error) setGithubStats(data) - }) - .catch(() => {}) - .finally(() => setLoading(prev => ({ ...prev, github: false }))) + } catch {} + finally { setLoading(prev => ({ ...prev, github: false })) } + })() ) requests.push( - fetch('/api/hermes') - .then(async (res) => { - if (!res.ok) return - const data = await res.json() + (async () => { + try { + const data = await apiFetch('/api/hermes') if (data?.cronJobCount != null) setHermesCronJobCount(data.cronJobCount) - }) - .catch(() => {}) + } catch {} + })() ) } else { setLoading(prev => ({ ...prev, claude: false, github: false })) diff --git a/src/components/dashboard/empty-state-launchpad.tsx b/src/components/dashboard/empty-state-launchpad.tsx index f6427aa13c..a8d836a738 100644 --- a/src/components/dashboard/empty-state-launchpad.tsx +++ b/src/components/dashboard/empty-state-launchpad.tsx @@ -2,6 +2,7 @@ import { useState, useEffect } from 'react' import { Button } from '@/components/ui/button' +import { apiFetch } from '@/lib/api-client' interface RuntimeStatus { id: string @@ -25,26 +26,23 @@ export function EmptyStateLaunchpad({ agentCount, taskCount, onNavigate }: Props useEffect(() => { // Try the agent-runtimes API first, fall back to capabilities endpoint - fetch('/api/agent-runtimes') - .then(r => r.ok ? r.json() : null) - .then(d => { + ;(async () => { + try { + const d = await apiFetch('/api/agent-runtimes') if (d?.runtimes) { setRuntimes(d.runtimes) + setLoaded(true) return } - // Fallback: use capabilities endpoint for detection - return fetch('/api/status?action=capabilities') - .then(r => r.ok ? r.json() : {}) - .then((caps: Record) => { - const detected: RuntimeStatus[] = [] - if (caps.openclawHome) detected.push({ id: 'openclaw', name: 'OpenClaw', installed: true }) - if (caps.hermesInstalled) detected.push({ id: 'hermes', name: 'Hermes Agent', installed: true }) - if (caps.claudeHome) detected.push({ id: 'claude', name: 'Claude Code', installed: true }) - setRuntimes(detected) - }) - }) - .catch(() => {}) - .finally(() => setLoaded(true)) + const caps = await apiFetch>('/api/status?action=capabilities') + const detected: RuntimeStatus[] = [] + if (caps.openclawHome) detected.push({ id: 'openclaw', name: 'OpenClaw', installed: true }) + if (caps.hermesInstalled) detected.push({ id: 'hermes', name: 'Hermes Agent', installed: true }) + if (caps.claudeHome) detected.push({ id: 'claude', name: 'Claude Code', installed: true }) + setRuntimes(detected) + } catch {} + setLoaded(true) + })() }, []) const installed = runtimes.filter(r => r.installed) diff --git a/src/components/dashboard/sidebar.tsx b/src/components/dashboard/sidebar.tsx index 24b646c44d..1a23cbce5a 100644 --- a/src/components/dashboard/sidebar.tsx +++ b/src/components/dashboard/sidebar.tsx @@ -5,6 +5,7 @@ import { useEffect, useState } from 'react' import { useMissionControl } from '@/store' import { useNavigateToPanel } from '@/lib/navigation' import { createClientLogger } from '@/lib/client-logger' +import { apiFetch } from '@/lib/api-client' import { Button } from '@/components/ui/button' const log = createClientLogger('Sidebar') @@ -70,10 +71,12 @@ export function Sidebar() { useEffect(() => { let cancelled = false - fetch('/api/status?action=overview') - .then(res => res.json()) - .then(data => { if (!cancelled) setSystemStats(readSystemStats(data)) }) - .catch(err => log.error('Failed to fetch system status:', err)) + ;(async () => { + try { + const data = await apiFetch('/api/status?action=overview') + if (!cancelled) setSystemStats(readSystemStats(data)) + } catch (err) { log.error('Failed to fetch system status:', err) } + })() return () => { cancelled = true } }, []) diff --git a/src/components/dashboard/widgets/onboarding-checklist-widget.tsx b/src/components/dashboard/widgets/onboarding-checklist-widget.tsx index d92c90400f..3f6bcc58e2 100644 --- a/src/components/dashboard/widgets/onboarding-checklist-widget.tsx +++ b/src/components/dashboard/widgets/onboarding-checklist-widget.tsx @@ -4,6 +4,7 @@ import { useState, useEffect, useCallback } from 'react' import { Button } from '@/components/ui/button' import { useMissionControl } from '@/store' import { useNavigateToPanel } from '@/lib/navigation' +import { apiFetch } from '@/lib/api-client' interface ChecklistItem { id: string @@ -26,10 +27,7 @@ export function OnboardingChecklistWidget() { let cancelled = false async function check() { try { - const onboardingRes = await fetch('/api/onboarding') - if (cancelled) return - - const onboardingData = onboardingRes.ok ? await onboardingRes.json() : null + const onboardingData = await apiFetch('/api/onboarding') const completed = onboardingData?.completed === true const skipped = onboardingData?.skipped === true @@ -68,11 +66,7 @@ export function OnboardingChecklistWidget() { setCelebrating(true) const timer = setTimeout(async () => { try { - await fetch('/api/onboarding', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ action: 'dismiss_checklist' }), - }) + await apiFetch('/api/onboarding', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ action: 'dismiss_checklist' }) }) } catch {} setVisible(false) }, 3000) @@ -83,7 +77,7 @@ export function OnboardingChecklistWidget() { const handleDismiss = useCallback(async () => { setDismissing(true) try { - await fetch('/api/onboarding', { + await apiFetch('/api/onboarding', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ action: 'dismiss_checklist' }), diff --git a/src/components/dashboard/widgets/security-audit-widget.tsx b/src/components/dashboard/widgets/security-audit-widget.tsx index 8c2b3c4af0..6fb082ac36 100644 --- a/src/components/dashboard/widgets/security-audit-widget.tsx +++ b/src/components/dashboard/widgets/security-audit-widget.tsx @@ -3,6 +3,7 @@ import { useState, useEffect, useCallback } from 'react' import { StatRow, type DashboardData } from '../widget-primitives' import { useNavigateToPanel } from '@/lib/navigation' +import { apiFetch } from '@/lib/api-client' interface PostureInfo { score: number @@ -23,11 +24,8 @@ export function SecurityAuditWidget({ data }: { data: DashboardData }) { const fetchPosture = useCallback(async () => { try { - const res = await fetch('/api/security-audit?timeframe=day') - if (res.ok) { - const json = await res.json() - if (json.posture) setPosture(json.posture) - } + const json = await apiFetch('/api/security-audit?timeframe=day') + if (json?.posture) setPosture(json.posture) } catch { // Silent } diff --git a/src/components/panels/cost-tracker-panel.tsx b/src/components/panels/cost-tracker-panel.tsx index 2ad68bdf67..4f2b328709 100644 --- a/src/components/panels/cost-tracker-panel.tsx +++ b/src/components/panels/cost-tracker-panel.tsx @@ -6,6 +6,7 @@ import { Button } from '@/components/ui/button' import { Loader } from '@/components/ui/loader' import { useMissionControl } from '@/store' import { createClientLogger } from '@/lib/client-logger' +import { apiFetch } from '@/lib/api-client' import { PieChart, Pie, Cell, LineChart, Line, XAxis, YAxis, CartesianGrid, Tooltip, Legend, ResponsiveContainer, BarChart, Bar, @@ -117,14 +118,11 @@ export function CostTrackerPanel() { const loadData = useCallback(async () => { setIsLoading(true) try { - const [statsRes, trendRes, byAgentRes, taskRes] = await Promise.all([ - fetch(`/api/tokens?action=stats&timeframe=${timeframe}`), - fetch(`/api/tokens?action=trends&timeframe=${timeframe}`), - fetch(`/api/tokens/by-agent?days=${timeframeToDays(timeframe)}`), - fetch(`/api/tokens?action=task-costs&timeframe=${timeframe}`), - ]) const [statsJson, trendJson, byAgentJson, taskJson] = await Promise.all([ - statsRes.json(), trendRes.json(), byAgentRes.json(), taskRes.json(), + apiFetch(`/api/tokens?action=stats&timeframe=${timeframe}`), + apiFetch(`/api/tokens?action=trends&timeframe=${timeframe}`), + apiFetch(`/api/tokens/by-agent?days=${timeframeToDays(timeframe)}`), + apiFetch(`/api/tokens?action=task-costs&timeframe=${timeframe}`), ]) setUsageStats(statsJson) setTrendData(trendJson) @@ -139,8 +137,7 @@ export function CostTrackerPanel() { const loadSessionCosts = useCallback(async () => { try { - const res = await fetch(`/api/tokens?action=session-costs&timeframe=${timeframe}`) - const data = await res.json() + const data = await apiFetch<{ sessions: SessionCostEntry[] }>(`/api/tokens?action=session-costs&timeframe=${timeframe}`) if (Array.isArray(data?.sessions)) { setSessionCosts(data.sessions) } else if (usageStats?.sessions) { @@ -171,7 +168,7 @@ export function CostTrackerPanel() { const exportData = async (format: 'json' | 'csv') => { setIsExporting(true) try { - const res = await fetch(`/api/tokens?action=export&timeframe=${timeframe}&format=${format}`) + const res = await apiFetch(`/api/tokens?action=export&timeframe=${timeframe}&format=${format}`, { raw: true }) if (!res.ok) throw new Error('Export failed') const blob = await res.blob() const url = window.URL.createObjectURL(blob) diff --git a/src/lib/__tests__/api-client.test.ts b/src/lib/__tests__/api-client.test.ts new file mode 100644 index 0000000000..a8f3bcee28 --- /dev/null +++ b/src/lib/__tests__/api-client.test.ts @@ -0,0 +1,114 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import { apiFetch, ApiError } from '../api-client' + +const realFetch = global.fetch +const realLocation = window.location + +function mockResponse(status: number, body: unknown = {}, opts: { json?: boolean } = { json: true }) { + return new Response(opts.json ? JSON.stringify(body) : String(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }) +} + +describe('apiFetch — global 401 / 403 / 5xx / network handling', () => { + let dispatched: CustomEvent[] = [] + let originalHref = '' + let authExpiredListener: ((e: Event) => void) | null = null + + beforeEach(() => { + dispatched = [] + authExpiredListener = (e) => dispatched.push(e as CustomEvent) + window.addEventListener('mc:auth-expired', authExpiredListener) + + // Stub location so redirect-to-login is observable + originalHref = window.location.href + Object.defineProperty(window, 'location', { + writable: true, + value: { + ...realLocation, + pathname: '/cost-tracker', + search: '', + href: 'http://127.0.0.1:3000/cost-tracker', + }, + }) + }) + + afterEach(() => { + if (authExpiredListener) { + window.removeEventListener('mc:auth-expired', authExpiredListener) + authExpiredListener = null + } + global.fetch = realFetch + Object.defineProperty(window, 'location', { writable: true, value: realLocation }) + window.location.href = originalHref + vi.restoreAllMocks() + }) + + it('returns parsed JSON on 200', async () => { + global.fetch = vi.fn().mockResolvedValue(mockResponse(200, { ok: true, count: 42 })) + const data = await apiFetch<{ ok: boolean; count: number }>('/api/tokens') + expect(data).toEqual({ ok: true, count: 42 }) + }) + + it('emits mc:auth-expired and redirects to /login on 401', async () => { + global.fetch = vi.fn().mockResolvedValue(mockResponse(401, { error: 'Authentication required' })) + await expect(apiFetch('/api/tokens')).rejects.toMatchObject({ + code: 'UNAUTHENTICATED', + status: 401, + }) + expect(dispatched).toHaveLength(1) + expect(dispatched[0].detail).toMatchObject({ path: '/api/tokens', status: 401 }) + expect(window.location.href).toContain('/login?from=%2Fcost-tracker') + }) + + it('does NOT redirect when redirectOnUnauthenticated=false', async () => { + global.fetch = vi.fn().mockResolvedValue(mockResponse(401, { error: 'Authentication required' })) + await expect( + apiFetch('/api/tokens', { redirectOnUnauthenticated: false }) + ).rejects.toThrow(ApiError) + expect(window.location.href).toBe('http://127.0.0.1:3000/cost-tracker') + }) + + it('throws FORBIDDEN on 403 without redirecting', async () => { + global.fetch = vi.fn().mockResolvedValue(mockResponse(403, { error: 'Requires admin role' })) + await expect(apiFetch('/api/tokens')).rejects.toMatchObject({ + code: 'FORBIDDEN', + status: 403, + }) + expect(window.location.href).toBe('http://127.0.0.1:3000/cost-tracker') + }) + + it('throws SERVER_ERROR on 500 with upstream message', async () => { + global.fetch = vi.fn().mockResolvedValue(mockResponse(500, { error: 'database is locked' })) + await expect(apiFetch('/api/tokens')).rejects.toMatchObject({ + code: 'SERVER_ERROR', + status: 500, + message: 'database is locked', + }) + }) + + it('throws NETWORK_ERROR when fetch rejects', async () => { + global.fetch = vi.fn().mockRejectedValue(new TypeError('Failed to fetch')) + await expect(apiFetch('/api/tokens')).rejects.toMatchObject({ + code: 'NETWORK_ERROR', + status: 0, + }) + }) + + it('does not redirect when already on /login (avoid infinite loop)', async () => { + Object.defineProperty(window, 'location', { + writable: true, + value: { ...realLocation, pathname: '/login', search: '', href: 'http://127.0.0.1:3000/login' }, + }) + global.fetch = vi.fn().mockResolvedValue(mockResponse(401)) + await expect(apiFetch('/api/auth/login', { method: 'POST' })).rejects.toThrow(ApiError) + expect(window.location.href).toBe('http://127.0.0.1:3000/login') + }) + + it('returns undefined for 204 No Content', async () => { + global.fetch = vi.fn().mockResolvedValue(new Response(null, { status: 204 })) + const data = await apiFetch('/api/sessions/123', { method: 'DELETE' }) + expect(data).toBeUndefined() + }) +}) diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts new file mode 100644 index 0000000000..1e3edcc46f --- /dev/null +++ b/src/lib/api-client.ts @@ -0,0 +1,145 @@ +/** + * API client with global 401 / 403 / network handling. + * + * Why this exists + * --------------- + * Mission Control had no global auth-failure handler. When a session expired, + * `fetch('/api/...')` returned 401 silently and panels (cost-tracker, dashboard, + * activities) stuck on loading skeletons forever — users perceived "the service + * died" but the backend was healthy. Root cause logged in: + * src/lib/auth.ts:629 requireRole() + * + * Contract + * -------- + * apiFetch(path, init?) -> Promise + * + * - Always sends cookies (credentials: 'include') + * - On 401: emits an `mc:auth-expired` CustomEvent, redirects to /login?from=… + * - On 403: throws ApiError with code='FORBIDDEN' + * - On 5xx: throws ApiError with code='SERVER_ERROR' and the upstream message + * - On network failure: throws ApiError with code='NETWORK_ERROR' + * + * Listen once at app root: + * window.addEventListener('mc:auth-expired', () => showToast('Session expired')) + */ + +export type ApiErrorCode = + | 'UNAUTHENTICATED' + | 'FORBIDDEN' + | 'NOT_FOUND' + | 'SERVER_ERROR' + | 'NETWORK_ERROR' + | 'PARSE_ERROR' + +export class ApiError extends Error { + readonly code: ApiErrorCode + readonly status: number + readonly payload: unknown + + constructor(code: ApiErrorCode, status: number, message: string, payload?: unknown) { + super(message) + this.name = 'ApiError' + this.code = code + this.status = status + this.payload = payload + } +} + +// Browser-only redirect to /login. SSR / unit tests skip it. +function redirectToLogin(): void { + if (typeof window === 'undefined') return + const from = window.location.pathname + window.location.search + // Avoid redirect loops if user is already on /login + if (window.location.pathname === '/login') return + window.location.href = `/login?from=${encodeURIComponent(from)}` +} + +function emitAuthExpired(detail: { path: string; status: number }): void { + if (typeof window === 'undefined') return + window.dispatchEvent(new CustomEvent('mc:auth-expired', { detail })) +} +export interface ApiFetchOptions extends RequestInit { + /** When true (default) and the response is 401, redirect to /login. */ + redirectOnUnauthenticated?: boolean + /** When true, return raw Response instead of parsed JSON. */ + raw?: boolean +} + +export async function apiFetch( + path: string, + options: ApiFetchOptions = {} +): Promise { + const { + redirectOnUnauthenticated = true, + raw = false, + headers, + ...rest + } = options + + let response: Response + try { + response = await fetch(path, { + credentials: 'include', + headers: { + Accept: 'application/json', + ...(rest.body && !(rest.body instanceof FormData) + ? { 'Content-Type': 'application/json' } + : {}), + ...headers, + }, + ...rest, + }) + } catch (err) { + throw new ApiError( + 'NETWORK_ERROR', + 0, + err instanceof Error ? err.message : 'Network request failed' + ) + } + + // 401 — emit event, optionally redirect, always throw + if (response.status === 401) { + emitAuthExpired({ path, status: 401 }) + if (redirectOnUnauthenticated) redirectToLogin() + throw new ApiError('UNAUTHENTICATED', 401, 'Authentication required') + } + + // 403 — throw, do NOT redirect (user is logged in but lacks role) + if (response.status === 403) { + const payload = await safeParseJson(response) + throw new ApiError('FORBIDDEN', 403, 'Insufficient permissions', payload) + } + + if (response.status === 404) { + throw new ApiError('NOT_FOUND', 404, `Not found: ${path}`) + } + + if (response.status >= 500) { + const payload = await safeParseJson(response) + const msg = + (typeof payload === 'object' && payload !== null && 'error' in payload && + typeof (payload as { error: unknown }).error === 'string' + ? (payload as { error: string }).error + : null) || `Server error ${response.status}` + throw new ApiError('SERVER_ERROR', response.status, msg, payload) + } + + if (raw) return response as unknown as T + + if (response.status === 204) return undefined as T + + try { + return (await response.json()) as T + } catch { + throw new ApiError('PARSE_ERROR', response.status, 'Response was not valid JSON') + } +} + +async function safeParseJson(res: Response): Promise { + try { + return await res.json() + } catch { + return null + } +} +