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
}
41 changes: 16 additions & 25 deletions src/components/panels/memory-browser-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Loader } from '@/components/ui/loader'
import { useMissionControl } from '@/store'
import { createClientLogger } from '@/lib/client-logger'
import { MemoryGraph } from './memory-graph'
import { apiFetch } from '@/lib/api-client'

const log = createClientLogger('MemoryBrowser')

Expand Down Expand Up @@ -148,8 +149,7 @@ export function MemoryBrowserPanel() {
const params = new URLSearchParams({ action: 'tree' })
if (typeof options?.depth === 'number') params.set('depth', String(options.depth))
if (options?.path) params.set('path', options.path)
const response = await fetch(`/api/memory?${params.toString()}`)
return response.json()
return apiFetch<any>(`/api/memory?${params.toString()}`)
}, [])

const loadFileTree = useCallback(async () => {
Expand Down Expand Up @@ -194,8 +194,7 @@ export function MemoryBrowserPanel() {
const loadFileContent = async (filePath: string) => {
setIsLoading(true)
try {
const response = await fetch(`/api/memory?action=content&path=${encodeURIComponent(filePath)}`)
const data = await response.json()
const data = await apiFetch<any>(`/api/memory?action=content&path=${encodeURIComponent(filePath)}`)
if (data.content !== undefined) {
setSelectedMemoryFile(filePath)
setMemoryContent(data.content)
Expand All @@ -208,8 +207,7 @@ export function MemoryBrowserPanel() {
incoming: [],
outgoing: [],
})
fetch(`/api/memory/links?file=${encodeURIComponent(filePath)}`)
.then((r) => r.json())
apiFetch<any>(`/api/memory/links?file=${encodeURIComponent(filePath)}`)
.then((linkData) => {
setMemoryFileLinks({
wikiLinks: linkData.wikiLinks || data.wikiLinks,
Expand All @@ -234,8 +232,7 @@ export function MemoryBrowserPanel() {
if (!searchQuery.trim()) return
setIsSearching(true)
try {
const response = await fetch(`/api/memory?action=search&query=${encodeURIComponent(searchQuery)}`)
const data = await response.json()
const data = await apiFetch<{ results?: any[] }>(`/api/memory?action=search&query=${encodeURIComponent(searchQuery)}`)
setSearchResults(data.results || [])
} catch (error) {
log.error('Search failed:', error)
Expand Down Expand Up @@ -264,12 +261,11 @@ export function MemoryBrowserPanel() {
if (!selectedMemoryFile) return
setIsSaving(true)
try {
const response = await fetch('/api/memory', {
const data = await apiFetch<any>('/api/memory', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ action: 'save', path: selectedMemoryFile, content: editedContent })
body: JSON.stringify({ action: 'save', path: selectedMemoryFile, content: editedContent }),
})
const data = await response.json()
if (data.success) {
setMemoryContent(editedContent)
setIsEditing(false)
Expand All @@ -286,12 +282,11 @@ export function MemoryBrowserPanel() {

const createNewFile = async (filePath: string, content: string = '') => {
try {
const response = await fetch('/api/memory', {
const data = await apiFetch<any>('/api/memory', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ action: 'create', path: filePath, content })
body: JSON.stringify({ action: 'create', path: filePath, content }),
})
const data = await response.json()
if (data.success) {
loadFileTree()
loadFileContent(filePath)
Expand All @@ -304,12 +299,11 @@ export function MemoryBrowserPanel() {
const deleteFile = async () => {
if (!selectedMemoryFile) return
try {
const response = await fetch('/api/memory', {
const data = await apiFetch<any>('/api/memory', {
method: 'DELETE',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ action: 'delete', path: selectedMemoryFile })
body: JSON.stringify({ action: 'delete', path: selectedMemoryFile }),
})
const data = await response.json()
if (data.success) {
setSelectedMemoryFile('')
setMemoryContent('')
Expand All @@ -325,8 +319,7 @@ export function MemoryBrowserPanel() {
const loadHealth = useCallback(async () => {
setIsLoadingHealth(true)
try {
const response = await fetch('/api/memory/health')
const data = await response.json()
const data = await apiFetch<any>('/api/memory/health')
if (data.categories) {
setHealthReport(data)
setMemoryHealth(data)
Expand All @@ -346,15 +339,14 @@ export function MemoryBrowserPanel() {

useEffect(() => {
if (hermesInstalled === null) {
fetch('/api/hermes').then(r => r.json()).then(d => setHermesInstalled(d.installed === true)).catch(() => setHermesInstalled(false))
apiFetch<{ installed?: boolean }>('/api/hermes').then(d => setHermesInstalled(d.installed === true)).catch(() => setHermesInstalled(false))
}
}, [hermesInstalled])

useEffect(() => {
if (activeView === 'hermes' && !hermesMemory && !isLoadingHermes) {
setIsLoadingHermes(true)
fetch('/api/hermes/memory')
.then(r => r.json())
apiFetch<any>('/api/hermes/memory')
.then(d => setHermesMemory(d))
.catch(() => {})
.finally(() => setIsLoadingHermes(false))
Expand All @@ -366,12 +358,11 @@ export function MemoryBrowserPanel() {
setPipelineResult(null)
setMocGroups([])
try {
const response = await fetch('/api/memory/process', {
const data = await apiFetch<any>('/api/memory/process', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ action })
body: JSON.stringify({ action }),
})
const data = await response.json()
if (action === 'generate-moc') {
setMocGroups(data.groups || [])
} else {
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