Fix race condition in async requests (#882)

CHANGELOG.md

@@ -5,6 +5,16 @@ All notable changes to Turbo EA are documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/).
 
+## [2.32.1] - 2026-07-27
+
+### Fixed
+- **Changing an inventory filter while a slower request was still running could leave the grid showing the wrong cards.** Clearing the type filter fetches the whole repository; picking a specific type straight afterwards came back first, and then the slow response landed and overwrote the grid — so the grid listed every card while the sidebar said «Application». Filter-driven requests are now cancelled when a newer one starts, and a superseded response is discarded instead of applied.
+- **The Matrix report could draw one pair of card types under the labels of another.** It never cleared the previous data, so after switching an axis the old grid stayed on screen beneath the new row and column headings, with nothing to indicate the two disagreed. Switching axes now shows the loading indicator until the matching data arrives.
+- **Typing in the inventory search box fired a full repository request per keystroke.** The search term now settles for a moment before it is sent, and the grid keeps its loading indicator from the very first keystroke, so it never looks finished while it is still showing results for what you typed before. Type, approval and archived toggles stay instant.
+- **The same stale-result problem affected the Cost, Dependency, Capability Map and Lifecycle reports, the Todos list and the PPM portfolio.** Every view whose data reloads when you change a picker now discards results for a selection you have moved on from.
+- **The portfolio reports could sit on a loading spinner forever.** Switching card type cleared the chart before the new data arrived, and a request that failed left nothing to bring it back. A failure now shows an error message instead.
+- **Card pickers could get stuck showing results for a search you had already replaced.** Changing the type filter or search term while a request was in flight dropped the new request outright and never retried it. This affected relation, parent and vendor pickers throughout the app, and the search fields in the survey builder, calculation tester, survey response form and End-of-Life linking.
+
 ## [2.32.0] - 2026-07-27
 
 ### Added

CLAUDE.md

@@ -199,6 +199,8 @@ The **Extension Store** (Admin → Extensions) installs vendor-signed, licensed
 - Feature-specific components go in `src/features/{feature}/`.
 - Use `api.get()`, `api.post()`, `api.patch()`, `api.delete()` from `src/api/client.ts` for all API calls.
 - JWT token is in `sessionStorage` (not localStorage). Use `setToken()`/`clearToken()` from `client.ts`.
+- **Filter-driven GETs must go through the request hooks — never a bare `api.get` in a `useEffect`.** Any fetch keyed on user-controlled state (a type picker, a search box, a tab, a metric toggle) races: the request for the *previous* value can land after the one for the current value and overwrite it, leaving the grid or chart showing data the controls say you are not looking at (#882). Use `useApiQuery(path | null)` (`frontend/src/hooks/useApiQuery.ts`) when one payload feeds one state; `useAbortableEffect(fn, deps)` (`frontend/src/hooks/useLatestRequest.ts`) when the effect writes several states or fans out over `Promise.all`; and the `useLatestRequest()` primitive when the fetch must also be callable imperatively — `InventoryPage`'s `loadData`, which 13 mutation handlers refresh through. All three abort the predecessor **and** bump a generation token. The token is not redundant: an already-resolved response cannot be aborted, and test doubles ignore `AbortSignal` entirely. Two rules are routinely missed and are exactly what re-breaks this: **(1)** guard *every* `setState` with `isCurrent()`, and **(2)** clear the loading flag **only** when `isCurrent()` — an unconditional `finally { setLoading(false) }` lets the aborted predecessor clear the spinner while the winner is still in flight, flashing "done" over stale rows. Debounce free-text search with `useDebouncedValue(value, 300)` and OR its `pending` flag into the loading prop, so a grid never looks settled while it is still showing the previous query's rows; keep discrete toggles instant. `api.get(path, { signal })` rejects with an `AbortError` — if you call it outside these hooks you own that rejection, so check `isAbortError(err)` from `src/api/client.ts`. `signal` is deliberately **not** offered on `post`/`patch`/`put`/`delete`: aborting a mutation does not undo it server-side.
+
 - **Boot-time singleton hooks must use the inflight-promise pattern.** Hooks that fetch a process-wide value once (`useMetamodel`, `useDateFormat`, `useCurrency`, `useBpmEnabled`, `usePpmEnabled`, `useTurboLensReady`, …) keep a module-level `_cache` plus an `_inflight: Promise | null`. The fetch helper checks **both** — if `_cache` has the value, return it; if `_inflight` is non-null, return that promise; only otherwise start a new fetch and store it in `_inflight`, clearing it on settle. The naive `if (_cache === null) _fetch()` pattern races: when several components mount in the same tick they each see an empty cache and each fire their own request. Add an inflight guard to any new singleton hook of this shape.
 - **For new boot-time settings, prefer adding to `/settings/bootstrap` over creating a new singleton hook.** The frontend's `primeBootstrap()` (in `src/api/bootstrap.ts`) calls `GET /settings/bootstrap` once after login and pushes each value into the corresponding hook's singleton cache via the hook's top-level `invalidate*(value)` export. Every per-hook GET that fires before bootstrap arrives is a wasted round-trip; every value bootstrap doesn't carry is a duplicate fetch. When you add a settings hook, also add a top-level `invalidate*(value)` export, extend `BootstrapResponse` in `bootstrap.ts`, and call your invalidator from `primeBootstrap()`.
 - All TypeScript interfaces live in `src/types/index.ts`.

VERSION

@@ -1 +1 @@
-2.32.0
+2.32.1

frontend/UI_GUIDELINES.md

@@ -227,6 +227,12 @@ Always render status through one of these:
 
 Skeleton loaders are not used today — that's a deliberate choice. If introduced, add a section here.
 
+**Control-driven surfaces** (a grid or chart whose data reloads when a picker, tab or search box changes) have three additional rules — see the request-hook conventions in `CLAUDE.md` for the mechanics:
+
+- **The loading indicator starts when the control changes, not when the request does.** If the input is debounced, the debounce window is part of the loading state. A surface that looks settled while it is still showing the previous query's rows reads as a wrong answer, not a pending one.
+- **A failed fetch renders an `<Alert severity="error">`, never an endless spinner.** Keep the previous data behind the alert where there is any; blanking the view loses the user's context for no benefit.
+- **Don't blank to a spinner on every picker change when the previous render is still meaningful.** The exception is a view whose labels are drawn from the *current* selection while the body comes from the response — a matrix, a cross-tab — where showing the previous payload under the new headers is worse than a spinner, because nothing on screen signals the mismatch.
+
 ### 3.9 Icons
 
 - Use `<MaterialSymbol icon="..." size={...} color={...}/>` — never raw SVG.

frontend/src/api/client.test.ts

@@ -8,6 +8,7 @@ import {
   isAuthenticated,
   setAuthenticated,
   auth,
+  isAbortError,
 } from "./client";
 
 // ---------------------------------------------------------------------------
@@ -294,3 +295,62 @@ describe("auth helpers", () => {
     expect(mockFetch.mock.calls[0][0]).toBe("/api/v1/auth/logout");
   });
 });
+
+// ---------------------------------------------------------------------------
+// Request cancellation (#882)
+// ---------------------------------------------------------------------------
+
+describe("AbortSignal support", () => {
+  it("forwards a signal from api.get into fetch", async () => {
+    mockFetch.mockReturnValueOnce(jsonResponse({ ok: true }));
+    const ctrl = new AbortController();
+
+    await api.get("/cards", { signal: ctrl.signal });
+
+    const init = mockFetch.mock.calls[0][1];
+    expect(init.signal).toBe(ctrl.signal);
+    // The signal must not displace the standard request configuration.
+    expect(init.credentials).toBe("same-origin");
+    expect(init.headers["X-Turbo-EA-Origin"]).toBe("web");
+  });
+
+  it("forwards a signal from api.getRaw into fetch", async () => {
+    mockFetch.mockReturnValueOnce(jsonResponse({ ok: true }));
+    const ctrl = new AbortController();
+
+    await api.getRaw("/cards/export/csv", { signal: ctrl.signal });
+
+    expect(mockFetch.mock.calls[0][1].signal).toBe(ctrl.signal);
+  });
+
+  it("omits the signal when no options are passed", async () => {
+    mockFetch.mockReturnValueOnce(jsonResponse({ ok: true }));
+
+    await api.get("/cards");
+
+    expect(mockFetch.mock.calls[0][1].signal).toBeUndefined();
+  });
+
+  it("propagates the abort rejection rather than swallowing it", async () => {
+    // `request()` must reject so `finally { setLoading(false) }` still runs.
+    // Only call sites that opted in by passing a signal can ever observe this.
+    const abortErr = new DOMException("The user aborted a request.", "AbortError");
+    mockFetch.mockReturnValueOnce(Promise.reject(abortErr));
+
+    await expect(api.get("/cards", { signal: new AbortController().signal })).rejects.toBe(
+      abortErr,
+    );
+  });
+
+  it("isAbortError recognises an abort and nothing else", () => {
+    expect(isAbortError(new DOMException("aborted", "AbortError"))).toBe(true);
+    // Duck-typed on purpose — jsdom/undici and test doubles may not be DOMException.
+    expect(isAbortError({ name: "AbortError" })).toBe(true);
+
+    expect(isAbortError(new ApiError("boom", 500, null))).toBe(false);
+    expect(isAbortError(new Error("network"))).toBe(false);
+    expect(isAbortError("AbortError")).toBe(false);
+    expect(isAbortError(null)).toBe(false);
+    expect(isAbortError(undefined)).toBe(false);
+  });
+});

frontend/src/api/client.ts

@@ -1,5 +1,28 @@
 const BASE = "/api/v1";
 
+/**
+ * Options accepted by the read helpers. Deliberately narrower than `RequestInit`
+ * so call sites can't override method / credentials / headers.
+ */
+export interface GetOptions {
+  signal?: AbortSignal;
+}
+
+/**
+ * True for a fetch aborted via an AbortSignal. A superseded request is not an
+ * error — never surface one to the user.
+ *
+ * Matches on `name` rather than `instanceof DOMException` so it also holds under
+ * jsdom/undici and for anything a test double throws.
+ */
+export function isAbortError(err: unknown): boolean {
+  return (
+    typeof err === "object" &&
+    err !== null &&
+    (err as { name?: unknown }).name === "AbortError"
+  );
+}
+
 /** Error with HTTP status and structured detail from the API. */
 export class ApiError extends Error {
   status: number;
@@ -92,8 +115,16 @@ async function requestRaw(path: string, options: RequestInit = {}): Promise<Resp
 }
 
 export const api = {
-  get: <T>(path: string) => request<T>(path),
-  getRaw: (path: string) => requestRaw(path),
+  // Passing a `signal` means you own the resulting AbortError rejection.
+  // Prefer the request hooks in `src/hooks/` (`useApiQuery`,
+  // `useAbortableEffect`, `useLatestRequest`) — they abort the predecessor,
+  // discard superseded responses and swallow the abort for you.
+  //
+  // Deliberately not offered on the mutating helpers: aborting a POST/PATCH
+  // does not undo it server-side, so exposing `signal` there would advertise a
+  // cancellation guarantee we cannot make.
+  get: <T>(path: string, opts?: GetOptions) => request<T>(path, opts),
+  getRaw: (path: string, opts?: GetOptions) => requestRaw(path, opts),
   post: <T>(path: string, body?: unknown) =>
     request<T>(path, { method: "POST", body: JSON.stringify(body) }),
   patch: <T>(path: string, body?: unknown) =>

frontend/src/components/CreateCardDialog.tsx

@@ -36,6 +36,8 @@ import {
   useSubtypeLabel,
 } from "@/hooks/useResolveLabel";
 import { useAiStatus } from "@/hooks/useAiStatus";
+import { useAbortableEffect } from "@/hooks/useLatestRequest";
+import { useDebouncedValue } from "@/hooks/useDebouncedValue";
 import { api, ApiError } from "@/api/client";
 import type {
   FieldDef,
@@ -201,33 +203,41 @@ export default function CreateCardDialog({
     }
   }, [open, initialType]);
 
-  // Auto-search EOL when name changes (debounced)
-  useEffect(() => {
-    if (!isEolEligible || !name.trim() || name.trim().length < 2) {
-      setEolSuggestions([]);
-      setEolAutoSearchDone(false);
-      return;
-    }
-    // Don't auto-search if already linked
-    if (eolProduct && eolCycle) return;
+  // Auto-search EOL when name changes (debounced). Cancelling the timer never
+  // cancelled a dispatched request, so suggestions for a half-typed name could
+  // land last and replace the ones for the finished name (#882).
+  const [debouncedName] = useDebouncedValue(name, 600);
+  useAbortableEffect(
+    async ({ signal, isCurrent }) => {
+      const trimmed = debouncedName.trim();
+      if (!isEolEligible || !trimmed || trimmed.length < 2) {
+        setEolSuggestions([]);
+        setEolAutoSearchDone(false);
+        setEolSearching(false);
+        return;
+      }
+      // Don't auto-search if already linked
+      if (eolProduct && eolCycle) return;
 
-    const timer = setTimeout(async () => {
       setEolSearching(true);
       try {
         const results = await api.get<EolProductMatch[]>(
-          `/eol/products/fuzzy?search=${encodeURIComponent(name.trim())}&limit=5`
+          `/eol/products/fuzzy?search=${encodeURIComponent(trimmed)}&limit=5`,
+          { signal },
         );
+        if (!isCurrent()) return;
         setEolSuggestions(results);
         setEolAutoSearchDone(true);
       } catch {
+        if (!isCurrent()) return;
         setEolSuggestions([]);
         setEolAutoSearchDone(true);
       } finally {
-        setEolSearching(false);
+        if (isCurrent()) setEolSearching(false);
       }
-    }, 600);
-    return () => clearTimeout(timer);
-  }, [name, isEolEligible, eolProduct, eolCycle]);
+    },
+    [debouncedName, isEolEligible, eolProduct, eolCycle],
+  );
 
   const setAttr = (key: string, value: unknown) => {
     setAttributes((prev) => ({ ...prev, [key]: value }));

frontend/src/components/EolLinkSection.tsx

@@ -24,6 +24,8 @@ import LinearProgress from "@mui/material/LinearProgress";
 import MaterialSymbol from "@/components/MaterialSymbol";
 import { useSyncedExpanded } from "@/hooks/useSyncedExpanded";
 import { api } from "@/api/client";
+import { useAbortableEffect } from "@/hooks/useLatestRequest";
+import { useDebouncedValue } from "@/hooks/useDebouncedValue";
 import { STATUS_COLORS } from "@/theme/tokens";
 import type { Card, EolProduct, EolCycle, EolProductMatch } from "@/types";
 
@@ -129,53 +131,64 @@ function EolPicker({ onSelect, onCancel, initialProduct, resetKey, cardName }: E
   }, [resetKey]);  // eslint-disable-line react-hooks/exhaustive-deps
 
   // Auto-fuzzy-search using card name (mirrors CreateCardDialog pattern)
-  useEffect(() => {
-    const trimmed = (cardName || "").trim();
-    if (!trimmed || trimmed.length < 2 || selectedProduct) {
-      setEolSuggestions([]);
-      setEolAutoSearchDone(false);
-      return;
-    }
-    if (productSearch && productSearch !== trimmed) return;
-    const timer = setTimeout(async () => {
+  const [debouncedCardName] = useDebouncedValue(cardName, 600);
+  useAbortableEffect(
+    async ({ signal, isCurrent }) => {
+      const trimmed = (debouncedCardName || "").trim();
+      if (!trimmed || trimmed.length < 2 || selectedProduct) {
+        setEolSuggestions([]);
+        setEolAutoSearchDone(false);
+        setEolSearching(false);
+        return;
+      }
+      if (productSearch && productSearch !== trimmed) return;
       setEolSearching(true);
       try {
         const res = await api.get<EolProductMatch[]>(
           `/eol/products/fuzzy?search=${encodeURIComponent(trimmed)}&limit=5`,
+          { signal },
         );
+        if (!isCurrent()) return;
         setEolSuggestions(res);
         setEolAutoSearchDone(true);
       } catch {
+        if (!isCurrent()) return;
         setEolSuggestions([]);
         setEolAutoSearchDone(true);
       } finally {
-        setEolSearching(false);
+        if (isCurrent()) setEolSearching(false);
       }
-    }, 600);
-    return () => clearTimeout(timer);
-  }, [cardName, productSearch, selectedProduct]);
+    },
+    [debouncedCardName, productSearch, selectedProduct],
+  );
 
-  // Search products
-  useEffect(() => {
-    if (productSearch.length < 2) {
-      setProductOptions([]);
-      return;
-    }
-    const timer = setTimeout(async () => {
+  // Search products. The timer was cancelled on each keystroke but a dispatched
+  // request was not, so a stale response could replace newer options (#882).
+  const [debouncedProductSearch] = useDebouncedValue(productSearch, 300);
+  useAbortableEffect(
+    async ({ signal, isCurrent }) => {
+      if (debouncedProductSearch.length < 2) {
+        setProductOptions([]);
+        setProductLoading(false);
+        return;
+      }
       setProductLoading(true);
       try {
         const res = await api.get<EolProduct[]>(
-          `/eol/products?search=${encodeURIComponent(productSearch)}`
+          `/eol/products?search=${encodeURIComponent(debouncedProductSearch)}`,
+          { signal },
         );
+        if (!isCurrent()) return;
         setProductOptions(res);
       } catch {
+        if (!isCurrent()) return;
         setProductOptions([]);
       } finally {
-        setProductLoading(false);
+        if (isCurrent()) setProductLoading(false);
       }
-    }, 300);
-    return () => clearTimeout(timer);
-  }, [productSearch]);
+    },
+    [debouncedProductSearch],
+  );
 
   // Fetch cycles when product is selected
   useEffect(() => {