fix(spa): #271 disable all family rows on click (family-global lock parity)

Cold-review nit fixes on PR #298:
- useCollectionsScan.scan() now guards on the whole family being busy (active OR
  any pending key of that family), not just the exact key — a sibling source of a
  family with a scan in flight no longer fires a redundant (benign-409) POST.
- LibrariesScreen ExternalCollectionsSection disables every row of a family that
  has a pending or active scan (derives pendingFamilies from pendingKeys), matching
  Blazor's instant all-rows-disabled behavior instead of waiting a poll RTT.
- Rewrite the promote test to actually observe the optimistic-pending window via a
  deferred POST (was only asserting the promoted end state), and add a test proving
  a sibling-source click fires no second POST while the family is pending.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

web/src/api/libraries.test.ts

@@ -1,4 +1,4 @@
-import { renderHook, waitFor } from '@testing-library/react';
+import { act, renderHook, waitFor } from '@testing-library/react';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 import {
   getCollectionsScanStatus,
@@ -24,31 +24,41 @@ function lastCall(fetchMock: ReturnType<typeof vi.spyOn>) {
 // Stateful collections backend mock: the scan-collections POST acquires the family-global lock
 // synchronously (as the real controller does before returning 202/409), so a subsequent status GET
 // reports that family active. `postStatus` overrides the POST response for error-path tests.
-function mockCollectionsBackend(opts: { postStatus?: number; seededFamilies?: string[] } = {}) {
+function mockCollectionsBackend(opts: { deferPost?: boolean; postStatus?: number; seededFamilies?: string[] } = {}) {
   const active = new Set<string>(opts.seededFamilies ?? []);
-  const fetchMock = vi.spyOn(window, 'fetch').mockImplementation((input, init) => {
+  // When deferPost is set, the scan-collections POST does not resolve until releasePost() is called -
+  // this lets a test observe the optimistic-pending window before the POST completes.
+  let releasePost = () => {};
+  const postGate = opts.deferPost ? new Promise<void>((resolve) => (releasePost = resolve)) : Promise.resolve();
+
+  const fetchMock = vi.spyOn(window, 'fetch').mockImplementation(async (input, init) => {
     const url = String(input);
     const method = (init?.method ?? 'GET').toUpperCase();
 
     if (url.includes('/api/media-sources/collections-scan-status')) {
-      return Promise.resolve(json([...active].map((family) => ({ family }))));
+      return json([...active].map((family) => ({ family })));
     }
 
     const match = url.match(/\/api\/media-sources\/(\w+)\/\d+\/scan-collections/);
     if (match && method === 'POST') {
+      await postGate;
       const status = opts.postStatus ?? 202;
       if (status === 202 || status === 409) {
         // Both mean a collections scan is now running for this family -> the lock is held.
         active.add(match[1]);
-        return Promise.resolve(status === 202 ? noContent() : json({ detail: 'already scanning' }, 409));
+        return status === 202 ? noContent() : json({ detail: 'already scanning' }, 409);
       }
-      return Promise.resolve(json({ detail: 'not found' }, status));
+      return json({ detail: 'not found' }, status);
     }
 
-    return Promise.resolve(json({ detail: 'unexpected' }, 500));
+    return json({ detail: 'unexpected' }, 500);
   });
 
-  return { active, fetchMock };
+  function scanPostCount(): number {
+    return fetchMock.mock.calls.filter((call) => String(call[0]).includes('scan-collections')).length;
+  }
+
+  return { active, fetchMock, releasePost: () => releasePost(), scanPostCount };
 }
 
 describe('libraries api client', () => {
@@ -120,20 +130,50 @@ describe('useCollectionsScan', () => {
     expect(result.current.pendingKeys.size).toBe(0);
   });
 
-  it('optimistically marks pending on scan, then promotes to active once the family is observed', async () => {
-    mockCollectionsBackend();
+  it('bridges the click->observed-active gap with optimistic pending, then promotes', async () => {
+    const backend = mockCollectionsBackend({ deferPost: true });
     const { result } = renderHook(() => useCollectionsScan());
 
     await waitFor(() => expect(result.current.activeFamilies.size).toBe(0));
 
-    await result.current.scan('jellyfin', 7, false);
+    let scanPromise: Promise<void> = Promise.resolve();
+    act(() => {
+      scanPromise = result.current.scan('jellyfin', 7, false);
+    });
 
-    // The scan()'s 202 poll observes the (synchronously-locked) family active and reconciles.
+    // Optimistic pending is visible immediately, BEFORE the (deferred) POST resolves.
+    await waitFor(() => expect(result.current.pendingKeys.has('jellyfin:7')).toBe(true));
+    expect(result.current.activeFamilies.has('jellyfin')).toBe(false);
+
+    // Release the POST (lock now held) -> the follow-up poll promotes the family to active.
+    act(() => backend.releasePost());
+    await scanPromise;
     await waitFor(() => expect(result.current.activeFamilies.has('jellyfin')).toBe(true));
     expect(result.current.pendingKeys.has('jellyfin:7')).toBe(false);
     expect(result.current.error).toBeNull();
   });
 
+  it('does not fire a second scan for a sibling source while the family is already pending', async () => {
+    const backend = mockCollectionsBackend({ deferPost: true });
+    const { result } = renderHook(() => useCollectionsScan());
+
+    await waitFor(() => expect(result.current.activeFamilies.size).toBe(0));
+
+    act(() => {
+      void result.current.scan('plex', 1, false);
+    });
+    await waitFor(() => expect(result.current.pendingKeys.has('plex:1')).toBe(true));
+    const postsAfterFirst = backend.scanPostCount();
+
+    // Sibling source of the same (family-global-locked) family - must be ignored, no second POST.
+    await act(async () => {
+      await result.current.scan('plex', 2, false);
+    });
+    expect(backend.scanPostCount()).toBe(postsAfterFirst);
+
+    backend.releasePost();
+  });
+
   it('keeps the family disabled and surfaces no error on a 409 (already scanning)', async () => {
     mockCollectionsBackend({ postStatus: 409 });
     const { result } = renderHook(() => useCollectionsScan());

web/src/api/libraries.ts

@@ -430,7 +430,7 @@ export function collectionsScanKey(source: CollectionsScanSource, sourceId: numb
 }
 
 // Recover the family from a `${family}:${sourceId}` key (the family segment has no colon).
-function collectionsScanKeyFamily(key: string): CollectionsScanSource {
+export function collectionsScanKeyFamily(key: string): CollectionsScanSource {
   return key.slice(0, key.indexOf(':')) as CollectionsScanSource;
 }
 
@@ -522,9 +522,13 @@ export function useCollectionsScan(pollMs = 10000): CollectionsScanState {
   const scan = useCallback(
     (source: CollectionsScanSource, sourceId: number, deep: boolean): Promise<void> => {
       const key = collectionsScanKey(source, sourceId);
-      if (pendingKeysRef.current.has(key) || activeFamiliesRef.current.has(source)) {
-        // Already pending, or the whole family is already scanning (family-global lock) - ignore the
-        // duplicate submission. Quick and deep scans share the same lock, so both gate on this.
+      const familyBusy =
+        activeFamiliesRef.current.has(source) ||
+        [...pendingKeysRef.current].some((pendingKey) => collectionsScanKeyFamily(pendingKey) === source);
+      if (familyBusy) {
+        // The whole family is already scanning or has a pending scan - ignore the duplicate. The lock
+        // is family-global, so a sibling source of a busy family would only earn a benign 409; don't
+        // fire it. (Covers the same-key case too - a pending key implies its family is pending.)
         return Promise.resolve();
       }