Merge pull request 'feat(editor): restore reading scroll position on reload (#266)' (#267) from feat/266-scroll-position into develop

Reviewed-on: #267

CHANGELOG.md

@@ -514,6 +514,7 @@ knowledge layer, an embedded MCP server, and the Gitmost rebrand.
 - Build: drop the private EE submodule, retarget CI to GHCR, and update the
   Docker image to the GHCR registry.
 
-[Unreleased]: https://github.com/vvzvlad/gitmost/compare/v0.93.0...HEAD
+[Unreleased]: https://github.com/vvzvlad/gitmost/compare/v0.94.0...HEAD
+[0.94.0]: https://github.com/vvzvlad/gitmost/compare/v0.93.0...v0.94.0
 [0.93.0]: https://github.com/vvzvlad/gitmost/compare/v0.91.0...v0.93.0
 [0.91.0]: https://github.com/vvzvlad/gitmost/compare/v0.90.1...v0.91.0

apps/client/src/features/dictation/hooks/use-streaming-dictation.test.tsx

@@ -0,0 +1,206 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { renderHook, act } from "@testing-library/react";
+
+// Shared, hoisted test state the module mocks write into. `onSpeechEnd` is the
+// VAD callback the hook registers on MicVAD.new — capturing it lets us drive
+// "a speech segment ended" deterministically. `pending` collects the deferred
+// transcription promises so the test controls their resolution order, which is
+// the whole point: out-of-order HTTP responses must NOT scramble the emitted
+// text (the in-order emitter under test).
+const h = vi.hoisted(() => {
+  return {
+    onSpeechEnd: null as null | ((audio: Float32Array) => void),
+    pending: [] as { resolve: (s: string) => void; reject: (e: unknown) => void }[],
+    notify: null as null | ReturnType<typeof Object>,
+  };
+});
+
+// Lazy-imported VAD: capture the onSpeechEnd handler and hand back a no-op
+// instance (start/pause/destroy all resolve).
+vi.mock("@ricky0123/vad-web", () => ({
+  MicVAD: {
+    new: vi.fn(async (opts: { onSpeechEnd: (a: Float32Array) => void }) => {
+      h.onSpeechEnd = opts.onSpeechEnd;
+      return {
+        start: vi.fn(async () => {}),
+        pause: vi.fn(async () => {}),
+        destroy: vi.fn(async () => {}),
+      };
+    }),
+  },
+}));
+
+// Each transcribeAudio call returns a promise we resolve/reject by index.
+vi.mock("@/features/dictation/services/dictation-service", () => ({
+  transcribeAudio: vi.fn(
+    () =>
+      new Promise<string>((resolve, reject) => {
+        h.pending.push({ resolve, reject });
+      }),
+  ),
+}));
+
+// Avoid real WAV encoding; the segment payload is irrelevant to ordering.
+vi.mock("@/features/dictation/utils/encode-wav", () => ({
+  encodeWavPcm16: vi.fn(() => new Blob()),
+}));
+
+const notifyShow = vi.fn();
+vi.mock("@mantine/notifications", () => ({
+  notifications: { show: (...args: unknown[]) => notifyShow(...args) },
+}));
+
+vi.mock("react-i18next", () => ({
+  useTranslation: () => ({ t: (s: string) => s }),
+}));
+
+import { useStreamingDictation } from "./use-streaming-dictation";
+
+// jsdom has no AudioContext; the hook constructs one and calls resume(). A
+// trivial stub is enough — the real audio path is irrelevant to ordering.
+class FakeAudioContext {
+  state = "running";
+  resume() {
+    return Promise.resolve();
+  }
+  close() {
+    this.state = "closed";
+    return Promise.resolve();
+  }
+}
+
+async function startRecording(onText: (t: string) => void) {
+  const hook = renderHook(() => useStreamingDictation({ onText }));
+  await act(async () => {
+    await hook.result.current.start();
+  });
+  // The VAD registered its onSpeechEnd and start() resolved into "recording".
+  expect(h.onSpeechEnd).toBeTypeOf("function");
+  expect(hook.result.current.status).toBe("recording");
+  return hook;
+}
+
+// Fire N ended speech segments (seq 0..N-1), each kicking off one transcription.
+async function emitSegments(n: number) {
+  await act(async () => {
+    for (let i = 0; i < n; i++) h.onSpeechEnd!(new Float32Array(8));
+  });
+}
+
+describe("useStreamingDictation — in-order segment emitter", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    h.onSpeechEnd = null;
+    h.pending = [];
+    notifyShow.mockClear();
+    (window as unknown as { AudioContext: unknown }).AudioContext =
+      FakeAudioContext;
+  });
+
+  it("emits transcriptions in segment order even when responses resolve out of order", async () => {
+    const emitted: string[] = [];
+    await startRecording((t) => emitted.push(t));
+    await emitSegments(3);
+    expect(h.pending).toHaveLength(3);
+
+    // Resolve seq 1 FIRST: it must be buffered, not emitted, because seq 0 is
+    // still outstanding (nextEmit == 0).
+    await act(async () => {
+      h.pending[1].resolve("second");
+    });
+    expect(emitted).toEqual([]);
+
+    // Resolve seq 0: this unblocks the buffer and flushes 0 then 1 in order.
+    await act(async () => {
+      h.pending[0].resolve("first");
+    });
+    expect(emitted).toEqual(["first", "second"]);
+
+    // seq 2 resolves last and flushes immediately (it is now next).
+    await act(async () => {
+      h.pending[2].resolve("third");
+    });
+    expect(emitted).toEqual(["first", "second", "third"]);
+  });
+
+  it("trims whitespace and drops empty/whitespace-only transcriptions while still advancing", async () => {
+    const emitted: string[] = [];
+    await startRecording((t) => emitted.push(t));
+    await emitSegments(3);
+
+    await act(async () => {
+      h.pending[0].resolve("  hello  "); // leading/trailing space trimmed
+      h.pending[1].resolve("   "); // whitespace-only -> not emitted, but seq advances
+      h.pending[2].resolve("world");
+    });
+
+    expect(emitted).toEqual(["hello", "world"]);
+  });
+
+  it("a failed segment shows one notification and is skipped so later segments still flush in order", async () => {
+    const emitted: string[] = [];
+    await startRecording((t) => emitted.push(t));
+    await emitSegments(2);
+
+    // seq 0 fails: the user sees a notification and the emitter advances past it.
+    await act(async () => {
+      h.pending[0].reject({ message: "boom" });
+    });
+    expect(notifyShow).toHaveBeenCalledTimes(1);
+    expect(emitted).toEqual([]);
+
+    // seq 1 still flushes (it is now next), proving one failure did not stall.
+    await act(async () => {
+      h.pending[1].resolve("survivor");
+    });
+    expect(emitted).toEqual(["survivor"]);
+  });
+
+  it("an OUT-OF-ORDER failed segment is buffered as empty and skipped without stalling later text", async () => {
+    const emitted: string[] = [];
+    await startRecording((t) => emitted.push(t));
+    await emitSegments(3);
+
+    // seq 1 (NOT next-to-emit) fails first: it takes the else branch — an empty
+    // placeholder is buffered (resultsRef.set(seq, "")) so the emitter can later
+    // skip it. One notification, nothing emitted yet (seq 0 still gates).
+    await act(async () => {
+      h.pending[1].reject({ message: "boom" });
+    });
+    expect(notifyShow).toHaveBeenCalledTimes(1);
+    expect(emitted).toEqual([]);
+
+    // seq 0 flushes; the drain then reaches the buffered empty seq 1 and SKIPS
+    // past it to seq 2.
+    await act(async () => {
+      h.pending[0].resolve("alpha");
+    });
+    expect(emitted).toEqual(["alpha"]);
+
+    // seq 2 emits — proving the empty placeholder let the emitter advance past
+    // the failed seq 1. Without the else branch's placeholder the drain would
+    // stall at the missing seq 1 and "gamma" would never flush.
+    await act(async () => {
+      h.pending[2].resolve("gamma");
+    });
+    expect(emitted).toEqual(["alpha", "gamma"]);
+  });
+
+  it("ignores a transcription that resolves AFTER cancel() (stale epoch — no emit)", async () => {
+    const emitted: string[] = [];
+    const hook = await startRecording((t) => emitted.push(t));
+    await emitSegments(1);
+
+    // Hard discard the session: the in-flight request is now stale.
+    act(() => {
+      hook.result.current.cancel();
+    });
+    expect(hook.result.current.status).toBe("idle");
+
+    // Its late resolution must be dropped (no emit into the new/empty session).
+    await act(async () => {
+      h.pending[0].resolve("late");
+    });
+    expect(emitted).toEqual([]);
+  });
+});

apps/client/src/features/editor/components/link/internal-link-paste.test.ts

@@ -0,0 +1,194 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+// Mock the page-service so importing the module under test does not pull in the
+// axios/api-client chain. `createMentionAction` is wired to `getPageById`; the
+// spy lets us assert that wiring without any network. `vi.hoisted` keeps the spy
+// available inside the hoisted vi.mock factory.
+const { getPageById } = vi.hoisted(() => ({ getPageById: vi.fn() }));
+vi.mock("@/features/page/services/page-service.ts", () => ({
+  getPageById,
+}));
+
+// `uuid` v7 is used for the mention node id; pin only v7 so assertions are
+// stable, keeping the rest (e.g. `validate`, used by extractPageSlugId) real.
+vi.mock("uuid", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("uuid")>()),
+  v7: () => "fixed-mention-uuid",
+}));
+
+import {
+  handleInternalLink,
+  createMentionAction,
+} from "./internal-link-paste";
+
+// Minimal ProseMirror-ish EditorView fake. We record what handleInternalLink
+// builds and dispatches without standing up a real schema/state.
+function makeView() {
+  const tr = {
+    replaceWith: vi.fn(function (this: unknown) {
+      return tr;
+    }),
+    insertText: vi.fn(function (this: unknown) {
+      return tr;
+    }),
+    addMark: vi.fn(function (this: unknown) {
+      return tr;
+    }),
+  };
+  const schema = {
+    nodes: {
+      mention: {
+        // Echo the attrs back so we can assert exactly what was created.
+        create: vi.fn((attrs: Record<string, unknown>) => ({
+          type: "mention",
+          attrs,
+        })),
+      },
+    },
+    marks: {
+      link: {
+        create: vi.fn((attrs: Record<string, unknown>) => ({
+          type: "link",
+          attrs,
+        })),
+      },
+    },
+  };
+  const view = {
+    state: { schema, tr },
+    dispatch: vi.fn(),
+  };
+  return { view, tr, schema };
+}
+
+describe("handleInternalLink", () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  it("does nothing when validateFn rejects the url (no resolve, no dispatch)", async () => {
+    const onResolveLink = vi.fn();
+    const validateFn = vi.fn(() => false);
+    const { view } = makeView();
+
+    await handleInternalLink({ validateFn, onResolveLink })(
+      "any-url",
+      view as never,
+      3,
+      "creator-1",
+    );
+
+    expect(validateFn).toHaveBeenCalledWith("any-url", view);
+    expect(onResolveLink).not.toHaveBeenCalled();
+    expect(view.dispatch).not.toHaveBeenCalled();
+  });
+
+  it("on resolve: inserts a mention node carrying the resolved page + anchor and dispatches replaceWith at pos", async () => {
+    const page = {
+      id: "page-id-99",
+      title: "My Page",
+      slugId: "slugABC",
+    };
+    const onResolveLink = vi.fn().mockResolvedValue(page);
+    const { view, tr, schema } = makeView();
+
+    // extractPageSlugId("doc-slug-xyz789") -> "xyz789" (last hyphen segment).
+    await handleInternalLink({ validateFn: () => true, onResolveLink })(
+      "doc-slug-xyz789",
+      view as never,
+      5,
+      "creator-7",
+      "anchor-42",
+    );
+
+    // The linked page id is the extracted slug-id, not the whole url.
+    expect(onResolveLink).toHaveBeenCalledWith("xyz789", "creator-7");
+    expect(schema.nodes.mention.create).toHaveBeenCalledWith({
+      id: "fixed-mention-uuid",
+      label: "My Page",
+      entityType: "page",
+      entityId: "page-id-99",
+      slugId: "slugABC",
+      creatorId: "creator-7",
+      anchorId: "anchor-42",
+    });
+    expect(tr.replaceWith).toHaveBeenCalledWith(5, 5, {
+      type: "mention",
+      attrs: expect.objectContaining({ entityId: "page-id-99" }),
+    });
+    expect(tr.insertText).not.toHaveBeenCalled();
+    expect(view.dispatch).toHaveBeenCalledTimes(1);
+    expect(view.dispatch).toHaveBeenCalledWith(tr);
+  });
+
+  it("falls back to 'Untitled' label when the resolved page has no title", async () => {
+    const onResolveLink = vi
+      .fn()
+      .mockResolvedValue({ id: "p", title: "", slugId: "s" });
+    const { view, schema } = makeView();
+
+    await handleInternalLink({ validateFn: () => true, onResolveLink })(
+      "abc-id1",
+      view as never,
+      0,
+      "c",
+    );
+
+    expect(schema.nodes.mention.create).toHaveBeenCalledWith(
+      expect.objectContaining({ label: "Untitled" }),
+    );
+  });
+
+  it("on reject: inserts the raw url as plain text with a link mark and dispatches", async () => {
+    const onResolveLink = vi.fn().mockRejectedValue(new Error("not found"));
+    const { view, tr, schema } = makeView();
+
+    await handleInternalLink({ validateFn: () => true, onResolveLink })(
+      "http://x/page-id2",
+      view as never,
+      4,
+      "creator-1",
+    );
+
+    // No mention node on the failure path.
+    expect(schema.nodes.mention.create).not.toHaveBeenCalled();
+    expect(tr.insertText).toHaveBeenCalledWith("http://x/page-id2", 4);
+    expect(schema.marks.link.create).toHaveBeenCalledWith({
+      href: "http://x/page-id2",
+    });
+    // Mark spans exactly the inserted url text: [pos, pos + url.length].
+    expect(tr.addMark).toHaveBeenCalledWith(4, 4 + "http://x/page-id2".length, {
+      type: "link",
+      attrs: { href: "http://x/page-id2" },
+    });
+    expect(view.dispatch).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe("createMentionAction", () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  it("resolves the link via getPageById and inserts the mention", async () => {
+    getPageById.mockResolvedValue({
+      id: "real-page",
+      title: "Real",
+      slugId: "rslug",
+    });
+    const { view, schema } = makeView();
+
+    await createMentionAction("ref-pageABC", view as never, 2, "creator-9");
+
+    expect(getPageById).toHaveBeenCalledWith({ pageId: "pageABC" });
+    expect(schema.nodes.mention.create).toHaveBeenCalledWith(
+      expect.objectContaining({ entityId: "real-page", label: "Real" }),
+    );
+  });
+
+  it("propagates a getPageById failure to the plain-link fallback", async () => {
+    getPageById.mockRejectedValue(new Error("404"));
+    const { view, tr } = makeView();
+
+    await createMentionAction("ref-pageABC", view as never, 1, "creator-9");
+
+    // Failure path: the url is inserted as text, not as a mention node.
+    expect(tr.insertText).toHaveBeenCalledWith("ref-pageABC", 1);
+  });
+});

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

@@ -0,0 +1,243 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import { renderHook, act } from "@testing-library/react";
+import { useScrollPosition } from "./use-scroll-position";
+
+const KEY_PREFIX = "gitmost:scroll-position:";
+
+function setScrollY(value: number): void {
+  Object.defineProperty(window, "scrollY", {
+    configurable: true,
+    value,
+  });
+}
+
+function setScrollHeight(value: number): void {
+  Object.defineProperty(document.documentElement, "scrollHeight", {
+    configurable: true,
+    value,
+  });
+}
+
+function setInnerHeight(value: number): void {
+  Object.defineProperty(window, "innerHeight", {
+    configurable: true,
+    value,
+  });
+}
+
+describe("useScrollPosition", () => {
+  beforeEach(() => {
+    window.sessionStorage.clear();
+    setScrollY(0);
+    setScrollHeight(0);
+    setInnerHeight(800);
+    // jsdom does not implement window.scrollTo; stub it.
+    window.scrollTo = vi.fn();
+    // Ensure no anchor leaks between tests.
+    window.location.hash = "";
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+    vi.useRealTimers();
+    window.location.hash = "";
+  });
+
+  it("(a) saves window.scrollY to sessionStorage under the pageId key, throttled", () => {
+    vi.useFakeTimers();
+    const { unmount } = renderHook(() => useScrollPosition("p1"));
+
+    // Leading-edge save fires immediately.
+    setScrollY(123);
+    act(() => {
+      window.dispatchEvent(new Event("scroll"));
+    });
+    expect(window.sessionStorage.getItem(`${KEY_PREFIX}p1`)).toBe("123");
+
+    // Within the throttle window the next scroll is suppressed.
+    setScrollY(456);
+    act(() => {
+      window.dispatchEvent(new Event("scroll"));
+    });
+    expect(window.sessionStorage.getItem(`${KEY_PREFIX}p1`)).toBe("123");
+
+    // After the throttle window elapses, the next scroll persists again.
+    act(() => {
+      vi.advanceTimersByTime(250);
+    });
+    setScrollY(789);
+    act(() => {
+      window.dispatchEvent(new Event("scroll"));
+    });
+    expect(window.sessionStorage.getItem(`${KEY_PREFIX}p1`)).toBe("789");
+
+    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");
+    // Content is ALREADY tall enough (maxScroll = 2000 - 800 = 1200 >= 500), so
+    // without the hash guard tryRestore would call scrollTo synchronously on the
+    // first tick. The assertion below therefore genuinely proves the hash guard
+    // short-circuits before any scroll (not just that the poll has not fired).
+    setScrollHeight(2000);
+    window.location.hash = "#some-heading";
+
+    const { result } = renderHook(() => useScrollPosition("p2"));
+    act(() => {
+      result.current.restoreScrollPosition();
+      vi.advanceTimersByTime(5000);
+    });
+
+    expect(window.scrollTo).not.toHaveBeenCalled();
+  });
+
+  it("(f) cancels the in-flight restore poll on unmount (no scroll on the next page)", () => {
+    vi.useFakeTimers();
+    window.sessionStorage.setItem(`${KEY_PREFIX}p7`, "500");
+    setInnerHeight(800);
+    setScrollHeight(100); // maxScroll = -700: target not reachable yet, so it polls.
+
+    const { result, unmount } = renderHook(() => useScrollPosition("p7"));
+    act(() => {
+      result.current.restoreScrollPosition();
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled(); // still polling
+
+    // Navigate away (the hook unmounts) BEFORE the content grows tall enough.
+    unmount();
+
+    // Content of the NEXT page becomes tall; advancing time must NOT resurrect
+    // the cancelled poll (without the cleanup it would scroll the new page).
+    setScrollHeight(2000);
+    act(() => {
+      vi.advanceTimersByTime(5000);
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled();
+  });
+
+  it("(c) does nothing when nothing is saved or the saved value is <= 0", () => {
+    // Nothing saved.
+    const a = renderHook(() => useScrollPosition("nope"));
+    act(() => {
+      a.result.current.restoreScrollPosition();
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled();
+
+    // Saved value <= 0.
+    window.sessionStorage.setItem(`${KEY_PREFIX}zero`, "0");
+    const b = renderHook(() => useScrollPosition("zero"));
+    act(() => {
+      b.result.current.restoreScrollPosition();
+    });
+    expect(window.scrollTo).not.toHaveBeenCalled();
+  });
+
+  it("(d) scrolls to the saved Y once the content is tall enough", () => {
+    vi.useFakeTimers();
+    window.sessionStorage.setItem(`${KEY_PREFIX}p4`, "500");
+    setInnerHeight(800);
+    setScrollHeight(100); // maxScroll = -700, target not yet reachable.
+
+    const { result } = renderHook(() => useScrollPosition("p4"));
+    act(() => {
+      result.current.restoreScrollPosition();
+    });
+
+    // Still polling: content not laid out yet.
+    expect(window.scrollTo).not.toHaveBeenCalled();
+
+    // Content becomes tall enough: maxScroll = 2000 - 800 = 1200 >= 500.
+    setScrollHeight(2000);
+    act(() => {
+      vi.advanceTimersByTime(100);
+    });
+
+    expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
+  });
+
+  it("(d2) clamps to the max reachable position after the timeout", () => {
+    vi.useFakeTimers();
+    window.sessionStorage.setItem(`${KEY_PREFIX}p5`, "5000");
+    setInnerHeight(800);
+    setScrollHeight(1000); // maxScroll stays 200, never reaches 5000.
+
+    const { result } = renderHook(() => useScrollPosition("p5"));
+    act(() => {
+      result.current.restoreScrollPosition();
+    });
+
+    // Advance past the 5s timeout; restore should fire clamped to maxScroll.
+    act(() => {
+      vi.advanceTimersByTime(5000);
+    });
+
+    expect(window.scrollTo).toHaveBeenCalledWith({ top: 200, behavior: "auto" });
+  });
+
+  it("(e) never throws when storage access throws", () => {
+    const err = new Error("storage denied");
+    vi.spyOn(window.sessionStorage, "getItem").mockImplementation(() => {
+      throw err;
+    });
+    vi.spyOn(window.sessionStorage, "setItem").mockImplementation(() => {
+      throw err;
+    });
+
+    expect(() => {
+      const { result, unmount } = renderHook(() => useScrollPosition("p6"));
+      act(() => {
+        setScrollY(42);
+        window.dispatchEvent(new Event("scroll"));
+        result.current.restoreScrollPosition();
+      });
+      unmount();
+    }).not.toThrow();
+  });
+});

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

@@ -0,0 +1,177 @@
+import { useCallback, useEffect, useRef } from "react";
+
+// Throttle interval for persisting the scroll position while the user reads.
+const SAVE_THROTTLE_MS = 250;
+// Give up polling for the live content height after this long and restore to
+// the furthest reachable position (handles "collab never finishes laying out").
+const MAX_RESTORE_WAIT_MS = 5000;
+// How often to re-check the document height while waiting for content to load.
+const RESTORE_POLL_MS = 100;
+
+// sessionStorage key prefix. sessionStorage survives an F5 in the same tab and
+// is cleared on tab close, which is exactly the lifetime we want for an MVP
+// "remember where I was reading" feature (self-limiting, no cross-tab leak).
+const STORAGE_PREFIX = "gitmost:scroll-position:";
+
+function storageKey(pageId: string): string {
+  return `${STORAGE_PREFIX}${pageId}`;
+}
+
+// All storage access is wrapped: private mode / quota / disabled storage must
+// never throw out of the hook and break the page.
+function readStorage(pageId: string): number | null {
+  try {
+    const raw = window.sessionStorage.getItem(storageKey(pageId));
+    if (raw === null) return null;
+    const value = Number.parseInt(raw, 10);
+    return Number.isFinite(value) ? value : null;
+  } 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;
+  }
+}
+
+function writeStorage(pageId: string, scrollY: number): void {
+  try {
+    window.sessionStorage.setItem(storageKey(pageId), String(Math.round(scrollY)));
+  } 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);
+  }
+}
+
+/**
+ * Persists and restores the window scroll position per page so a reader keeps
+ * their place across a reload (F5) or reopening the document.
+ *
+ * Returns `restoreScrollPosition`, which the page editor calls once the live
+ * (non-static) content is laid out. The two scroll mechanisms are mutually
+ * exclusive: if the URL has a `#hash` anchor, the existing anchor-scroll logic
+ * wins and restore is a no-op.
+ */
+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".
+  const initialTargetRef = useRef<number | null>(null);
+  // Guards so restore runs at most once per page mount.
+  const hasRestoredRef = useRef(false);
+  // Holds the in-flight restore poll timer so the cleanup can cancel it: without
+  // this, a fast SPA navigation away mid-poll would let the old page's poll fire
+  // window.scrollTo against the NEW page's document (visible wrong-page scroll).
+  const pollTimerRef = useRef<number | null>(null);
+
+  // Capture the previously-saved value synchronously during render, before the
+  // effect below registers handlers that would persist the current (0) scrollY.
+  if (initialTargetRef.current === null) {
+    const saved = readStorage(pageId);
+    // Store 0 when nothing is saved so the "already captured" check (!== null)
+    // holds; restore treats targetY <= 0 as a no-op anyway.
+    initialTargetRef.current = saved ?? 0;
+  }
+
+  useEffect(() => {
+    let throttleTimer: number | null = null;
+
+    const save = () => {
+      writeStorage(pageId, window.scrollY);
+    };
+
+    // Throttle the high-frequency scroll handler: persist immediately on the
+    // leading edge, then at most once per SAVE_THROTTLE_MS.
+    const onScroll = () => {
+      if (throttleTimer !== null) return;
+      save();
+      throttleTimer = window.setTimeout(() => {
+        throttleTimer = null;
+      }, SAVE_THROTTLE_MS);
+    };
+
+    // pagehide fires on reload/navigation (more reliable than unload); save now.
+    const onPageHide = () => {
+      save();
+    };
+
+    // Save when the tab is being backgrounded — covers mobile where pagehide is
+    // not always emitted.
+    const onVisibilityChange = () => {
+      if (document.visibilityState === "hidden") {
+        save();
+      }
+    };
+
+    window.addEventListener("scroll", onScroll, { passive: true });
+    window.addEventListener("pagehide", onPageHide);
+    document.addEventListener("visibilitychange", onVisibilityChange);
+
+    return () => {
+      window.removeEventListener("scroll", onScroll);
+      window.removeEventListener("pagehide", onPageHide);
+      document.removeEventListener("visibilitychange", onVisibilityChange);
+      if (throttleTimer !== null) {
+        window.clearTimeout(throttleTimer);
+        throttleTimer = null;
+      }
+      // Cancel any in-flight restore poll so it cannot scroll the next page.
+      if (pollTimerRef.current !== null) {
+        window.clearTimeout(pollTimerRef.current);
+        pollTimerRef.current = null;
+      }
+      // SPA navigation away from this page: persist the final position.
+      save();
+    };
+  }, [pageId]);
+
+  const restoreScrollPosition = useCallback(() => {
+    // Run at most once per page mount.
+    if (hasRestoredRef.current) return;
+    hasRestoredRef.current = true;
+
+    // Anchor priority: a `#hash` in the URL is handled by useEditorScroll.
+    if (window.location.hash) return;
+
+    const targetY = initialTargetRef.current ?? 0;
+    // Nothing meaningful to restore to.
+    if (targetY <= 0) return;
+
+    const start = Date.now();
+
+    const tryRestore = () => {
+      const maxScroll =
+        document.documentElement.scrollHeight - window.innerHeight;
+      const timedOut = Date.now() - start >= MAX_RESTORE_WAIT_MS;
+
+      // Restore once the content is tall enough to reach the target, or bail out
+      // after the timeout and scroll as far as currently possible.
+      if (maxScroll >= targetY || timedOut) {
+        window.scrollTo({
+          top: Math.min(targetY, Math.max(maxScroll, 0)),
+          behavior: "auto",
+        });
+        pollTimerRef.current = null;
+        return;
+      }
+
+      // Stored in a ref so the effect cleanup can cancel it on unmount.
+      pollTimerRef.current = window.setTimeout(tryRestore, RESTORE_POLL_MS);
+    };
+
+    tryRestore();
+  }, []);
+
+  return { restoreScrollPosition };
+}

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

@@ -77,6 +77,7 @@ import { PageEditMode } from "@/features/user/types/user.types.ts";
 import { jwtDecode } from "jwt-decode";
 import { searchSpotlight } from "@/features/search/constants.ts";
 import { useEditorScroll } from "./hooks/use-editor-scroll";
+import { useScrollPosition } from "./hooks/use-scroll-position";
 import { EditorLinkMenu } from "@/features/editor/components/link/link-menu";
 import ColumnsMenu from "@/features/editor/components/columns/columns-menu.tsx";
 import { TransclusionLookupProvider } from "@/features/editor/components/transclusion/transclusion-lookup-context";
@@ -141,6 +142,7 @@ export default function PageEditor({
     [isComponentMounted],
   );
   const { handleScrollTo } = useEditorScroll({ canScroll });
+  const { restoreScrollPosition } = useScrollPosition(pageId);
   // Providers only created once per pageId
   const providersRef = useRef<{
     local: IndexeddbPersistence;
@@ -479,6 +481,11 @@ export default function PageEditor({
     }
   }, [yjsConnectionStatus, isSynced]);
 
+  // Restore the saved reading position once the live content is laid out.
+  useEffect(() => {
+    if (!showStatic && editor) restoreScrollPosition();
+  }, [showStatic, editor, restoreScrollPosition]);
+
   return (
     <TransclusionLookupProvider>
       <PageEmbedLookupProvider>

apps/server/src/collaboration/extensions/persistence-store.spec.ts

@@ -422,4 +422,51 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
     expect(historyQueue.add).not.toHaveBeenCalled();
     expect(aiQueue.add).not.toHaveBeenCalled();
   });
+
+  // #260 — when the collab doc name carries a SLUGID (`page.<slugId>`) the
+  // post-store side effects must use the resolved page.id (a UUID), NOT the
+  // slugId. The transclusion sync + embedding reindex write uuid-typed columns,
+  // so a slugId there threw Postgres 22P02; the contributors key must also match
+  // the PAGE_HISTORY job, which is enqueued with page.id.
+  it('uses the canonical page.id (not the slugId doc name) for post-store side effects (#260)', async () => {
+    const SLUG = 'slug-1'; // persistedHumanPage.slugId; findById resolves it
+    const document = ydocFor(doc('NEW AGENT CONTENT'));
+    pageRepo.findById.mockResolvedValue(persistedHumanPage('NEW AGENT CONTENT'));
+    pageHistoryRepo.findPageLastHistory.mockResolvedValue(null);
+
+    // A `page.<slugId>` document name (the bug's smoking gun), agent store over
+    // a human page so the in-tx history-boundary read is also exercised.
+    await ext.onStoreDocument({
+      documentName: `page.${SLUG}`,
+      document,
+      context: { user: { id: USER_ID, name: 'Alice' }, actor: 'agent' },
+    } as any);
+
+    // findById was queried with the slugId (it resolves either id or slugId).
+    expect(pageRepo.findById).toHaveBeenCalledWith(SLUG, expect.anything());
+
+    // The in-tx history-boundary read uses the canonical UUID, never the slugId.
+    expect(pageHistoryRepo.findPageLastHistory).toHaveBeenCalledWith(
+      PAGE_ID,
+      expect.anything(),
+    );
+
+    // Transclusion sync (uuid-typed columns) must receive the UUID.
+    expect(transclusionService.syncPageTransclusions.mock.calls[0][0]).toBe(
+      PAGE_ID,
+    );
+    expect(transclusionService.syncPageReferences.mock.calls[0][0]).toBe(
+      PAGE_ID,
+    );
+    expect(
+      transclusionService.syncPageTemplateReferences.mock.calls[0][0],
+    ).toBe(PAGE_ID);
+
+    // Embedding reindex job keyed by the UUID (slugId there threw 22P02).
+    expect(aiQueue.add).toHaveBeenCalledTimes(1);
+    expect(aiQueue.add.mock.calls[0][1].pageIds).toEqual([PAGE_ID]);
+
+    // Contributors keyed by the UUID so they match the PAGE_HISTORY job (page.id).
+    expect(collabHistory.addContributors.mock.calls[0][0]).toBe(PAGE_ID);
+  });
 });

apps/server/src/collaboration/extensions/persistence.extension.ts

@@ -329,8 +329,10 @@ export class PersistenceExtension implements Extension {
             lastUpdatedSource === 'agent' &&
             page.lastUpdatedSource !== 'agent'
           ) {
+            // pageHistory.pageId is uuid-typed; use page.id (never the doc-name
+            // slugId) so a `page.<slugId>` doc cannot throw 22P02 here (#260).
             const lastHistory = await this.pageHistoryRepo.findPageLastHistory(
-              pageId,
+              page.id,
               { includeContent: true, trx },
             );
             const humanBaselineMissing =
@@ -398,11 +400,16 @@ export class PersistenceExtension implements Extension {
         }),
       );
 
-      await this.syncTransclusion(pageId, page.workspaceId, tiptapJson);
+      // Use the canonical page UUID (page.id), not the doc-name id, which may be
+      // a slugId for a `page.<slugId>` doc (#260). The transclusion/reference
+      // syncs write uuid-typed columns, so a slugId here threw Postgres 22P02.
+      await this.syncTransclusion(page.id, page.workspaceId, tiptapJson);
     }
 
     if (page) {
-      await this.collabHistory.addContributors(pageId, editingUserIds);
+      // Key contributors by the page UUID so they MATCH the PAGE_HISTORY job,
+      // which is enqueued with page.id and pops contributors by page.id (#260).
+      await this.collabHistory.addContributors(page.id, editingUserIds);
 
       const mentions = extractMentions(tiptapJson);
 
@@ -420,14 +427,17 @@ export class PersistenceExtension implements Extension {
             creatorId: m.creatorId,
           })),
           oldMentionedUserIds,
-          pageId,
+          // Canonical UUID, never the doc-name slugId (#260).
+          pageId: page.id,
           spaceId: page.spaceId,
           workspaceId: page.workspaceId,
         } as IPageMentionNotificationJob);
       }
 
       await this.aiQueue.add(QueueJob.PAGE_CONTENT_UPDATED, {
-        pageIds: [pageId],
+        // Canonical UUID: the embedding reindex resolves pages by uuid, so a
+        // slugId here threw Postgres 22P02 invalid-uuid (#260).
+        pageIds: [page.id],
         workspaceId: page.workspaceId,
       });
 

packages/mcp/build/client.js

@@ -37,6 +37,15 @@ const MIME_TO_EXT = {
     "image/webp": ".webp",
     "image/svg+xml": ".svg",
 };
+// Canonical UUID shape (versions 1–8, matching the `uuid` package's `validate`
+// that the server's isValidUUID uses). page.repo.ts treats any non-UUID pageId
+// as a slugId, so the MCP detects a UUID locally and skips a /pages/info
+// round-trip in resolvePageId. A 10-char nanoid slugId never contains dashes,
+// so it can never be misread as a UUID here.
+const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
+function isUuid(value) {
+    return typeof value === "string" && UUID_RE.test(value);
+}
 export class DocmostClient {
     client;
     token = null;
@@ -64,6 +73,11 @@ export class DocmostClient {
     // can all call login() at once. Memoizing a single promise collapses that
     // thundering herd into ONE /auth/login request that everyone awaits.
     loginPromise = null;
+    // Canonical-UUID cache for resolvePageId: maps an agent-supplied slugId to the
+    // page's canonical UUID, so repeated collab edits on the same page do not
+    // re-fetch /pages/info. A UUID input short-circuits before this cache (see
+    // resolvePageId), so only slugId->uuid entries are stored/read here.
+    pageIdCache = new Map();
     constructor(configOrBaseURL, email, password) {
         // Normalize the legacy positional form into the object union.
         const config = typeof configOrBaseURL === "string"
@@ -572,6 +586,35 @@ export class DocmostClient {
         const response = await this.client.post("/pages/info", { pageId });
         return response.data?.data ?? response.data;
     }
+    /**
+     * Resolve an agent-supplied pageId to the page's CANONICAL UUID (`page.id`),
+     * so every collaboration document the MCP opens is named `page.<uuid>` — the
+     * SAME name the web editor always uses (`page.${page.id}`).
+     *
+     * The agent commonly passes a 10-char public slugId (from URLs/listings) as
+     * the pageId. The web editor opens the collab doc by UUID, but the MCP used to
+     * pass that slugId straight into the collab doc name (`page.<slugId>`). For one
+     * DB row that produced TWO independent Yjs documents whose debounced stores
+     * clobbered each other — the agent's edit was silently lost (#260).
+     *
+     * A UUID input short-circuits with no network round-trip. A slugId is resolved
+     * once via getPageRaw and cached (both slugId->uuid and uuid->uuid), so
+     * repeated edits on the same page add no extra request.
+     */
+    async resolvePageId(pageId) {
+        if (isUuid(pageId))
+            return pageId;
+        const cached = this.pageIdCache.get(pageId);
+        if (cached)
+            return cached;
+        const data = await this.getPageRaw(pageId);
+        const uuid = data?.id;
+        if (typeof uuid !== "string" || !uuid) {
+            throw new Error(`Could not resolve a canonical page id for "${pageId}"`);
+        }
+        this.pageIdCache.set(pageId, uuid);
+        return uuid;
+    }
     async getPage(pageId) {
         await this.ensureAuthenticated();
         const resultData = await this.getPageRaw(pageId);
@@ -863,10 +906,12 @@ export class DocmostClient {
     async tableInsertRow(pageId, tableRef, cells, index) {
         await this.ensureAuthenticated();
         const collabToken = await this.getCollabTokenWithReauth();
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
         // Track insertion in an outer var, reset per-transform, so a collab retry
         // recomputes it cleanly (mirrors insertNode's pattern).
         let inserted = false;
-        const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => {
+        const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, (liveDoc) => {
             inserted = false;
             const { doc: nd, inserted: ins } = insertTableRow(liveDoc, tableRef, cells, index);
             inserted = ins;
@@ -892,8 +937,10 @@ export class DocmostClient {
     async tableDeleteRow(pageId, tableRef, index) {
         await this.ensureAuthenticated();
         const collabToken = await this.getCollabTokenWithReauth();
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
         let deleted = false;
-        const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => {
+        const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, (liveDoc) => {
             deleted = false;
             const { doc: nd, deleted: del } = deleteTableRow(liveDoc, tableRef, index);
             deleted = del;
@@ -921,8 +968,10 @@ export class DocmostClient {
     async tableUpdateCell(pageId, tableRef, row, col, text) {
         await this.ensureAuthenticated();
         const collabToken = await this.getCollabTokenWithReauth();
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
         let updated = false;
-        const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => {
+        const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, (liveDoc) => {
             updated = false;
             const { doc: nd, updated: upd } = updateTableCell(liveDoc, tableRef, row, col, text);
             updated = upd;
@@ -1034,6 +1083,10 @@ export class DocmostClient {
      */
     async updatePage(pageId, content, title) {
         await this.ensureAuthenticated();
+        // Open the collab doc by the canonical UUID, never the slugId (#260). The
+        // REST /pages/update title write below keeps the agent-supplied id (the
+        // server resolves a slugId there).
+        const pageUuid = await this.resolvePageId(pageId);
         // Write the BODY first, then the title (#159 split-brain). If the collab
         // body write fails (e.g. a persist timeout), the title must be left
         // UNTOUCHED so the page never ends up with a new title over its old body.
@@ -1043,7 +1096,7 @@ export class DocmostClient {
         let mutation;
         try {
             collabToken = await this.getCollabTokenWithReauth();
-            mutation = await updatePageContentRealtime(pageId, content, collabToken, this.apiUrl);
+            mutation = await updatePageContentRealtime(pageUuid, content, collabToken, this.apiUrl);
         }
         catch (error) {
             // Verbose diagnostics (incl. anything that could expose a token prefix)
@@ -1259,7 +1312,9 @@ export class DocmostClient {
         // Write the BODY first, then the title (#159 split-brain): a failed body
         // write (e.g. persist timeout) must not leave a new title over the old body.
         const collabToken = await this.getCollabTokenWithReauth();
-        const mutation = await this.replacePage(pageId, doc, collabToken, this.apiUrl);
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
+        const mutation = await this.replacePage(pageUuid, doc, collabToken, this.apiUrl);
         // Body persisted successfully — now it is safe to set the title.
         if (title) {
             await this.client.post("/pages/update", { pageId, title });
@@ -1294,8 +1349,10 @@ export class DocmostClient {
             throw new Error("insert_footnote: text is required");
         }
         const collabToken = await this.getCollabTokenWithReauth();
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
         let result = null;
-        const mutation = await this.mutatePage(pageId, collabToken, this.apiUrl, (liveDoc) => {
+        const mutation = await this.mutatePage(pageUuid, collabToken, this.apiUrl, (liveDoc) => {
             const r = insertInlineFootnote(liveDoc, { anchorText, text });
             if (!r.inserted) {
                 // Abort the page-locked write by throwing: mutatePageContent does not
@@ -1383,7 +1440,9 @@ export class DocmostClient {
         // PAGE import: canonicalize footnotes (see markdownToProseMirrorCanonical).
         const doc = await markdownToProseMirrorCanonical(body);
         const collabToken = await this.getCollabTokenWithReauth();
-        const mutation = await replacePageContent(pageId, doc, collabToken, this.apiUrl);
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
+        const mutation = await replacePageContent(pageUuid, doc, collabToken, this.apiUrl);
         // Collect distinct comment ids that actually became comment marks in the doc.
         const collectCommentIds = (node, acc) => {
             if (!node || typeof node !== "object")
@@ -1467,7 +1526,9 @@ export class DocmostClient {
         // to the target (parity with the other full-doc write paths).
         const canonical = canonicalizeFootnotes(content);
         const collabToken = await this.getCollabTokenWithReauth();
-        const mutation = await this.replacePage(targetPageId, canonical, collabToken, this.apiUrl);
+        // Open the TARGET collab doc by its canonical UUID, never the slugId (#260).
+        const targetUuid = await this.resolvePageId(targetPageId);
+        const mutation = await this.replacePage(targetUuid, canonical, collabToken, this.apiUrl);
         return {
             success: true,
             sourcePageId,
@@ -1483,6 +1544,8 @@ export class DocmostClient {
     async editPageText(pageId, edits) {
         await this.ensureAuthenticated();
         const collabToken = await this.getCollabTokenWithReauth();
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
         // Apply the edits against the LIVE synced document, not the debounced REST
         // snapshot, so concurrent human edits/comments are preserved. applyTextEdits
         // records per-edit match problems in `failed` instead of throwing, and
@@ -1495,7 +1558,7 @@ export class DocmostClient {
         // we must NOT write (no spurious history version) and must not claim a write
         // happened.
         let wrote = false;
-        const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => {
+        const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, (liveDoc) => {
             wrote = false;
             const r = applyTextEdits(liveDoc, edits);
             results = r.results;
@@ -1580,10 +1643,12 @@ export class DocmostClient {
             target.attrs.id = nodeId;
         }
         const collabToken = await this.getCollabTokenWithReauth();
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
         // Track the replacement count in an outer var, reset per-transform, so a
         // collab retry recomputes it cleanly (mirrors replaceImage's pattern).
         let replaced = 0;
-        const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => {
+        const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, (liveDoc) => {
             replaced = 0;
             const { doc: nd, replaced: r } = replaceNodeById(liveDoc, nodeId, target);
             replaced = r;
@@ -1636,10 +1701,12 @@ export class DocmostClient {
             }
         }
         const collabToken = await this.getCollabTokenWithReauth();
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
         // Track insertion in an outer var, reset per-transform, so a collab retry
         // recomputes it cleanly (mirrors replaceImage's pattern).
         let inserted = false;
-        const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => {
+        const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, (liveDoc) => {
             inserted = false;
             const { doc: nd, inserted: ins } = insertNodeRelative(liveDoc, node, opts);
             inserted = ins;
@@ -1675,10 +1742,12 @@ export class DocmostClient {
     async deleteNode(pageId, nodeId) {
         await this.ensureAuthenticated();
         const collabToken = await this.getCollabTokenWithReauth();
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
         // Track the deletion count in an outer var, reset per-transform, so a
         // collab retry recomputes it cleanly (mirrors replaceImage's pattern).
         let deleted = 0;
-        const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => {
+        const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, (liveDoc) => {
             deleted = 0;
             const { doc: nd, deleted: d } = deleteNodeById(liveDoc, nodeId);
             deleted = d;
@@ -1921,7 +1990,10 @@ export class DocmostClient {
         let anchored = false;
         try {
             const collabToken = await this.getCollabTokenWithReauth();
-            const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => {
+            // Open the collab doc by the canonical UUID, never the slugId (#260). The
+            // /comments/create REST call above keeps the agent-supplied id.
+            const pageUuid = await this.resolvePageId(pageId);
+            const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, (liveDoc) => {
                 const doc = liveDoc && liveDoc.type === "doc"
                     ? liveDoc
                     : { type: "doc", content: [] };
@@ -2324,6 +2396,9 @@ export class DocmostClient {
         if (opts.alt)
             node.attrs.alt = opts.alt;
         const collabToken = await this.getCollabTokenWithReauth();
+        // Open the collab doc by the canonical UUID, never the slugId (#260). The
+        // uploadImage /files/upload call above keeps the agent-supplied id.
+        const pageUuid = await this.resolvePageId(pageId);
         // Recursively collect the plain text of a top-level block.
         const blockText = (n) => {
             let out = "";
@@ -2337,7 +2412,7 @@ export class DocmostClient {
         // concurrent edits/comments/images are preserved and parallel insert_image
         // calls (serialized by the per-page lock) each see the previous insertion.
         let placement;
-        const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, (liveDoc) => {
+        const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, (liveDoc) => {
             const doc = liveDoc && liveDoc.type === "doc"
                 ? liveDoc
                 : { type: "doc", content: [] };
@@ -2424,6 +2499,13 @@ export class DocmostClient {
      */
     async replaceImage(pageId, oldAttachmentId, url, opts = {}) {
         const collabToken = await this.getCollabTokenWithReauth();
+        // Open the collab doc by the canonical UUID, never the slugId (#260). The
+        // page lock must ALSO key on the UUID so this operation serializes against
+        // other writes to the same page (mutatePageContent now locks by the resolved
+        // UUID too); locking by the raw slugId here would desync the mutex key and
+        // reopen the TOCTOU/orphan-attachment window the lock closes. uploadImage
+        // keeps the agent-supplied id (it hits REST, not the collab doc).
+        const pageUuid = await this.resolvePageId(pageId);
         // Hold ONE per-page lock for the WHOLE operation (scan -> upload -> write).
         // Previously the scan and the write were two separate mutatePageContent
         // calls, each acquiring + releasing the lock, with the upload happening in
@@ -2435,7 +2517,7 @@ export class DocmostClient {
         // reentrant, so the self-locking mutatePageContent would deadlock here)
         // closes that TOCTOU window. uploadImage hits /files/upload over plain HTTP
         // and does not touch the page lock, so it is safe to call while held.
-        return withPageLock(pageId, async () => {
+        return withPageLock(pageUuid, async () => {
             // STEP 1: read-only live check. Scan the live document for any image node
             // matching oldAttachmentId BEFORE uploading anything, so a wrong/stale id
             // throws without ever creating an orphan attachment.
@@ -2453,7 +2535,7 @@ export class DocmostClient {
                         scan(node.content);
                 }
             };
-            await this.mutateLiveContentUnlocked(pageId, collabToken, (liveDoc) => {
+            await this.mutateLiveContentUnlocked(pageUuid, collabToken, (liveDoc) => {
                 matchFound = false; // reset per-transform (collab may retry the read).
                 const doc = liveDoc && liveDoc.type === "doc"
                     ? liveDoc
@@ -2501,7 +2583,7 @@ export class DocmostClient {
                         walk(node.content);
                 }
             };
-            const mutation = await this.mutateLiveContentUnlocked(pageId, collabToken, (liveDoc) => {
+            const mutation = await this.mutateLiveContentUnlocked(pageUuid, collabToken, (liveDoc) => {
                 // Reset per-transform so collab retries recompute cleanly (no double-count).
                 replaced = 0;
                 const doc = liveDoc && liveDoc.type === "doc"
@@ -2598,7 +2680,10 @@ export class DocmostClient {
         // JSON write path) before writing it back.
         this.validateDocUrls(version.content);
         const collabToken = await this.getCollabTokenWithReauth();
-        const mutation = await mutatePageContent(version.pageId, collabToken, this.apiUrl, () => version.content);
+        // version.pageId is the page entity id (already a UUID); resolvePageId
+        // short-circuits a UUID with no round-trip, so this is defensive only (#260).
+        const pageUuid = await this.resolvePageId(version.pageId);
+        const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, () => version.content);
         return {
             pageId: version.pageId,
             restoredFrom: historyId,
@@ -2767,7 +2852,9 @@ export class DocmostClient {
         }
         // Apply atomically against the live doc.
         const collabToken = await this.getCollabTokenWithReauth();
-        const mutation = await mutatePageContent(pageId, collabToken, this.apiUrl, runTransform);
+        // Open the collab doc by the canonical UUID, never the slugId (#260).
+        const pageUuid = await this.resolvePageId(pageId);
+        const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, runTransform);
         // Optionally delete consumed comments (best-effort; a delete failure must
         // not undo the successful write).
         const deletedComments = [];

packages/mcp/src/client.ts

@@ -133,6 +133,18 @@ export type DocmostMcpConfig = { apiUrl: string } & (
     };
   };
 
+// Canonical UUID shape (versions 1–8, matching the `uuid` package's `validate`
+// that the server's isValidUUID uses). page.repo.ts treats any non-UUID pageId
+// as a slugId, so the MCP detects a UUID locally and skips a /pages/info
+// round-trip in resolvePageId. A 10-char nanoid slugId never contains dashes,
+// so it can never be misread as a UUID here.
+const UUID_RE =
+  /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
+
+function isUuid(value: string): boolean {
+  return typeof value === "string" && UUID_RE.test(value);
+}
+
 export class DocmostClient {
   private client: AxiosInstance;
   private token: string | null = null;
@@ -160,6 +172,11 @@ export class DocmostClient {
   // can all call login() at once. Memoizing a single promise collapses that
   // thundering herd into ONE /auth/login request that everyone awaits.
   private loginPromise: Promise<void> | null = null;
+  // Canonical-UUID cache for resolvePageId: maps an agent-supplied slugId to the
+  // page's canonical UUID, so repeated collab edits on the same page do not
+  // re-fetch /pages/info. A UUID input short-circuits before this cache (see
+  // resolvePageId), so only slugId->uuid entries are stored/read here.
+  private pageIdCache = new Map<string, string>();
 
   // Two construction forms:
   //  - new DocmostClient(config)                  // discriminated union (current)
@@ -751,6 +768,36 @@ export class DocmostClient {
     return response.data?.data ?? response.data;
   }
 
+  /**
+   * Resolve an agent-supplied pageId to the page's CANONICAL UUID (`page.id`),
+   * so every collaboration document the MCP opens is named `page.<uuid>` — the
+   * SAME name the web editor always uses (`page.${page.id}`).
+   *
+   * The agent commonly passes a 10-char public slugId (from URLs/listings) as
+   * the pageId. The web editor opens the collab doc by UUID, but the MCP used to
+   * pass that slugId straight into the collab doc name (`page.<slugId>`). For one
+   * DB row that produced TWO independent Yjs documents whose debounced stores
+   * clobbered each other — the agent's edit was silently lost (#260).
+   *
+   * A UUID input short-circuits with no network round-trip. A slugId is resolved
+   * once via getPageRaw and cached (both slugId->uuid and uuid->uuid), so
+   * repeated edits on the same page add no extra request.
+   */
+  private async resolvePageId(pageId: string): Promise<string> {
+    if (isUuid(pageId)) return pageId;
+    const cached = this.pageIdCache.get(pageId);
+    if (cached) return cached;
+    const data = await this.getPageRaw(pageId);
+    const uuid = data?.id;
+    if (typeof uuid !== "string" || !uuid) {
+      throw new Error(
+        `Could not resolve a canonical page id for "${pageId}"`,
+      );
+    }
+    this.pageIdCache.set(pageId, uuid);
+    return uuid;
+  }
+
   async getPage(pageId: string) {
     await this.ensureAuthenticated();
     const resultData = await this.getPageRaw(pageId);
@@ -1083,12 +1130,14 @@ export class DocmostClient {
   ) {
     await this.ensureAuthenticated();
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
 
     // Track insertion in an outer var, reset per-transform, so a collab retry
     // recomputes it cleanly (mirrors insertNode's pattern).
     let inserted = false;
     const mutation = await mutatePageContent(
-      pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       (liveDoc) => {
@@ -1126,10 +1175,12 @@ export class DocmostClient {
   async tableDeleteRow(pageId: string, tableRef: string, index: number) {
     await this.ensureAuthenticated();
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
 
     let deleted = false;
     const mutation = await mutatePageContent(
-      pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       (liveDoc) => {
@@ -1174,10 +1225,12 @@ export class DocmostClient {
   ) {
     await this.ensureAuthenticated();
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
 
     let updated = false;
     const mutation = await mutatePageContent(
-      pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       (liveDoc) => {
@@ -1313,6 +1366,10 @@ export class DocmostClient {
    */
   async updatePage(pageId: string, content: string, title?: string) {
     await this.ensureAuthenticated();
+    // Open the collab doc by the canonical UUID, never the slugId (#260). The
+    // REST /pages/update title write below keeps the agent-supplied id (the
+    // server resolves a slugId there).
+    const pageUuid = await this.resolvePageId(pageId);
 
     // Write the BODY first, then the title (#159 split-brain). If the collab
     // body write fails (e.g. a persist timeout), the title must be left
@@ -1324,7 +1381,7 @@ export class DocmostClient {
     try {
       collabToken = await this.getCollabTokenWithReauth();
       mutation = await updatePageContentRealtime(
-        pageId,
+        pageUuid,
         content,
         collabToken,
         this.apiUrl,
@@ -1587,8 +1644,10 @@ export class DocmostClient {
     // Write the BODY first, then the title (#159 split-brain): a failed body
     // write (e.g. persist timeout) must not leave a new title over the old body.
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
     const mutation = await this.replacePage(
-      pageId,
+      pageUuid,
       doc,
       collabToken,
       this.apiUrl,
@@ -1630,9 +1689,11 @@ export class DocmostClient {
       throw new Error("insert_footnote: text is required");
     }
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
     let result: { footnoteId: string; reused: boolean } | null = null;
     const mutation = await this.mutatePage(
-      pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       (liveDoc: any) => {
@@ -1740,8 +1801,10 @@ export class DocmostClient {
     // PAGE import: canonicalize footnotes (see markdownToProseMirrorCanonical).
     const doc = await markdownToProseMirrorCanonical(body);
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
     const mutation = await replacePageContent(
-      pageId,
+      pageUuid,
       doc,
       collabToken,
       this.apiUrl,
@@ -1840,8 +1903,10 @@ export class DocmostClient {
     const canonical = canonicalizeFootnotes(content);
 
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the TARGET collab doc by its canonical UUID, never the slugId (#260).
+    const targetUuid = await this.resolvePageId(targetPageId);
     const mutation = await this.replacePage(
-      targetPageId,
+      targetUuid,
       canonical,
       collabToken,
       this.apiUrl,
@@ -1864,6 +1929,8 @@ export class DocmostClient {
     await this.ensureAuthenticated();
 
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
 
     // Apply the edits against the LIVE synced document, not the debounced REST
     // snapshot, so concurrent human edits/comments are preserved. applyTextEdits
@@ -1878,7 +1945,7 @@ export class DocmostClient {
     // happened.
     let wrote = false;
     const mutation = await mutatePageContent(
-      pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       (liveDoc) => {
@@ -1978,12 +2045,14 @@ export class DocmostClient {
     }
 
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
 
     // Track the replacement count in an outer var, reset per-transform, so a
     // collab retry recomputes it cleanly (mirrors replaceImage's pattern).
     let replaced = 0;
     const mutation = await mutatePageContent(
-      pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       (liveDoc) => {
@@ -2066,12 +2135,14 @@ export class DocmostClient {
     }
 
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
 
     // Track insertion in an outer var, reset per-transform, so a collab retry
     // recomputes it cleanly (mirrors replaceImage's pattern).
     let inserted = false;
     const mutation = await mutatePageContent(
-      pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       (liveDoc) => {
@@ -2120,12 +2191,14 @@ export class DocmostClient {
     await this.ensureAuthenticated();
 
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
 
     // Track the deletion count in an outer var, reset per-transform, so a
     // collab retry recomputes it cleanly (mirrors replaceImage's pattern).
     let deleted = 0;
     const mutation = await mutatePageContent(
-      pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       (liveDoc) => {
@@ -2414,8 +2487,11 @@ export class DocmostClient {
     let anchored = false;
     try {
       const collabToken = await this.getCollabTokenWithReauth();
+      // Open the collab doc by the canonical UUID, never the slugId (#260). The
+      // /comments/create REST call above keeps the agent-supplied id.
+      const pageUuid = await this.resolvePageId(pageId);
       const mutation = await mutatePageContent(
-        pageId,
+        pageUuid,
         collabToken,
         this.apiUrl,
         (liveDoc) => {
@@ -2893,6 +2969,9 @@ export class DocmostClient {
     if (opts.alt) node.attrs.alt = opts.alt;
 
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260). The
+    // uploadImage /files/upload call above keeps the agent-supplied id.
+    const pageUuid = await this.resolvePageId(pageId);
 
     // Recursively collect the plain text of a top-level block.
     const blockText = (n: any): string => {
@@ -2907,7 +2986,7 @@ export class DocmostClient {
     // calls (serialized by the per-page lock) each see the previous insertion.
     let placement: "replaced" | "after" | "appended" | undefined;
     const mutation = await mutatePageContent(
-      pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       (liveDoc) => {
@@ -3019,6 +3098,13 @@ export class DocmostClient {
     opts: { align?: "left" | "center" | "right"; alt?: string } = {},
   ) {
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260). The
+    // page lock must ALSO key on the UUID so this operation serializes against
+    // other writes to the same page (mutatePageContent now locks by the resolved
+    // UUID too); locking by the raw slugId here would desync the mutex key and
+    // reopen the TOCTOU/orphan-attachment window the lock closes. uploadImage
+    // keeps the agent-supplied id (it hits REST, not the collab doc).
+    const pageUuid = await this.resolvePageId(pageId);
 
     // Hold ONE per-page lock for the WHOLE operation (scan -> upload -> write).
     // Previously the scan and the write were two separate mutatePageContent
@@ -3031,7 +3117,7 @@ export class DocmostClient {
     // reentrant, so the self-locking mutatePageContent would deadlock here)
     // closes that TOCTOU window. uploadImage hits /files/upload over plain HTTP
     // and does not touch the page lock, so it is safe to call while held.
-    return withPageLock(pageId, async () => {
+    return withPageLock(pageUuid, async () => {
       // STEP 1: read-only live check. Scan the live document for any image node
       // matching oldAttachmentId BEFORE uploading anything, so a wrong/stale id
       // throws without ever creating an orphan attachment.
@@ -3050,7 +3136,7 @@ export class DocmostClient {
         }
       };
 
-      await this.mutateLiveContentUnlocked(pageId, collabToken, (liveDoc) => {
+      await this.mutateLiveContentUnlocked(pageUuid, collabToken, (liveDoc) => {
         matchFound = false; // reset per-transform (collab may retry the read).
         const doc =
           liveDoc && liveDoc.type === "doc"
@@ -3105,7 +3191,7 @@ export class DocmostClient {
       };
 
       const mutation = await this.mutateLiveContentUnlocked(
-        pageId,
+        pageUuid,
         collabToken,
         (liveDoc) => {
           // Reset per-transform so collab retries recompute cleanly (no double-count).
@@ -3214,8 +3300,11 @@ export class DocmostClient {
     // JSON write path) before writing it back.
     this.validateDocUrls(version.content);
     const collabToken = await this.getCollabTokenWithReauth();
+    // version.pageId is the page entity id (already a UUID); resolvePageId
+    // short-circuits a UUID with no round-trip, so this is defensive only (#260).
+    const pageUuid = await this.resolvePageId(version.pageId);
     const mutation = await mutatePageContent(
-      version.pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       () => version.content,
@@ -3414,8 +3503,10 @@ export class DocmostClient {
 
     // Apply atomically against the live doc.
     const collabToken = await this.getCollabTokenWithReauth();
+    // Open the collab doc by the canonical UUID, never the slugId (#260).
+    const pageUuid = await this.resolvePageId(pageId);
     const mutation = await mutatePageContent(
-      pageId,
+      pageUuid,
       collabToken,
       this.apiUrl,
       runTransform,

packages/mcp/test/mock/ambiguous-node-id.test.mjs

@@ -132,7 +132,7 @@ test("patch_node REFUSES an ambiguous (duplicate) id without writing to collab",
 
   await assert.rejects(
     () =>
-      client.patchNode("page-1", DUP_ID, {
+      client.patchNode("11111111-1111-4111-8111-111111111111", DUP_ID, {
         type: "paragraph",
         content: [{ type: "text", text: "replacement" }],
       }),
@@ -152,7 +152,7 @@ test("delete_node REFUSES an ambiguous (duplicate) id without writing to collab"
   const client = new DocmostClient(baseURL, "user@example.com", "pw");
 
   await assert.rejects(
-    () => client.deleteNode("page-2", DUP_ID),
+    () => client.deleteNode("22222222-2222-4222-8222-222222222222", DUP_ID),
     /ambiguous/i,
     "delete_node must reject a duplicate-id target with an 'ambiguous' error",
   );

packages/mcp/test/mock/insert-footnote-wrapper.test.mjs

@@ -37,6 +37,11 @@ function makeClient(liveDoc) {
     async getCollabTokenWithReauth() {
       return "collab-token";
     }
+    // Identity resolution: this test isolates the footnote wrapper, so the
+    // slugId->uuid resolution (#260) is stubbed to a no-op and "p1" stays "p1".
+    async resolvePageId(pageId) {
+      return pageId;
+    }
     async mutatePage(pageId, token, apiUrl, transform) {
       calls.pageId = pageId;
       calls.token = token;

packages/mcp/test/mock/resolve-page-id-collab-doc-name.test.mjs

@@ -0,0 +1,387 @@
+// Mock collab regression for the #260 data-loss bug: the MCP must open every
+// collaboration document by the page's CANONICAL UUID (`page.<uuid>`) — the same
+// name the web editor uses — even when the agent supplies a public slugId.
+//
+// Root cause: the agent commonly passes a 10-char slugId (from URLs/listings) as
+// pageId. The web tab opens `page.<uuid>`, but the MCP used to pass the slugId
+// straight into the collab doc name (`page.<slugId>`), so one DB page ended up
+// with TWO independent Yjs documents whose debounced stores clobbered each other
+// — the agent's edit was silently lost on reload.
+//
+// We stand up a real Hocuspocus server (like ambiguous-node-id.test.mjs) and
+// capture the EXACT documentName each connection requests via onLoadDocument.
+// The /pages/info mock resolves the slugId -> uuid, and counts its own hits so we
+// can also prove the UUID short-circuit + cache (no redundant resolve round-trip).
+import { test, after } from "node:test";
+import assert from "node:assert/strict";
+import http from "node:http";
+import { WebSocketServer } from "ws";
+import { Hocuspocus } from "@hocuspocus/server";
+import { DocmostClient } from "../../build/client.js";
+import { buildYDoc } from "../../build/lib/collaboration.js";
+// Import the SAME page-lock module instance that build/client.js imports. ESM
+// caches modules by resolved URL, so this `withPageLock` shares the very
+// per-page mutex map (`chains`) the client uses — letting the replaceImage test
+// probe which key the operation actually locks on (see that test for details).
+import { withPageLock } from "../../build/lib/page-lock.js";
+
+const SLUG = "dwzDdgPep2"; // 10-char nanoid public id (no dashes)
+const UUID = "11111111-1111-4111-8111-111111111111"; // canonical page.id
+
+// A simple one-paragraph document; "hello world" gives editPageText a match and
+// insertFootnote an anchor. No table node, so tableInsertRow aborts with
+// "no table found" — but the collab doc was still OPENED by then, which is what
+// we assert (the doc NAME is fixed at connect time, before any transform runs).
+function seedDoc() {
+  return {
+    type: "doc",
+    content: [
+      {
+        type: "paragraph",
+        attrs: { id: "p1" },
+        content: [{ type: "text", text: "hello world" }],
+      },
+    ],
+  };
+}
+
+// Same shape as seedDoc but with one image node carrying attachmentId "att-old"
+// (mirrors what client.addImage emits). replaceImage scans the live doc for this
+// node, so it must survive the Yjs round-trip with attachmentId intact.
+function seedDocWithImage() {
+  return {
+    type: "doc",
+    content: [
+      {
+        type: "paragraph",
+        attrs: { id: "p1" },
+        content: [{ type: "text", text: "hello world" }],
+      },
+      {
+        type: "image",
+        attrs: {
+          src: "/api/files/att-old/old.png",
+          attachmentId: "att-old",
+          size: 10,
+          align: "center",
+          width: null,
+        },
+      },
+    ],
+  };
+}
+
+function readBody(req) {
+  return new Promise((resolve) => {
+    let raw = "";
+    req.on("data", (c) => (raw += c));
+    req.on("end", () => resolve(raw));
+  });
+}
+
+// Stand up an HTTP server that authenticates, hands out a collab token, serves
+// /pages/info (slugId -> uuid resolution), and upgrades /collab to a Hocuspocus
+// instance whose onLoadDocument records the requested documentName.
+// opts.seed: a function returning the ProseMirror doc the collab server loads
+// (defaults to seedDoc). opts.onUpload: an optional async hook invoked when
+// /files/upload is hit, letting a test GATE the upload (hold replaceImage inside
+// its page lock). Existing callers pass no opts and are unaffected.
+async function spawnCollabStack(opts = {}) {
+  const seed = opts.seed ?? seedDoc;
+  const state = { docNames: [], pagesInfoCalls: [] };
+
+  const hocuspocus = new Hocuspocus({
+    quiet: true,
+    async onLoadDocument({ documentName }) {
+      state.docNames.push(documentName);
+      return buildYDoc(seed());
+    },
+  });
+
+  const wss = new WebSocketServer({ noServer: true });
+
+  const server = http.createServer(async (req, res) => {
+    const raw = await readBody(req);
+    if (req.url === "/api/auth/login") {
+      res.writeHead(200, {
+        "Content-Type": "application/json",
+        "Set-Cookie": "authToken=t; Path=/; HttpOnly",
+      });
+      res.end(JSON.stringify({ success: true }));
+      return;
+    }
+    if (req.url === "/api/auth/collab-token") {
+      res.writeHead(200, { "Content-Type": "application/json" });
+      res.end(JSON.stringify({ data: { token: "collab-jwt" } }));
+      return;
+    }
+    if (req.url === "/api/pages/info") {
+      let pageId;
+      try {
+        pageId = JSON.parse(raw)?.pageId;
+      } catch {
+        pageId = undefined;
+      }
+      state.pagesInfoCalls.push(pageId);
+      // Always resolve to the SAME canonical record, mirroring the server's
+      // findById (which accepts either the uuid or the slugId).
+      res.writeHead(200, { "Content-Type": "application/json" });
+      res.end(
+        JSON.stringify({
+          data: {
+            id: UUID,
+            slugId: SLUG,
+            title: "Doc",
+            spaceId: "space-1",
+            content: seedDoc(),
+          },
+        }),
+      );
+      return;
+    }
+    if (req.url && req.url.endsWith(".png")) {
+      // Serve image bytes for fetchRemoteImage (replaceImage downloads the new
+      // image before uploading it). Any non-empty image/* body is enough;
+      // fetchRemoteImage does not validate PNG magic bytes.
+      res.writeHead(200, { "Content-Type": "image/png" });
+      res.end(Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]));
+      return;
+    }
+    if (req.url === "/api/files/upload") {
+      // Optional gate: a test can hold replaceImage parked here (inside its page
+      // lock, after the scan) to probe the lock key. Default: respond at once.
+      if (opts.onUpload) await opts.onUpload();
+      res.writeHead(200, { "Content-Type": "application/json" });
+      res.end(
+        JSON.stringify({
+          data: { id: "att-new", fileName: "replacement.png", fileSize: 8 },
+        }),
+      );
+      return;
+    }
+    // Title writes (/pages/update) and anything else: succeed quietly.
+    res.writeHead(200, { "Content-Type": "application/json" });
+    res.end(JSON.stringify({ data: {} }));
+  });
+
+  // buildCollabWsUrl maps http://host:port/api -> ws://host:port/collab.
+  server.on("upgrade", (request, socket, head) => {
+    if (!request.url || !request.url.startsWith("/collab")) {
+      socket.destroy();
+      return;
+    }
+    wss.handleUpgrade(request, socket, head, (ws) => {
+      hocuspocus.handleConnection(ws, request);
+    });
+  });
+
+  const baseURL = await new Promise((resolve) => {
+    server.listen(0, "127.0.0.1", () => {
+      const { port } = server.address();
+      resolve(`http://127.0.0.1:${port}/api`);
+    });
+  });
+
+  openStacks.push({ server, hocuspocus });
+  return { state, baseURL };
+}
+
+const openStacks = [];
+after(async () => {
+  await Promise.all(
+    openStacks.map(
+      ({ server, hocuspocus }) =>
+        new Promise((resolve) => {
+          server.close(() => {
+            Promise.resolve(hocuspocus.destroy?.()).finally(resolve);
+          });
+        }),
+    ),
+  );
+});
+
+test("editPageText with a slugId opens the collab doc by the resolved UUID (#260)", async () => {
+  const { state, baseURL } = await spawnCollabStack();
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  const res = await client.editPageText(SLUG, [
+    { find: "hello", replace: "hi" },
+  ]);
+  assert.equal(res.success, true);
+
+  assert.ok(
+    state.docNames.includes(`page.${UUID}`),
+    `collab doc must be opened as page.${UUID}, got ${JSON.stringify(state.docNames)}`,
+  );
+  assert.ok(
+    !state.docNames.includes(`page.${SLUG}`),
+    "collab doc must NEVER be opened by the slugId (that is the data-loss bug)",
+  );
+  // The slugId had to be resolved via /pages/info at least once.
+  assert.ok(state.pagesInfoCalls.length >= 1);
+});
+
+test("tableInsertRow with a slugId opens the collab doc by the resolved UUID (#260)", async () => {
+  const { state, baseURL } = await spawnCollabStack();
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  // No table in the seed doc, so this aborts with "no table found" — but the
+  // collab doc has ALREADY been opened (by UUID) before the transform decides.
+  await assert.rejects(
+    () => client.tableInsertRow(SLUG, "#0", ["a", "b"]),
+    /no table/i,
+  );
+
+  assert.deepEqual(
+    state.docNames,
+    [`page.${UUID}`],
+    "tableInsertRow must open the collab doc by the resolved UUID",
+  );
+});
+
+test("the generic mutate (insert_footnote) with a slugId opens by the resolved UUID (#260)", async () => {
+  const { state, baseURL } = await spawnCollabStack();
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  const res = await client.insertFootnote(SLUG, "world", "a note");
+  assert.equal(res.success, true);
+
+  assert.deepEqual(
+    state.docNames,
+    [`page.${UUID}`],
+    "insert_footnote (via the mutatePage seam) must open the collab doc by UUID",
+  );
+});
+
+test("a UUID input is passed through unchanged and triggers NO /pages/info fetch (short-circuit)", async () => {
+  const { state, baseURL } = await spawnCollabStack();
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  const res = await client.editPageText(UUID, [
+    { find: "hello", replace: "hi" },
+  ]);
+  assert.equal(res.success, true);
+
+  assert.deepEqual(state.docNames, [`page.${UUID}`]);
+  assert.equal(
+    state.pagesInfoCalls.length,
+    0,
+    "a UUID input must short-circuit resolvePageId with no /pages/info round-trip",
+  );
+});
+
+test("a repeated slugId edit resolves the UUID only once (cache)", async () => {
+  const { state, baseURL } = await spawnCollabStack();
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  // Each mock connection re-seeds a fresh "hello world" doc (the mock does not
+  // persist across connects), so both edits target "hello". The cache assertion
+  // only concerns the slugId->uuid resolution, not the document content.
+  await client.editPageText(SLUG, [{ find: "hello", replace: "hi" }]);
+  await client.editPageText(SLUG, [{ find: "hello", replace: "hey" }]);
+
+  assert.deepEqual(state.docNames, [`page.${UUID}`, `page.${UUID}`]);
+  assert.equal(
+    state.pagesInfoCalls.length,
+    1,
+    "the slugId->uuid resolution must be cached across edits on the same page",
+  );
+});
+
+// PR#265 reviewer finding F1. replaceImage is the one path where the resolved
+// UUID gates BOTH (a) the collab-doc OPEN (mutateLiveContentUnlocked ->
+// page.<uuid>) AND (b) the per-page mutex key withPageLock(uuid). The lock
+// serializes the whole scan -> upload -> write against other writes to the same
+// page (which now also lock by the resolved UUID), closing a TOCTOU/orphan-
+// attachment window. A regression that re-keys this lock by the raw slugId would
+// desync it from mutatePageContent's UUID key and silently reopen that window.
+// This test pins both invariants and FAILS under either regression:
+//   - open by slugId  -> assertion (a) sees page.<slug> in docNames;
+//   - lock by slugId   -> assertion (b)'s UUID-keyed probe is no longer blocked.
+test("replaceImage opens by the resolved UUID AND keys its page lock by that UUID, not the slugId (#260 / PR#265 F1)", async () => {
+  // A gate that holds the /files/upload response open, so replaceImage parks
+  // INSIDE its page lock (after the read-only scan, mid-upload) until released.
+  let releaseUpload;
+  const uploadReleased = new Promise((r) => (releaseUpload = r));
+  let uploadHit;
+  const uploadStarted = new Promise((r) => (uploadHit = r));
+
+  const { state, baseURL } = await spawnCollabStack({
+    seed: seedDocWithImage,
+    onUpload: async () => {
+      uploadHit(); // replaceImage is now holding its page lock...
+      await uploadReleased; // ...and stays parked until the test releases it.
+    },
+  });
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+
+  // Kick off the replace but DO NOT await: it resolves SLUG->UUID, takes
+  // withPageLock(UUID), scan-opens page.<UUID>, finds the seeded "att-old"
+  // image, then blocks in uploadImage on our gate while still holding the lock.
+  // The image URL is served as image/png by the mock (the ".png" route above).
+  const imageUrl = `${baseURL}/x.png`;
+  const replacePromise = client.replaceImage(SLUG, "att-old", imageUrl);
+
+  await uploadStarted; // deterministic: replaceImage now holds its page lock.
+
+  // (a) OPEN BY UUID: the only collab doc opened so far (the scan pass) used the
+  // canonical UUID, never the slugId. (The write pass opens a second time after
+  // we release the gate; asserted at the end.)
+  assert.deepEqual(
+    state.docNames,
+    [`page.${UUID}`],
+    "replaceImage must scan-open the collab doc by the resolved UUID, never the slugId",
+  );
+
+  // (b) LOCK KEY == UUID (the distinct invariant). We share the SAME page-lock
+  // module instance as build/client.js, so enqueuing on key=UUID contends on the
+  // very chain replaceImage holds. Because replaceImage is deterministically
+  // parked mid-upload (still holding the lock), a UUID-keyed probe MUST stay
+  // queued; it cannot run until the lock frees. The contention here is pure
+  // in-memory promise-chain microtask scheduling (no timers, no socket I/O), so
+  // a single macrotask flush is a sufficient and deterministic observation.
+  // If replaceImage were reverted to lock by the slugId, the UUID chain would be
+  // free and this probe would run during the flush -> probeRan === true -> FAIL.
+  let probeRan = false;
+  const probeDone = withPageLock(UUID, async () => {
+    probeRan = true;
+  });
+  // setImmediate runs after the microtask queue fully drains, so a probe on a
+  // FREE chain would already have run by the time this resolves.
+  await new Promise((r) => setImmediate(r));
+  assert.equal(
+    probeRan,
+    false,
+    "a probe on key=UUID must stay blocked while replaceImage holds the lock; " +
+      "if it ran, replaceImage locked by a different key (e.g. the raw slugId)",
+  );
+
+  // Non-vacuity guard: a probe on an UNRELATED key DOES run after the same
+  // single flush. This proves the flush actually executes queued callbacks, so
+  // probeRan === false above means "blocked", not "the flush never ran anyone".
+  let freeRan = false;
+  const freeDone = withPageLock(`page.free-${UUID}`, async () => {
+    freeRan = true;
+  });
+  await new Promise((r) => setImmediate(r));
+  assert.equal(
+    freeRan,
+    true,
+    "sanity: a probe on a FREE key must run after one flush (the UUID probe was blocked by the held key, not by an inert flush)",
+  );
+
+  // Release the gate; replaceImage finishes and the queued UUID probe can run.
+  releaseUpload();
+  const res = await replacePromise;
+  await probeDone;
+  await freeDone;
+
+  assert.equal(res.success, true);
+  assert.equal(res.replaced, 1, "the one seeded image must be repointed");
+  // Both opens (scan pass + write pass) used the UUID; the slugId never appears.
+  assert.deepEqual(state.docNames, [`page.${UUID}`, `page.${UUID}`]);
+  assert.ok(
+    !state.docNames.includes(`page.${SLUG}`),
+    "replaceImage must NEVER open the collab doc by the slugId (the #260 bug)",
+  );
+});

packages/mcp/test/mock/write-order.test.mjs

@@ -66,6 +66,14 @@ function makeServer() {
       sendJson(res, 200, { data: { token: "collab-jwt" } });
       return;
     }
+    if (req.url === "/api/pages/info") {
+      // Resolve the pageId -> canonical UUID (#260) so the test exercises the
+      // real body-write failure (no WS upgrade) rather than a resolve failure.
+      sendJson(res, 200, {
+        data: { id: "11111111-1111-4111-8111-111111111111", slugId: "page-1" },
+      });
+      return;
+    }
     if (req.url === "/api/pages/update") {
       state.titlePosted = true;
       sendJson(res, 200, { data: {} });