test/refactor(#266): cover anti-clobber capture + once-guard; log storage errors

Review round 1 on the scroll-position feature:
- F1: add two tests for the hook's subtlest invariants — (a2) the restore
  target is captured synchronously at mount and survives a fresh scroll@0
  overwriting storage on load (a regression moving the capture into an effect
  would now fail); (a3) restore runs at most once per mount even when called
  again (the wiring effect can re-run).
- F2: log instead of silently swallowing sessionStorage errors in
  readStorage/writeStorage (AGENTS.md "errors must never be swallowed" rule);
  no user notification since a missed scroll restore is not actionable.
- F3: document the hard dependency on PageEditor remounting per page
  (key={page.id}) at the refs declaration — the per-mount refs are not reset
  on an in-place pageId change, so removing that key would break restore on
  the 2nd page.

vitest 9/9, tsc 0, eslint 0.

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

apps/client/src/features/editor/hooks/use-scroll-position.test.ts

@@ -74,6 +74,51 @@ describe("useScrollPosition", () => {
     unmount();
   });
 
+  it("(a2) the restore target is captured at mount and survives a fresh scroll@0 clobber", () => {
+    vi.useFakeTimers();
+    // A previous session saved 500.
+    window.sessionStorage.setItem(`${KEY_PREFIX}clob`, "500");
+
+    const { result } = renderHook(() => useScrollPosition("clob"));
+
+    // On load the page is at the top; a scroll@0 fires and overwrites storage
+    // with 0. This is exactly the clobber the synchronous mount-capture defends
+    // against: the stored value becomes "0", but the target was already captured.
+    setScrollY(0);
+    act(() => {
+      window.dispatchEvent(new Event("scroll"));
+    });
+    expect(window.sessionStorage.getItem(`${KEY_PREFIX}clob`)).toBe("0");
+
+    // Restore still scrolls to 500 (the captured target), NOT the clobbered 0.
+    // If the capture were moved into an effect (after handlers register), it
+    // would read the clobbered 0 and this assertion would fail.
+    setScrollHeight(2000); // maxScroll = 1200 >= 500
+    act(() => {
+      result.current.restoreScrollPosition();
+    });
+    expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
+  });
+
+  it("(a3) restores at most once per mount even if called again", () => {
+    vi.useFakeTimers();
+    window.sessionStorage.setItem(`${KEY_PREFIX}once`, "500");
+    setScrollHeight(2000); // tall enough to restore synchronously
+
+    const { result } = renderHook(() => useScrollPosition("once"));
+    act(() => {
+      result.current.restoreScrollPosition();
+    });
+    expect(window.scrollTo).toHaveBeenCalledTimes(1);
+
+    // A second call (e.g. the wiring effect re-running on [showStatic, editor,
+    // restoreScrollPosition]) must NOT scroll again and yank the reader.
+    act(() => {
+      result.current.restoreScrollPosition();
+    });
+    expect(window.scrollTo).toHaveBeenCalledTimes(1);
+  });
+
   it("(b) does not restore when the URL has a #hash anchor", () => {
     vi.useFakeTimers();
     window.sessionStorage.setItem(`${KEY_PREFIX}p2`, "500");

apps/client/src/features/editor/hooks/use-scroll-position.ts

@@ -25,7 +25,11 @@ function readStorage(pageId: string): number | null {
     if (raw === null) return null;
     const value = Number.parseInt(raw, 10);
     return Number.isFinite(value) ? value : null;
-  } catch {
+  } catch (err) {
+    // Best-effort feature: storage may be unavailable (private mode / quota).
+    // No user-facing notification (a missed scroll restore is not actionable),
+    // but log per the AGENTS.md "errors must never be swallowed" rule.
+    console.warn("[useScrollPosition] sessionStorage read failed", err);
     return null;
   }
 }
@@ -33,8 +37,10 @@ function readStorage(pageId: string): number | null {
 function writeStorage(pageId: string, scrollY: number): void {
   try {
     window.sessionStorage.setItem(storageKey(pageId), String(Math.round(scrollY)));
-  } catch {
-    // Silently ignore: storage unavailable (private mode / quota exceeded).
+  } catch (err) {
+    // Storage unavailable (private mode / quota). Non-actionable for the user,
+    // but log it rather than swallow silently (AGENTS.md error-handling rule).
+    console.warn("[useScrollPosition] sessionStorage write failed", err);
   }
 }
 
@@ -50,6 +56,14 @@ function writeStorage(pageId: string, scrollY: number): void {
 export function useScrollPosition(pageId: string): {
   restoreScrollPosition: () => void;
 } {
+  // CONTRACT: this hook assumes PageEditor REMOUNTS per page — page.tsx renders
+  // `<MemoizedFullEditor key={page.id} ...>`, so switching pages creates a fresh
+  // hook instance with fresh refs. These refs latch per-mount and are NOT reset
+  // when `pageId` changes in place (only the effect re-runs on [pageId]). If that
+  // `key={page.id}` is ever removed, restore would silently break on the 2nd page
+  // (refs would hold the first page's target / already-restored flag) — in that
+  // case the refs must be reset on a pageId change.
+  //
   // The target Y captured synchronously at mount, BEFORE any scroll/visibility
   // handler can overwrite the stored value with a fresh 0 (the page starts
   // scrolled to top on load). `null` means "not yet captured".