test(830): pin which ARM the call site reaches, not only that the hook has one

Review measured M6: swapping `reportFailure(...)` for `onAddFailed(...)` in
`AddItemsDialog`'s catch left all 490 tests across the 42 `src/screens` files
green. Under that mutant a failure that lands while the dialog is STILL OPEN
renders into `ManualItemsView`'s screen banner, which sits behind the dialog's
`createPortal` panel with `aria-modal="true"` — covered for sighted users,
hidden from AT, and the surface the user is actually looking at stays blank.
That is the exact shape the decision record calls "its own defect", and the
whole gap was the call-site wiring: the hook's inline branch is pinned at unit
level in `hooks.test.tsx`, but a unit test of the hook cannot see which arm a
consumer reaches.

Adds the integration assertion: fail the POST with the dialog still up, assert
the message is inside `[role="dialog"]` and appears exactly once in the tree.
Re-executed the mutation with it in place — 1 failed / 490 passed, and the red
is this test alone.

Records the new pin as mechanics (5) on
`spa.dismissible-write-failure-reporting` and the general form in
`spa-conventions.md` §3c, so the next site wired to the hook pins both arms at
its call site rather than inheriting the hook's unit coverage.

Refs #830

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

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

@@ -7,7 +7,7 @@ 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 four 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.'
+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.**
@@ -48,7 +48,9 @@ opened next. That split is only this simple where the parent has no competing cl
 **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.
+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,

docs/spa-conventions.md

@@ -456,6 +456,14 @@ one will be, because `MediaDetailScreen` wires no outcome callbacks at all); the
 `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

web/src/screens/CollectionsScreen.test.tsx

@@ -464,6 +464,43 @@ 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,