fix(offline): address PR #120 review (comment 2513)

CHANGELOG: stop presenting the service-worker API cache as an active offline
store (/api is NetworkOnly) — describe it as a defensive purge of the legacy
api-get-cache from older clients; add an explicit upgrade note that the new CORS
allowlist rejects previously-allowed cross-domain REST clients until their origin
is added to CORS_ALLOWED_ORIGINS.

test(offline): cover make-offline ancestor-walk + dedup — a real-ancestor case
exercising the ancestorId===pageId guard (page warmed once), the dedup of
repeated tree failures into a single "tree" label, and the "breadcrumbs" label
when the breadcrumbs lookup rejects.

test(auth): cover clearOfflineCache in handleLogout — purged exactly once before
window.location.replace, and a thrown purge error does not block the redirect.

conventions: use pageKeys.detail() instead of raw ["pages", …] literals in
title-editor and use-page-collab-providers.

cleanup: remove the dead emit() in title-editor (the gateway ignores it; the
cross-user tree refresh is server-side via the Yjs title fragment); drop the
trivial Array.isArray(tiptapExtensions) test (schema is exercised transitively).

refactor: extract the shared page.<id> Yjs doc-name convention into
pageYdocName()/PAGE_YDOC_NAME_PREFIX so the editor providers, offline warm, and
offline purge can no longer drift apart.

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

CHANGELOG.md

@@ -71,9 +71,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   children, and comments are cached in IndexedDB (TanStack Query persister plus
   `y-indexeddb` for the page's Yjs document), and a PWA service worker
   (vite-plugin-pwa) serves an app shell so previously opened pages stay readable
-  offline. The offline cache (persisted query cache, Yjs page documents, and the
-  service-worker API cache) is cleared on logout AND on sign-in so a previous
-  user's private data does not remain in the browser.
+  offline. The two offline stores (the persisted query cache and the Yjs page
+  documents) are cleared on logout AND on sign-in so a previous user's private
+  data does not remain in the browser; the same purge also defensively drops any
+  legacy service-worker `api-get-cache` left by older clients (current builds
+  serve `/api` as NetworkOnly, so there is no active service-worker API cache).
 - **Mobile bootstrap**: a `returnToken` opt-in on login so native/mobile clients
   can request the access JWT in the response body (`data.authToken`) in addition
   to the httpOnly cookie (the web client stays cookie-only); an optional
@@ -87,7 +89,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   separately-hosted cross-domain client must now be listed in
   `CORS_ALLOWED_ORIGINS` (native Capacitor/Ionic/localhost WebView origins are
   allowed automatically). Requests with no `Origin` header (server-to-server)
-  are still allowed.
+  are still allowed. **Upgrade note:** the old bare `app.enableCors()` reflected
+  *any* origin (with `credentials:false`), so any previously-working cross-domain
+  REST/browser client is now rejected until its origin is added to
+  `CORS_ALLOWED_ORIGINS` (see `.env.example`).
 
 ### Fixed
 

apps/client/src/features/auth/hooks/use-auth.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, vi, beforeEach } from "vitest";
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import { renderHook, act } from "@testing-library/react";
 
 // react-i18next: identity t() so the hook renders without an i18n provider.
@@ -12,11 +12,12 @@ vi.mock("react-router-dom", () => ({
   useNavigate: () => navigateMock,
 }));
 
-// The auth service is the network boundary; stub login per test.
+// The auth service is the network boundary; stub login/logout per test.
 const loginMock = vi.fn();
+const logoutMock = vi.fn();
 vi.mock("@/features/auth/services/auth-service", () => ({
   login: (...args: unknown[]) => loginMock(...args),
-  logout: vi.fn(),
+  logout: (...args: unknown[]) => logoutMock(...args),
   forgotPassword: vi.fn(),
   passwordReset: vi.fn(),
   setupWorkspace: vi.fn(),
@@ -50,6 +51,8 @@ beforeEach(() => {
   navigateMock.mockReset();
   loginMock.mockReset();
   loginMock.mockResolvedValue(undefined);
+  logoutMock.mockReset();
+  logoutMock.mockResolvedValue(undefined);
   clearOfflineCacheMock.mockReset();
   clearOfflineCacheMock.mockResolvedValue(undefined);
 });
@@ -89,3 +92,63 @@ describe("useAuth.handleSignIn", () => {
     expect(navigateMock).toHaveBeenCalledWith("/home");
   });
 });
+
+describe("useAuth.handleLogout", () => {
+  const replaceMock = vi.fn();
+  let originalLocation: Location;
+
+  beforeEach(() => {
+    replaceMock.mockReset();
+    // window.location.replace is the post-logout redirect. jsdom's real `replace`
+    // is a non-configurable method that warns "not implemented", so swap the
+    // whole location object for one whose `replace` we can capture.
+    originalLocation = window.location;
+    Object.defineProperty(window, "location", {
+      configurable: true,
+      writable: true,
+      value: { replace: replaceMock },
+    });
+  });
+
+  afterEach(() => {
+    Object.defineProperty(window, "location", {
+      configurable: true,
+      writable: true,
+      value: originalLocation,
+    });
+  });
+
+  it("purges the offline cache exactly once BEFORE redirecting (cross-user leak guard)", async () => {
+    const order: string[] = [];
+    clearOfflineCacheMock.mockImplementation(async () => {
+      order.push("clear");
+    });
+    replaceMock.mockImplementation((url: string) => {
+      order.push(`replace:${url}`);
+    });
+
+    const { result } = renderHook(() => useAuth());
+    await act(async () => {
+      await result.current.logout();
+    });
+
+    expect(clearOfflineCacheMock).toHaveBeenCalledTimes(1);
+    // Purge must complete before the redirect (which would otherwise interrupt
+    // the async cleanup).
+    expect(order).toEqual(["clear", "replace:/login?logout=1"]);
+  });
+
+  it("still redirects when the cache purge throws (best-effort, never blocks logout)", async () => {
+    clearOfflineCacheMock.mockRejectedValue(new Error("idb unavailable"));
+
+    const { result } = renderHook(() => useAuth());
+    await act(async () => {
+      await result.current.logout();
+    });
+
+    // The thrown purge error is swallowed and the redirect still fires.
+    expect(clearOfflineCacheMock).toHaveBeenCalledTimes(1);
+    expect(replaceMock).toHaveBeenCalledTimes(1);
+    expect(replaceMock).toHaveBeenCalledWith("/login?logout=1");
+  });
+});

apps/client/src/features/editor/hooks/use-page-collab-providers.ts

@@ -25,6 +25,8 @@ import { useParams } from "react-router-dom";
 import { extractPageSlugId } from "@/lib";
 import { FIVE_MINUTES } from "@/lib/constants.ts";
 import { collabTokenNeedsRefresh } from "@/features/editor/hooks/collab-token";
+import { pageYdocName } from "@/features/editor/page-ydoc-name";
+import { pageKeys } from "@/features/page/queries/page-query";
 
 export interface PageCollabProviders {
   ydoc: Y.Doc | null;
@@ -72,7 +74,7 @@ export function usePageCollabProviders(pageId: string): PageCollabProviders {
 
   useEffect(() => {
     if (!providersRef.current) {
-      const documentName = `page.${pageId}`;
+      const documentName = pageYdocName(pageId);
       const ydoc = new Y.Doc();
       const local = new IndexeddbPersistence(documentName, ydoc);
       const socket = new HocuspocusProviderWebsocket({
@@ -91,9 +93,11 @@ export function usePageCollabProviders(pageId: string): PageCollabProviders {
         try {
           const message = JSON.parse(payload);
           if (message?.type !== "page.updated" || !message.updatedAt) return;
-          const pageData = queryClient.getQueryData<IPage>(["pages", slugId]);
+          const pageData = queryClient.getQueryData<IPage>(
+            pageKeys.detail(slugId),
+          );
           if (pageData) {
-            queryClient.setQueryData(["pages", slugId], {
+            queryClient.setQueryData(pageKeys.detail(slugId), {
               ...pageData,
               updatedAt: message.updatedAt,
               ...(message.lastUpdatedBy && {

apps/client/src/features/editor/page-ydoc-name.ts

@@ -0,0 +1,14 @@
+/**
+ * Single source of truth for the IndexedDB / Hocuspocus document name of a
+ * page's collaborative Yjs doc.
+ *
+ * The `page.<id>` convention is shared knowledge across three call sites: the
+ * live editor providers (`use-page-collab-providers`), the offline warm path
+ * (`make-offline`), and the offline purge (`clear-offline-cache`, which matches
+ * the databases to delete by this prefix). Centralizing it here stops those
+ * sites from silently drifting apart.
+ */
+export const PAGE_YDOC_NAME_PREFIX = "page.";
+
+export const pageYdocName = (pageId: string): string =>
+  `${PAGE_YDOC_NAME_PREFIX}${pageId}`;

apps/client/src/features/editor/title-editor.tsx

@@ -11,10 +11,9 @@ import {
   pageEditorAtom,
   titleEditorAtom,
 } from "@/features/editor/atoms/editor-atoms";
-import { updatePageData } from "@/features/page/queries/page-query";
+import { pageKeys, updatePageData } from "@/features/page/queries/page-query";
 import { useDebouncedCallback, getHotkeyHandler } from "@mantine/hooks";
 import { useAtom } from "jotai";
-import { useQueryEmit } from "@/features/websocket/use-query-emit.ts";
 import { Collaboration } from "@tiptap/extension-collaboration";
 import { shouldPropagateTitleChange } from "@/features/editor/title-collab";
 import { buildPageUrl } from "@/features/page/page.utils.ts";
@@ -48,7 +47,6 @@ export function TitleEditor({
   const { t } = useTranslation();
   const pageEditor = useAtomValue(pageEditorAtom);
   const [, setTitleEditor] = useAtom(titleEditorAtom);
-  const emit = useQueryEmit();
   const navigate = useNavigate();
   const currentPageEditMode = useAtomValue(currentPageEditModeAtom);
 
@@ -145,8 +143,8 @@ export function TitleEditor({
     });
 
     const page =
-      queryClient.getQueryData<IPage>(["pages", slugId]) ??
-      queryClient.getQueryData<IPage>(["pages", pageId]);
+      queryClient.getQueryData<IPage>(pageKeys.detail(slugId)) ??
+      queryClient.getQueryData<IPage>(pageKeys.detail(pageId));
     if (!page) return;
 
     const updatedPage: IPage = { ...page, title: titleText };
@@ -165,8 +163,11 @@ export function TitleEditor({
     };
 
     updatePageData(updatedPage);
+    // Drive the local (same-tab) tree/breadcrumb update. The cross-user tree
+    // refresh is handled server-side: the collab process extracts the renamed
+    // 'title' Yjs fragment and broadcasts a treeUpdate. The previous socket
+    // `emit(event)` here was a no-op (the gateway ignores it) and was removed.
     localEmitter.emit("message", event);
-    emit(event);
   }, 500);
 
   useEffect(() => {

apps/client/src/features/offline/clear-offline-cache.ts

@@ -2,6 +2,7 @@ import { del } from "idb-keyval";
 
 import { queryClient } from "@/main.tsx";
 import { OFFLINE_CACHE_KEY } from "./query-persister";
+import { PAGE_YDOC_NAME_PREFIX } from "@/features/editor/page-ydoc-name";
 
 /**
  * Best-effort purge of all of the current user's offline data from the browser.
@@ -55,7 +56,8 @@ export async function clearOfflineCache(): Promise<void> {
       const dbs = await indexedDB.databases();
       for (const db of dbs) {
         const name = db?.name;
-        if (typeof name !== "string" || !name.startsWith("page.")) continue;
+        if (typeof name !== "string" || !name.startsWith(PAGE_YDOC_NAME_PREFIX))
+          continue;
         try {
           // Fire-and-forget delete; await a thin wrapper so a slow delete does
           // not race the page teardown, but never reject on it.

apps/client/src/features/offline/make-offline.test.ts

@@ -219,6 +219,113 @@ describe("makePageAvailableOffline", () => {
 
     errorSpy.mockRestore();
   });
+
+  // Helper: the page-ids passed to the sidebar-children warm (its query key is
+  // ["sidebar-pages", { pageId, spaceId }]) — i.e. which nodes were prefetched.
+  const warmedSidebarIds = () =>
+    prefetchQuery.mock.calls
+      .map((c) => c[0])
+      .filter((opts: any) => opts?.queryKey?.[0] === "sidebar-pages")
+      .map((opts: any) => opts.queryKey[1]?.pageId);
+
+  it("warms the page + every ancestor's children once and skips the self-ancestor guard", async () => {
+    (getPageById as ReturnType<typeof vi.fn>).mockResolvedValue(okPage);
+    // Breadcrumbs include two real ancestors, the page's OWN id (must be skipped
+    // by the ancestorId === pageId guard so it is not warmed twice), and a
+    // malformed entry with no id (also skipped).
+    (getPageBreadcrumbs as ReturnType<typeof vi.fn>).mockResolvedValue([
+      { id: "anc-1" },
+      { id: "uuid-1" }, // === pageId -> guard
+      { id: "anc-2" },
+      {}, // no id -> skipped
+    ]);
+    (getSidebarPages as ReturnType<typeof vi.fn>).mockResolvedValue({
+      items: [],
+      meta: { nextCursor: null },
+    });
+    (getPageComments as ReturnType<typeof vi.fn>).mockResolvedValue({
+      items: [],
+      meta: { nextCursor: null },
+    });
+
+    const result = await makePageAvailableOffline({
+      pageId: "uuid-1",
+      spaceId: "space-uuid",
+    });
+
+    const ids = warmedSidebarIds();
+    // The page's own children (warmSidebarChildren(pageId)) plus each real
+    // ancestor — exactly once each. The self-ancestor (uuid-1 in breadcrumbs) is
+    // NOT a second warm: uuid-1 appears once (from the page's own children call).
+    expect(ids).toEqual(["uuid-1", "anc-1", "anc-2"]);
+    expect(ids.filter((id: string) => id === "uuid-1")).toHaveLength(1);
+    expect(result).toEqual({ ok: true, failed: [] });
+  });
+
+  it("dedupes repeated tree failures into a single 'tree' label", async () => {
+    const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    (getPageById as ReturnType<typeof vi.fn>).mockResolvedValue(okPage);
+    (getPageBreadcrumbs as ReturnType<typeof vi.fn>).mockResolvedValue([
+      { id: "anc-1" },
+      { id: "anc-2" },
+    ]);
+    (getSidebarPages as ReturnType<typeof vi.fn>).mockResolvedValue({
+      items: [],
+      meta: { nextCursor: null },
+    });
+    (getPageComments as ReturnType<typeof vi.fn>).mockResolvedValue({
+      items: [],
+      meta: { nextCursor: null },
+    });
+    // Fail ONLY the sidebar-children prefetches (page-own + both ancestors = 3
+    // failures); the currentUser/space prefetches still resolve.
+    prefetchQuery.mockImplementation(async (opts: any) => {
+      if (opts?.queryKey?.[0] === "sidebar-pages") throw new Error("network");
+      return undefined;
+    });
+
+    const result = await makePageAvailableOffline({
+      pageId: "uuid-1",
+      spaceId: "space-uuid",
+    });
+
+    // Three node warms failed but the contract collapses them to one "tree".
+    expect(result.ok).toBe(false);
+    expect(result.failed).toEqual(["tree"]);
+    expect(errorSpy).toHaveBeenCalled();
+
+    errorSpy.mockRestore();
+  });
+
+  it("records 'breadcrumbs' (not 'tree') when the breadcrumbs lookup rejects", async () => {
+    const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    (getPageById as ReturnType<typeof vi.fn>).mockResolvedValue(okPage);
+    // Ancestor discovery fails -> the ancestor-walk is recorded as "breadcrumbs".
+    (getPageBreadcrumbs as ReturnType<typeof vi.fn>).mockRejectedValue(
+      new Error("network"),
+    );
+    (getSidebarPages as ReturnType<typeof vi.fn>).mockResolvedValue({
+      items: [],
+      meta: { nextCursor: null },
+    });
+    (getPageComments as ReturnType<typeof vi.fn>).mockResolvedValue({
+      items: [],
+      meta: { nextCursor: null },
+    });
+
+    const result = await makePageAvailableOffline({
+      pageId: "uuid-1",
+      spaceId: "space-uuid",
+    });
+
+    // The page's own children still warmed fine (prefetch resolves), so the only
+    // failure is the breadcrumbs lookup.
+    expect(result.ok).toBe(false);
+    expect(result.failed).toEqual(["breadcrumbs"]);
+    expect(errorSpy).toHaveBeenCalled();
+
+    errorSpy.mockRestore();
+  });
 });
 
 describe("warmPageYdoc", () => {

apps/client/src/features/offline/make-offline.ts

@@ -19,6 +19,7 @@ import { getMyInfo } from "@/features/user/services/user-service";
 import { userKeys } from "@/features/user/hooks/use-current-user";
 import { IPage } from "@/features/page/types/page.types";
 import { IPagination } from "@/lib/types.ts";
+import { pageYdocName } from "@/features/editor/page-ydoc-name";
 
 /**
  * Fully paginate an infinite query and write the @tanstack InfiniteData cache
@@ -258,7 +259,7 @@ export async function warmPageYdoc(
   let remote: HocuspocusProvider | null = null;
 
   try {
-    const documentName = `page.${pageId}`;
+    const documentName = pageYdocName(pageId);
     ydoc = new Y.Doc();
     local = new IndexeddbPersistence(documentName, ydoc);
     remote = new HocuspocusProvider({

apps/server/src/collaboration/collaboration.util.spec.ts

@@ -7,7 +7,6 @@ import {
   prosemirrorNodeToYElement,
   buildTitleSeedYdoc,
   jsonToText,
-  tiptapExtensions,
 } from './collaboration.util';
 import { Node } from '@tiptap/pm/model';
 
@@ -284,10 +283,4 @@ describe('buildTitleSeedYdoc', () => {
     expect(text1).toBe(title);
     expect(text2).toBe(text1);
   });
-
-  // Touch tiptapExtensions so the import is exercised (mirrors the brief's import
-  // list and guards against accidental tree-shaking of the schema dependency).
-  it('uses the shared tiptap extensions schema', () => {
-    expect(Array.isArray(tiptapExtensions)).toBe(true);
-  });
 });