Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,37 @@ const config = [
'react-hooks/immutability': 'off',
},
},
// P1.5 — Discourage bare `fetch('/api/...')`.
// The team must migrate to `apiFetch<T>()` 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<T>() 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<T>() from '@/lib/api-client' instead of bare fetch(`/api/...`). It handles 401 redirect, 403/5xx typed errors, and network failures uniformly.",
},
],
},
},
]

export default config
2 changes: 2 additions & 0 deletions src/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -114,6 +115,7 @@ export default async function RootLayout({
disableTransitionOnChange
>
<ThemeBackground />
<AuthExpiredListener />
<div className="h-screen overflow-hidden bg-background text-foreground">
{children}
</div>
Expand Down
33 changes: 33 additions & 0 deletions src/components/auth-expired-listener.tsx
Original file line number Diff line number Diff line change
@@ -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
}
23 changes: 11 additions & 12 deletions src/components/onboarding/onboarding-wizard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { clampWizardStep, getWizardSteps, stepIdAt } from '@/lib/onboarding-flow
import { SecurityScanCard } from '@/components/onboarding/security-scan-card'
// StepAgentRuntimes removed — runtime management moved to Settings page
import { clearOnboardingReplayFromStart, markOnboardingDismissedThisSession, readOnboardingReplayFromStart } from '@/lib/onboarding-session'
import { apiFetch } from '@/lib/api-client'

interface StepInfo {
id: string
Expand Down Expand Up @@ -94,8 +95,7 @@ export function OnboardingWizard() {
const previousOverflow = document.body.style.overflow
document.body.style.overflow = 'hidden'

fetch('/api/onboarding')
.then(r => r.ok ? r.json() : null)
apiFetch<OnboardingState>('/api/onboarding')
.then(data => {
if (data) {
setState(data)
Expand All @@ -113,9 +113,9 @@ export function OnboardingWizard() {

// Fetch system capabilities and runtime status in parallel
Promise.allSettled([
fetch('/api/status?action=capabilities').then(r => r.ok ? r.json() : null),
fetch('/api/agents?limit=1').then(r => r.ok ? r.json() : null),
fetch('/api/agent-runtimes').then(r => r.ok ? r.json() : null),
apiFetch<any>('/api/status?action=capabilities').catch(() => null),
apiFetch<any>('/api/agents?limit=1').catch(() => null),
apiFetch<any>('/api/agent-runtimes').catch(() => null),
]).then(([statusResult, agentsResult, runtimesResult]) => {
const statusData = statusResult.status === 'fulfilled' ? statusResult.value : null
const agentsData = agentsResult.status === 'fulfilled' ? agentsResult.value : null
Expand Down Expand Up @@ -147,8 +147,7 @@ export function OnboardingWizard() {

useEffect(() => {
if (step !== credentialsStepIndex || credentialStatus) return
fetch('/api/diagnostics')
.then(r => r.ok ? r.json() : null)
apiFetch<{ security?: { checks?: DiagSecurityCheck[] } }>('/api/diagnostics')
.then(data => {
if (data?.security?.checks) {
const checks = data.security.checks as DiagSecurityCheck[]
Expand All @@ -161,7 +160,7 @@ export function OnboardingWizard() {
}, [step, credentialStatus, credentialsStepIndex])

const completeStep = useCallback(async (stepId: string) => {
await fetch('/api/onboarding', {
await apiFetch<{ ok?: boolean }>('/api/onboarding', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ action: 'complete_step', step: stepId }),
Expand All @@ -170,7 +169,7 @@ export function OnboardingWizard() {

const finish = useCallback(async () => {
setCompletionMessage(true)
await fetch('/api/onboarding', {
await apiFetch<{ ok?: boolean }>('/api/onboarding', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ action: 'complete' }),
Expand All @@ -184,7 +183,7 @@ export function OnboardingWizard() {

const skip = useCallback(async () => {
setClosing(true)
await fetch('/api/onboarding', {
await apiFetch<{ ok?: boolean }>('/api/onboarding', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ action: 'skip' }),
Expand Down Expand Up @@ -457,7 +456,7 @@ function StepInterfaceMode({ isGateway, onNext, onBack }: {
setSelected(mode)
setInterfaceMode(mode)
try {
await fetch('/api/settings', {
await apiFetch<{ ok?: boolean }>('/api/settings', {
method: 'PUT',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ settings: { 'general.interface_mode': mode } }),
Expand Down Expand Up @@ -555,7 +554,7 @@ function StepGatewayLink({ isGateway, registration, onNext, onBack }: {
const testConnection = async () => {
setTesting(true)
try {
const res = await fetch('/api/gateways/health', { method: 'POST' })
const res = await apiFetch<Response>('/api/gateways/health', { method: 'POST', raw: true })
setHealthOk(res.ok)
} catch {
setHealthOk(false)
Expand Down
114 changes: 114 additions & 0 deletions src/lib/__tests__/api-client.test.ts
Original file line number Diff line number Diff line change
@@ -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()
})
})
Loading