From 4ae952d4f3c2d1ae329be333f222799729f76aed Mon Sep 17 00:00:00 2001 From: fylorn <249551762+fylorn@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:10:11 +0800 Subject: [PATCH 1/4] Add TanStack Query to the web console Pull in @tanstack/react-query 5.102 and mount a shared client in main.tsx. The client's defaults live in src/lib/query-client.ts with their reasons: no retries, since nearly every failure here is a 4xx a retry cannot fix, and no refetch on window focus, since admin pages mount several queries at once. api() now announces successful writes (onSuccessfulWrite), and main.tsx has the cache drop the queries no screen is using. One write can change what several cached screens hold; this way none of them reopens onto data from before it, without each call site knowing what it touched. @tanstack/eslint-plugin-query joins the lint config: its exhaustive-deps keeps every value a queryFn reads in its key. Tests get renderWithQueryClient, which renders inside a fresh cache. Co-Authored-By: Claude Opus 5 --- web/eslint.config.js | 5 ++++ web/package.json | 2 ++ web/pnpm-lock.yaml | 39 +++++++++++++++++++++++++++++++ web/src/lib/api.test.ts | 46 +++++++++++++++++++++++++++++++++++++ web/src/lib/api.ts | 31 ++++++++++++++++++++++++- web/src/lib/query-client.ts | 37 +++++++++++++++++++++++++++++ web/src/main.tsx | 18 ++++++++++++--- web/src/test/render.tsx | 14 +++++++++++ 8 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 web/src/lib/query-client.ts create mode 100644 web/src/test/render.tsx diff --git a/web/eslint.config.js b/web/eslint.config.js index 478bf3ce..b294c94b 100644 --- a/web/eslint.config.js +++ b/web/eslint.config.js @@ -3,6 +3,7 @@ import globals from 'globals' import reactHooks from 'eslint-plugin-react-hooks' import reactRefresh from 'eslint-plugin-react-refresh' import reactCompiler from 'eslint-plugin-react-compiler' +import pluginQuery from '@tanstack/eslint-plugin-query' import tseslint from 'typescript-eslint' import { defineConfig, globalIgnores } from 'eslint/config' @@ -22,6 +23,10 @@ export default defineConfig([ tseslint.configs.recommended, reactHooks.configs.flat.recommended, reactRefresh.configs.vite, + // The query layer's counterpart to exhaustive effect deps: a value a + // `queryFn` reads but its `queryKey` omits makes two different + // requests share one cache entry. + pluginQuery.configs['flat/recommended'], ], rules: { // `warn` rather than `error` while we land the underlying diff --git a/web/package.json b/web/package.json index 96a6cf39..622ca804 100644 --- a/web/package.json +++ b/web/package.json @@ -20,6 +20,7 @@ "@codemirror/lang-json": "^6.0.2", "@fontsource-variable/geist": "^5.2.8", "@tailwindcss/vite": "^4.2.2", + "@tanstack/react-query": "^5.102.8", "@tanstack/react-router": "^1.168.10", "@uiw/react-codemirror": "^4.25.9", "class-variance-authority": "^0.7.1", @@ -44,6 +45,7 @@ "devDependencies": { "@eslint/js": "^10.0.1", "@playwright/test": "^1.59.1", + "@tanstack/eslint-plugin-query": "^5.102.8", "@testing-library/jest-dom": "^6.9.1", "@testing-library/react": "^16.3.2", "@testing-library/user-event": "^14.6.1", diff --git a/web/pnpm-lock.yaml b/web/pnpm-lock.yaml index f58904f0..a78aa5c9 100644 --- a/web/pnpm-lock.yaml +++ b/web/pnpm-lock.yaml @@ -17,6 +17,9 @@ importers: '@tailwindcss/vite': specifier: ^4.2.2 version: 4.3.3(vite@8.3.0(@types/node@25.9.6)(jiti@2.7.0)) + '@tanstack/react-query': + specifier: ^5.102.8 + version: 5.102.8(react@19.3.0) '@tanstack/react-router': specifier: ^1.168.10 version: 1.170.35(react-dom@19.3.0(react@19.3.0))(react@19.3.0) @@ -84,6 +87,9 @@ importers: '@playwright/test': specifier: ^1.59.1 version: 1.63.0 + '@tanstack/eslint-plugin-query': + specifier: ^5.102.8 + version: 5.102.8(eslint@10.10.0(jiti@2.7.0))(typescript@6.0.3) '@testing-library/jest-dom': specifier: ^6.9.1 version: 6.10.0(@testing-library/dom@10.4.1) @@ -1471,10 +1477,27 @@ packages: peerDependencies: vite: ^5.2.0 || ^6 || ^7 || ^8 + '@tanstack/eslint-plugin-query@5.102.8': + resolution: {integrity: sha512-zRjG2PL3zvoqnsSNJFyfX5Mo+OwRwTbLERwZVO3oNQQn82V949INwaLrprzsOW/ivuow7gASeAUUC24xH7kwSQ==} + peerDependencies: + eslint: ^8.57.0 || ^9.0.0 || ^10.0.0 + typescript: ^5.6.0 || ^6.0.0 || ^7.0.0 + peerDependenciesMeta: + typescript: + optional: true + '@tanstack/history@1.162.3': resolution: {integrity: sha512-zZTDxZxdDxZudHXGp42d221R/sEKoQPbZsW2ezpdWknHpOmJVLyiljW21pdWtKXfMnLq+Kg5VJpVBwOe18+ZzQ==} engines: {node: '>=20.19'} + '@tanstack/query-core@5.102.8': + resolution: {integrity: sha512-ZNjkJ33CqvPNec/6lZBnHqLc3EVGPZ9ySLhYahU9TcuRFdmwXewuj0c4hwSWcGHqEUwcSrKeZ+oGcvPBqXcQcg==} + + '@tanstack/react-query@5.102.8': + resolution: {integrity: sha512-TYBea4OuXWD7MhaSHq069TWbFe7rcwWN6kzT7JF0OKi1K6c1gTv2IzD6A6ExJsCMozdkqBWeuIUZmu4KQg0O5A==} + peerDependencies: + react: ^18 || ^19 + '@tanstack/react-router@1.170.35': resolution: {integrity: sha512-MiqKL692aSOFsbtEts5eFjfCixxrej+ko+gA2Y85gmWdxolgtfcBPOdJ6tMSi/zFePqwwPLo+CVSUWu/qXu8LQ==} engines: {node: '>=20.19'} @@ -5296,8 +5319,24 @@ snapshots: tailwindcss: 4.3.3 vite: 8.3.0(@types/node@25.9.6)(jiti@2.7.0) + '@tanstack/eslint-plugin-query@5.102.8(eslint@10.10.0(jiti@2.7.0))(typescript@6.0.3)': + dependencies: + '@typescript-eslint/utils': 8.70.0(eslint@10.10.0(jiti@2.7.0))(typescript@6.0.3) + eslint: 10.10.0(jiti@2.7.0) + optionalDependencies: + typescript: 6.0.3 + transitivePeerDependencies: + - supports-color + '@tanstack/history@1.162.3': {} + '@tanstack/query-core@5.102.8': {} + + '@tanstack/react-query@5.102.8(react@19.3.0)': + dependencies: + '@tanstack/query-core': 5.102.8 + react: 19.3.0 + '@tanstack/react-router@1.170.35(react-dom@19.3.0(react@19.3.0))(react@19.3.0)': dependencies: '@tanstack/history': 1.162.3 diff --git a/web/src/lib/api.test.ts b/web/src/lib/api.test.ts index 954ba7bd..ce5b8c93 100644 --- a/web/src/lib/api.test.ts +++ b/web/src/lib/api.test.ts @@ -147,3 +147,49 @@ describe('api client', () => { expect(result).toEqual({ data: 'refreshed' }) }) }) + +// main.tsx drops cached queries no screen is using on every successful +// write, so a screen never reopens onto data from before one. +describe('write notifications', () => { + const okFetch = () => + vi.fn().mockResolvedValue({ ok: true, status: 200, json: () => Promise.resolve({}) }) + + it('tells listeners about a successful write', async () => { + vi.stubGlobal('fetch', okFetch()) + const listener = vi.fn() + apiModule.onSuccessfulWrite(listener) + + await apiModule.api('/api/items', { method: 'POST', body: { name: 'a' } }) + + expect(listener).toHaveBeenCalledTimes(1) + }) + + it('stays quiet for reads and for writes that fail', async () => { + const listener = vi.fn() + apiModule.onSuccessfulWrite(listener) + + vi.stubGlobal('fetch', okFetch()) + await apiModule.api('/api/items') + + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: false, + status: 400, + statusText: 'Bad Request', + json: () => Promise.resolve({ error: { message: 'Invalid input' } }), + })) + await expect(apiModule.api('/api/items/1', { method: 'DELETE' })).rejects.toThrow('Invalid input') + + expect(listener).not.toHaveBeenCalled() + }) + + it('stops calling a listener that unsubscribed', async () => { + vi.stubGlobal('fetch', okFetch()) + const listener = vi.fn() + const unsubscribe = apiModule.onSuccessfulWrite(listener) + unsubscribe() + + await apiModule.api('/api/items/1', { method: 'PATCH', body: {} }) + + expect(listener).not.toHaveBeenCalled() + }) +}) diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index aaf27ecc..69f8b9cc 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -243,6 +243,31 @@ async function tryRefreshToken(): Promise { } } +// --- Write notifications --- +// +// Screens keep what they load in the query cache (src/lib/query-client.ts), +// and one write can change what several of them hold: adding a team member +// moves the team list's member count, the user list's team column and the +// team's own page. The screen making a write refreshes what it shows, but it +// rarely knows what else the write touched. Every write passes through +// `api()`, so this is where the cache hears about all of them. + +const writeListeners = new Set<() => void>(); + +/// Run `listener` after every successful request that isn't a GET. +/// Returns a function that unsubscribes it. +export function onSuccessfulWrite(listener: () => void): () => void { + writeListeners.add(listener); + return () => { + writeListeners.delete(listener); + }; +} + +function notifyWrite(method: string): void { + if (method.toUpperCase() === 'GET') return; + for (const listener of writeListeners) listener(); +} + // --- API Client --- function validate(path: string, json: unknown, schema?: ZodType): T { @@ -293,7 +318,10 @@ export async function api(path: string, options: ApiOptions = {}): Promise body: bodyStr, signal: options.signal, }); - if (retryRes.ok) return validate(path, await retryRes.json(), options.schema); + if (retryRes.ok) { + notifyWrite(method); + return validate(path, await retryRes.json(), options.schema); + } // After a successful refresh, the retry can still legitimately // return non-401 statuses — a 403 means "session is fine, but // this user can't do that"; a 500 means upstream broke. Don't @@ -339,6 +367,7 @@ export async function api(path: string, options: ApiOptions = {}): Promise throw new ApiError(serverMessage || 'Request failed', res.status, errorType); } + notifyWrite(method); return validate(path, await res.json(), options.schema); } diff --git a/web/src/lib/query-client.ts b/web/src/lib/query-client.ts new file mode 100644 index 00000000..48ad2ab2 --- /dev/null +++ b/web/src/lib/query-client.ts @@ -0,0 +1,37 @@ +import { QueryClient } from '@tanstack/react-query'; + +/** + * Build the query client the console shares. See "Data fetching" in + * web/README.md for how screens use it. + * + * Two defaults differ from the library's, and both keep the load pattern + * the console had before it went through a cache: + * + * · **No retries.** Almost every failure here is a 4xx — a missing + * permission, a row someone else deleted — that no retry can fix, and the + * library's three silent attempts would hold a spinner up for seconds + * before the page admitted the error. + * + * · **No refetch on window focus.** Admin pages mount several queries at + * once, some of them multi-page catalog loops; refiring all of them on + * every tab switch is a change in traffic, not a fix. A screen that needs + * fresher data asks for it — `refetchInterval` where it polls, an + * invalidation after a write. + * + * Cached queries no screen is using are dropped after every successful + * write — `onSuccessfulWrite` in `api.ts`, wired up in `main.tsx` — so no + * screen reopens onto data from before a write made in this tab. + * + * A factory rather than a singleton, so every test starts from an empty + * cache. + */ +export function createQueryClient(): QueryClient { + return new QueryClient({ + defaultOptions: { + queries: { + retry: false, + refetchOnWindowFocus: false, + }, + }, + }); +} diff --git a/web/src/main.tsx b/web/src/main.tsx index 4e8c4d2c..48fbe08e 100644 --- a/web/src/main.tsx +++ b/web/src/main.tsx @@ -1,14 +1,26 @@ import { StrictMode } from 'react' import { createRoot } from 'react-dom/client' +import { QueryClientProvider } from '@tanstack/react-query' import { TooltipProvider } from '@/components/ui/tooltip' +import { onSuccessfulWrite } from '@/lib/api' +import { createQueryClient } from '@/lib/query-client' import './i18n' import './index.css' import App from './App' +const queryClient = createQueryClient() + +// A write can change what any cached screen holds. The screen making it +// refreshes what it shows; this drops the cached queries no screen is using, +// so none of them reopens onto data from before the write. +onSuccessfulWrite(() => queryClient.removeQueries({ type: 'inactive' })) + createRoot(document.getElementById('root')!).render( - - - + + + + + , ) diff --git a/web/src/test/render.tsx b/web/src/test/render.tsx new file mode 100644 index 00000000..372ef56d --- /dev/null +++ b/web/src/test/render.tsx @@ -0,0 +1,14 @@ +import type { ReactElement } from 'react' +import { render } from '@testing-library/react' +import { QueryClientProvider } from '@tanstack/react-query' +import { createQueryClient } from '@/lib/query-client' + +/** + * `render` inside a query cache of its own. Screens load through TanStack + * Query, so they need a provider — and a fresh client per call keeps one + * test's responses out of the next. Pass `client` to start from a cache the + * test has seeded. + */ +export function renderWithQueryClient(ui: ReactElement, client = createQueryClient()) { + return render({ui}) +} From a3032f4903fc574a2492cb4ee2fb0325a7d128ff Mon Sep 17 00:00:00 2001 From: fylorn <249551762+fylorn@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:10:11 +0800 Subject: [PATCH 2/4] Load server state through TanStack Query Every load that carried a react-hooks/set-state-in-effect suppression goes through useQuery now, and writes invalidate what their screen shows instead of calling a loader again. The loading flags, the abort controllers, the fetch-token race guard and the 13 loader useCallback wrappers they needed are gone; query keys follow the endpoint. Behaviour that changes on purpose: - Logout, and the cross-tab logout broadcast, clear the cache along with the user. The signed-in user is one shared query, so signing out from Profile now signs out the whole UI. - A reload after a write refreshes in place instead of flashing the skeleton, and a load error clears on the next successful load. - RouteEditorDialog maps remote-models entries to ids: the endpoint returns objects, and the dialog handed them to React as children. - OutboxBacklogDialog drops onChanged: invalidating the webhook-outbox prefix also refreshes the parent's backlog counts. - Polling uses refetchInterval, which pauses while the tab is hidden. Where cached data can already be there on the first render, local state follows it through trackers that start empty: BatchImportDialog can mount open, the Models page's ?import= deeplink opens it during render, and the pricing form seeds once, from a fetch made after mount. BatchImportDialog also offers nothing until both lists are fresh, so a cached copy cannot offer models imported since. Co-Authored-By: Claude Opus 5 --- .../limits/user-limits-tab.test.tsx | 9 +- web/src/components/limits/user-limits-tab.tsx | 72 ++-- .../mcp/shared-credential-panel.tsx | 50 ++- .../components/mcp/wizard/use-wizard-state.ts | 194 +++++----- web/src/components/roles/RoleMembers.tsx | 47 +-- web/src/hooks/use-auth.test.tsx | 74 ++++ web/src/hooks/use-auth.ts | 87 +++-- web/src/router.tsx | 2 +- web/src/routes/admin/log-forwarders.tsx | 99 +++-- .../routes/admin/outbox-backlog-dialog.tsx | 72 ++-- web/src/routes/admin/roles.tsx | 130 ++++--- web/src/routes/admin/settings.test.tsx | 9 +- web/src/routes/admin/settings.tsx | 81 ++-- .../admin/settings/oidc/OidcWizardCard.tsx | 50 ++- web/src/routes/admin/team-detail.tsx | 111 +++--- web/src/routes/admin/teams.tsx | 40 +- web/src/routes/admin/trace.tsx | 58 +-- web/src/routes/admin/users.tsx | 124 +++--- web/src/routes/analytics/costs.tsx | 127 +++---- web/src/routes/analytics/usage.tsx | 51 ++- web/src/routes/api-keys.test.tsx | 11 +- web/src/routes/api-keys.tsx | 161 ++++---- web/src/routes/connections.tsx | 48 +-- .../gateway/models/BatchImportDialog.test.tsx | 78 ++++ .../gateway/models/BatchImportDialog.tsx | 188 ++++----- .../gateway/models/RouteEditorDialog.tsx | 59 ++- web/src/routes/gateway/models/index.tsx | 359 ++++++++---------- web/src/routes/gateway/providers.tsx | 45 +-- web/src/routes/logs.tsx | 71 ++-- web/src/routes/mcp/servers.tsx | 46 +-- web/src/routes/mcp/store.tsx | 77 ++-- web/src/routes/mcp/tools.tsx | 55 +-- 32 files changed, 1317 insertions(+), 1368 deletions(-) create mode 100644 web/src/hooks/use-auth.test.tsx create mode 100644 web/src/routes/gateway/models/BatchImportDialog.test.tsx diff --git a/web/src/components/limits/user-limits-tab.test.tsx b/web/src/components/limits/user-limits-tab.test.tsx index 3b1552b0..45d5a645 100644 --- a/web/src/components/limits/user-limits-tab.test.tsx +++ b/web/src/components/limits/user-limits-tab.test.tsx @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -import { render, screen, waitFor } from '@testing-library/react' +import { screen, waitFor } from '@testing-library/react' +import { renderWithQueryClient } from '@/test/render' import { UserLimitsTab } from './user-limits-tab' vi.mock('@/lib/api', () => ({ @@ -55,7 +56,7 @@ describe('UserLimitsTab — remaining / exceeded labels', () => { dashboard({ ruleMax: 1000, ruleCurrent: 250, capLimit: 1_000_000, capCurrent: 600_000 }), ) - render() + renderWithQueryClient() await waitFor(() => { expect(screen.getByText('25%')).toBeInTheDocument() @@ -72,7 +73,7 @@ describe('UserLimitsTab — remaining / exceeded labels', () => { dashboard({ ruleMax: 100, ruleCurrent: 100, capLimit: 500, capCurrent: 600 }), ) - render() + renderWithQueryClient() await waitFor(() => { // Rule pct caps at 100 because of Math.min, cap pct also caps at 100 @@ -103,7 +104,7 @@ describe('UserLimitsTab — remaining / exceeded labels', () => { recent_events: [], }) - render() + renderWithQueryClient() await waitFor(() => { expect(screen.getByText('0%')).toBeInTheDocument() diff --git a/web/src/components/limits/user-limits-tab.tsx b/web/src/components/limits/user-limits-tab.tsx index a37720a8..4aeaf14c 100644 --- a/web/src/components/limits/user-limits-tab.tsx +++ b/web/src/components/limits/user-limits-tab.tsx @@ -31,8 +31,9 @@ // POST /api/admin/limits/bulk/{rules|budgets}/{disable|delete} — bulk // ============================================================================ -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; +import { useQuery, useQueryClient } from '@tanstack/react-query'; import { useNow } from '@/hooks/use-now'; import { AlertCircle, Plus, Trash2, PowerOff, RotateCw, Pencil } from 'lucide-react'; import { Button } from '@/components/ui/button'; @@ -174,10 +175,18 @@ interface DrawerInit { export function UserLimitsTab({ userId }: UserLimitsTabProps) { const { t } = useTranslation(); - const [data, setData] = useState(null); - const [loading, setLoading] = useState(true); - const [error, setError] = useState(''); - const [selected, setSelected] = useState>(new Set()); + const queryClient = useQueryClient(); + const dashboardQuery = useQuery({ + queryKey: ['admin', 'users', userId, 'limits-dashboard'], + queryFn: ({ signal }) => + api(`/api/admin/users/${userId}/limits-dashboard`, { signal }), + }); + const data = dashboardQuery.data ?? null; + const loading = dashboardQuery.isPending; + const error = dashboardQuery.error?.message ?? ''; + const reload = () => + queryClient.invalidateQueries({ queryKey: ['admin', 'users', userId, 'limits-dashboard'] }); + const [selection, setSelection] = useState>(new Set()); const [bulkAction, setBulkAction] = useState<'disable' | 'delete' | null>(null); const [drawer, setDrawer] = useState(null); const [resetTarget, setResetTarget] = useState< @@ -186,39 +195,6 @@ export function UserLimitsTab({ userId }: UserLimitsTabProps) { | null >(null); - const reload = useCallback(async () => { - setError(''); - try { - const res = await api(`/api/admin/users/${userId}/limits-dashboard`); - setData(res); - // Drop stale selections. - setSelected((prev) => { - const live = new Set(); - res.rules.forEach((r) => { - const k = rowKey(r); - if (k) live.add(k); - }); - res.caps.forEach((c) => { - const k = rowKey(c); - if (k) live.add(k); - }); - return new Set(Array.from(prev).filter((k) => live.has(k))); - }); - } catch (e) { - setError(e instanceof Error ? e.message : t('common.operationFailed')); - } finally { - setLoading(false); - } - }, [userId, t]); - - useEffect(() => { - // Hand-rolled load: the spinner flag is the first half of "start a - // fetch" and belongs with it. See "Data fetching" in web/README.md — - // this goes away with a data-fetching layer, not by moving the flag. - // eslint-disable-next-line react-hooks/set-state-in-effect - reload(); - }, [reload]); - const overrideKeys = useMemo(() => { if (!data) return { rules: new Set(), caps: new Set() }; return { @@ -229,10 +205,26 @@ export function UserLimitsTab({ userId }: UserLimitsTabProps) { }; }, [data]); + // Drop stale selections: an override the latest load no longer lists + // stops counting as selected. + const selected = useMemo(() => { + if (!data) return selection; + const live = new Set(); + data.rules.forEach((r) => { + const k = rowKey(r); + if (k) live.add(k); + }); + data.caps.forEach((c) => { + const k = rowKey(c); + if (k) live.add(k); + }); + return new Set(Array.from(selection).filter((k) => live.has(k))); + }, [data, selection]); + const someSelected = selected.size > 0; const toggleOne = (key: string) => - setSelected((prev) => { + setSelection((prev) => { const next = new Set(prev); if (next.has(key)) next.delete(key); else next.add(key); @@ -259,7 +251,7 @@ export function UserLimitsTab({ userId }: UserLimitsTabProps) { count: selected.size, }), ); - setSelected(new Set()); + setSelection(new Set()); await reload(); } catch (e) { toast.error(e instanceof Error ? e.message : t('common.operationFailed')); diff --git a/web/src/components/mcp/shared-credential-panel.tsx b/web/src/components/mcp/shared-credential-panel.tsx index bc9949be..1e2d3bef 100644 --- a/web/src/components/mcp/shared-credential-panel.tsx +++ b/web/src/components/mcp/shared-credential-panel.tsx @@ -1,11 +1,12 @@ -import { useCallback, useEffect, useState } from 'react'; +import { useState } from 'react'; import { useTranslation } from 'react-i18next'; +import { useQuery, useQueryClient } from '@tanstack/react-query'; import { CheckCircle2, AlertTriangle } from 'lucide-react'; import { Alert, AlertDescription } from '@/components/ui/alert'; import { Button } from '@/components/ui/button'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; -import { apiDelete, apiGet, apiPost, apiPut } from '@/lib/api'; +import { api, apiDelete, apiPost, apiPut } from '@/lib/api'; import { toast } from 'sonner'; interface SharedCredentialStatus { @@ -45,35 +46,28 @@ export function SharedCredentialPanel({ const oauthCapable = authShape === 'oauth'; const allowStaticToken = authShape === 'static'; const { t } = useTranslation(); - const [status, setStatus] = useState(null); - const [loading, setLoading] = useState(true); + const queryClient = useQueryClient(); + const statusQuery = useQuery({ + queryKey: ['admin', 'mcp', 'servers', serverId, 'shared-credential'], + queryFn: ({ signal }) => + api(`/api/admin/mcp/servers/${serverId}/shared-credential`, { + signal, + }).catch((err: unknown) => { + // Status endpoint is read-only; failure here just leaves the + // panel reading "not configured" — the user can save and retry. + if (!signal.aborted) console.warn('shared-credential status fetch failed', err); + throw err; + }), + }); + const status = statusQuery.data ?? null; + const loading = statusQuery.isPending; + const refresh = () => + queryClient.invalidateQueries({ + queryKey: ['admin', 'mcp', 'servers', serverId, 'shared-credential'], + }); const [pasted, setPasted] = useState(''); const [submitting, setSubmitting] = useState(false); - const refresh = useCallback(async () => { - setLoading(true); - try { - const s = await apiGet( - `/api/admin/mcp/servers/${serverId}/shared-credential`, - ); - setStatus(s); - } catch (err) { - // Status endpoint is read-only; failure here just leaves the - // panel in "loading" state — the user can save and retry. - console.warn('shared-credential status fetch failed', err); - } finally { - setLoading(false); - } - }, [serverId]); - - useEffect(() => { - // Hand-rolled load: the spinner flag is the first half of "start a - // fetch" and belongs with it. See "Data fetching" in web/README.md — - // this goes away with a data-fetching layer, not by moving the flag. - // eslint-disable-next-line react-hooks/set-state-in-effect - void refresh(); - }, [refresh, serverId]); - const startOAuth = async () => { setSubmitting(true); try { diff --git a/web/src/components/mcp/wizard/use-wizard-state.ts b/web/src/components/mcp/wizard/use-wizard-state.ts index dd72c6e5..cd01284c 100644 --- a/web/src/components/mcp/wizard/use-wizard-state.ts +++ b/web/src/components/mcp/wizard/use-wizard-state.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useState } from 'react'; -import { apiDelete, apiGet } from '@/lib/api'; +import { skipToken, useQuery, useQueryClient } from '@tanstack/react-query'; +import { api, apiDelete } from '@/lib/api'; import { PersistedWizardStateSchema, type PersistedWizardState, @@ -270,24 +271,10 @@ export function useWizardState(): WizardController { }; }); + const queryClient = useQueryClient(); const [state, setState] = useState(init.initialState); - const [resumeChecking, setResumeChecking] = useState(init.resumed); - // Template fetch is deferred to a useEffect (network call) — track - // the in-flight state so Step 1 can show a spinner instead of - // letting the admin type into a URL field that's about to be - // overwritten by the template's `endpoint_template`. - const [templateLoading, setTemplateLoading] = useState( - init.templateSlug !== null, - ); - - // Strip `#wizard_resume=` from the URL on resume mounts. We - // CANNOT strip `?template=` here because React 18 strict mode - // double-invokes effects: the first mount would fetch + strip, - // its setState gets cancelled by the cleanup, and the second - // mount has no slug left in the URL to re-fetch from. So - // `?template=` gets stripped inside the fetch effect AFTER the - // setState lands — see below. + // Strip `#wizard_resume=` from the URL on resume mounts. useEffect(() => { if (!init.resumed) return; if (typeof window !== 'undefined') { @@ -298,95 +285,110 @@ export function useWizardState(): WizardController { // Template prefill — fetch the template by slug and apply its // defaults onto the wizard state. Runs once on mount when the wizard // was opened from `/mcp/store` via `/mcp/servers/new?template=...`. - useEffect(() => { - const slug = init.templateSlug; - if (!slug) return; - let alive = true; - (async () => { - try { - const tmpl = await apiGet( - `/api/mcp/store/${encodeURIComponent(slug)}`, - ); - if (!alive) return; - setState((s) => applyTemplateDefaults(s, tmpl)); - // NOW strip the query — only after the prefill landed. If we - // stripped earlier and React 18 strict-mode unmounted us, - // the second mount would have an empty URL and never re-fetch. - if (typeof window !== 'undefined') { - window.history.replaceState(null, '', window.location.pathname); - } - } catch (err) { - // Slug doesn't exist (404) or backend hiccup — leave the - // wizard in its empty default state. The admin can still - // register a server manually; we just can't claim it came - // from this template. Log to console so a misrouted slug or - // backend regression isn't entirely silent in DevTools. - console.warn( - `[wizard] template prefill failed for slug=${slug}:`, - err, - ); - } finally { - if (alive) setTemplateLoading(false); - } - })(); - return () => { - alive = false; - }; - }, [init.templateSlug]); + const slug = init.templateSlug; + const templateQuery = useQuery({ + queryKey: ['mcp', 'store', slug], + queryFn: slug + ? ({ signal }) => + api(`/api/mcp/store/${encodeURIComponent(slug)}`, { + signal, + }).catch((err: unknown) => { + // Slug doesn't exist (404) or backend hiccup — leave the + // wizard in its empty default state. The admin can still + // register a server manually; we just can't claim it came + // from this template. Log to console so a misrouted slug or + // backend regression isn't entirely silent in DevTools. + if (!signal.aborted) { + console.warn(`[wizard] template prefill failed for slug=${slug}:`, err); + } + throw err; + }) + : skipToken, + // Applied once, onto a fresh wizard — a refetch would have nowhere to go. + staleTime: Infinity, + }); + // In flight — Step 1 shows a spinner instead of letting the admin type + // into a URL field that's about to be overwritten by the template's + // `endpoint_template`. + const templateLoading = templateQuery.isLoading; + + // Apply the defaults during the render the template arrives in, so no + // frame shows Step 1 without them. + const template = templateQuery.data; + const [templateApplied, setTemplateApplied] = useState(false); + if (template && !templateApplied) { + setTemplateApplied(true); + setState((s) => applyTemplateDefaults(s, template)); + } - // Resume probe — confirm the OAuth dance landed a credential blob. + // Strip `?template=` only once the prefill has landed: a failed prefill + // leaves the slug in the URL, so a refresh tries it again. useEffect(() => { - if (!init.resumed) return; - if (state.credential_owner !== 'admin_shared') { - // Hand-rolled load: the spinner flag is the first half of "start a - // fetch" and belongs with it. See "Data fetching" in web/README.md — - // this goes away with a data-fetching layer, not by moving the flag. - // eslint-disable-next-line react-hooks/set-state-in-effect - setResumeChecking(false); - return; + if (!templateApplied) return; + if (typeof window !== 'undefined') { + window.history.replaceState(null, '', window.location.pathname); } - let alive = true; - (async () => { - try { - const status = await apiGet<{ - credential_type: string; - upstream_subject?: string | null; - expires_at?: string | null; - scopes: string[]; - }>( - `/api/admin/mcp/wizards/${encodeURIComponent( - init.sessionId, - )}/credential-status`, - ); - if (!alive) return; - const sharedPending: SharedPending = { - kind: 'oauth_done', - upstream_subject: status.upstream_subject ?? null, - expires_at: status.expires_at ?? null, - scopes: status.scopes ?? [], - }; - setState((s) => ({ ...s, shared_pending: sharedPending, step: 3 })); - } catch { - // 404 = blob not there (TTL expired, or callback hasn't run). - // Leave shared_pending null; Step 3 will show "OAuth flow - // didn't complete — re-run authorize". - } finally { - if (alive) setResumeChecking(false); - } - })(); - return () => { - alive = false; + }, [templateApplied]); + + // Resume probe — confirm the OAuth dance landed a credential blob. Only + // an admin_shared resume has one to confirm, and switching the owner back + // to admin_shared probes again: switching away cleared `shared_pending`. + const probeCredential = init.resumed && state.credential_owner === 'admin_shared'; + const credentialQuery = useQuery({ + queryKey: ['admin', 'mcp', 'wizards', init.sessionId, 'credential-status'], + queryFn: probeCredential + ? ({ signal }) => + api<{ + credential_type: string; + upstream_subject?: string | null; + expires_at?: string | null; + scopes: string[]; + }>(`/api/admin/mcp/wizards/${encodeURIComponent(init.sessionId)}/credential-status`, { + signal, + }) + : skipToken, + // A landed probe moves the wizard to Step 3. A network reconnect + // must not refetch it and pull the admin back there mid-wizard. + refetchOnReconnect: false, + }); + const resumeChecking = probeCredential && credentialQuery.isLoading; + + // Each successful probe lands in the wizard state once, during the + // render it arrives in. A 404 = blob not there (TTL expired, or callback + // hasn't run): shared_pending stays null, and Step 3 will show "OAuth + // flow didn't complete — re-run authorize". + const credential = credentialQuery.data; + const [landedProbeAt, setLandedProbeAt] = useState(0); + if (probeCredential && credential && credentialQuery.dataUpdatedAt !== landedProbeAt) { + setLandedProbeAt(credentialQuery.dataUpdatedAt); + const sharedPending: SharedPending = { + kind: 'oauth_done', + upstream_subject: credential.upstream_subject ?? null, + expires_at: credential.expires_at ?? null, + scopes: credential.scopes ?? [], }; - }, [init.resumed, init.sessionId, state.credential_owner]); + setState((s) => ({ ...s, shared_pending: sharedPending, step: 3 })); + } // Persist on every mutation. useEffect(() => { writeStorage(state); }, [state]); - const patch = useCallback((partial: Partial) => { - setState((s) => ({ ...s, ...partial })); - }, []); + const patch = useCallback( + (partial: Partial) => { + // Leaving admin_shared discards the resume probe, one still in flight + // included: its answer is about the owner the admin just left, and + // switching back probes again from scratch. + if (partial.credential_owner && partial.credential_owner !== 'admin_shared') { + queryClient.removeQueries({ + queryKey: ['admin', 'mcp', 'wizards', init.sessionId, 'credential-status'], + }); + } + setState((s) => ({ ...s, ...partial })); + }, + [queryClient, init.sessionId], + ); const patchOAuth = useCallback((partial: Partial) => { setState((s) => ({ ...s, oauth: { ...s.oauth, ...partial } })); diff --git a/web/src/components/roles/RoleMembers.tsx b/web/src/components/roles/RoleMembers.tsx index fef43bbc..5c1bee15 100644 --- a/web/src/components/roles/RoleMembers.tsx +++ b/web/src/components/roles/RoleMembers.tsx @@ -1,5 +1,6 @@ -import { useCallback, useEffect, useState } from 'react'; +import { useState } from 'react'; import { useTranslation } from 'react-i18next'; +import { useQuery, useQueryClient } from '@tanstack/react-query'; import { Plus, Trash2 } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { Badge } from '@/components/ui/badge'; @@ -42,37 +43,19 @@ interface RoleMembersProps { */ export function RoleMembers({ role, teamsById, onMembersChanged }: RoleMembersProps) { const { t } = useTranslation(); - const [members, setMembers] = useState(null); - const [membersError, setMembersError] = useState(false); - - const reloadMembers = useCallback(async () => { - try { - const res = await api<{ items: RoleMember[] }>(`/api/admin/roles/${role.id}/members`); - setMembers(res.items); - setMembersError(false); - } catch { - setMembersError(true); - } - }, [role.id]); - - // Blank the list when the *role* changes, not on every reload: the other - // callers of `reloadMembers` run after an add or a remove, and flashing a - // skeleton there would make a one-row change look like a full reload. - const [shownRole, setShownRole] = useState(role.id); - if (shownRole !== role.id) { - setShownRole(role.id); - setMembers(null); - setMembersError(false); - } - - useEffect(() => { - // Async loader: its first statement is the `await`, so every setState - // inside runs in the continuation — never synchronously with this - // effect, and never as a cascading render. The rule's cross-function - // analysis does not model `await`. - // eslint-disable-next-line react-hooks/set-state-in-effect - reloadMembers(); - }, [reloadMembers]); + const queryClient = useQueryClient(); + // Keyed by role, so switching roles never shows the previous role's + // members, while a reload after an add or a remove keeps the rows on + // screen — a one-row change should not look like a full reload. + const membersQuery = useQuery({ + queryKey: ['admin', 'roles', role.id, 'members'], + queryFn: ({ signal }) => + api<{ items: RoleMember[] }>(`/api/admin/roles/${role.id}/members`, { signal }), + }); + const members = membersQuery.data?.items ?? null; + const membersError = membersQuery.isError; + const reloadMembers = () => + queryClient.invalidateQueries({ queryKey: ['admin', 'roles', role.id, 'members'] }); const [users, setUsers] = useState(null); const [pickerOpen, setPickerOpen] = useState(false); diff --git a/web/src/hooks/use-auth.test.tsx b/web/src/hooks/use-auth.test.tsx new file mode 100644 index 00000000..3b81ebab --- /dev/null +++ b/web/src/hooks/use-auth.test.tsx @@ -0,0 +1,74 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { ReactNode } from 'react' +import { act, renderHook, waitFor } from '@testing-library/react' +import { QueryClientProvider } from '@tanstack/react-query' +import { createQueryClient } from '@/lib/query-client' +import { useAuth } from './use-auth' + +vi.mock('@/lib/api', () => ({ + api: vi.fn(), + apiPost: vi.fn(), + broadcastLogout: vi.fn(), + clearCachedPermissions: vi.fn(), + registerKeyPair: vi.fn(), + setCachedPermissions: vi.fn(), +})) + +// logout() loads the key store lazily, and jsdom has no IndexedDB behind it. +vi.mock('@/lib/crypto-store', () => ({ clearSigningKey: vi.fn() })) + +import { api, apiPost } from '@/lib/api' + +const signedIn = { + id: 'user-1', + email: 'admin@example.com', + permissions: [], + denied_permissions: [], +} +const teamsKey = ['admin', 'teams'] + +function renderAuth() { + const client = createQueryClient() + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ) + return { client, ...renderHook(() => useAuth(), { wrapper }) } +} + +beforeEach(() => { + vi.clearAllMocks() + vi.mocked(api).mockResolvedValue(signedIn) + vi.mocked(apiPost).mockResolvedValue({}) +}) + +// The query cache outlives a session in the tab. Ending a session has to +// take every cached screen with it, or the next person to sign in here +// would be shown the previous user's data until each screen refetched. +// +// The cache is swept synchronously; the hook's own re-render arrives with +// the query layer's next notification, hence `waitFor` for `user`. +describe('useAuth — ending a session', () => { + it('logout signs the user out and drops everything cached for them', async () => { + const { client, result } = renderAuth() + await waitFor(() => expect(result.current.user).toEqual(signedIn)) + client.setQueryData(teamsKey, [{ id: 'team-1', name: 'engineering' }]) + + await act(() => result.current.logout()) + + expect(client.getQueryData(teamsKey)).toBeUndefined() + await waitFor(() => expect(result.current.user).toBeNull()) + }) + + it('a logout in another tab ends the session here too', async () => { + const { client, result } = renderAuth() + await waitFor(() => expect(result.current.user).toEqual(signedIn)) + client.setQueryData(teamsKey, [{ id: 'team-1', name: 'engineering' }]) + + act(() => { + window.dispatchEvent(new CustomEvent('thinkwatch:logged-out')) + }) + + expect(client.getQueryData(teamsKey)).toBeUndefined() + await waitFor(() => expect(result.current.user).toBeNull()) + }) +}) diff --git a/web/src/hooks/use-auth.ts b/web/src/hooks/use-auth.ts index d1beb1db..859dac78 100644 --- a/web/src/hooks/use-auth.ts +++ b/web/src/hooks/use-auth.ts @@ -1,4 +1,5 @@ -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect } from 'react'; +import { hashKey, useQuery, useQueryClient, type QueryClient } from '@tanstack/react-query'; import { api, apiPost, @@ -27,50 +28,65 @@ interface PowSolution { nonce: string; } +/// The signed-in user, or `null` once the server has said there is none. +const ME_KEY = ['auth', 'me']; + +/** + * Forget the signed-in user and everything that was loaded on their behalf. + * + * The query cache outlives a session in this tab. Without the sweep, the + * next person to sign in here would be shown the previous user's cached + * pages — member lists, keys, audit rows — until each one refetched. + */ +function endSession(queryClient: QueryClient) { + clearCachedPermissions(); + queryClient.setQueryData(ME_KEY, null); + const me = hashKey(ME_KEY); + queryClient.removeQueries({ predicate: (query) => query.queryHash !== me }); +} + export function useAuth() { - const [user, setUser] = useState(null); - const [loading, setLoading] = useState(true); + const queryClient = useQueryClient(); - const fetchUser = useCallback(async () => { + const { data: user = null, isPending: loading } = useQuery({ + queryKey: ME_KEY, // No localStorage check anymore — the access cookie is opaque // from JS, so the only way to know if we're logged in is to // ask the server. /api/auth/me returns 401 if the cookie is // missing or invalid, which the api client handles via the // 401 → refresh → logout flow. - try { - const u = await api('/api/auth/me', { no401Redirect: true, schema: UserResponseSchema }); - setUser(u); - setCachedPermissions(u.permissions, u.denied_permissions); - } catch { - setUser(null); - clearCachedPermissions(); - } finally { - setLoading(false); - } - }, []); - - // `fetchUser` is async and its first statement is the `await`, so every - // setState inside it runs in the continuation — never synchronously with - // this effect, and never as a cascading render. The rule's cross-function - // analysis does not model `await`, so it reads the documented - // fetch-on-mount shape as a violation. - // eslint-disable-next-line react-hooks/set-state-in-effect - useEffect(() => { fetchUser(); }, [fetchUser]); + // + // No abort signal: every failure below reads as "signed out", and a + // request cancelled because a component unmounted is not one. + queryFn: async () => { + try { + const u = await api('/api/auth/me', { no401Redirect: true, schema: UserResponseSchema }); + setCachedPermissions(u.permissions, u.denied_permissions); + return u; + } catch { + clearCachedPermissions(); + return null; + } + }, + // Who is signed in only changes through this hook — login, logout, the + // SSO callback — and each of those updates the entry itself. Left to go + // stale, every component mounting the hook and every network reconnect + // would refetch it, and one refetch failing on a flaky connection would + // drop the admin to the login page mid-session. + staleTime: Infinity, + }); // Listen for cross-tab logout broadcasts so this tab drops its - // React user state (and the admin UI unmounts) immediately, + // signed-in user (and the admin UI unmounts) immediately, // instead of waiting for the next request to 401. The broadcast // origin is api.ts's BroadcastChannel handler; it also clears the // signing key + permission cache, but the React tree only resets - // when we flip `user` to null here. + // when the user entry flips to null here. useEffect(() => { - const handler = () => { - setUser(null); - clearCachedPermissions(); - }; + const handler = () => endSession(queryClient); window.addEventListener('thinkwatch:logged-out', handler); return () => window.removeEventListener('thinkwatch:logged-out', handler); - }, []); + }, [queryClient]); const login = async ( email: string, @@ -100,7 +116,7 @@ export function useAuth() { // register the public key with the server. await registerKeyPair(); setCachedPermissions(res.permissions, res.denied_permissions); - await fetchUser(); + await queryClient.refetchQueries({ queryKey: ME_KEY }); return res; }; @@ -112,17 +128,16 @@ export function useAuth() { } const { clearSigningKey } = await import('@/lib/crypto-store'); await clearSigningKey(); - clearCachedPermissions(); broadcastLogout(); - setUser(null); - }, []); + endSession(queryClient); + }, [queryClient]); const handleSsoCallback = useCallback(async () => { // SSO redirect set the auth cookies. Generate an ECDSA key pair // and register the public key with the server. await registerKeyPair(); - await fetchUser(); - }, [fetchUser]); + await queryClient.refetchQueries({ queryKey: ME_KEY }); + }, [queryClient]); return { user, loading, login, logout, handleSsoCallback }; } diff --git a/web/src/router.tsx b/web/src/router.tsx index ac9ec514..4b900592 100644 --- a/web/src/router.tsx +++ b/web/src/router.tsx @@ -297,7 +297,7 @@ const registerRoute = createRoute({ { // Hard navigate so RootComponent remounts and picks up - // the freshly-set auth cookies via useAuth → fetchUser. + // the freshly-set auth cookies when useAuth loads /api/auth/me. window.location.href = '/'; }} /> diff --git a/web/src/routes/admin/log-forwarders.tsx b/web/src/routes/admin/log-forwarders.tsx index 96a50ff6..88a34482 100644 --- a/web/src/routes/admin/log-forwarders.tsx +++ b/web/src/routes/admin/log-forwarders.tsx @@ -1,5 +1,6 @@ -import { useEffect, useState, useCallback } from 'react'; +import { useState } from 'react'; import { useTranslation } from 'react-i18next'; +import { useQuery, useQueryClient } from '@tanstack/react-query'; import { Card, CardContent } from '@/components/ui/card'; import { Button } from '@/components/ui/button'; import { Badge } from '@/components/ui/badge'; @@ -93,12 +94,37 @@ function formatTime(ts: string | null): string { return new Date(ts).toLocaleString(); } +const NO_BACKLOG_COUNTS: Record = {}; + +function countsByForwarder(rows: Array<{ forwarder_id: string; count: number }>) { + const map: Record = {}; + for (const r of rows) map[r.forwarder_id] = r.count; + return map; +} + export function LogForwardersPage() { const { t } = useTranslation(); - const [forwarders, setForwarders] = useState([]); - const [loading, setLoading] = useState(true); + const queryClient = useQueryClient(); + const forwardersQuery = useQuery({ + queryKey: ['admin', 'log-forwarders'], + queryFn: ({ signal }) => api('/api/admin/log-forwarders', { signal }), + }); + const forwarders = forwardersQuery.data ?? []; + const loading = forwardersQuery.isPending; const pager = useClientPagination(forwarders, 20); + const invalidateForwarders = () => + queryClient.invalidateQueries({ queryKey: ['admin', 'log-forwarders'] }); + + // One dismissable banner for load and action failures alike. Each load + // outcome writes to it once: a failure shows its message, a success + // clears whatever the banner held. const [error, setError] = useState(''); + const loadOutcomeAt = Math.max(forwardersQuery.dataUpdatedAt, forwardersQuery.errorUpdatedAt); + const [seenLoadOutcomeAt, setSeenLoadOutcomeAt] = useState(0); + if (loadOutcomeAt !== seenLoadOutcomeAt) { + setSeenLoadOutcomeAt(loadOutcomeAt); + setError(forwardersQuery.error?.message ?? ''); + } const [dialogOpen, setDialogOpen] = useState(false); const [testing, setTesting] = useState(null); const [testResult, setTestResult] = useState(null); @@ -136,50 +162,20 @@ export function LogForwardersPage() { const [deleteTargetId, setDeleteTargetId] = useState(null); // Per-forwarder outbox backlog counts + the drawer triaging one of them. - const [backlogCounts, setBacklogCounts] = useState>({}); - const [backlogForwarderId, setBacklogForwarderId] = useState(null); - - const loadForwarders = useCallback(async () => { - try { - const data = await api('/api/admin/log-forwarders'); - setForwarders(data); - setError(''); - } catch (err) { - setError(err instanceof Error ? err.message : t('common.error')); - } finally { - setLoading(false); - } - }, [t]); - - const loadBacklogCounts = useCallback(async () => { - try { - const rows = await api>( - '/api/admin/webhook-outbox/counts', - ); - const map: Record = {}; - for (const r of rows) map[r.forwarder_id] = r.count; - setBacklogCounts(map); - } catch { - // Non-critical — leave the column blank if the endpoint hiccups. - } - }, []); - - useEffect(() => { - // Async loader: its first statement is the `await`, so every setState - // inside runs in the continuation — never synchronously with this - // effect, and never as a cascading render. The rule's cross-function - // analysis does not model `await`. - // eslint-disable-next-line react-hooks/set-state-in-effect - loadForwarders(); - loadBacklogCounts(); - }, [loadForwarders, loadBacklogCounts]); - - // Poll backlog counts on the same cadence as the drain worker so the + // The counts poll on the same cadence as the drain worker so the // operator watching a stuck destination sees it drain down live. - useEffect(() => { - const id = window.setInterval(loadBacklogCounts, 10_000); - return () => window.clearInterval(id); - }, [loadBacklogCounts]); + // Non-critical — the column stays blank if the endpoint hiccups. + const backlogCountsQuery = useQuery({ + queryKey: ['admin', 'webhook-outbox', 'counts'], + queryFn: ({ signal }) => + api>('/api/admin/webhook-outbox/counts', { + signal, + }), + select: countsByForwarder, + refetchInterval: 10_000, + }); + const backlogCounts = backlogCountsQuery.data ?? NO_BACKLOG_COUNTS; + const [backlogForwarderId, setBacklogForwarderId] = useState(null); const resetForm = () => { setFormName(''); @@ -227,7 +223,7 @@ export function LogForwardersPage() { }); setDialogOpen(false); resetForm(); - loadForwarders(); + void invalidateForwarders(); toast.success(t('logForwarders.toast.created')); } catch (err) { setError(err instanceof Error ? err.message : t('common.error')); @@ -244,7 +240,7 @@ export function LogForwardersPage() { await apiPost(`/api/admin/log-forwarders/${id}/toggle`, { enabled: !currentlyEnabled, }); - loadForwarders(); + void invalidateForwarders(); toast.success(t('logForwarders.toast.toggled')); } catch (err) { setError(err instanceof Error ? err.message : t('common.error')); @@ -256,7 +252,7 @@ export function LogForwardersPage() { await apiDelete(`/api/admin/log-forwarders/${id}`); setDeleteDialogOpen(false); setDeleteTargetId(null); - loadForwarders(); + void invalidateForwarders(); } catch (err) { setError(err instanceof Error ? err.message : t('common.error')); } @@ -278,7 +274,7 @@ export function LogForwardersPage() { const handleResetStats = async (id: string) => { try { await apiPost(`/api/admin/log-forwarders/${id}/reset-stats`, {}); - loadForwarders(); + void invalidateForwarders(); toast.success(t('logForwarders.toast.statsReset')); } catch (err) { setError(err instanceof Error ? err.message : t('common.error')); @@ -360,7 +356,7 @@ export function LogForwardersPage() { }); setEditDialogOpen(false); setEditForwarder(null); - loadForwarders(); + void invalidateForwarders(); toast.success(t('logForwarders.toast.updated')); } catch (err) { setError(err instanceof Error ? err.message : t('common.error')); @@ -820,7 +816,6 @@ export function LogForwardersPage() { onOpenChange={(open) => { if (!open) setBacklogForwarderId(null); }} - onChanged={loadBacklogCounts} /> ); diff --git a/web/src/routes/admin/outbox-backlog-dialog.tsx b/web/src/routes/admin/outbox-backlog-dialog.tsx index b858f60b..fd02f878 100644 --- a/web/src/routes/admin/outbox-backlog-dialog.tsx +++ b/web/src/routes/admin/outbox-backlog-dialog.tsx @@ -1,7 +1,7 @@ -import { useCallback, useEffect, useState } from 'react'; +import { useState } from 'react'; import { useNow } from '@/hooks/use-now'; import { useTranslation } from 'react-i18next'; -import { useResetOnChange } from '@/hooks/use-reset-on-change'; +import { skipToken, useQuery, useQueryClient } from '@tanstack/react-query'; import { Button } from '@/components/ui/button'; import { Badge } from '@/components/ui/badge'; import { Checkbox } from '@/components/ui/checkbox'; @@ -42,9 +42,6 @@ interface OutboxBacklogDialogProps { /// already has it loaded. forwarderName?: string; onOpenChange: (open: boolean) => void; - /// Called after a retry / delete / natural drain so the parent can - /// refresh its backlog counts column without a separate poll. - onChanged?: () => void; } /// Per-forwarder backlog triage — the content that used to live on the @@ -58,61 +55,39 @@ export function OutboxBacklogDialog({ forwarderId, forwarderName, onOpenChange, - onChanged, }: OutboxBacklogDialogProps) { const { t, i18n } = useTranslation(); - const [data, setData] = useState(null); - const [error, setError] = useState(''); - const [loading, setLoading] = useState(false); + const queryClient = useQueryClient(); const [busyId, setBusyId] = useState(null); const [autoRefresh, setAutoRefresh] = useState(true); const now = useNow(); - const load = useCallback( - async (isInitial: boolean) => { - if (!forwarderId) return; - if (isInitial) setLoading(true); - setError(''); - try { - const res = await api( - `/api/admin/webhook-outbox?forwarder_id=${forwarderId}`, - ); - setData(res); - } catch (err) { - setError(err instanceof Error ? err.message : t('common.error')); - } finally { - if (isInitial) setLoading(false); - } - }, - [forwarderId, t], - ); - - useResetOnChange(forwarderId, () => { - if (!forwarderId) setData(null); + const outboxQuery = useQuery({ + queryKey: ['admin', 'webhook-outbox', { forwarder_id: forwarderId }], + queryFn: forwarderId + ? ({ signal }) => + api(`/api/admin/webhook-outbox?forwarder_id=${forwarderId}`, { + signal, + }) + : skipToken, + refetchInterval: autoRefresh ? 10_000 : false, }); + const data = outboxQuery.data ?? null; + // Only the first load shows the skeleton; the 10s poll refreshes in place. + const loading = outboxQuery.isLoading; + const error = outboxQuery.error?.message ?? ''; - useEffect(() => { - if (!forwarderId) return; - // Hand-rolled load: the spinner flag is the first half of "start a - // fetch" and belongs with it. See "Data fetching" in web/README.md — - // this goes away with a data-fetching layer, not by moving the flag. - // eslint-disable-next-line react-hooks/set-state-in-effect - void load(true); - }, [forwarderId, load]); - - useEffect(() => { - if (!forwarderId || !autoRefresh) return; - const id = window.setInterval(() => void load(false), 10_000); - return () => window.clearInterval(id); - }, [forwarderId, autoRefresh, load]); + /// A retry or a delete changes this list and the parent's backlog counts, + /// which live under the same prefix — one invalidation refreshes both. + const invalidateOutbox = () => + queryClient.invalidateQueries({ queryKey: ['admin', 'webhook-outbox'] }); const handleRetry = async (id: string) => { setBusyId(id); try { await apiPost(`/api/admin/webhook-outbox/${id}/retry`, {}); toast.success(t('webhookOutbox.retryQueued')); - await load(false); - onChanged?.(); + await invalidateOutbox(); } catch (err) { toast.error(err instanceof Error ? err.message : t('common.error')); } finally { @@ -126,8 +101,7 @@ export function OutboxBacklogDialog({ try { await apiDelete(`/api/admin/webhook-outbox/${id}`); toast.success(t('webhookOutbox.deleted')); - await load(false); - onChanged?.(); + await invalidateOutbox(); } catch (err) { toast.error(err instanceof Error ? err.message : t('common.error')); } finally { @@ -178,7 +152,7 @@ export function OutboxBacklogDialog({