Compare commits

...

15 Commits

Author SHA1 Message Date
2524f39a36 Merge pull request 'docs: how to bring up a local dev stand (+ gotchas), referenced from AGENTS.md' (#272) from docs/dev-stand-guide into develop
Reviewed-on: #272
2026-07-01 18:32:35 +03:00
claude code agent 227
ef173f022d docs: add "Running a local dev stand" guide + reference it from AGENTS.md
Captures the non-obvious gotchas that make bringing up a local instance
painful: the collaboration server is a THIRD process (pnpm dev starts only
API + client) that must be built before running (tsx/ts-node fail on NestJS
DI); APP_SECRET must be identical between the API and collab servers or every
realtime connection is rejected with "Invalid collab token"; Vite binds
localhost so LAN access needs --host; a stale @docmost/editor-ext white-
screens the client; pgvector is mandatory; migrations don't auto-run in dev.
Also documents that demo/test passwords should be a simple one-word
alphanumeric (no special chars, which get mangled through shells/JSON/URLs).

Referenced from AGENTS.md (Commands + Two-server-processes sections).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-01 03:21:41 +03:00
38f9a7938a Merge pull request 'feat(editor): restore reading scroll position on reload (#266)' (#267) from feat/266-scroll-position into develop
Reviewed-on: #267
2026-06-30 19:59:50 +03:00
claude code agent 227
30cdd65b92 test/refactor(#266): cover anti-clobber capture + once-guard; log storage errors
Review round 1 on the scroll-position feature:
- F1: add two tests for the hook's subtlest invariants — (a2) the restore
  target is captured synchronously at mount and survives a fresh scroll@0
  overwriting storage on load (a regression moving the capture into an effect
  would now fail); (a3) restore runs at most once per mount even when called
  again (the wiring effect can re-run).
- F2: log instead of silently swallowing sessionStorage errors in
  readStorage/writeStorage (AGENTS.md "errors must never be swallowed" rule);
  no user notification since a missed scroll restore is not actionable.
- F3: document the hard dependency on PageEditor remounting per page
  (key={page.id}) at the refs declaration — the per-mount refs are not reset
  on an in-place pageId change, so removing that key would break restore on
  the 2nd page.

vitest 9/9, tsc 0, eslint 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 12:13:44 +03:00
claude code agent 227
b601c78c21 feat(editor): restore reading scroll position on reload (#266)
Adds useScrollPosition(pageId): saves window.scrollY to sessionStorage
(key gitmost:scroll-position:<pageId>) on throttled scroll / pagehide /
visibilitychange / cleanup, capturing the previously-saved value
synchronously at mount before any handler can overwrite it with the fresh 0.
restoreScrollPosition() (wired in page-editor.tsx to fire once the live
content is laid out, !showStatic && editor) yields to a #hash anchor, then
polls the document height and scrolls to the saved Y once the content is
tall enough, with a 5s timeout clamped to the max reachable position. All
storage access is try/caught so a disabled/quota'd Storage never breaks the
page. The in-flight restore poll is held in a ref and cancelled on unmount,
so a fast SPA navigation can't scroll the next page. closes #266

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 11:43:14 +03:00
79394b3ef8 Merge pull request 'test(#244): dictation ordered-emitter + internal-link paste (Phase 2 tail)' (#263) from test/244-phase2-tail into develop
Reviewed-on: #263
2026-06-30 11:21:17 +03:00
e3ec9a2965 Merge pull request 'fix(#262): reindex counter polls past the stale pre-reindex snapshot' (#264) from fix/262-reindex-progress-realtime into develop
Reviewed-on: #264
2026-06-30 11:21:01 +03:00
449a304657 Merge pull request 'fix(#260): open MCP collab docs by canonical UUID (slugId doc-name split)' (#265) from fix/260-collab-docname-slugid into develop
Reviewed-on: #265
2026-06-30 11:20:51 +03:00
claude code agent 227
e04afee629 test(#260): cover replaceImage's UUID lock-key invariant; drop dead cache line
Reviewer round 1 on the #260 collab-doc-name fix:

- F1: replaceImage is the one path where the resolved UUID gates BOTH the
  collab-doc open AND the per-page mutex key (withPageLock(pageUuid)). Add a
  deterministic test to resolve-page-id-collab-doc-name.test.mjs: it gates
  /files/upload so replaceImage parks mid-upload holding its lock, asserts the
  doc opened as page.<uuid> (never page.<slug>), and probes the SHARED
  page-lock chain — a withPageLock(UUID) probe must stay blocked while
  replaceImage holds it (with a free-key probe as a non-vacuity guard). The
  test fails if the lock key is reverted to the slugId (verified).
- F2: drop the dead `pageIdCache.set(uuid, uuid)` — resolvePageId returns on
  the isUuid() short-circuit before the cache is ever read with a uuid key, so
  only slugId->uuid entries are stored/read. Comment corrected to match.

MCP suite 430/430, tsc 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 10:46:07 +03:00
claude code agent 227
3b80285d57 fix(#260): open MCP collab docs by canonical UUID (slugId doc-name split)
Real root cause of the silent MCP edit loss: the web editor always opens the
collaboration document by the page UUID (`page.${page.id}`), but the MCP
opened it by the agent-supplied id — usually a slugId — so `page.${pageId}`
became `page.<slugId>`. For one DB page that is TWO independent Yjs documents;
both persist to the same `pages` row (findById/updatePage resolve id or
slugId), so the human tab's debounced store overwrites the agent edit
(last-store-wins) — gone after reload, never shown live. The slugId doc also
made the server's transclusion sync + embedding reindex throw Postgres 22P02.

Fix:
- MCP (primary): resolvePageId(pageId) returns the canonical UUID — a UUID
  short-circuits with no network call, a slugId resolves once via getPageRaw
  and is cached both ways. Every collab-write path (mutatePageContent /
  updatePageContentRealtime / replacePageContent and the mutate/replace/
  unlocked seams) now opens by the resolved UUID, so the MCP and the editor
  share ONE Yjs doc. replaceImage's whole-operation page lock also keys on the
  UUID so it serializes against the other (now-UUID-keyed) writes.
- Server (defense + kills the 22P02 noise): onStoreDocument passes the resolved
  page.id — not the raw doc-name id — to syncTransclusion, the embedding queue,
  the mention-notification job, addContributors, and the in-tx history read.
  Content store and the empty-guard are untouched.

Tests: a new MCP test stands up a real Hocuspocus server and asserts a slugId
input opens `page.<uuid>` (never `page.<slugId>`), with UUID short-circuit and
single-resolve caching; the server spec asserts the side-effects receive the
UUID for a `page.<slugId>` doc. closes #260

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 10:04:49 +03:00
claude code agent 227
42a1fa1d3a test(#244): cover the out-of-order failure branch of the dictation emitter (F1)
The reviewer noted the in-order emitter's else branch (a NOT-next-to-emit
segment failing → buffer an empty placeholder so the drain can skip it,
use-streaming-dictation.ts:215-218) was the one reachable ordering branch
left uncovered. Add a non-vacuous case: with 3 segments, reject seq 1
(out of order) → one notification, nothing emitted; resolve seq 0 → "alpha";
resolve seq 2 → "gamma". The seq-2 flush proves the empty placeholder let the
emitter advance PAST the failed seq 1 — without the else branch the drain
would stall at the missing seq 1 and "gamma" would never emit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 10:01:49 +03:00
claude code agent 227
67312a3753 fix(#262): keep polling the reindex counter past the stale pre-reindex snapshot
After "Reindex now" the "Indexed X of Y" counter froze at 0 until a manual
reload. Root cause is purely client-side: right after the mutation the
client still holds the PRE-reindex settings snapshot, which for an already
fully-indexed workspace reads reindexing=false, indexed>=total. The
deadline-clearing effect evaluated isReindexComplete() against that stale
snapshot, read it as "done", and cleared the poll deadline before the first
post-reindex poll ever landed — so polling never ran and the counter stayed
at 0 (a reload just fetched one fresh snapshot).

Gate completion on having actually observed the active run: a
reindexSeenActiveRef, reset on each new reindex (mutation onSuccess, before
setting the deadline) and latched true once a poll reports reindexing=true.
isReindexComplete(status, seenActive) and nextReindexPollInterval now require
seenActive, so the stale fully-indexed snapshot no longer reads as finished.
The server pre-seeds reindexing=true from enqueue time, so seenActive latches
early and a genuine completion still stops polling promptly; the
REINDEX_POLL_CAP_MS cap is checked first and always wins, so polling can
never run away. closes #262

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 09:12:15 +03:00
claude code agent 227
ef27b6d440 test(#244): cover dictation ordered-emitter + internal-link paste (Phase 2 tail)
Backfill the two genuinely-uncovered infra-free units from the #244 Part B
test backlog (the rest was already covered by #248/#257):

- use-streaming-dictation: the in-order transcription emitter. Drives the
  real hook via renderHook with mocked VAD + deferred transcribeAudio so the
  test controls response order. Asserts out-of-order HTTP responses still
  emit text in segment order; whitespace trimmed and empty results dropped
  while the sequence advances; a failed segment shows one notification and is
  skipped so later segments still flush; a response resolving after cancel()
  is dropped (stale-epoch guard).
- internal-link-paste (handleInternalLink / createMentionAction): validateFn
  reject → no resolve/dispatch; resolve → mention node with the resolved page
  + anchor dispatched via replaceWith at pos; "Untitled" fallback; reject →
  raw url inserted as text under a link mark; createMentionAction wiring to
  getPageById on success + failure.

Test-only; no production code changed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 09:07:45 +03:00
c4842367af Merge pull request 'docs(changelog): sync compare-links for 0.94.0 (#258)' (#261) from fix/258-changelog-compare-links into develop
Reviewed-on: #261
2026-06-30 09:02:22 +03:00
claude_code
96b9ec11d6 ci: use mirror.gcr.io for postgres and redis
Update GitHub workflow services to pull PostgreSQL and Redis images from `mirror.gcr.io` instead of Docker Hub. This avoids anonymous pull rate‑limit failures on shared GitHub runner IPs by using the Docker Hub pull‑through cache.
2026-06-30 08:50:00 +03:00
19 changed files with 1773 additions and 75 deletions

View File

@@ -75,7 +75,9 @@ jobs:
APP_URL: http://localhost:3000
services:
postgres:
image: pgvector/pgvector:pg18
# via mirror.gcr.io (Docker Hub pull-through cache; avoids Hub anonymous
# pull rate-limit that randomly fails on shared GitHub runner IPs).
image: mirror.gcr.io/pgvector/pgvector:pg18
env:
POSTGRES_DB: docmost
POSTGRES_USER: docmost
@@ -88,7 +90,8 @@ jobs:
--health-timeout 5s
--health-retries 20
redis:
image: redis:7
# via mirror.gcr.io (see postgres note above).
image: mirror.gcr.io/library/redis:7
ports:
- 6379:6379
options: >-
@@ -135,7 +138,9 @@ jobs:
NODE_ENV: production
services:
postgres:
image: pgvector/pgvector:pg18
# via mirror.gcr.io (Docker Hub pull-through cache; avoids Hub anonymous
# pull rate-limit that randomly fails on shared GitHub runner IPs).
image: mirror.gcr.io/pgvector/pgvector:pg18
env:
POSTGRES_DB: docmost
POSTGRES_USER: docmost
@@ -148,7 +153,8 @@ jobs:
--health-timeout 5s
--health-retries 20
redis:
image: redis:7
# via mirror.gcr.io (see postgres note above).
image: mirror.gcr.io/library/redis:7
ports:
- 6379:6379
options: >-

View File

@@ -27,7 +27,9 @@ jobs:
# TEST_*_URL overrides are needed.
services:
postgres:
image: pgvector/pgvector:pg18
# via mirror.gcr.io (Docker Hub pull-through cache; avoids Hub anonymous
# pull rate-limit that randomly fails on shared GitHub runner IPs).
image: mirror.gcr.io/pgvector/pgvector:pg18
env:
POSTGRES_USER: docmost
POSTGRES_PASSWORD: docmost_dev_pw
@@ -40,7 +42,8 @@ jobs:
--health-timeout 5s
--health-retries 5
redis:
image: redis:7
# via mirror.gcr.io (see postgres note above).
image: mirror.gcr.io/library/redis:7
ports:
- 6379:6379
options: >-

View File

@@ -197,6 +197,12 @@ pnpm workspace (`pnpm@10.4.0`) orchestrated by **Nx**. Four workspace packages:
Run from the repo root unless noted. The dev workflow needs **Postgres (with the `pgvector` extension) and Redis** reachable per `.env` (copy `.env.example``.env`).
> **Bringing up a full local stand** (API + client + the separate realtime
> collaboration process) has several non-obvious gotchas — a missing collab
> server, `APP_SECRET` mismatch between processes, a stale `editor-ext` white-
> screening the client, LAN exposure. See **[docs/dev-stand.md](docs/dev-stand.md)**
> for the step-by-step and the traps.
```bash
pnpm install # install all workspaces (uses pnpm patches; see package.json `pnpm.patchedDependencies`)
pnpm dev # client (Vite) + server (Nest watch) concurrently — primary dev loop
@@ -241,6 +247,8 @@ Migration files live in `apps/server/src/database/migrations/` and are named `YY
- **API server** — `dist/main` (`apps/server/src/main.ts`), the Fastify HTTP app (`AppModule`).
- **Collaboration server** — `dist/collaboration/server/collab-main` (`pnpm collab`), a Hocuspocus/Yjs WebSocket server (`apps/server/src/collaboration/`) handling real-time document editing, persistence, and page-history snapshots. It listens on `COLLAB_PORT` (default `3001`), separate from the API server's `PORT` (default `3000`), and shares state with the API server through Redis.
`pnpm dev` starts **only** the API server + client — the collaboration process is separate and must be started too, or the editor never connects. See **[docs/dev-stand.md](docs/dev-stand.md)** for running both locally (and why `APP_SECRET` must match between them).
The API server is a Fastify app with a global `/api` prefix (`main.ts` excludes `robots.txt`, public share pages, and `mcp` from the prefix). A `preHandler` hook enforces that a resolved `workspaceId` exists for most `/api` routes (multi-tenant by hostname/subdomain via `DomainMiddleware`). `GET /api/sb/:id` (the anonymous blob-sandbox read route) is listed in that preHandler's `excludedPaths`, so it is exempt from workspace resolution and carries no session auth at all (its capability is the unguessable UUID + TTL + TLS) — unlike `/api/files/public/...`, which still resolves a workspace and requires a workspace-bound attachment JWT. Auth is JWT (cookie + bearer); authorization is **CASL** (`core/casl`) — every data access is scoped to the user's abilities.
### Module structure (server)

View File

@@ -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([]);
});
});

View File

@@ -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);
});
});

View File

@@ -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();
});
});

View File

@@ -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 };
}

View File

@@ -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>

View File

@@ -77,7 +77,9 @@ describe('resolveKeyField (write-only key payload)', () => {
describe('nextReindexPollInterval', () => {
const INTERVAL = 5000;
const base = { now: 1_000, intervalMs: INTERVAL };
// `seenActive: true` is the steady state for most of a run — a poll has
// observed `reindexing === true` (the server pre-seeds it from enqueue time).
const base = { now: 1_000, intervalMs: INTERVAL, seenActive: true };
it('does not poll when no reindex deadline is set', () => {
expect(
@@ -111,7 +113,7 @@ describe('nextReindexPollInterval', () => {
).toBe(INTERVAL);
});
it('stops once the run is finished AND fully indexed', () => {
it('stops once the run is finished AND fully indexed (after having been active)', () => {
expect(
nextReindexPollInterval({
...base,
@@ -121,11 +123,29 @@ describe('nextReindexPollInterval', () => {
).toBe(false);
});
it('does NOT stop on the stale pre-reindex snapshot (fully indexed, never seen active)', () => {
// Regression for #262: right after "Reindex now" the client still holds the
// PRE-reindex settings (an already fully-indexed workspace reads as
// reindexing=false, indexed>=total). Without the seenActive gate this looked
// "done" and stopped polling on the very first tick, freezing the counter at
// 0 until a manual reload. The fresh window has not observed the active run,
// so polling must continue until the first real poll lands.
expect(
nextReindexPollInterval({
...base,
seenActive: false,
deadline: 10_000,
status: { reindexing: false, indexedPages: 478, totalPages: 478 },
}),
).toBe(INTERVAL);
});
it('keeps polling within the deadline when not yet done and no active flag', () => {
// First poll right after enqueue, before the worker publishes progress.
expect(
nextReindexPollInterval({
...base,
seenActive: false,
deadline: 10_000,
status: { reindexing: false, indexedPages: 0, totalPages: 478 },
}),
@@ -138,12 +158,15 @@ describe('nextReindexPollInterval', () => {
deadline: 1_000,
now: 2_000, // past the deadline
intervalMs: INTERVAL,
seenActive: true,
status: { reindexing: true, indexedPages: 200, totalPages: 478 },
}),
).toBe(false);
});
it('stops on an empty workspace (0 of 0) once the run is finished', () => {
// The pre-seed publishes reindexing=true even for 0 pages, so a poll sees the
// run active before the worker clears -> seenActive latches true.
expect(
nextReindexPollInterval({
...base,
@@ -156,26 +179,46 @@ describe('nextReindexPollInterval', () => {
describe('isReindexComplete', () => {
it('false when no status yet', () => {
expect(isReindexComplete(undefined)).toBe(false);
expect(isReindexComplete(undefined, true)).toBe(false);
});
it('false while a run is still active (even at indexed==total)', () => {
expect(
isReindexComplete({ reindexing: true, indexedPages: 478, totalPages: 478 }),
isReindexComplete(
{ reindexing: true, indexedPages: 478, totalPages: 478 },
true,
),
).toBe(false);
});
it('false when finished but not yet fully indexed', () => {
expect(
isReindexComplete({ reindexing: false, indexedPages: 120, totalPages: 478 }),
isReindexComplete(
{ reindexing: false, indexedPages: 120, totalPages: 478 },
true,
),
).toBe(false);
});
it('true once finished and fully indexed', () => {
it('true once finished and fully indexed (after having been active)', () => {
expect(
isReindexComplete({ reindexing: false, indexedPages: 478, totalPages: 478 }),
isReindexComplete(
{ reindexing: false, indexedPages: 478, totalPages: 478 },
true,
),
).toBe(true);
});
it('false on the stale pre-reindex snapshot: finished+fully indexed but never seen active', () => {
// The just-started edge: the gate keeps this from clearing the poll deadline
// before the first post-reindex poll arrives.
expect(
isReindexComplete(
{ reindexing: false, indexedPages: 478, totalPages: 478 },
false,
),
).toBe(false);
});
});
describe('isReindexButtonLoading', () => {

View File

@@ -1,4 +1,4 @@
import { useEffect, useState } from "react";
import { useEffect, useRef, useState } from "react";
import { z } from "zod/v4";
import {
ActionIcon,
@@ -185,14 +185,23 @@ type ReindexStatus = Pick<
* has finished AND everything is indexed (server cleared its progress record and
* fell back to the DB coverage count), or the deadline cap is hit — the cap
* always wins so a stuck/never-clearing progress record can't poll forever.
*
* `seenActive` guards the just-started window: right after "Reindex now" the
* client still holds the PRE-reindex settings snapshot, which for an already
* fully-indexed workspace reads as `reindexing=false, indexed>=total`. Treating
* that stale snapshot as "done" would stop polling before the first post-reindex
* poll ever lands (counter frozen at 0). So completion is only honored once a
* poll has actually observed the active run (the enqueue-time pre-seed makes
* `reindexing=true` visible from the first poll until the run truly clears).
*/
export function nextReindexPollInterval(args: {
deadline: number | null;
now: number;
intervalMs: number;
status?: ReindexStatus;
seenActive: boolean;
}): number | false {
const { deadline, now, intervalMs, status } = args;
const { deadline, now, intervalMs, status, seenActive } = args;
if (deadline === null) return false;
// Cap always wins.
if (now > deadline) return false;
@@ -200,20 +209,33 @@ export function nextReindexPollInterval(args: {
if (status?.reindexing) return intervalMs;
// Finished and fully indexed (incl. an empty workspace, 0 >= 0) → stop. Reuse
// isReindexComplete so the completeness check lives in exactly one place.
if (isReindexComplete(status)) return false;
if (isReindexComplete(status, seenActive)) return false;
// Within the deadline and not yet done → keep polling.
return intervalMs;
}
/**
* Whether the reindex poll deadline should be cleared: the server reports no
* active run AND the count is complete. The single source of truth for the
* "reindex finished" check — `nextReindexPollInterval` reuses it for its stop
* condition (sans the cap, which the effect handles via time).
* Whether the reindex poll deadline should be cleared: a poll has observed the
* active run (`seenActive`) AND the server now reports no active run AND the
* count is complete. The single source of truth for the "reindex finished"
* check — `nextReindexPollInterval` reuses it for its stop condition (sans the
* cap, which the effect handles via time).
*
* The `seenActive` requirement is what keeps the STALE pre-reindex snapshot
* (already fully indexed → `reindexing=false, indexed>=total`) from being read
* as "finished" in the window before the first post-reindex poll arrives. Once
* a poll has seen `reindexing=true` (guaranteed by the server's enqueue-time
* pre-seed for the whole run), this flips to a genuine completion check.
*/
export function isReindexComplete(status?: ReindexStatus): boolean {
export function isReindexComplete(
status: ReindexStatus | undefined,
seenActive: boolean,
): boolean {
return (
!!status && !status.reindexing && status.indexedPages >= status.totalPages
seenActive &&
!!status &&
!status.reindexing &&
status.indexedPages >= status.totalPages
);
}
@@ -290,6 +312,14 @@ export default function AiProviderSettings() {
const REINDEX_POLL_INTERVAL = 5000; // ms between refetches while indexing
const REINDEX_POLL_CAP_MS = 120000; // ~2 min hard cap
const [reindexDeadline, setReindexDeadline] = useState<number | null>(null);
// Whether any poll in the CURRENT window has actually observed the active run
// (`reindexing === true`). Reset when a new reindex is kicked off. Gates the
// completion check so the STALE pre-reindex snapshot (an already fully-indexed
// workspace reads as `reindexing=false, indexed>=total`) can't be mistaken for
// "finished" before the first post-reindex poll lands — which would freeze the
// counter at 0 until a manual reload. A ref (not state) because it must not
// trigger a render and is only ever read where `reindexing` is already false.
const reindexSeenActiveRef = useRef(false);
// Only admins may read the (masked) AI settings; the server enforces this too.
const { data: settings, isLoading } = useAiSettingsQuery(isAdmin, (query) =>
@@ -298,6 +328,7 @@ export default function AiProviderSettings() {
now: Date.now(),
intervalMs: REINDEX_POLL_INTERVAL,
status: query.state.data,
seenActive: reindexSeenActiveRef.current,
}),
);
@@ -305,12 +336,17 @@ export default function AiProviderSettings() {
// unmount because the deadline state goes away with the component.
useEffect(() => {
if (reindexDeadline === null) return;
// "Done" matches the refetchInterval stop condition: the server reports no
// active run AND the count is complete (indexed >= total, incl. an empty
// workspace 0 >= 0), so the deadline clears promptly instead of waiting out
// the cap. While `reindexing` is still true we keep the deadline so polling
// continues for the whole run.
if (isReindexComplete(settings)) {
// Latch "we have seen the active run" the moment a poll reports it, so the
// completion check below (and the refetchInterval's) only fires once the run
// has genuinely started — never on the stale pre-reindex snapshot.
if (settings?.reindexing) reindexSeenActiveRef.current = true;
// "Done" matches the refetchInterval stop condition: a poll has observed the
// active run AND the server now reports no active run AND the count is
// complete (indexed >= total, incl. an empty workspace 0 >= 0), so the
// deadline clears promptly instead of waiting out the cap. While `reindexing`
// is still true (or no poll has seen it active yet) we keep the deadline so
// polling continues for the whole run.
if (isReindexComplete(settings, reindexSeenActiveRef.current)) {
setReindexDeadline(null);
return;
}
@@ -1117,8 +1153,13 @@ export default function AiProviderSettings() {
reindexMutation.mutate(undefined, {
// Begin bounded polling so the counter climbs as the async
// background job indexes (it does not update on its own).
onSuccess: () =>
setReindexDeadline(Date.now() + REINDEX_POLL_CAP_MS),
// Clear the "seen active" latch first so this fresh window
// doesn't inherit a previous run's completion state and stop
// immediately.
onSuccess: () => {
reindexSeenActiveRef.current = false;
setReindexDeadline(Date.now() + REINDEX_POLL_CAP_MS);
},
})
}
>

View File

@@ -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);
});
});

View File

@@ -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,
});

135
docs/dev-stand.md Normal file
View File

@@ -0,0 +1,135 @@
# Running a local dev stand
How to bring up a working local instance (API + client + realtime collaboration)
and the non-obvious gotchas that will otherwise eat an hour. Written from real
setup pain — read the **Gotchas** section before you start.
## Prerequisites
- **Node 20+ / pnpm 10+.**
- **Postgres with pgvector.** Use the `pgvector/pgvector` image (e.g.
`pgvector/pgvector:pg18`). The stock `postgres` image will FAIL the
`CREATE EXTENSION vector` migration — the RAG feature stores embeddings in
`page_embeddings`.
- **Redis** — backs caching, BullMQ queues, the Socket.IO adapter, and collab
sync.
## 1. Environment (`.env`)
The client (`apps/client/vite.config.ts`) and both server processes read env via
`envPath` → the **workspace root `.env`**. Keep a single source of truth. Minimum:
```dotenv
APP_URL=http://localhost:3000
PORT=3000
APP_SECRET=<one long secret — SAME value everywhere, see gotcha #3>
DATABASE_URL="postgresql://<user>:<pass>@localhost:5432/<db>?schema=public"
REDIS_URL=redis://127.0.0.1:6379
COLLAB_URL=http://localhost:3001 # where the CLIENT connects for realtime
COLLAB_PORT=3001 # where the COLLAB server listens
STORAGE_DRIVER=local
DISABLE_TELEMETRY=true
```
> If you also keep an `apps/server/.env`, its `APP_SECRET` **must match** the
> root one (see gotcha #3).
## 2. Migrations
Migrations do **not** auto-run in local dev. After a fresh checkout or switching
branches, apply them yourself or endpoints touching a new column/table will 500:
```bash
pnpm --filter server migration:latest
```
## 3. Bring it up — THREE processes, not two
`pnpm dev` starts only the **API server** (Nest, `:3000`) and the **client**
(Vite). Realtime collaboration is a **separate process** and `pnpm dev` does NOT
start it. You need all three:
```bash
# 1) API + client (from the repo root)
pnpm dev
# → API http://localhost:3000
# → client http://localhost:5173 (Vite; localhost-only by default)
# 2) Collaboration server — SEPARATE process. Build first (see gotcha #2), then:
pnpm --filter server build # produces dist/collaboration/server/collab-main.js
pnpm collab:dev # node dist/.../collab-main → listens on :3001 (0.0.0.0)
```
Without step 2 the editor shows **"Real-time editor connection lost. Retrying…"**,
stays in read-only *static* mode, and anything that only mounts in the *live*
editor won't appear.
## Seeding a login
Register through the UI, or reset an existing user's password directly in the DB
(the server hashes with `bcrypt`):
```js
// node -e '...' with pg + bcrypt from the repo's node_modules
const bcrypt = require("bcrypt");
const { Client } = require("pg");
(async () => {
const hash = await bcrypt.hash("demopass", 10);
const c = new Client({ /* DATABASE_URL parts */ });
await c.connect();
await c.query("update users set password=$1 where email=$2", [hash, "admin@example.com"]);
await c.end();
})();
```
> **Use a simple one-word password with no special characters** (e.g. `demopass`,
> not `Str0ng!Pass@2026`). Demo/test credentials get passed through shells, JSON
> payloads, and URLs by scripts and automation, where `!` `@` `$` `&` etc. get
> mangled or need escaping — a plain alphanumeric word avoids a whole class of
> "wrong password" confusion.
## Gotchas (the грабли)
1. **Collaboration is a third process.** `pnpm dev` runs API + client only.
Start `pnpm collab:dev` (on `:3001`) separately or the live editor never
connects. The client connects to `COLLAB_URL` directly (default
`http://localhost:3001`), NOT through the Vite `/collab` proxy — the API
server on `:3000` does **not** serve the collab websocket.
2. **The collab server must be built — you can't run it from source.**
`collab:dev` runs `node dist/collaboration/server/collab-main.js`, so run
`pnpm --filter server build` first. Running the entry via `tsx`/`ts-node`
fails with a NestJS DI error ("dependency … appears to be undefined at
runtime") because direct TS execution doesn't emit the decorator metadata the
built output has.
3. **`APP_SECRET` must be identical for the API server and the collab server.**
The API issues a collab-token (JWT signed with `APP_SECRET`); the collab
server validates it with `APP_SECRET`. If they load different values (e.g. a
root `.env` and an `apps/server/.env` with different secrets), every realtime
connection is rejected with **`[onAuthenticate] Invalid collab token`** and
the editor shows "connection lost". Keep one secret everywhere.
4. **Vite binds localhost only.** To reach the stand from another machine on the
LAN, start the client with `--host` (`pnpm --filter client exec vite --host`)
and use the box's LAN IP. The `/api`, `/socket.io`, and `/collab` Vite proxies
forward to `APP_URL`, so the API just works over the LAN; realtime needs
`COLLAB_URL` reachable from the browser (point it at the LAN IP:3001, and run
collab on `0.0.0.0` — it does by default).
5. **A stale `@docmost/editor-ext` white-screens the client.** The client imports
from `@docmost/editor-ext` (a workspace package). If that package's source is
behind (missing a newer export, e.g. `Spoiler`), the client dies at load with
*"The requested module … does not provide an export named 'Spoiler'"* → blank
page. Make sure the workspace `packages/editor-ext` is current for the branch
you're running (a stale sibling checkout resolved through a shared
`node_modules` symlink is the usual cause).
6. **pgvector, not stock postgres** (see Prerequisites) — the `vector` extension
migration fails otherwise.
7. **Migrations don't auto-run in dev** — run `migration:latest` after every pull
or branch switch.
See also the **Commands** and **Architecture → Two server processes** sections in
[`AGENTS.md`](../AGENTS.md).

View File

@@ -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 = [];

View File

@@ -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,

View File

@@ -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",
);

View File

@@ -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;

View File

@@ -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)",
);
});

View File

@@ -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: {} });