refactor(account): address account-pin re-hydrate review — extract module, harden guards

Fixes the 5 findings from /code-review high on the account-pin work:
- Extract the reactive-account primitives (ACTIVE_ACCOUNT_KEY, activeAccountId,
  notifyActiveAccountChange, useActiveAccountId) into a new dependency-free
  lib/activeAccount.ts, breaking the storage.ts <-> useAccountRehydrate.ts import cycle.
  storage.ts / api/core.ts / useAccountRehydrate.ts now import from it.
- NavigationProvider: the page re-hydrate now PRESERVES existing history.state markers
  (spread, not overwrite), and openChannel/enterYtSearch markDirty so a sub-view opened
  before the account pins isn't clobbered by the page override.
- useAccountRehydrate: reset the dirty flag after each account transition, so a future
  non-reloading account switch can't skip the re-hydrate and leak the prior account's state.
- Doc-link useAccountRehydrate to the pre-existing useEffect([me?.id]) server-adopt pattern.
- Tests: model a mutable pinned account and add useSortState integration tests for the
  null->id re-hydrate + the markDirty no-clobber guard (vitest 115->117).

gate-green: vite build + vitest 117 + eslint-0 + knip + prettier. Self-E2E'd on a fresh
tab: still lands on Plex with Movies/In-progress/Year restored, zero console errors.

frontend/src/components/NavigationProvider.tsx

@@ -114,7 +114,10 @@ export function NavigationProvider({ children }: { children: ReactNode }) {
     if (stored === pageRef.current) return;
     setPageState(stored);
     setAccountRaw(PAGE_KEY, stored); // no-op re-write, keeps storage authoritative
-    window.history.replaceState({ sfPage: stored }, "");
+    // Correct the CURRENT entry (replace, not push) and PRESERVE any markers already on it — a channel
+    // (_chan) or YT search (_yt) opened in the pre-pin window must survive, or its overlay is orphaned
+    // from history and Back can't close it. (Those setters also markDirty, so this usually no-ops then.)
+    window.history.replaceState({ ...(window.history.state || {}), sfPage: stored }, "");
   });
 
   const setPage = useCallback(
@@ -134,27 +137,35 @@ export function NavigationProvider({ children }: { children: ReactNode }) {
     [markDirty],
   );
 
-  const openChannel = useCallback((id: string, name?: string) => {
+  const openChannel = useCallback(
-    setChannelView({ id, name });
+    (id: string, name?: string) => {
-    const st = window.history.state || {};
+      markDirty(); // opening a sub-view is a navigation — don't let a late account-pin re-hydrate override the page
-    if (st._chan) window.history.replaceState({ ...st, _chan: id, _chanName: name ?? null }, "");
+      setChannelView({ id, name });
-    else window.history.pushState({ ...st, _chan: id, _chanName: name ?? null }, "");
+      const st = window.history.state || {};
-  }, []);
+      if (st._chan) window.history.replaceState({ ...st, _chan: id, _chanName: name ?? null }, "");
+      else window.history.pushState({ ...st, _chan: id, _chanName: name ?? null }, "");
+    },
+    [markDirty],
+  );
 
   const closeChannel = useCallback(() => {
     if (window.history.state?._chan) window.history.back();
     else setChannelView(null);
   }, []);
 
-  const enterYtSearch = useCallback((q: string) => {
+  const enterYtSearch = useCallback(
-    setYtSearch(q);
+    (q: string) => {
-    // Drop any prior _ytScrape marker — the new search's source isn't known yet; Feed re-stamps it
+      markDirty(); // a sub-view navigation — see openChannel
-    // once the results resolve, so a reload only restores a confirmed scrape-sourced search.
+      setYtSearch(q);
-    const { _ytScrape: _drop, ...st } = window.history.state || {};
+      // Drop any prior _ytScrape marker — the new search's source isn't known yet; Feed re-stamps it
-    if (st._yt)
+      // once the results resolve, so a reload only restores a confirmed scrape-sourced search.
-      window.history.replaceState({ ...st, _yt: q }, ""); // refine current search
+      const { _ytScrape: _drop, ...st } = window.history.state || {};
-    else window.history.pushState({ ...st, sfPage: "feed", _yt: q }, ""); // new sub-view entry
+      if (st._yt)
-  }, []);
+        window.history.replaceState({ ...st, _yt: q }, ""); // refine current search
+      else window.history.pushState({ ...st, sfPage: "feed", _yt: q }, ""); // new sub-view entry
+    },
+    [markDirty],
+  );
 
   const exitYtSearch = useCallback(() => {
     // Pop our search entry (so Forward still works); fall back to a plain clear if we don't own one.

frontend/src/lib/activeAccount.ts

@@ -0,0 +1,39 @@
+import { useSyncExternalStore } from "react";
+
+// The account a browser TAB is acting as (per-tab, in sessionStorage — see api.ts), plus its reactive
+// wrapper. Its OWN module (imports nothing internal) so it can be shared by both `storage.ts` (which
+// keys per-account localStorage on it) and `useAccountRehydrate.ts` without a circular import — the
+// account id is a lower-level primitive than the storage helpers that consume it.
+//
+// ACTIVE_ACCOUNT_KEY is duplicated here rather than imported from api.ts so this stays dependency-free.
+export const ACTIVE_ACCOUNT_KEY = "siftlode.activeAccount";
+
+export function activeAccountId(): number | null {
+  try {
+    const raw = sessionStorage.getItem(ACTIVE_ACCOUNT_KEY);
+    if (!raw) return null;
+    const n = Number(raw);
+    return Number.isFinite(n) ? n : null;
+  } catch {
+    return null;
+  }
+}
+
+// Reactive active-account. setActiveAccount/clearActiveAccount write sessionStorage — a silent op that
+// triggers no React render — so this tab's per-account state (sort/filter/view/layout, all keyed by the
+// account) would never re-read once the account is finally known. That bites exactly once, on first
+// load: the provider tree above <App> mounts with NO account, then <App> pins it after /api/me resolves.
+// The account setters call notifyActiveAccountChange() so useActiveAccountId subscribers can re-hydrate.
+const accountListeners = new Set<() => void>();
+function subscribeActiveAccount(cb: () => void): () => void {
+  accountListeners.add(cb);
+  return () => accountListeners.delete(cb);
+}
+export function notifyActiveAccountChange(): void {
+  for (const cb of [...accountListeners]) cb();
+}
+/** The current tab's active account id, as reactive state — re-renders when it is (re)pinned. The
+ * snapshot is a primitive (number | null), so useSyncExternalStore's Object.is check is stable. */
+export function useActiveAccountId(): number | null {
+  return useSyncExternalStore(subscribeActiveAccount, activeAccountId, activeAccountId);
+}

frontend/src/lib/api/core.ts

@@ -5,7 +5,7 @@
 import { notify, remove } from "../notifications";
 import i18n from "../../i18n";
 import { reportError } from "../errorDialog";
-import { activeAccountId, ACTIVE_ACCOUNT_KEY, notifyActiveAccountChange } from "../storage";
+import { activeAccountId, ACTIVE_ACCOUNT_KEY, notifyActiveAccountChange } from "../activeAccount";
 
 export class HttpError extends Error {
   status: number;

frontend/src/lib/storage.ts

@@ -1,4 +1,5 @@
-import { useCallback, useState, useSyncExternalStore } from "react";
+import { useCallback, useState } from "react";
+import { activeAccountId } from "./activeAccount";
 import { useAccountRehydrate } from "./useAccountRehydrate";
 
 // Central registry of every localStorage key the app uses. One place to see them all, rename
@@ -68,40 +69,8 @@ export const LS = {
 } as const;
 
 // --- per-account scoping -----------------------------------------------------------------
-// The account a browser TAB is acting as (per-tab, in sessionStorage — see api.ts). Duplicated
+// The active-account primitive (activeAccountId / useActiveAccountId / notify) lives in ./activeAccount
-// here (rather than imported from api.ts) so storage.ts stays dependency-free and usable by the
+// so it can be shared without a cycle; the helpers below key localStorage on it.
-// low-level helpers below.
-export const ACTIVE_ACCOUNT_KEY = "siftlode.activeAccount";
-
-export function activeAccountId(): number | null {
-  try {
-    const raw = sessionStorage.getItem(ACTIVE_ACCOUNT_KEY);
-    if (!raw) return null;
-    const n = Number(raw);
-    return Number.isFinite(n) ? n : null;
-  } catch {
-    return null;
-  }
-}
-
-// Reactive active-account. setActiveAccount/clearActiveAccount write sessionStorage — a silent op that
-// triggers no React render — so this tab's per-account state (sort/filter/view/layout, all keyed by the
-// account) would never re-read once the account is finally known. That bites exactly once, on first
-// load: the provider tree above <App> mounts with NO account, then <App> pins it after /api/me resolves.
-// The account setters call notifyActiveAccountChange() so useActiveAccountId subscribers can re-hydrate.
-const accountListeners = new Set<() => void>();
-function subscribeActiveAccount(cb: () => void): () => void {
-  accountListeners.add(cb);
-  return () => accountListeners.delete(cb);
-}
-export function notifyActiveAccountChange(): void {
-  for (const cb of [...accountListeners]) cb();
-}
-/** The current tab's active account id, as reactive state — re-renders when it is (re)pinned. The
- * snapshot is a primitive (number | null), so useSyncExternalStore's Object.is check is stable. */
-export function useActiveAccountId(): number | null {
-  return useSyncExternalStore(subscribeActiveAccount, activeAccountId, activeAccountId);
-}
 
 /** Suffix a base localStorage key with an account id so per-account UI state (filters, selected
  * playlist, notifications, tab positions, cached prefs…) is NOT shared across the accounts signed

frontend/src/lib/useAccountRehydrate.dom.test.ts

@@ -6,7 +6,7 @@ import { describe, it, expect, beforeEach, vi } from "vitest";
 import { renderHook, act } from "@testing-library/react";
 
 let account: number | null = null;
-vi.mock("./storage", () => ({
+vi.mock("./activeAccount", () => ({
   useActiveAccountId: () => account,
 }));
 

frontend/src/lib/useAccountRehydrate.ts

@@ -1,5 +1,5 @@
 import { useCallback, useEffect, useRef } from "react";
-import { useActiveAccountId } from "./storage";
+import { useActiveAccountId } from "./activeAccount";
 
 // Re-read per-account persisted state when this tab's active account becomes known (or changes). The
 // provider tree above <App> mounts before the account is pinned, so its useState initializers seed from
@@ -9,6 +9,11 @@ import { useActiveAccountId } from "./storage";
 //
 // Keyed on the account id ONLY — the user's own writes go through setAccountRaw, which does not change
 // the account id, so this effect can't loop on them.
+//
+// This is the localStorage-only counterpart of the `useEffect([me?.id])` server-prefs adopt pattern in
+// FeedFiltersProvider / FeedViewProvider / PrefsProvider (and App's shell prefs): those re-load from the
+// SERVER when the account query resolves; this re-loads from localStorage when the account is pinned.
+// A provider needs one or the other — not both — to survive a fresh tab's first paint.
 export function useAccountRehydrate(rehydrate: () => void): () => void {
   const accountId = useActiveAccountId();
   const dirty = useRef(false);
@@ -22,6 +27,10 @@ export function useAccountRehydrate(rehydrate: () => void): () => void {
     if (accountId === prev.current) return; // mount, and renders with no real transition
     prev.current = accountId;
     if (!dirty.current) fn.current();
+    // Reset per account: `dirty` means "the user edited under THIS account". Today a real account
+    // switch full-reloads (fresh instance), but clearing it here keeps the guard correct even if a
+    // future switch stops reloading — the next account starts clean rather than inheriting dirtiness.
+    dirty.current = false;
   }, [accountId]);
 
   return useCallback(() => {

frontend/src/lib/useFilterState.dom.test.ts

@@ -12,10 +12,10 @@ vi.mock("./storage", () => ({
   setAccountRaw: (k: string, v: string) => {
     store.set(k, v);
   },
-  // A stable account id keeps useAccountRehydrate a no-op here (no null→id transition); the re-hydrate
-  // path itself is covered in useAccountRehydrate.dom.test.ts.
-  useActiveAccountId: () => 1,
 }));
+// A stable account id keeps useAccountRehydrate a no-op here (no null→id transition); the re-hydrate
+// path itself is covered in useAccountRehydrate.dom.test.ts + useSortState.dom.test.ts.
+vi.mock("./activeAccount", () => ({ useActiveAccountId: () => 1 }));
 
 import { useFilterState } from "./useFilterState";
 

frontend/src/lib/useSortState.dom.test.ts

@@ -6,21 +6,27 @@
 import { describe, it, expect, beforeEach, vi } from "vitest";
 import { renderHook, act } from "@testing-library/react";
 
-const { store } = vi.hoisted(() => ({ store: new Map<string, string>() }));
+// `ctl.account` models the tab's pinned account: getAccountRaw returns null while it's null (mirrors the
-vi.mock("./storage", () => ({
+// real accountKey → no key → null), so a fresh-tab mount reads defaults until the account is pinned.
-  getAccountRaw: (k: string) => store.get(k) ?? null,
+const { store, ctl } = vi.hoisted(() => ({
-  setAccountRaw: (k: string, v: string) => {
+  store: new Map<string, string>(),
-    store.set(k, v);
+  ctl: { account: 1 as number | null },
-  },
-  // A stable account id keeps useAccountRehydrate a no-op here (no null→id transition); the re-hydrate
-  // path itself is covered in useAccountRehydrate.dom.test.ts.
-  useActiveAccountId: () => 1,
 }));
+vi.mock("./storage", () => ({
+  getAccountRaw: (k: string) => (ctl.account == null ? null : (store.get(k) ?? null)),
+  setAccountRaw: (k: string, v: string) => {
+    if (ctl.account != null) store.set(k, v);
+  },
+}));
+vi.mock("./activeAccount", () => ({ useActiveAccountId: () => ctl.account }));
 
 import { useSortState } from "./useSortState";
 
 describe("useSortState", () => {
-  beforeEach(() => store.clear());
+  beforeEach(() => {
+    store.clear();
+    ctl.account = 1;
+  });
 
   it("hydrates to the fallback when storage is empty", () => {
     const { result } = renderHook(() => useSortState("k"));
@@ -56,4 +62,29 @@ describe("useSortState", () => {
     const { result } = renderHook(() => useSortState("k"));
     expect(result.current[0]).toEqual({ key: "anything", dir: "desc" });
   });
+
+  // The account-pin re-hydrate (via useAccountRehydrate) integrated into useSortState.
+  it("re-hydrates the stored sort once the account is pinned after mount", () => {
+    ctl.account = null; // fresh tab: no account at mount
+    store.set("k", JSON.stringify({ key: "views", dir: "desc" }));
+    const { result, rerender } = renderHook(() => useSortState("k"));
+    expect(result.current[0]).toBeNull(); // mount read with no account → default
+    act(() => {
+      ctl.account = 1; // <App> pins the account
+      rerender();
+    });
+    expect(result.current[0]).toEqual({ key: "views", dir: "desc" });
+  });
+
+  it("does NOT clobber a pre-pin user edit when the account is then pinned (markDirty guard)", () => {
+    ctl.account = null;
+    store.set("k", JSON.stringify({ key: "views", dir: "desc" })); // the stored value
+    const { result, rerender } = renderHook(() => useSortState("k"));
+    act(() => result.current[1]({ key: "name", dir: "asc" })); // user edits before the pin
+    act(() => {
+      ctl.account = 1;
+      rerender();
+    });
+    expect(result.current[0]).toEqual({ key: "name", dir: "asc" }); // the edit survives, not "views"
+  });
 });