Merge develop into fix/page-template-demo-issues (#45)

Resolve conflicts from the parallel page-embed refactor that landed in develop
via #49:
- page-embed-view.tsx: keep develop's canonical decideEmbedState for the
  cycle/depth/availability guard; keep #45's #39 chrome cleanup (single source
  link, IconFileText fallback) and #40 refresh remount key. Drop #45's now-unused
  isPageEmbedCycle/isPageEmbedTooDeep wiring.
- page-embed-picker.tsx: use develop's excludeHost util; drop #45's duplicate
  filterPageEmbedOptions and its test.
- page-embed-ancestry-context.test.tsx: keep #45's superset suite.
- page-template-access.spec.ts: keep develop's constructor args; update the two
  deleteByReferenceAndSources assertions to the new 4-arg workspace-scoped
  signature introduced by #45 (#36 defense-in-depth).

Full suite green: server 624, client 219, editor-ext 56, mcp 247.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

apps/client/src/features/editor/components/page-embed/decide-embed-state.test.ts

@@ -0,0 +1,141 @@
+import { describe, it, expect } from "vitest";
+import { decideEmbedState } from "./decide-embed-state";
+import { PAGE_EMBED_MAX_DEPTH } from "./page-embed-ancestry-context";
+import type { PageTemplateLookup } from "@/features/page-embed/types/page-embed.types";
+
+const okResult: PageTemplateLookup = {
+  sourcePageId: "p1",
+  slugId: "slug-p1",
+  title: "Template",
+  icon: null,
+  content: { type: "doc" },
+  sourceUpdatedAt: "2026-01-01T00:00:00.000Z",
+};
+
+describe("decideEmbedState", () => {
+  it("returns no_source when sourcePageId is null", () => {
+    expect(
+      decideEmbedState({
+        sourcePageId: null,
+        chain: [],
+        hostPageId: null,
+        available: true,
+        result: null,
+      }),
+    ).toBe("no_source");
+  });
+
+  it("returns cycle when sourcePageId is already in the ancestor chain", () => {
+    expect(
+      decideEmbedState({
+        sourcePageId: "p1",
+        chain: ["root", "p1"],
+        hostPageId: "host",
+        available: true,
+        result: okResult,
+      }),
+    ).toBe("cycle");
+  });
+
+  it("returns cycle when sourcePageId equals the host page id (top-level self-embed)", () => {
+    expect(
+      decideEmbedState({
+        sourcePageId: "host",
+        chain: [],
+        hostPageId: "host",
+        available: true,
+        result: okResult,
+      }),
+    ).toBe("cycle");
+  });
+
+  it("returns too_deep when chain length reaches PAGE_EMBED_MAX_DEPTH", () => {
+    const chain = Array.from({ length: PAGE_EMBED_MAX_DEPTH }, (_, i) => `a${i}`);
+    expect(
+      decideEmbedState({
+        sourcePageId: "p1",
+        chain,
+        hostPageId: "host",
+        available: true,
+        result: okResult,
+      }),
+    ).toBe("too_deep");
+  });
+
+  it("cycle wins over too_deep when both apply (cycle checked first)", () => {
+    const chain = Array.from(
+      { length: PAGE_EMBED_MAX_DEPTH },
+      (_, i) => `a${i}`,
+    );
+    chain[0] = "p1"; // also a cycle
+    expect(
+      decideEmbedState({
+        sourcePageId: "p1",
+        chain,
+        hostPageId: "host",
+        available: true,
+        result: okResult,
+      }),
+    ).toBe("cycle");
+  });
+
+  it("returns unavailable when no lookup context is mounted", () => {
+    expect(
+      decideEmbedState({
+        sourcePageId: "p1",
+        chain: [],
+        hostPageId: "host",
+        available: false,
+        result: null,
+      }),
+    ).toBe("unavailable");
+  });
+
+  it("returns loading when available but the result is not back yet", () => {
+    expect(
+      decideEmbedState({
+        sourcePageId: "p1",
+        chain: [],
+        hostPageId: "host",
+        available: true,
+        result: null,
+      }),
+    ).toBe("loading");
+  });
+
+  it("returns no_access when the result status is no_access", () => {
+    expect(
+      decideEmbedState({
+        sourcePageId: "p1",
+        chain: [],
+        hostPageId: "host",
+        available: true,
+        result: { sourcePageId: "p1", status: "no_access" },
+      }),
+    ).toBe("no_access");
+  });
+
+  it("returns not_found when the result status is not_found", () => {
+    expect(
+      decideEmbedState({
+        sourcePageId: "p1",
+        chain: [],
+        hostPageId: "host",
+        available: true,
+        result: { sourcePageId: "p1", status: "not_found" },
+      }),
+    ).toBe("not_found");
+  });
+
+  it("returns ok for a resolved template (happy path)", () => {
+    expect(
+      decideEmbedState({
+        sourcePageId: "p1",
+        chain: [],
+        hostPageId: "host",
+        available: true,
+        result: okResult,
+      }),
+    ).toBe("ok");
+  });
+});

apps/client/src/features/editor/components/page-embed/decide-embed-state.ts

@@ -0,0 +1,58 @@
+import { PAGE_EMBED_MAX_DEPTH } from "./page-embed-ancestry-context";
+import type { PageTemplateLookup } from "@/features/page-embed/types/page-embed.types";
+
+/**
+ * The render outcome of a single pageEmbed node, decided BEFORE rendering a
+ * nested editor. Kept pure (no React) so the cycle / depth / access / not-found
+ * branch logic is unit-testable in isolation; the node view maps each outcome
+ * to a placeholder or the embedded content.
+ */
+export type EmbedState =
+  | "no_source" // no sourcePageId picked yet
+  | "cycle" // self-embed or an ancestor already shows this page
+  | "too_deep" // nesting depth limit reached
+  | "unavailable" // no lookup context (e.g. public share)
+  | "loading" // context present, result not back yet
+  | "ok" // resolved template content to render
+  | "no_access" // server says the viewer can't see the page
+  | "not_found"; // server says the page no longer exists
+
+export interface DecideEmbedStateInput {
+  sourcePageId: string | null;
+  /** sourcePageIds of every ancestor pageEmbed up the render tree. */
+  chain: string[];
+  /** Host page id; a top-level self-embed must be caught against it. */
+  hostPageId: string | null;
+  /** Whether a lookup context is mounted (false on public shares in MVP). */
+  available: boolean;
+  /** The lookup result, or null while still loading. */
+  result: PageTemplateLookup | null;
+}
+
+/**
+ * Decide what a pageEmbed should render. The order matters: cycle and depth
+ * guards run first (before any lookup is even consulted), then availability,
+ * then the resolved result. Mirrors the branch ladder in PageEmbedBody.
+ */
+export function decideEmbedState({
+  sourcePageId,
+  chain,
+  hostPageId,
+  available,
+  result,
+}: DecideEmbedStateInput): EmbedState {
+  if (!sourcePageId) return "no_source";
+
+  // Self-embed or a source already present in the ancestor chain → cycle.
+  const isCycle = chain.includes(sourcePageId) || hostPageId === sourcePageId;
+  if (isCycle) return "cycle";
+
+  if (chain.length >= PAGE_EMBED_MAX_DEPTH) return "too_deep";
+
+  if (!available) return "unavailable";
+  if (!result) return "loading";
+
+  if (!("status" in result)) return "ok";
+  if (result.status === "no_access") return "no_access";
+  return "not_found";
+}

apps/client/src/features/editor/components/page-embed/page-embed-lookup-context.test.tsx

@@ -0,0 +1,162 @@
+import {
+  describe,
+  it,
+  expect,
+  vi,
+  beforeEach,
+  afterEach,
+} from "vitest";
+import { act, render } from "@testing-library/react";
+import type { PageTemplateLookup } from "@/features/page-embed/types/page-embed.types";
+
+// Mock the API module the provider calls. Hoisted by vitest before the import.
+const lookupTemplate = vi.fn();
+vi.mock("@/features/page-embed/services/page-embed-api", () => ({
+  lookupTemplate: (...args: unknown[]) => lookupTemplate(...args),
+}));
+
+// Imported AFTER the mock is declared so the provider picks up the mock.
+import {
+  PageEmbedLookupProvider,
+  usePageEmbedLookup,
+} from "./page-embed-lookup-context";
+
+function ok(id: string): PageTemplateLookup {
+  return {
+    sourcePageId: id,
+    slugId: `slug-${id}`,
+    title: `T-${id}`,
+    icon: null,
+    content: { type: "doc" },
+    sourceUpdatedAt: "2026-01-01T00:00:00.000Z",
+  };
+}
+
+// Probe that subscribes to a sourceId and exposes its latest result + refresh.
+function Probe({
+  id,
+  sink,
+}: {
+  id: string;
+  sink: (api: ReturnType<typeof usePageEmbedLookup>) => void;
+}) {
+  const api = usePageEmbedLookup(id);
+  sink(api);
+  return <div>{api.result ? "loaded" : "pending"}</div>;
+}
+
+describe("PageEmbedLookupProvider (batching / dedup / refresh)", () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+    lookupTemplate.mockReset();
+  });
+
+  afterEach(() => {
+    vi.runOnlyPendingTimers();
+    vi.useRealTimers();
+  });
+
+  it("dedups two subscribers for the same id into a single lookup call; both get the result", async () => {
+    let a: ReturnType<typeof usePageEmbedLookup> | null = null;
+    let b: ReturnType<typeof usePageEmbedLookup> | null = null;
+    lookupTemplate.mockResolvedValue({ items: [ok("p1")] });
+
+    render(
+      <PageEmbedLookupProvider>
+        <Probe id="p1" sink={(x) => (a = x)} />
+        <Probe id="p1" sink={(x) => (b = x)} />
+      </PageEmbedLookupProvider>,
+    );
+
+    // Subscriptions run in effects + the 10ms debounce batches them together.
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(20);
+    });
+
+    expect(lookupTemplate).toHaveBeenCalledTimes(1);
+    expect(lookupTemplate).toHaveBeenCalledWith({ sourcePageIds: ["p1"] });
+    expect(a!.result).toEqual(ok("p1"));
+    expect(b!.result).toEqual(ok("p1"));
+  });
+
+  it("batches two distinct ids subscribed within the window into one call", async () => {
+    lookupTemplate.mockResolvedValue({ items: [ok("p1"), ok("p2")] });
+
+    render(
+      <PageEmbedLookupProvider>
+        <Probe id="p1" sink={() => {}} />
+        <Probe id="p2" sink={() => {}} />
+      </PageEmbedLookupProvider>,
+    );
+
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(20);
+    });
+
+    expect(lookupTemplate).toHaveBeenCalledTimes(1);
+    expect(lookupTemplate.mock.calls[0][0]).toEqual({
+      sourcePageIds: ["p1", "p2"],
+    });
+  });
+
+  it("refresh() clears the cache and re-fetches", async () => {
+    let a: ReturnType<typeof usePageEmbedLookup> | null = null;
+    lookupTemplate.mockResolvedValue({ items: [ok("p1")] });
+
+    render(
+      <PageEmbedLookupProvider>
+        <Probe id="p1" sink={(x) => (a = x)} />
+      </PageEmbedLookupProvider>,
+    );
+
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(20);
+    });
+    expect(lookupTemplate).toHaveBeenCalledTimes(1);
+
+    // refresh resolves once the next batch flush completes.
+    await act(async () => {
+      const p = a!.refresh();
+      await vi.advanceTimersByTimeAsync(20);
+      await p;
+    });
+
+    expect(lookupTemplate).toHaveBeenCalledTimes(2);
+  });
+
+  it("a rejected lookup resolves refresh() waiters, clears inFlight, and logs the error (not swallowed)", async () => {
+    const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    let a: ReturnType<typeof usePageEmbedLookup> | null = null;
+    lookupTemplate.mockRejectedValueOnce(new Error("boom"));
+
+    render(
+      <PageEmbedLookupProvider>
+        <Probe id="p1" sink={(x) => (a = x)} />
+      </PageEmbedLookupProvider>,
+    );
+
+    // Initial subscription enqueues a lookup that rejects.
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(20);
+    });
+
+    expect(errSpy).toHaveBeenCalled();
+    // The error message is surfaced, not swallowed.
+    expect(errSpy.mock.calls[0][0]).toContain("[pageEmbed] template lookup failed");
+
+    // inFlight was cleared on failure, so a refresh re-enqueues and resolves.
+    lookupTemplate.mockResolvedValueOnce({ items: [ok("p1")] });
+    let resolved = false;
+    await act(async () => {
+      const p = a!.refresh().then(() => {
+        resolved = true;
+      });
+      await vi.advanceTimersByTimeAsync(20);
+      await p;
+    });
+    expect(resolved).toBe(true);
+    expect(a!.result).toEqual(ok("p1"));
+
+    errSpy.mockRestore();
+  });
+});

apps/client/src/features/editor/components/page-embed/page-embed-picker.test.ts

@@ -1,44 +0,0 @@
-import { describe, it, expect } from "vitest";
-import { filterPageEmbedOptions } from "./page-embed-picker";
-
-type Page = { id: string; title?: string };
-
-describe("filterPageEmbedOptions", () => {
-  const pages: Page[] = [
-    { id: "p1", title: "One" },
-    { id: "host", title: "Host" },
-    { id: "p2", title: "Two" },
-  ];
-
-  it("excludes the host page from the options (self-embed guard)", () => {
-    const result = filterPageEmbedOptions(pages, "host");
-    expect(result.map((p) => p.id)).toEqual(["p1", "p2"]);
-  });
-
-  it("keeps all pages when the host id matches nothing", () => {
-    const result = filterPageEmbedOptions(pages, "other");
-    expect(result.map((p) => p.id)).toEqual(["p1", "host", "p2"]);
-  });
-
-  it("keeps all pages when no host id is provided", () => {
-    const result = filterPageEmbedOptions(pages, undefined);
-    expect(result.map((p) => p.id)).toEqual(["p1", "host", "p2"]);
-  });
-
-  it("drops nullish entries defensively", () => {
-    const dirty = [
-      { id: "p1" },
-      null as unknown as Page,
-      undefined as unknown as Page,
-      { id: "p2" },
-    ];
-    const result = filterPageEmbedOptions(dirty, "host");
-    expect(result.map((p) => p.id)).toEqual(["p1", "p2"]);
-  });
-
-  it("returns an empty array for nullish input", () => {
-    expect(
-      filterPageEmbedOptions(null as unknown as Page[], "host"),
-    ).toEqual([]);
-  });
-});

apps/client/src/features/editor/components/page-embed/page-embed-picker.tsx

@@ -6,21 +6,10 @@ import { IconFileText, IconSearch } from "@tabler/icons-react";
 import type { Editor, Range } from "@tiptap/core";
 import { searchSuggestions } from "@/features/search/services/search-service";
 import type { IPage } from "@/features/page/types/page.types";
+import { buildPickerQuery, excludeHost } from "./page-embed-picker.utils";
 
 export const PAGE_EMBED_PICKER_EVENT = "open-page-embed-picker";
 
-/**
- * Pure filter excluding the host page (and any nullish entries) from the picker
- * results. Extracted so the self-embed guard at insertion time is unit-testable
- * without mounting the modal/search query.
- */
-export function filterPageEmbedOptions<T extends { id: string }>(
-  pages: T[],
-  hostPageId?: string,
-): T[] {
-  return (pages ?? []).filter((p) => p && p.id !== hostPageId);
-}
-
 type PickerDetail = {
   editor: Editor;
   range: Range;
@@ -55,19 +44,13 @@ export default function PageEmbedPicker() {
 
   const { data, isFetching } = useQuery({
     queryKey: ["page-embed-template-picker", query],
-    queryFn: () =>
-      searchSuggestions({
-        query,
-        includePages: true,
-        onlyTemplates: true,
-        limit: 20,
-      }),
+    queryFn: () => searchSuggestions(buildPickerQuery(query)),
     enabled: opened,
     staleTime: 30 * 1000,
   });
 
   const hostPageId = detailRef.current?.hostPageId;
-  const pages = filterPageEmbedOptions((data?.pages ?? []) as IPage[], hostPageId);
+  const pages = excludeHost((data?.pages ?? []) as IPage[], hostPageId);
 
   const handleSelect = (page: IPage) => {
     const detail = detailRef.current;

apps/client/src/features/editor/components/page-embed/page-embed-picker.utils.test.ts

@@ -0,0 +1,43 @@
+import { describe, it, expect } from "vitest";
+import { excludeHost, buildPickerQuery } from "./page-embed-picker.utils";
+import type { IPage } from "@/features/page/types/page.types";
+
+function page(id: string): IPage {
+  return { id, title: id, slugId: `slug-${id}` } as IPage;
+}
+
+describe("excludeHost", () => {
+  it("drops the host page from the results (self-embed guard)", () => {
+    const result = excludeHost([page("a"), page("host"), page("b")], "host");
+    expect(result.map((p) => p.id)).toEqual(["a", "b"]);
+  });
+
+  it("returns all pages when hostPageId is undefined", () => {
+    const result = excludeHost([page("a"), page("b")], undefined);
+    expect(result.map((p) => p.id)).toEqual(["a", "b"]);
+  });
+
+  it("drops null/blank entries", () => {
+    const result = excludeHost(
+      [page("a"), null as unknown as IPage, page("b")],
+      "host",
+    );
+    expect(result.map((p) => p.id)).toEqual(["a", "b"]);
+  });
+});
+
+describe("buildPickerQuery", () => {
+  it("passes onlyTemplates:true with the query and page inclusion", () => {
+    expect(buildPickerQuery("foo")).toEqual({
+      query: "foo",
+      includePages: true,
+      onlyTemplates: true,
+      limit: 20,
+    });
+  });
+
+  it("preserves an empty query", () => {
+    expect(buildPickerQuery("").query).toBe("");
+    expect(buildPickerQuery("").onlyTemplates).toBe(true);
+  });
+});

apps/client/src/features/editor/components/page-embed/page-embed-picker.utils.ts

@@ -0,0 +1,27 @@
+import type { IPage } from "@/features/page/types/page.types";
+import type { SearchSuggestionParams } from "@/features/search/types/search.types";
+
+/**
+ * Self-embed guard at insertion time: drop the host page (and any null/blank
+ * entries) from the picker results so the current page can't embed itself.
+ */
+export function excludeHost(
+  pages: IPage[],
+  hostPageId: string | undefined,
+): IPage[] {
+  return pages.filter((p) => p && p.id !== hostPageId);
+}
+
+/**
+ * Build the search-suggestions query for the template picker. Always restricts
+ * to template-flagged pages (`onlyTemplates`) and includes pages, mirroring the
+ * inline query args in PageEmbedPicker.
+ */
+export function buildPickerQuery(query: string): SearchSuggestionParams {
+  return {
+    query,
+    includePages: true,
+    onlyTemplates: true,
+    limit: 20,
+  };
+}

apps/client/src/features/editor/components/page-embed/page-embed-view.tsx

@@ -20,9 +20,8 @@ import { usePageEmbedLookup } from "./page-embed-lookup-context";
 import {
   PageEmbedAncestryProvider,
   usePageEmbedAncestry,
-  isPageEmbedCycle,
-  isPageEmbedTooDeep,
 } from "./page-embed-ancestry-context";
+import { decideEmbedState } from "./decide-embed-state";
 import PageEmbedContent from "./page-embed-content";
 
 function Placeholder({
@@ -99,14 +98,15 @@ function PageEmbedBody({
     }
   };
 
-  // --- Cycle / depth guard (evaluated before any lookup is rendered) ---------
-  // Self-embed or a source already present in the ancestor chain → cycle.
-  const isCycle = isPageEmbedCycle(
-    ancestry.chain,
-    ancestry.hostPageId,
+  // --- Cycle / depth / availability decision (pure, unit-tested) ------------
+  // Evaluated before any nested editor is rendered.
+  const embedState = decideEmbedState({
     sourcePageId,
-  );
-  const isTooDeep = isPageEmbedTooDeep(ancestry.chain);
+    chain: ancestry.chain,
+    hostPageId: ancestry.hostPageId,
+    available,
+    result,
+  });
 
   const sourceTitle =
     result && !("status" in result) ? result.title : null;
@@ -179,28 +179,28 @@ function PageEmbedBody({
     ) : null;
 
   let body: React.ReactNode;
-  if (!sourcePageId) {
+  if (embedState === "no_source") {
     body = (
       <Placeholder
         icon={<IconInfoCircle size={18} stroke={1.6} />}
         label={t("No page selected")}
       />
     );
-  } else if (isCycle) {
+  } else if (embedState === "cycle") {
     body = (
       <Placeholder
         icon={<IconRepeat size={18} stroke={1.6} />}
         label={t("Circular embed: this page is already shown above")}
       />
     );
-  } else if (isTooDeep) {
+  } else if (embedState === "too_deep") {
     body = (
       <Placeholder
         icon={<IconRepeat size={18} stroke={1.6} />}
         label={t("Embed nesting limit reached")}
       />
     );
-  } else if (!available) {
+  } else if (embedState === "unavailable") {
     // No lookup context (e.g. public share) → placeholder, no fetch in MVP.
     body = (
       <Placeholder
@@ -208,9 +208,9 @@ function PageEmbedBody({
         label={t("Embedded page is not available here")}
       />
     );
-  } else if (!result) {
+  } else if (embedState === "loading") {
     body = <div style={{ minHeight: 24 }} />;
-  } else if (!("status" in result)) {
+  } else if (embedState === "ok" && result && !("status" in result)) {
     body = (
       <PageEmbedAncestryProvider
         sourcePageId={sourcePageId}
@@ -229,7 +229,7 @@ function PageEmbedBody({
         />
       </PageEmbedAncestryProvider>
     );
-  } else if (result.status === "no_access") {
+  } else if (embedState === "no_access") {
     body = (
       <Placeholder
         icon={<IconEyeOff size={18} stroke={1.6} />}