Clear 127 lint findings and gate lint in CI

.github/workflows/ci.yml

@@ -102,6 +102,12 @@ jobs:
       - name: i18n parity
         run: pnpm check:i18n
 
+      # Added once the tree was clean. It had never been gated, and 127
+      # findings — 90 of them errors — had accumulated behind that: a check
+      # nobody runs only moves one direction.
+      - name: Lint
+        run: pnpm lint
+
       - name: Test
         run: pnpm test
 

web/README.md

@@ -21,6 +21,31 @@ pnpm test         # Run tests
 pnpm exec tsc --noEmit  # Type check
 ```
 
+## Data fetching
+
+Loads are hand-rolled: a `useCallback` that fetches and sets state, plus a
+`useEffect` that calls it. There is no data-fetching layer in this app.
+
+That shape trips `react-hooks/set-state-in-effect`, and the sites that it
+flags carry a one-line suppression saying which of two things is going on:
+
+- **The rule is wrong.** `useEffect(() => { load(); }, [load])` where `load`
+  is async and its first statement is the `await`. Every setState inside
+  runs in the continuation — never synchronously with the effect, never a
+  cascading render. The rule's cross-function analysis does not model
+  `await`.
+- **The rule is right and the fix is architectural.** The loader's first
+  statement flips a spinner. Hoisting that flag out to render-time silences
+  the rule, but it splits "start a load" across two places and leaves every
+  other caller of the loader responsible for remembering half of it.
+
+**Both go away with a data-fetching layer** (TanStack Query or equivalent),
+which owns the loading flag and the cache and removes the effect entirely.
+That is a deliberate piece of work, not something to fold into a lint pass.
+Until then, do not add new suppressions of this rule without one of the two
+reasons above — every other finding it reports is a real one, and the rest
+of the codebase is clean of them.
+
 ## Project Structure
 
 ```

web/e2e/fixtures.ts

@@ -65,6 +65,7 @@ async function loginAdmin(page: Page) {
       `Login failed for ${ADMIN_EMAIL} — set PW_ADMIN_EMAIL and PW_ADMIN_PASSWORD to match your dev DB.\n` +
         `Defaults: admin@thinkwatch.local / Admin_pass_1!\n` +
         `Underlying error: ${e instanceof Error ? e.message : e}`,
+      { cause: e },
     );
   }
 }

web/eslint.config.js

@@ -27,10 +27,56 @@ export default defineConfig([
       // `warn` rather than `error` while we land the underlying
       // refactors; flip to error once the existing finds are cleaned up.
       'react-compiler/react-compiler': 'warn',
+      // A leading underscore already means "deliberately unused" throughout
+      // this codebase — constructor parameters that exist only to match an
+      // upstream signature, `{ _clientId: _, ...rest }` to drop a field.
+      // Without this the convention reads as five defects.
+      '@typescript-eslint/no-unused-vars': [
+        'error',
+        {
+          argsIgnorePattern: '^_',
+          varsIgnorePattern: '^_',
+          caughtErrorsIgnorePattern: '^_',
+          destructuredArrayIgnorePattern: '^_',
+          ignoreRestSiblings: true,
+        },
+      ],
     },
     languageOptions: {
       ecmaVersion: 2020,
       globals: globals.browser,
     },
   },
+  {
+    // Playwright fixtures, not React. `base.extend({ adminPage: async
+    // ({ page }, use) => ... })` hands the fixture a callback named `use`,
+    // and the hooks rule reads that call as a `use()` hook outside a
+    // component. There is no React in this directory at all.
+    files: ['e2e/**'],
+    rules: {
+      'react-hooks/rules-of-hooks': 'off',
+    },
+  },
+  {
+    // `src/components/ui/` is shadcn output, not code we write. Its house
+    // style deliberately ships a component and its variants from one file
+    // (`Button` + `buttonVariants`, `Sidebar` + `useSidebar`), which costs
+    // Fast Refresh on those modules.
+    //
+    // **Splitting them would not survive.** The next `pnpm dlx shadcn add`
+    // overwrites the file and the finding comes straight back, so enforcing
+    // the rule here buys a warning that has to be re-fixed forever. The
+    // files are leaf primitives that rarely change; losing HMR on them is
+    // the cheaper side of the trade.
+    //
+    // The compiler rule is off here for the same reason: shadcn's sidebar
+    // writes `document.cookie` inside a `useCallback` to persist the open
+    // state. That is the upstream implementation, and editing it has the
+    // same problem — the next `add` puts it back.
+    files: ['src/components/ui/**'],
+    rules: {
+      'react-refresh/only-export-components': 'off',
+      'react-compiler/react-compiler': 'off',
+    },
+  },
 ])

web/src/components/command-palette.tsx

@@ -1,6 +1,7 @@
 import { Fragment, useEffect, useMemo, useState } from 'react';
 import { useNavigate } from '@tanstack/react-router';
 import { useTranslation } from 'react-i18next';
+import { useResetOnChange } from '@/hooks/use-reset-on-change';
 import { Dialog, DialogContent } from '@/components/ui/dialog';
 import { Input } from '@/components/ui/input';
 import {
@@ -45,11 +46,15 @@ interface RecentGatewayResponse {
 /// users without `logs:read_all` just won't see the section.
 function useRecentTraces(open: boolean): CmdAction[] {
   const [items, setItems] = useState<CmdAction[]>([]);
+  // Clearing on close is state alignment, not a side effect — doing it
+  // during render means the palette never paints yesterday's traces for a
+  // frame when it is reopened.
+  useResetOnChange(open, () => {
+    if (!open) setItems([]);
+  });
+
   useEffect(() => {
-    if (!open) {
-      setItems([]);
-      return;
-    }
+    if (!open) return;
     let cancelled = false;
     api<RecentGatewayResponse>('/api/gateway/logs?limit=5&offset=0', {
       no401Redirect: true,
@@ -125,12 +130,12 @@ export function CommandPalette() {
   }, []);
 
   // Reset state on open
-  useEffect(() => {
+  useResetOnChange(open, () => {
     if (open) {
       setQuery('');
       setActiveIdx(0);
     }
-  }, [open]);
+  });
 
   const recentTraces = useRecentTraces(open);
 

web/src/components/confirm-dialog.tsx

@@ -9,7 +9,7 @@ import {
   DialogTitle,
 } from '@/components/ui/dialog';
 import { Input } from '@/components/ui/input';
-import { useState, useEffect } from 'react';
+import { useState } from 'react';
 
 interface ConfirmDialogProps {
   open: boolean;
@@ -40,9 +40,15 @@ export function ConfirmDialog({
   const { t } = useTranslation();
   const [inputValue, setInputValue] = useState('');
 
-  useEffect(() => {
+  // Clear the typed confirmation when the dialog closes, adjusted during
+  // render rather than in an effect: an effect would paint the stale text
+  // for one frame on the way out, and React re-runs this render before
+  // anything reaches the screen.
+  const [wasOpen, setWasOpen] = useState(open);
+  if (wasOpen !== open) {
+    setWasOpen(open);
     if (!open) setInputValue('');
-  }, [open]);
+  }
 
   const canConfirm = requireInput ? inputValue === requireInput : true;
 

web/src/components/dashboard/card-timeout.ts

@@ -0,0 +1,31 @@
+// Split out of `stat-cards.tsx` so that file exports only components:
+// Fast Refresh gives up on a module that mixes the two, and losing HMR on
+// the dashboard cards is a real cost while iterating on them.
+
+// 12s is past every realistic CH analytics query (P99 < 3s on the
+// existing dashboards) but short enough that a frozen result lands
+// the per-card error fallback well before a user gives up scrolling.
+export const DASHBOARD_CARD_TIMEOUT_MS = 12_000;
+
+/**
+ * Race a promise against a deadline; on timeout reject with a
+ * labelled Error that the ErrorBoundary surfaces. The cleared timer
+ * keeps the JS heap clean when the underlying request resolves first.
+ */
+export function withTimeout<T>(p: Promise<T>, ms: number, label: string): Promise<T> {
+  return new Promise<T>((resolve, reject) => {
+    const id = setTimeout(() => {
+      reject(new Error(`Timed out fetching ${label} (>${ms}ms)`));
+    }, ms);
+    p.then(
+      (v) => {
+        clearTimeout(id);
+        resolve(v);
+      },
+      (e) => {
+        clearTimeout(id);
+        reject(e);
+      },
+    );
+  });
+}

web/src/components/dashboard/getting-started-card.tsx

@@ -1,4 +1,4 @@
-import { useEffect, useState } from 'react';
+import { useState } from 'react';
 import { useTranslation } from 'react-i18next';
 import { Link } from '@tanstack/react-router';
 import { X, KeyRound, Plug, Users } from 'lucide-react';
@@ -24,14 +24,17 @@ export function GettingStartedCard({
   signals: { hasApiKeys: boolean; hasProviders: boolean };
 }) {
   const { t } = useTranslation();
-  const [dismissed, setDismissed] = useState(false);
-  useEffect(() => {
+  // Read in the initialiser, not an effect: an effect renders the card
+  // once before hiding it, so a user who dismissed it still sees it flash
+  // on every page load.
+  const [dismissed, setDismissed] = useState(() => {
     try {
-      if (window.localStorage.getItem(DISMISSED_KEY) === '1') setDismissed(true);
+      return window.localStorage.getItem(DISMISSED_KEY) === '1';
     } catch {
-      // ignore
+      // Private windows and blocked site data both throw here.
+      return false;
     }
-  }, []);
+  });
 
   if (dismissed) return null;
   // Auto-suppress once the platform is past first-run state — the

web/src/components/dashboard/live-log-panel.tsx

@@ -9,6 +9,7 @@
 
 import { memo, useEffect, useRef, useState } from 'react';
 import { useTranslation } from 'react-i18next';
+import { useResetOnChange } from '@/hooks/use-reset-on-change';
 import { Pause, Play } from 'lucide-react';
 
 import { Card } from '@/components/ui/card';
@@ -171,26 +172,18 @@ export function LiveLogPanel({
 }) {
   const { t } = useTranslation();
   const [snapshot, setSnapshot] = useState<LiveLogRow[] | null>(null);
-  useEffect(() => {
-    if (paused) {
-      setSnapshot(rows);
-    } else {
-      setSnapshot(null);
-    }
-    // Intentionally ignore `rows` here — we only snapshot on the
-    // pause edge. The `rows === null` reset below handles the case
-    // where the upstream `live` state gets cleared (range toggle).
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [paused]);
+  // Snapshot on the pause edge only — keyed on `paused`, so a new batch of
+  // rows arriving while paused does not overwrite what the user froze.
+  useResetOnChange(paused, () => {
+    setSnapshot(paused ? rows : null);
+  });
   // If the live stream is reset mid-pause (range change clears `live`),
   // drop the snapshot too so the panel doesn't keep showing old-window
   // rows under the new range's eyebrow. Re-pause on the next WS frame
   // re-captures from the new window.
-  useEffect(() => {
-    if (paused && rows === null) {
-      setSnapshot(null);
-    }
-  }, [paused, rows]);
+  useResetOnChange(rows, () => {
+    if (paused && rows === null) setSnapshot(null);
+  });
   const displayed = paused ? snapshot : rows;
 
   // Mirror what the row layout will be so headers and rows align perfectly.

web/src/components/dashboard/provider-health-panel.tsx

@@ -41,7 +41,7 @@ export function ProviderFilterTabs({
   const onKeyDown = (e: ReactKeyboardEvent<HTMLButtonElement>) => {
     const idx = tabs.findIndex((t) => t.key === value);
     if (idx < 0) return;
-    let next = idx;
+    let next: number;
     if (e.key === 'ArrowRight') next = (idx + 1) % tabs.length;
     else if (e.key === 'ArrowLeft') next = (idx - 1 + tabs.length) % tabs.length;
     else if (e.key === 'Home') next = 0;

web/src/components/dashboard/stat-cards.tsx

@@ -11,15 +11,7 @@
  * rejected fetch and falls back to a terminal card variant.
  */
 
-import {
-  Suspense,
-  use,
-  useEffect,
-  useMemo,
-  useRef,
-  useState,
-  type ReactNode,
-} from 'react';
+import { Suspense, type ReactNode, use, useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { useTranslation } from 'react-i18next';
 import { Area, AreaChart } from 'recharts';
 import { toast } from 'sonner';
@@ -44,34 +36,6 @@ import type {
   UsageStats,
 } from './types';
 
-// 12s is past every realistic CH analytics query (P99 < 3s on the
-// existing dashboards) but short enough that a frozen result lands
-// the per-card error fallback well before a user gives up scrolling.
-export const DASHBOARD_CARD_TIMEOUT_MS = 12_000;
-
-/**
- * Race a promise against a deadline; on timeout reject with a
- * labelled Error that the ErrorBoundary surfaces. The cleared timer
- * keeps the JS heap clean when the underlying request resolves first.
- */
-export function withTimeout<T>(p: Promise<T>, ms: number, label: string): Promise<T> {
-  return new Promise<T>((resolve, reject) => {
-    const id = setTimeout(() => {
-      reject(new Error(`Timed out fetching ${label} (>${ms}ms)`));
-    }, ms);
-    p.then(
-      (v) => {
-        clearTimeout(id);
-        resolve(v);
-      },
-      (e) => {
-        clearTimeout(id);
-        reject(e);
-      },
-    );
-  });
-}
-
 const STAT_ORDER_KEY = 'dashboard.stat-order.v1';
 
 /**
@@ -88,15 +52,15 @@ export function StatCardGrid({ cards }: { cards: Record<string, ReactNode> }) {
   const { t } = useTranslation();
   const defaultOrder = useMemo(() => Object.keys(cards), [cards]);
 
-  const mergeOrder = (saved: unknown): string[] => {
+  const mergeOrder = useCallback((saved: unknown): string[] => {
     if (!Array.isArray(saved) || !saved.every((k) => typeof k === 'string')) {
       return defaultOrder;
     }
     // Drop ids that no longer exist (card removed); append new ones at end.
     const known = saved.filter((k) => k in cards);
     for (const k of defaultOrder) if (!known.includes(k)) known.push(k);
     return known;
-  };
+  }, [cards, defaultOrder]);
 
   const [order, setOrder] = useState<string[]>(() => {
     try {
@@ -132,8 +96,7 @@ export function StatCardGrid({ cards }: { cards: Record<string, ReactNode> }) {
     };
     // Intentionally only reconcile once per mount — subsequent drags
     // write through to the server, so there's no race to rehydrate.
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+  }, [mergeOrder]);
 
   const persist = (next: string[]) => {
     setOrder(next);