fix(#238): review — trakt sub-route no-op + data-driven wiring test

Cold-review follow-ups (both non-blocking):
- Low: the TopBar "Add Trakt List" button was a silent no-op on the
  /app/trakt-lists/{id} detail sub-route (setAddOpen state isn't rendered by
  the editor branch, and the screen is keyed by pathname so the state
  wouldn't survive a navigate). Guard on editingId: route back to the list
  from the detail view, open the dialog from the list.
- Nit: the invariant test only spot-checked 2 screens. Replaced with a
  data-driven it.each over the 4 URL-navigating create screens (channels,
  filler, ffmpeg, watermarks) asserting each banner actually navigates — a
  typo'd route id now fails red. Docs wording corrected to match.

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

docs/decisions.md

@@ -1716,4 +1716,5 @@ very dead-button class #238 fixes. Wiring only the single-create-flow screens gi
 post-#295 (the screen displays the machine key + changes the local password; there is nothing to "save") —
 relabelled to reflect the #295 reality. Convention + hook documented in `spa-conventions.md` §10. The
 implicit route-label ⟺ screen-subscription coupling is the kind of thing #247 (shell extraction) will
-formalize; #238 keeps it a documented convention guarded by an `App.test.tsx` invariant test. Refs #238 #247.
+formalize; #238 keeps it a documented convention guarded by a data-driven `App.test.tsx` test over the URL-navigating
+create screens (a typo'd route id → the banner navigates nowhere → red). Refs #238 #247.

docs/spa-conventions.md

@@ -352,9 +352,11 @@ active screen. The TopBar has no reference to the screen component, so the click
   (`usePrimaryAction('schedules', () => setForm('create'))`).
 - **The route must ALSO declare a matching non-empty `primaryAction` label** in the `routes` table. The
   TopBar renders the button **only when `route.primaryAction` is non-empty** — so declaring a label without
-  subscribing renders a dead button, and subscribing without a label renders nothing. Keep the two in lockstep
-  (invariant covered by the `#238` test in `App.test.tsx`: every non-empty-`primaryAction` route is a create
-  screen whose banner works; a no-`primaryAction` route shows no banner button).
+  subscribing renders a dead button, and subscribing without a label renders nothing. Keep the two in lockstep.
+  The `#238` tests in `App.test.tsx` guard this: a data-driven `it.each` asserts each URL-navigating create
+  screen's banner actually navigates (so a typo'd route id → a button that navigates nowhere → red), plus a
+  drop test that an action screen shows no banner button. The dialog/editor create screens (schedules, multi/
+  rerun collections, trakt) are exercised by their own create-flow tests.
 - **When to WIRE vs DROP (issue #238).** The Plus icon makes the button semantically a *"create new item"*
   affordance. **Keep + wire it only on list screens with a single, unambiguous create flow** ("Add Channel",
   "Add Schedule", "Add Multi-Collection", "Add Rerun Collection", "Add Trakt List", "Add Filler Preset", "Add

web/src/App.test.tsx

@@ -38,16 +38,26 @@ describe('ChicoryTV SPA scaffold', () => {
     expect(designSystemStylesheet).toBe('../../design-system/styles.css');
   });
 
-  it('wires the TopBar primary-action button on a create screen and drops it elsewhere (#238)', async () => {
+  // Data-driven over every WIRED create screen whose primary action navigates to a URL — this
+  // catches a typo'd route-id in usePrimaryAction on any of them (the button would render but do
+  // nothing). The button lives in the shell header (role="banner"), scoped so an in-body Add
+  // control can't satisfy the assertion. (#238)
+  it.each([
+    ['/app/channels', 'Add Channel', '/app/new-channel'],
+    ['/app/filler-presets', 'Add Filler Preset', '/app/filler-presets/add'],
+    ['/app/ffmpeg-profiles', 'Add Profile', '/app/ffmpeg-profiles/add'],
+    ['/app/watermarks', 'Add Watermark', '/app/watermarks/add']
+  ])('wires the TopBar create button on %s → %s (#238)', async (path, label, target) => {
+    window.history.pushState(null, '', path);
     render(<App />);
 
-    // A "create list" screen (Channels) shows a working "+ Add Channel" TopBar button that
-    // navigates to the builder; the button lives in the shell header (role="banner").
-    fireEvent.click(screen.getByRole('link', { name: 'Channels' }));
-    expect(await screen.findByRole('heading', { name: 'Channels' })).toBeInTheDocument();
-    const banner = screen.getByRole('banner');
-    fireEvent.click(within(banner).getByRole('button', { name: 'Add Channel' }));
-    expect(window.location.pathname).toBe('/app/new-channel');
+    const banner = await screen.findByRole('banner');
+    fireEvent.click(within(banner).getByRole('button', { name: label }));
+    expect(window.location.pathname).toBe(target);
+  });
+
+  it('drops the TopBar primary-action button on action screens (#238)', async () => {
+    render(<App />);
 
     // An "action" screen (Logs) declares no primaryAction → no TopBar button at all
     // (its Refresh control lives in-body, where the "+" primary styling would be wrong).

web/src/screens/TraktListsScreen.tsx

@@ -277,8 +277,17 @@ export function TraktListsScreen() {
   const [rowError, setRowError] = useState<string | null>(null);
   const activeRef = useRef(true);
 
-  // TopBar "Add Trakt List" primary action.
-  usePrimaryAction('traktLists', () => setAddOpen(true));
+  // TopBar "Add Trakt List" primary action. On the list, open the add dialog. On the
+  // detail sub-route (/app/trakt-lists/{id}) the add dialog isn't mounted and this component
+  // is keyed by pathname (so state wouldn't survive a navigate), so route back to the list
+  // instead of flipping a state nobody renders — avoids a silent no-op button (#238 review).
+  usePrimaryAction('traktLists', () => {
+    if (editingId != null) {
+      navigateToPath(TRAKT_BASE_PATH);
+    } else {
+      setAddOpen(true);
+    }
+  });
 
   // Fetch only; state updates happen in the async callbacks (never synchronously in the
   // effect body). `loading` starts true and flips false in `finally`, so the mount effect