fix(830): a write failure reports to a surface that outlives the dialog that started it (#878)

fixes #830

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

docs/decisions/README.md

@@ -195,6 +195,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
 | `spa.collection-custom-order-ui` | Collection custom ordering uses per-row Move up/Move down buttons (not drag) and is offered for any manual collection with custom order enabled, not just movies-only. | 2026-07-09 | [link](records/spa/collection-custom-order-ui.md) |
 | `spa.datetime-local-input` | The channel-mode date/time input uses a native `<input type="datetime-local">` instead of free-text Chronic natural-language parsing. | 2026-07-09 | [link](records/spa/datetime-local-input.md) |
 | `spa.deco-templates-table` | The deco-templates editor also renders its day/deco assignment as a table, extending (not replacing) the templates-editor-table convention. | 2026-07-09 | [link](records/spa/deco-templates-table.md) |
+| `spa.dismissible-write-failure-reporting` | A component that starts an async WRITE from inside a dismissible surface (`Dialog`, `SlideOver`, `ConfirmDialog`) must report failure through `useDismissSafeError` (`web/src/hooks.ts`), which renders the message INLINE while the surface is mounted and hands it to a caller-supplied `onFailed` once the surface is gone. Guarding the `setError` with an is-mounted check and stopping there is NOT sufficient: it converts a silent data-loss risk into a silent NO-OP the user reads as success. All three primitives dismiss through Escape and a backdrop/scrim click (`useOverlayBehavior`) plus a header close button, and NONE of those consult a busy flag — disabling the footer Cancel button, which every one of these dialogs does, looks like it closes the hole and does not. Report the OUTCOME past dismissal in both directions — a success callback gated on an is-mounted check makes a completed write silent too (measured on `AddToCollectionDialog`, #877) — but gate the DISMISS request (`onClose`) separately, because closing a surface that is no longer yours closes whatever replaced it. The surviving surface belongs to the PARENT, so the mechanism is a PROP CONTRACT rather than a rendering decision; as of 2026-08-29 exactly ONE screen is wired (`CollectionsScreen`, into its screen-level `role="alert"` banner), and the `notice`+`Toast` pair on `MediaBrowseScreen`/`SearchScreen` is a CANDIDATE second surface, not a wired one; there is no global toast host in this SPA and this decision does not add one. The reporting prop is REQUIRED where the host has a surface (`AddItemsDialog.onAddFailed`), so a failure cannot be dropped by forgetting to wire it; it is optional only where some host genuinely has nowhere to report, and there an omitted callback drops the failure exactly as before — a KNOWN remaining gap, not a claim of coverage. | 2026-08-29 | [link](records/spa/dismissible-write-failure-reporting.md) |
 | `spa.download-sample-gate` | The SPA disables both Download Media Sample and Download Results while a troubleshooting session is starting/running (Blazor only gated Download Results). | 2026-07-09 | [link](records/spa/download-sample-gate.md) |
 | `spa.field-progressive-disclosure` | A consequential settings field explains itself through one shared `FieldHelp` icon trigger beside the field name — never the label itself, never a widened `Tooltip` — with the paragraph declared `as const` in the screen's own `FIELD_HELP` record and the panel portalled to `document.body`. | 2026-08-26 | [link](records/spa/field-progressive-disclosure.md) |
 | `spa.legacy-redirect-matcher` | `LegacyUiRedirects.TryGetRedirect` is a two-tier matcher — an exact `OrdinalIgnoreCase` `Map` (Tier 1) then an ordered segment-template pattern list (Tier 2, first-match-wins) — collision-free by construction, with a guard invariant that no rule may prefix-match `/api`, `/artwork`, `/docs`, `/openapi`, `/iptv`, `/app`, or `/media/sources`. | 2026-07-11 | [link](records/spa/legacy-redirect-matcher.md) |

docs/decisions/records/spa/dismissible-write-failure-reporting.md

@@ -0,0 +1,100 @@
+---
+key: spa.dismissible-write-failure-reporting
+title: '2026-08-29 — A write failure reports to a surface that OUTLIVES the dismissible surface that started it (#830)'
+status: active
+since: '2026-08-29'
+supersedes: none
+superseded-by: none
+rule: 'A component that starts an async WRITE from inside a dismissible surface (`Dialog`, `SlideOver`, `ConfirmDialog`) must report failure through `useDismissSafeError` (`web/src/hooks.ts`), which renders the message INLINE while the surface is mounted and hands it to a caller-supplied `onFailed` once the surface is gone. Guarding the `setError` with an is-mounted check and stopping there is NOT sufficient: it converts a silent data-loss risk into a silent NO-OP the user reads as success. All three primitives dismiss through Escape and a backdrop/scrim click (`useOverlayBehavior`) plus a header close button, and NONE of those consult a busy flag — disabling the footer Cancel button, which every one of these dialogs does, looks like it closes the hole and does not. Report the OUTCOME past dismissal in both directions — a success callback gated on an is-mounted check makes a completed write silent too (measured on `AddToCollectionDialog`, #877) — but gate the DISMISS request (`onClose`) separately, because closing a surface that is no longer yours closes whatever replaced it. The surviving surface belongs to the PARENT, so the mechanism is a PROP CONTRACT rather than a rendering decision; as of 2026-08-29 exactly ONE screen is wired (`CollectionsScreen`, into its screen-level `role="alert"` banner), and the `notice`+`Toast` pair on `MediaBrowseScreen`/`SearchScreen` is a CANDIDATE second surface, not a wired one; there is no global toast host in this SPA and this decision does not add one. The reporting prop is REQUIRED where the host has a surface (`AddItemsDialog.onAddFailed`), so a failure cannot be dropped by forgetting to wire it; it is optional only where some host genuinely has nowhere to report, and there an omitted callback drops the failure exactly as before — a KNOWN remaining gap, not a claim of coverage.'
+signals: 'failed add silently swallowed · dialog closed mid-request · Escape backdrop close button ignore adding flag · error banner unmounts with the dialog · useDismissSafeError inline vs onFailed · success outlives dismissal but failure does not · onDone has no failure counterpart · paths: `web/src/hooks.ts`, `web/src/screens/CollectionsScreen.tsx`, `web/src/media/addTo/`, `web/src/components/overlay.tsx` · issues: #830, #877, #740, #685'
+mechanics: 'Pinned five ways. (1) `web/src/hooks.test.tsx` → "useDismissSafeError (#830)" pins both branches directly: mounted reports INLINE and does not call `onFailed`, unmounted calls `onFailed`, and the report goes through the LATEST callback rather than the one captured on first render. (2) `CollectionsScreen.test.tsx` → "reports a failed add on the screen when the dialog was dismissed before the request settled (#830)" drives the whole path — it parks the POST in flight, dismisses via Escape, then settles the request and asserts the message is on the screen and NOT inside a dialog. Executed: deleting `reportRef.current(message)` alone reddens it on `Unable to find an element with the text: Request failed with status 500`. That one clause is shared, so the same mutation reddens THREE tests (this one plus the two divert tests in (1)) — expect three reds, not one, when re-running it. (3) `CollectionsScreen.test.tsx` → "a late SUCCESS does not close the dialog the user reopened after dismissing (#830)" pins the OTHER half of the split: it dismisses mid-request, reopens the picker, then settles the request 204, and asserts the reopened dialog is still there — with an anti-vacuity check that the late response was actually processed (`onAdded` is `load`, so a second GET of the items endpoint must have happened), because otherwise "the dialog is still open" holds trivially. Executed: deleting the `if (mountedRef.current)` around `onClose()` reddens it alone. The POSITIVE direction is pinned separately, in the Song add test, because the negative one does not cover it: review measured that deleting the `onClose()` call ENTIRELY — so an ordinary successful add never closes the picker — left the whole suite green, so that test now asserts the dialog closes. (4) `CollectionsScreen.guards.test.tsx` counts is-mounted reads and moved from 2 to 1 when the catch''s guard migrated into the hook; its `...actual` module mock cannot see the hook''s internal `useIsMountedRef()`, which is why (1) exists. Executed: removing the surviving `finally` guard takes that count to 0 and reddens. (5) `CollectionsScreen.test.tsx` → "renders a failed add INSIDE the dialog while it is still open (#830)" pins which ARM the call site reaches, which (1) cannot: (1) proves the hook HAS an inline branch, not that this screen calls into it. It fails the POST with the dialog still up and asserts the message sits inside `[role="dialog"]` and appears exactly once in the tree. Executed 2026-09-04: swapping `reportFailure(...)` for `onAddFailed(...)` in the catch — the divert-while-open shape this record names as its own defect — reddens that test and NOTHING else (1 failed / 490 passed across the 42 `src/screens` files).'
+---
+
+**This record applies to ONE site, and the reason the rest were dropped is the useful part.**
+An earlier draft claimed SUCCESS already outlives dismissal "at every one of these sites", reasoning
+from `AddItemsDialog` — which does report success past dismissal, and says so in a comment — and
+generalising to the four `web/src/media/addTo/` dialogs without reading them. Adversarial review
+probed `AddToCollectionDialog` and MEASURED `onAdded` called 0 times after dismissal; the other
+three carry a visibly identical gate (read, not probed). So those four gate BOTH halves of the
+outcome behind their own `activeRef`, and dismiss-then-succeed is as silent there as
+dismiss-then-fail.
+
+Extending the mechanism to them was attempted and WITHDRAWN, which is why they are #877 and not this
+record. Removing that gate reports success correctly but also un-gates `onClose()`, and the two mean
+different things: `onAdded` is "tell the parent what happened", `onClose` is "close me" — addressed
+to a surface that no longer exists. Measured against the real `AddToMenu`: a late success from a
+DISMISSED dialog closed a dialog the user had since reopened. Read, not measured: the success
+handlers on `SearchScreen` / `MediaBrowseScreen` also call `clearSelection()` unconditionally, so
+the same late success would wipe a multi-select the user had rebuilt. Un-gating both is wrong,
+gating both is wrong, and gating only `onClose` still
+needs the parents to stop nulling the dialog themselves — three coupled edits across five files,
+plus an unresolved question about whether `clearSelection()` should fire for a write the user walked
+away from. That is a design decision, not a bug fix, so it goes to #877 with the measurements
+attached rather than riding along here.
+
+`AddToMenu` also has `onDone` and no failure counterpart at all, which is why a failed "Add to
+collection" from a media card reports nowhere. Same issue.
+
+**The outcome and the dismissal are different callbacks, and the exemplar proves it.** Review
+measured the cost of conflating them twice: gating both makes a completed write silent, un-gating
+both makes a late success close the dialog the user reopened to retry. `AddItemsDialog` now does
+each correctly — `onAdded()` unguarded, because it reloads the parent's list and that must survive;
+`onClose()` guarded, because after dismissal it is `setPickerOpen(false)` aimed at whatever the user
+opened next. That split is only this simple where the parent has no competing closer: here
+`onAdded` is `load`, which never touches `pickerOpen`. In the `media/addTo/` layer
+`AddToMenu.handleAdded` closes the dialog itself, so the same one-line gate is NOT sufficient there
+— measured — which is the concrete reason that half is #877 and not this record.
+
+**Why the inline branch is kept rather than always reporting to the parent.** While the dialog is
+up, inline is the better surface: it keeps the user's selections and the context they are looking
+at. Diverting to a parent banner in that case would be its own defect — the message would surface
+somewhere else while the dialog the user is staring at stays blank. Behind the dialog, in fact:
+the panel is a `createPortal` with `aria-modal="true"`, so the screen banner is hidden from AT and
+covered for everyone else. Pinned rather than argued — mechanics (5).
+
+**Why not simply gate dismissal on the busy flag.** That was considered and rejected: gating
+Escape/backdrop/close on `adding` traps the user behind an in-flight request with no cancel path,
+which is exactly why `Dialog` does not do it by default. The request is also genuinely still
+running — cancelling the UI would not cancel the write.
+
+**#740 guarded this and did not fix it, correctly.** That issue added the is-mounted guard so the
+`setError` could not fire on an unmounted tree. That is right and stays. Its scope was the async
+guards, and it deliberately did not change the UX — which left the guard making the drop
+*deliberate* rather than accidental. A deliberate drop is still a user who believes twelve items
+were added when none were, and that is the half this record closes.
+
+**Two limits of the mechanism, recorded rather than left to be rediscovered.** (1) `useIsMountedRef`
+clears its flag in a PASSIVE effect cleanup, so there is a narrow window in which the DOM node is
+detached but the flag still reads true — the message then renders inline into a dead tree instead of
+diverting. `useLayoutEffect` would close it deterministically, but that hook is shared by every
+async caller in the SPA (#578) and changing its timing is not something to do inside a bug fix;
+tracked with the rest in #877. (2) A limit the SECOND surface will have, stated now so it is
+not discovered when it is wired: `CollectionsScreen` reports into `role="alert"`, but the candidate
+surface on `MediaBrowseScreen`/`SearchScreen` is `Toast`, which is `role="status"` (polite) and a
+single last-writer-wins slot — so a failure diverted there would be announced less assertively and
+could be overwritten by a later success. Nothing diverts to those screens today; they receive no
+reporting callback.
+
+**The hook has exactly one consumer, and that is worth saying.** `useDismissSafeError` is a shared
+module with a single call site today. It earns the shape three ways: it is unit-tested directly in
+`hooks.test.tsx`, in a way that would be awkward to keep if the hook were inlined into
+`CollectionsScreen`; §3c prescribes it for the next site; and #877 is a queued second consumer. The
+caveat:
+#877 may conclude that A2's right answer is a shared reporting SURFACE rather than a per-site prop,
+in which case this hook may never gain that second consumer and stays a one-off with a convention
+section attached. That is an accepted risk, not an oversight.
+
+**Scope, stated rather than implied.** A sweep of all 68 `Dialog`/`ConfirmDialog`/`SlideOver` call
+sites (2026-08-29; 67 caller-owned plus `ConfirmDialog`'s own internal `<Dialog`) found three
+shapes: (A1) the surface hard-unmounts and takes its own error state
+with it — the shape fixed here, at `AddItemsDialog` ONLY — the four `web/src/media/addTo/` dialogs
+are the same shape and are NOT fixed (#877, see above);
+(A2) the error state lives in a parent that never unmounts, but the JSX rendering it is nested
+inside the same `open`/`target` condition dismissal just cleared, so it has no DOM node to render
+into — the majority pattern, mostly delete-confirm flows; (B) a genuinely surviving screen-level
+banner, which `DecoTemplatesScreen` and `BlocksScreen` already have. Only ONE A1 site is fixed here.
+A2 is
+LEFT OPEN deliberately: it is a larger, mostly-delete-confirm population whose right answer may be
+a shared reporting surface rather than a per-site prop, and folding it in would have made this PR a
+rewrite of twenty screens. It is tracked in #877, which carries the sweep's verified-vs-inferred
+split forward, rather than being recorded as done here.

docs/spa-conventions.md

@@ -404,6 +404,66 @@ double-invoke for the re-arm) **and** the *integration* (mock the hook module an
 component actually read `current` — and saw `false` — when the late response landed). Verify each by
 removing the mechanism and confirming the test fails.
 
+## 3c. A write failure must outlive the dismissible surface that started it (#830)
+
+`Dialog`, `SlideOver` and `ConfirmDialog` are all dismissible **mid-request** through three paths
+that never consult a busy flag: Escape and a backdrop/scrim click (both via `useOverlayBehavior`),
+and the header close button. Disabling the footer Cancel button — which nearly every dialog here
+does — looks like it closes that hole and does not.
+
+So an error rendered from the dismissible component's own state has nowhere to go once the user
+dismisses it. The request still runs, still fails, and the screen reloads unchanged: the user reads
+that as success. Guarding the `setError` with an is-mounted check (§3b) is necessary but **not
+sufficient** — it makes the drop deliberate rather than accidental, which is still a drop.
+
+**Use `useDismissSafeError` (`web/src/hooks.ts`).** It renders the message inline while the surface
+is mounted — the better surface, since it keeps the user's context — and hands it to a
+caller-supplied `onFailed` once the surface is gone:
+
+```tsx
+const { inlineError, reportFailure, setInlineError } = useDismissSafeError(onFailed);
+// ...
+} catch (err) {
+  reportFailure(messageFromXError(err, 'Unable to …'));
+}
+```
+
+The surviving surface belongs to the **parent**, so `onFailed` is a prop contract, not a rendering
+decision. `CollectionsScreen` reports into its screen-level `role="alert"` banner — the only wired
+site today. The other candidate surface is the `notice` + `Toast` pair that `MediaBrowseScreen` and
+`SearchScreen` already use for success. There is no global toast host in this SPA; do not add one
+for a single screen.
+
+The two are **not equivalent**, so pick deliberately if you wire the second: `Toast` is
+`role="status"` (polite, announced less assertively than `role="alert"`) and each screen has a
+single `notice` slot, so a later success Toast would overwrite a pending failure one. Nothing
+diverts to those screens today — they receive no reporting callback.
+
+**Both halves of the outcome must reach the surviving caller**, and they are not the same kind of
+callback. Report the OUTCOME (`onAdded` / `onDone` / `onFailed`) unguarded — gating a success
+callback on an is-mounted check makes a completed write silent, exactly like the failure case. But
+gate the DISMISS request (`onClose`): it means "close me", and after dismissal "me" is whatever the
+user opened next. The four `media/addTo/` dialogs currently gate BOTH (measured on
+`AddToCollectionDialog`: `onAdded` called 0 times after dismissal) and are tracked in #877 —
+separating the two there needs the parent screens to stop nulling the dialog themselves, so it is a
+design change rather than a one-line fix. When you add a success callback to a dialog, add its
+failure counterpart, and keep the close decision distinct from the report.
+
+Make the reporting prop **required** where the host screen has a surface for it —
+`AddItemsDialog.onAddFailed` is, so the failure cannot be dropped by forgetting to wire it. Make it
+optional only when some host genuinely has nowhere to report (the `media/addTo/` layer's eventual
+one will be, because `MediaDetailScreen` wires no outcome callbacks at all); there, an omitted
+`onFailed` drops the failure exactly as today — a known gap, not coverage (#877). Rationale and the
+full call-site sweep: `docs/decisions/records/spa/dismissible-write-failure-reporting.md`.
+
+**Pin the arm the CALL SITE reaches, not only the hook's.** A unit test of `useDismissSafeError`
+proves the inline branch exists; it cannot see a call site that reports through `onFailed` directly
+and so pushes the message onto the parent banner while the surface is still up — behind a
+`createPortal` panel with `aria-modal="true"`, i.e. covered for sighted users and hidden from AT.
+Add an integration assertion for each arm: fail the write with the surface still open and assert the
+message is inside `[role="dialog"]`, and fail it again across a dismissal and assert the message is
+on the screen and not in a dialog.
+
 ## 4. API client modules
 
 One file per domain in `web/src/api/`, e.g. `logs.ts`, `blocks.ts`, `playouts.ts`. Pattern (see
@@ -603,6 +663,13 @@ page's action row) and the `AddToCollectionDialog` / `AddToPlaylistDialog` / `Ad
 on grid screens is an explicit "Select" toggle (see `docs/decisions.md` 2026-07-10 for the rationale
 and the accepted deviations from Blazor).
 
+Each dialog in this layer is unmounted the moment it is dismissed, so an outcome the screen does not
+receive is an outcome the user never sees. As of 2026-08-29 the layer has **no failure channel at
+all** — `AddToMenu` exposes `onDone` and no counterpart — and its dialogs gate the success callback
+on their own unmount guard, so a write that settles after dismissal reports nothing in either
+direction. Adding one is #877; see §3c for the mechanism and for why the close decision has to stay
+separate from the report.
+
 ## 5d. Client-local preferences: `localStorage`, namespaced `ctv-*` keys
 
 Per-browser UI preferences (theme, an auth token, a screen's remembered page size) live in

web/src/hooks.test.tsx

@@ -1,7 +1,7 @@
-import { cleanup, render } from '@testing-library/react';
+import { act, cleanup, render } from '@testing-library/react';
 import { StrictMode, useEffect } from 'react';
 import { afterEach, describe, expect, it } from 'vitest';
-import { useIsMountedRef } from './hooks';
+import { useDismissSafeError, useIsMountedRef } from './hooks';
 
 afterEach(cleanup);
 
@@ -56,3 +56,70 @@ describe('useIsMountedRef (#578)', () => {
     expect((captured as unknown as { current: boolean }).current).toBe(true);
   });
 });
+
+describe('useDismissSafeError (#830)', () => {
+  interface Handle {
+    inlineError: string | null;
+    reportFailure: (message: string) => void;
+  }
+
+  function renderProbe(report: (message: string) => void) {
+    const handle: Handle = { inlineError: null, reportFailure: () => {} };
+
+    function Probe() {
+      const hook = useDismissSafeError(report);
+      handle.inlineError = hook.inlineError;
+      handle.reportFailure = hook.reportFailure;
+      return <span>{hook.inlineError}</span>;
+    }
+
+    return { handle, ...render(<Probe />) };
+  }
+
+  it('renders the failure INLINE while the surface is still mounted', async () => {
+    const reported: string[] = [];
+    const { handle, findByText } = renderProbe((message) => reported.push(message));
+
+    act(() => handle.reportFailure('nope'));
+
+    // Inline is the better surface while the dialog is up: it keeps the user's context. Diverting
+    // to the parent here would ALSO be a defect — the message would appear somewhere else while the
+    // dialog the user is looking at stays blank.
+    expect(await findByText('nope')).toBeInTheDocument();
+    expect(reported).toEqual([]);
+  });
+
+  it('diverts the failure to the surviving surface once the surface is gone', () => {
+    const reported: string[] = [];
+    const { handle, unmount } = renderProbe((message) => reported.push(message));
+
+    unmount();
+    handle.reportFailure('gone');
+
+    // This is the #830 defect itself: before the fix the message was dropped here and the user was
+    // left believing the write succeeded. Delete `reportRef.current(message)` and this goes red.
+    expect(reported).toEqual(['gone']);
+  });
+
+  it('reports through the LATEST callback, not the one captured on first render', () => {
+    const first: string[] = [];
+    const second: string[] = [];
+
+    function Probe({ sink }: { sink: (message: string) => void }) {
+      handle = useDismissSafeError(sink);
+      return null;
+    }
+
+    let handle: ReturnType<typeof useDismissSafeError> = null as never;
+    const { rerender, unmount } = render(<Probe sink={(m) => first.push(m)} />);
+    rerender(<Probe sink={(m) => second.push(m)} />);
+    unmount();
+
+    handle.reportFailure('latest');
+
+    // Parents hand down a fresh closure every render (the same reason `useOverlayBehavior` keeps a
+    // latest-ref). Capturing the first one would report into a stale closure.
+    expect(first).toEqual([]);
+    expect(second).toEqual(['latest']);
+  });
+});

web/src/hooks.ts

@@ -1,4 +1,4 @@
-import { useEffect, useRef } from 'react';
+import { useCallback, useEffect, useRef, useState } from 'react';
 
 // Shared is-mounted guard for async callbacks (#578). A monotonic `seqRef` drops a STALE response
 // (an older request resolving after a newer one) but says nothing about whether the component is
@@ -19,3 +19,58 @@ export function useIsMountedRef(): { readonly current: boolean } {
 
   return ref;
 }
+
+// A write failure must land on a surface that OUTLIVES the component that started it (#830).
+//
+// `Dialog` and `SlideOver` are dismissible mid-request through three paths that do not consult a
+// busy flag -- Escape and the backdrop/scrim click via `useOverlayBehavior`, plus the header close
+// button (`components/overlay.tsx`). Callers commonly disable only the footer Cancel button, which
+// looks like it closes the hole and does not. Dismissal unmounts the surface, so an error rendered
+// from ITS OWN state has nowhere to go: the request still runs, still fails, and the user is told
+// nothing while the list reloads unchanged.
+//
+// This hook carries the FAILURE half. `AddItemsDialog`, its one consumer, already reported SUCCESS
+// past dismissal -- `onAdded`/`onClose` belong to the parent, and it says so at the call site -- so
+// the fix routes failure through that same parent channel rather than adding a notification system.
+// The four `media/addTo/` dialogs gate BOTH halves behind their own `activeRef` and are NOT fixed;
+// separating the two there needs the parent screens to stop closing their own dialogs, so it is a
+// design change tracked in #877, not something this hook already solved.
+//
+// While mounted the error stays INLINE, which is the better surface: it keeps the user's selections
+// and their context. `reportFailure` only diverts once the surface is gone.
+//
+// `reportAfterDismiss` is read through a latest-ref for the same reason `useOverlayBehavior` does
+// it: parents hand down a fresh closure every render, and the ref must still be callable AFTER this
+// component unmounts, which is precisely when it is used. Note the reach is ONE level: the
+// reporting callback (`AddItemsDialog.onAddFailed`) targets whatever component owns the surviving
+// surface, so if THAT unmounts too -- navigating away,
+// or `CollectionsScreen`'s Back out of a collection while an add is in flight -- the setState is a
+// no-op on React 18+ and the message is still lost. Hoisting the banner further up is what would
+// shrink that, not a change here.
+export function useDismissSafeError(reportAfterDismiss: (message: string) => void): {
+  readonly inlineError: string | null;
+  readonly setInlineError: (message: string | null) => void;
+  readonly reportFailure: (message: string) => void;
+} {
+  const mountedRef = useIsMountedRef();
+  const [inlineError, setInlineError] = useState<string | null>(null);
+  const reportRef = useRef(reportAfterDismiss);
+
+  useEffect(() => {
+    reportRef.current = reportAfterDismiss;
+  });
+
+  const reportFailure = useCallback(
+    (message: string) => {
+      if (mountedRef.current) {
+        setInlineError(message);
+        return;
+      }
+
+      reportRef.current(message);
+    },
+    [mountedRef]
+  );
+
+  return { inlineError, reportFailure, setInlineError };
+}

web/src/screens/CollectionsScreen.guards.test.tsx

@@ -300,11 +300,20 @@ describe('AddItemsDialog async guards (#740)', () => {
     mountedReads.length = 0;
     parked[0].fail();
 
-    // `submit` has exactly TWO post-await writes to this component's own state — `setError` in the
-    // catch and `setAdding` in the finally — and the invariant is that BOTH consult the guard.
-    // Counting them is what keeps the two from masking each other: asserting only that SOME read
-    // saw `false` stays green with either one removed, because the survivor still reads the ref.
-    await waitFor(() => expect(mountedReads).toHaveLength(2));
+    // `submit` has exactly ONE post-await write left to state THIS component owns directly —
+    // `setAdding` in the finally. That is a census of OWN-STATE writes, not of `mountedRef` reads:
+    // the SUCCESS path has its own guarded `onClose()` (#830), which this failure-path test never
+    // reaches, so do not derive the guard population from this number. It was two until #830 moved the catch's report into
+    // `useDismissSafeError`, which keeps its own is-mounted guard and decides between the inline
+    // banner and the parent's surviving surface; that guard is invisible here because the mock
+    // above spreads `...actual`, so the hook's INTERNAL `useIsMountedRef()` call resolves to the
+    // real one rather than this proxy. Both halves of it are pinned directly by
+    // `hooks.test.tsx` → "useDismissSafeError (#830)".
+    //
+    // The exact count still does the anti-masking work it was added for: asserting only that SOME
+    // read saw `false` would stay green with the remaining guard deleted, because there would be no
+    // read left to contradict it. Pinning the number means removing this one gives 0 and reddens.
+    await waitFor(() => expect(mountedReads).toHaveLength(1));
     expect(mountedReads.every((read) => read === false)).toBe(true);
   });
 });

web/src/screens/CollectionsScreen.test.tsx

@@ -49,6 +49,9 @@ interface MockOptions {
   manual?: unknown[];
   smart?: unknown[];
   onRequest?: (url: string, method: string, body: unknown) => Response | null;
+  // Returns a promise the TEST settles, so a request can be left in flight while the surface that
+  // started it is dismissed (#830). `onRequest` cannot express that: it answers synchronously.
+  onRequestDeferred?: (url: string, method: string, body: unknown) => Promise<Response> | null;
 }
 
 function mockApi(options: MockOptions = {}) {
@@ -60,6 +63,14 @@ function mockApi(options: MockOptions = {}) {
     const method = (init?.method ?? 'GET').toUpperCase();
     const body = init?.body ? JSON.parse(String(init.body)) : undefined;
 
+    if (options.onRequestDeferred) {
+      const deferred = options.onRequestDeferred(url, method, body);
+
+      if (deferred) {
+        return deferred;
+      }
+    }
+
     if (options.onRequest) {
       const override = options.onRequest(url, method, body);
 
@@ -360,8 +371,9 @@ describe('CollectionsScreen', () => {
     RemoteStream: { id: 10, mediaType: 'RemoteStream', title: 'Live Feed' }
   };
 
-  function mockAddItemsApi() {
+  function mockAddItemsApi(onRequestDeferred?: MockOptions['onRequestDeferred']) {
     return mockApi({
+      onRequestDeferred,
       onRequest: (url) => {
         if (url.startsWith('/api/v1/library/browse')) {
           const params = new URL(url, 'http://localhost').searchParams;
@@ -452,6 +464,153 @@ describe('CollectionsScreen', () => {
     ).toBeInTheDocument();
   });
 
+  // #830, the arm the dismissal test below cannot see. `useDismissSafeError` has two of them and the
+  // call site picks one by calling `reportFailure` rather than `onAddFailed` directly. While the
+  // dialog is STILL UP the message belongs inline, beside the selections the user built: the
+  // screen's banner sits behind a `createPortal` panel carrying `aria-modal="true"`
+  // (`components/overlay.tsx`), so diverting there would leave the surface the user is staring at
+  // blank and hide the message from assistive tech. Measured: wiring `onAddFailed` at this call
+  // site instead of `reportFailure` leaves every other test in `src/screens` green.
+  it('renders a failed add INSIDE the dialog while it is still open (#830)', async () => {
+    mockAddItemsApi((url, method) =>
+      url === '/api/v1/collections/1/items' && method === 'POST'
+        ? Promise.resolve(new Response(null, { status: 500 }))
+        : null
+    );
+
+    const dialog = await openAddItemsDialog();
+    fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+      target: { value: 'za' }
+    });
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+    expect(await within(dialog).findByText('Zathura')).toBeInTheDocument();
+
+    fireEvent.click(within(dialog).getByText('Zathura'));
+    fireEvent.click(within(dialog).getByRole('button', { name: /Add 1 item/ }));
+
+    // The message lands in the dialog's own role="alert", not the screen's.
+    const inline = await within(dialog).findByText('Request failed with status 500');
+    expect(inline.closest('[role="dialog"]')).toBe(dialog);
+
+    // ...and NOWHERE else. One copy in the whole tree proves the parent banner did not also receive
+    // it, which is what a `reportFailure` -> `onAddFailed` swap at the call site would produce.
+    expect(screen.getAllByText('Request failed with status 500')).toHaveLength(1);
+
+    // Still up, with the selection intact — the reason inline is the right surface here at all.
+    expect(screen.getByRole('dialog')).toBe(dialog);
+    expect(within(dialog).getByRole('button', { name: /Add 1 item/ })).toBeInTheDocument();
+  });
+
+  // #830. The dialog is dismissible mid-request through three paths that never consult `adding`
+  // (Escape, backdrop click, header close button — `components/overlay.tsx`); only the footer Cancel
+  // is disabled, which looks like it closes the hole. Dismissal genuinely unmounts the instance,
+  // because the caller keys it on `add-${pickerOpen}`. #740 guarded the in-dialog `setError` so the
+  // drop became deliberate; the user-visible half is that a deliberate drop still reads as success.
+  //
+  // The request is left IN FLIGHT across the dismissal on purpose — settling it before the Escape
+  // would take the still-mounted path and prove nothing about the bug.
+  it('reports a failed add on the screen when the dialog was dismissed before the request settled (#830)', async () => {
+    let failAdd: (() => void) | undefined;
+    const addInFlight = new Promise<Response>((resolve) => {
+      failAdd = () => resolve(new Response(null, { status: 500 }));
+    });
+
+    const fetchMock = mockAddItemsApi((url, method) =>
+      url === '/api/v1/collections/1/items' && method === 'POST' ? addInFlight : null
+    );
+
+    const dialog = await openAddItemsDialog();
+    fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+      target: { value: 'za' }
+    });
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+    expect(await within(dialog).findByText('Zathura')).toBeInTheDocument();
+
+    fireEvent.click(within(dialog).getByText('Zathura'));
+    fireEvent.click(within(dialog).getByRole('button', { name: /Add 1 item/ }));
+
+    // The POST is out, and nothing has answered it yet.
+    await waitFor(() => {
+      expect(
+        fetchMock.mock.calls.some(
+          ([u, init]) => u === '/api/v1/collections/1/items' && (init?.method ?? '').toUpperCase() === 'POST'
+        )
+      ).toBe(true);
+    });
+
+    // Dismiss via Escape — the path the disabled Cancel button does not cover.
+    fireEvent.keyDown(document, { key: 'Escape' });
+    await waitFor(() => {
+      expect(screen.queryByRole('dialog')).toBeNull();
+    });
+
+    failAdd?.();
+
+    // The failure lands on the SCREEN's own role="alert" banner, which outlives the dialog. Asserting
+    // the message rather than merely "an alert exists" keeps this from passing on an unrelated banner.
+    const alert = await screen.findByText('Request failed with status 500');
+    expect(alert).toBeInTheDocument();
+
+    // ...and it is genuinely on the screen, not inside a dialog that somehow survived.
+    expect(screen.queryByRole('dialog')).toBeNull();
+    expect(alert.closest('[role="dialog"]')).toBeNull();
+  });
+
+  // #830, the other half of the same mechanism. `onAdded` and `onClose` are not the same kind of
+  // callback: the first REPORTS an outcome and must survive dismissal, the second REQUESTS A
+  // DISMISSAL and must not — after dismissal it closes whatever the user opened next. Un-gating both
+  // was measured wrong in review; this pins the split at the one site that ships it.
+  it('a late SUCCESS does not close the dialog the user reopened after dismissing (#830)', async () => {
+    let succeedAdd: (() => void) | undefined;
+    const addInFlight = new Promise<Response>((resolve) => {
+      succeedAdd = () => resolve(new Response(null, { status: 204 }));
+    });
+
+    const fetchMock = mockAddItemsApi((url, method) =>
+      url === '/api/v1/collections/1/items' && method === 'POST' ? addInFlight : null
+    );
+
+    const dialog = await openAddItemsDialog();
+    fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+      target: { value: 'za' }
+    });
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+    expect(await within(dialog).findByText('Zathura')).toBeInTheDocument();
+
+    fireEvent.click(within(dialog).getByText('Zathura'));
+    fireEvent.click(within(dialog).getByRole('button', { name: /Add 1 item/ }));
+    await waitFor(() => {
+      expect(
+        fetchMock.mock.calls.some(
+          ([u, init]) => u === '/api/v1/collections/1/items' && (init?.method ?? '').toUpperCase() === 'POST'
+        )
+      ).toBe(true);
+    });
+
+    fireEvent.keyDown(document, { key: 'Escape' });
+    await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull());
+
+    // The user reopens the picker to retry the add they believe failed.
+    fireEvent.click(screen.getByRole('button', { name: 'Add items' }));
+    const reopened = await screen.findByRole('dialog');
+
+    // ...and only now does the first request come back, successfully.
+    succeedAdd?.();
+
+    // ANTI-VACUITY: prove the late response was actually processed, so the assertion below is not
+    // just "nothing has happened yet". `onAdded` is `load`, which refetches the collection's items.
+    await waitFor(() => {
+      const itemReads = fetchMock.mock.calls.filter(
+        ([u, init]) => /\/api\/v1\/collections\/1\/items/.test(u.toString()) && (init?.method ?? 'GET').toUpperCase() === 'GET'
+      );
+      expect(itemReads.length).toBeGreaterThan(1);
+    });
+
+    // Remove the `if (mountedRef.current)` around `onClose()` and this dialog vanishes under the user.
+    expect(reopened).toBeInTheDocument();
+    expect(screen.queryByRole('dialog')).not.toBeNull();
+  });
+
   it('selecting a newly-addable kind filter (Song) surfaces it and buckets it correctly on add', async () => {
     const fetchMock = mockAddItemsApi();
 
@@ -479,6 +638,12 @@ describe('CollectionsScreen', () => {
       expect(addCall).toBeDefined();
       expect(JSON.parse(String(addCall?.[1]?.body)).songIds).toEqual([7]);
     });
+
+    // #830: the POSITIVE direction of the report/dismiss split. `onClose()` is guarded on
+    // `mountedRef` so a LATE success cannot close a dialog the user reopened; this pins that the
+    // guard did not make the ordinary close dead. Measured: without this line, deleting the
+    // `onClose()` call entirely left the whole suite green.
+    await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull());
   });
 
   it(

web/src/screens/CollectionsScreen.tsx

@@ -48,7 +48,7 @@ import {
   type MediaCollection,
   type SmartCollection
 } from '../api';
-import { useIsMountedRef } from '../hooks';
+import { useDismissSafeError, useIsMountedRef } from '../hooks';
 import { TYPE_LABEL } from '../media/mediaKinds';
 import { SmartCollectionDialog } from '../builder/SmartCollectionDialog';
 
@@ -224,11 +224,13 @@ function NameDialog({
 function AddItemsDialog({
   collection,
   onAdded,
+  onAddFailed,
   onClose,
   open
 }: {
   collection: MediaCollection;
   onAdded: () => void;
+  onAddFailed: (message: string) => void;
   onClose: () => void;
   open: boolean;
 }) {
@@ -242,7 +244,9 @@ function AddItemsDialog({
   const [submittedQuery, setSubmittedQuery] = useState('');
   const [searching, setSearching] = useState(false);
   const [selected, setSelected] = useState<Map<string, LibraryBrowseItem>>(() => new Map());
-  const [error, setError] = useState<string | null>(null);
+  // #830: `inlineError` renders here while the dialog is up; `reportFailure` diverts to
+  // `onAddFailed` once it has been dismissed, so a failed add cannot vanish with the surface.
+  const { inlineError, reportFailure, setInlineError } = useDismissSafeError(onAddFailed);
   const [adding, setAdding] = useState(false);
   const [kindFilter, setKindFilter] = useState<MediaKindFilter>('all');
   const seqRef = useRef(0);
@@ -271,7 +275,7 @@ function AddItemsDialog({
     const isCurrent = () => mountedRef.current && seqRef.current === seq;
 
     setSearching(true);
-    setError(null);
+    setInlineError(null);
 
     try {
       const kinds = filter === 'all' ? DEFAULT_SEARCH_KINDS : [filter];
@@ -298,7 +302,7 @@ function AddItemsDialog({
       if (!isCurrent()) {
         return;
       }
-      setError(messageFromCollectionError(searchError, 'Unable to search library'));
+      setInlineError(messageFromCollectionError(searchError, 'Unable to search library'));
       // A failed search must not leave a stale results/totalMatches pair rendering a confident
       // "Showing N of M matches" hint beside the error banner (#685 review).
       setResults([]);
@@ -342,25 +346,35 @@ function AddItemsDialog({
     }
 
     setAdding(true);
-    setError(null);
+    setInlineError(null);
 
     try {
       await addItemsToCollection(collection.id, toAddItemsRequest([...selected.values()]));
-      // NOT guarded: both belong to the still-mounted PARENT. `onAdded` reloads the collection's
-      // items, and dropping it because this dialog went away would leave the list stale after a
-      // write that succeeded.
+      // These two are NOT the same kind of callback, and only one of them survives dismissal.
+      //
+      // `onAdded` REPORTS what happened: it reloads the parent's item list, and dropping it because
+      // this dialog went away would leave that list stale after a write that succeeded. Unguarded.
+      //
+      // `onClose` REQUESTS A DISMISSAL, and after dismissal the thing it closes is no longer this
+      // instance -- `onClose` is `() => setPickerOpen(false)` on a parent that has since reopened
+      // the picker. Firing it from a dead instance closes the dialog the user reopened to retry,
+      // discarding the selection they just rebuilt (measured). Guarded. See `spa-conventions.md`
+      // §3c: report the outcome unguarded, gate the dismiss request.
       onAdded();
-      onClose();
-    } catch (addError) {
-      // Guarded: these two are this component's own state. The dialog is unmount-reachable while a
-      // request is in flight even though the Add button is disabled — Escape, a backdrop click and
-      // the header close button all reach `onClose` without consulting `adding`
-      // (`components/overlay.tsx`), and the caller remounts on `key={`add-${pickerOpen}`}`. This is
-      // the is-mounted guard's own trigger, unmount-while-in-flight, and it needs no `seq`: the
-      // disabled Add button means there is never a second submission to be superseded by.
+
       if (mountedRef.current) {
-        setError(messageFromCollectionError(addError, 'Unable to add items'));
+        onClose();
       }
+    } catch (addError) {
+      // The dialog is unmount-reachable while a request is in flight even though the Add button is
+      // disabled — Escape, a backdrop click and the header close button all reach `onClose` without
+      // consulting `adding` (`components/overlay.tsx`), and the caller remounts on
+      // `key={`add-${pickerOpen}`}`. #740 guarded this `setError` so the drop was deliberate rather
+      // than accidental; #830 is the missing half — a deliberate drop still leaves the user believing
+      // the add succeeded. `reportFailure` renders inline while the dialog is up and hands the
+      // message to the parent screen once it is gone. No `seq` is needed: the disabled Add button
+      // means there is never a second submission to supersede this one.
+      reportFailure(messageFromCollectionError(addError, 'Unable to add items'));
     } finally {
       if (mountedRef.current) {
         setAdding(false);
@@ -428,9 +442,9 @@ function AddItemsDialog({
       {/* Conditionally mounted, unlike the aria-live hint below — deliberately, not by oversight:
           `role="alert"` is the one live-region role screen readers reliably announce on INSERTION,
           so mounting it together with its content is right here and would be wrong there (#685). */}
-      {error && (
+      {inlineError && (
         <span className="ctv-field-error" role="alert">
-          {error}
+          {inlineError}
         </span>
       )}
       {submittedQuery !== '' && (
@@ -448,7 +462,7 @@ function AddItemsDialog({
         {totalMatches > results.length ? `Showing ${results.length} of ${totalMatches} matches — narrow your search.` : ''}
       </p>
       <div className="ctv-collections-picker-results">
-        {results.length === 0 && submittedQuery === '' && !error ? (
+        {results.length === 0 && submittedQuery === '' && !inlineError ? (
           // Keyed to submittedQuery, not the live input: without it, backspacing the query back
           // below the min-query length after a successful search wiped the rendered rows AND their
           // checkmarks while `selected` (and the Add button's count) still held them (#685) — and,
@@ -457,8 +471,8 @@ function AddItemsDialog({
           <div className="ctv-collections-picker-empty">
             Type at least {LIBRARY_PICKER_MIN_QUERY} characters to search.
           </div>
-        ) : results.length === 0 && !searching && !error ? (
-          // Also suppressed on `error`: "No results" asserts a search that COMPLETED and found
+        ) : results.length === 0 && !searching && !inlineError ? (
+          // Also suppressed on `inlineError`: "No results" asserts a search that COMPLETED and found
           // nothing, which is false when the request failed. The role="alert" banner above is the
           // whole message in that state (#685).
           <div className="ctv-collections-picker-empty">No results — try a search above.</div>
@@ -821,6 +835,9 @@ function ManualItemsView({
         collection={collection}
         key={`add-${pickerOpen}`}
         onAdded={load}
+        // #830: the surviving surface. This screen's own `role="alert"` banner outlives the dialog,
+        // so a failure that lands after dismissal is still reported where the user actually is.
+        onAddFailed={setError}
         onClose={() => setPickerOpen(false)}
         open={pickerOpen}
       />