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>
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>
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>
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>
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>
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>
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>
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>
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.
The [Unreleased] compare link still pointed at v0.93.0 even though the
0.94.0 release section already exists, and there was no [0.94.0]
link-reference at all (the header was unresolvable). Point [Unreleased] at
v0.94.0...HEAD and add [0.94.0]: v0.93.0...v0.94.0 so every version header
resolves. closes#258
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
stashPage is declared in the server's DocmostClientLike interface and
shipped as the stash_page MCP tool (client.ts, tool-specs.ts, index.ts),
but the hand-maintained HOST_CONTRACT_METHODS mirror in the contract test
was never updated — so the drift-guard test failed and broke CI's
unit-test job. Add the missing name; both directions now agree.
The floating bubble menu had no way to clear formatting, so in the
default configuration (fixed toolbar disabled) users could not reset
inline formatting at all. Mirror the fixed-toolbar action into the
bubble menu: a new "Clear formatting" item running unsetAllMarks().
- bubble-menu.tsx: import IconClearFormatting; append a non-toggle
"Clear formatting" item (isActive: () => false) to the items array.
- No i18n changes — the "Clear formatting" key already exists in all
locales.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Merging the image-captions (#221) and lossless-export branches each added
its own escapeHtmlAttr in turndown.utils.ts, producing two implementations
of the same function and breaking `tsc --build` (TS2393) — which failed the
Build editor-ext step across all CI jobs.
Drop the lighter image-captions duplicate (escapes & and ") and keep the
fuller version (escapes & " < >). It is a strict superset: both call sites
(serializeAttrs, the image rule) place the value inside a double-quoted HTML
attribute, where extra < > escaping is harmless and idempotent on re-import.
Verified: editor-ext builds; turndown.dataloss + image-markdown tests pass.
F1 (data loss): packages/mcp keeps its own copy of the document schema
(AGENTS.md), and the spoiler mark was only added to editor-ext + the server
tiptapExtensions, so a doc with a spoiler silently lost the mark through /mcp.
Add a local Spoiler mark to docmostExtensions (span[data-spoiler] parse,
data-spoiler="true"+class render) and a case "spoiler" in markdown-converter
emitting the same <span data-spoiler="true">…</span> as the editor-ext turndown
rule; add an MCP json->md->json round-trip test. Regenerated build/lib output.
F2: add the #259 inline-spoiler entry to CHANGELOG [Unreleased] Added.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The special-chars test only checked substrings (data-caption=/Tom/Jerry) that
survive even if escapeHtmlAttr stopped escaping " or double-encoded &. Assert
the exact escaped attribute in the intermediate Markdown
(data-caption="Tom & "Jerry"") and re-parse the rendered HTML to
confirm the recovered caption is exactly Tom & "Jerry".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an inline spoiler (Telegram/Discord-style hidden text): a TipTap mark
`spoiler` rendered as <span data-spoiler="true" class="spoiler">, blurred via
CSS and revealed on click (UI-only is-revealed class, never persisted).
- packages/editor-ext: the Spoiler mark (inclusive:false, set/toggle/unset
commands, ||text|| input rule), exported; a lossless turndown rule emitting
raw inline HTML; round-trip test.
- apps/client: SpoilerView mark-view (ReactMarkViewRenderer, Link pattern),
registration in extensions, bubble-menu toggle button (editable only), CSS
(blur + @media print reveal), en/ru i18n.
- apps/server: register Spoiler in collaboration.util tiptapExtensions so the
mark survives HTML<->JSON export/index/import/Yjs; a test proving the public
share keeps the spoiler (it isn't stripped with comments).
No keyboard shortcut: the proposed Mod-Shift-s collides with Strike (and
Mod-Shift-h with Highlight); the ||text|| input rule + the bubble-menu button
cover ergonomics.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The comment claimed vitest skips the file because it has no test cases; vitest
collects by filename glob, so the real reason is the name not matching
*.{test,spec}.ts. Reword to cite the glob and warn that adding test cases here
would not run them.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The schema + cell/row/table/doc builders + grid/stateFor/trFor were copied
verbatim into the 3 new table-utils test files (and the pre-existing
table-utils.test.ts) — a schema change would have to be synced across all four.
Move them into a shared table-test-helpers.ts (test-only, excluded from the
build like footnote-corpus.ts) and import it everywhere; cell uses the
(txt, attrs?) superset (a drop-in for the bare (txt) copies). No assertion
changes — test counts unchanged (223 passed + 3 expected-fail).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F1: add a test that empties a non-empty doc via a change-origin transaction
(ySyncPluginKey meta, the shape y-tiptap sets for remote/merge updates) and
asserts the intentional-clear signal is NOT emitted — pinning the
isChangeOrigin early-return that keeps remote emptiness from punching through
the #248 server guard. The 4 existing tests use local transactions and never
exercised that true-path (verified: removing the guard fails only this test).
F2: record the #248 empty-overwrite guard and the #251 intentional-clear in the
CHANGELOG [Unreleased] Fixed section.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F17: the header's content-clause literal omitted the [[:space:]]* tolerance;
copy page.repo.ts's exact '"type"[[:space:]]*:[[:space:]]*"text"' (jsonb::text
renders a space after the colon, which is why the tolerance exists).
F16: remove expect(ttl).toBeGreaterThan(0) — the twin of the F15 removal;
expect(ttl).toBe(120) strictly subsumes it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F14: the lockstep int-spec header still described the pre-F6 two-clause set with
'iff' — add the content-JSON text-node clause so it matches embeddablePredicate.
F15: remove the redundant expect(ttl).toBeLessThanOrEqual(120) that followed
expect(ttl).toBe(120).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F11: PRE_SEED_TTL_SECONDS 45->120 (= client REINDEX_POLL_CAP_MS). At concurrency
1 a queued reindex can wait past the old 45s; if the pre-seed expired while
pending, getMasked fell back to the COUNT and reported done, so the client
stopped polling and missed the climb. Tie the pre-seed TTL to the client cap.
F12: extend the lockstep integration spec — insertPage takes content; a
text_content=null + text-node-content page is IN and a math-only page is OUT,
pinning the structural "type":"text" clause (and the jsonb space-after-colon).
F13: list all three embeddable clauses in the reindex JSDoc/inline comments.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F6: extend embeddablePredicate to pages with body content but null text_content,
keyed on the text-node marker "type":"text" (not a bare "text": key, which
also matched math nodes' attrs.text and would leave math-only pages stuck
below 100%). Numerator and denominator share the predicate; tests assert the
compiled WHERE is byte-identical and a math-only doc is excluded.
F7: correct the start() JSDoc (both totals are the real page count).
F8: nextReindexPollInterval reuses isReindexComplete.
F9: getMasked reads progress first and skips the two COUNTs while a reindex is active.
F10: pre-seed the progress entry with a short 45s TTL so a deduped enqueue's
phantom "0 of N" expires quickly instead of sticking for the 1h TTL.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
uploadImage is internal to client.ts (called by insertImage/replaceImage);
the MCP transport (index.ts) does not call it directly. Remove it from the
comment's list of transport-called methods.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
guardedFetch blocks loopback/private/link-local/metadata IPs and never calls
fetch; decryptHeaders fails open (returns undefined, warns once, no blob leak).
yjs.util setYjsMark/removeYjsMarkByAttribute/updateYjsMarkAttribute on real
Y.Docs. SHARED_TOOL_SPECS<->in-app parity (name/desc/input-schema; a dropped or
renamed wiring fails). Replace the tautological storage.service spec with
driver-delegation checks across every public method.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
findBreadcrumbPath set node.name='Untitled' in place, mutating the shared
sidebar tree (treeData passed from resolveBreadcrumbNodes). Surface 'Untitled'
via a shallow copy on the returned chain only; input nodes stay untouched.
Add tests for the non-mutation invariant plus applyUpdateOne reducer,
formatRelativeTime buckets, and the pure tree mappers (sortPositionKeys,
pageToTreeNode).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extract pure extractAuthTokenFromSetCookie from performLogin (behavior-identical)
so cookie parsing is unit-testable without a network login. Add round-trip
coverage for media attrs (width/height/align/drawio/escaping) the existing
suite omitted; applyAnchorInDoc selection/ambiguity/atom-break cases; and a
cross-copy drift guard proving the vendored editor-ext recreate-transform and
the @fellow npm copy used by diff.ts emit identical steps (apply(diff)==target).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pub/sub error handlers were `(err) => () => {}` — a noop returning an
inner arrow that never runs, so socket.io redis client errors were silently
swallowed. Log them via Nest Logger. Adjacent pre-existing bug surfaced in
review of #255.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The WsRedisIoAdapter creates two ioredis clients (pubClient/subClient) for
@socket.io/redis-adapter but never closed them, leaking their TCP handles on
application shutdown (#255). The redis-adapter does not own these clients'
lifecycle, and the adapter is instantiated from main.ts (not a DI provider),
so no Nest lifecycle hook applied to it.
Keep references to both clients and override dispose(), which Nest's
SocketModule.close() invokes exactly once during shutdown after all socket.io
servers are closed. Use disconnect(false) to mirror the sibling pub/sub pair
in collaboration/extensions/redis-sync (onDestroy): immediate close, no QUIT
round-trip, no auto-reconnect. Refs are nulled to guard against double-close.
Runtime behavior is unchanged; only the shutdown path is added.
Verified with a script that boots connectToRedis() against a real Redis:
2 sockets to :6379 open after connect, 0 remain after dispose().
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The store-side empty-guard consumed the per-document intentional-clear flag
INSIDE the bounded retry loop. consumeIntentionalClear always deletes the
in-memory Map entry, but a tx rollback cannot un-delete it: attempt 1
consumed the flag then updatePage threw a transient error and rolled back;
attempt 2 re-read the page non-empty, saw the flag gone, and the empty-guard
silently BLOCKED the write — dropping the user's deliberate clear and
defeating the retry guarantee for clears.
Hoist the decision out of the loop (like consumeContributors /
consumeAgentTouched): consume once into `allowIntentionalClear` before the
`for`, and only read that boolean on the empty-over-non-empty branch. The
single hoisted consume still drops a pending flag for a non-empty store
(the "cleared then retyped" case), since every store consumes regardless of
incoming emptiness.
Add a regression test: arm via the real onStateless transport, updatePage
throws once then succeeds, assert it is called twice and the retry writes the
empty doc (the clear survives). It fails on the old consume-in-loop ordering
(updatePage called once) and passes after the hoist.
Document the known fail-safe limitation near the TTL constant: if document
ownership transfers / a node crashes between the stateless signal and the
debounced store, the in-memory flag is lost and the clear is silently not
applied (the doc reloads non-empty) — fail-safe, content is never destroyed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The full-AppModule e2e (apps/server/test/app.e2e-spec.ts) passed but jest
never exited, burning CI to its timeout. Diagnosis (process._getActiveHandles
after app.close()) showed exactly two ioredis sockets to :6379 still open after
shutdown; everything else (BullMQ queues/workers, @nestjs/schedule intervals,
nestjs-ioredis, nestjs-kysely pg pool, @nestjs/cache-manager Keyv store,
hocuspocus pub/sub) already closes on app.close().
The two leaks were owned-but-never-closed clients:
1. ThrottleModule passed a pre-built `new Redis(...)` instance to
ThrottlerStorageRedisService. With an instance, the lib sets
disconnectRequired=false, so its onModuleDestroy never disconnects.
Pass ioredis options instead so the service owns + disconnects the client.
2. CollaborationGateway created a source `new RedisClient(...)` that
RedisSyncExtension only duplicates into pub/sub; the extension's onDestroy
disconnects those duplicates but not the source. Keep a reference and
disconnect it after the hocuspocus onDestroy hook in destroy().
Both are real lifecycle fixes (production shutdown is now clean too), so no
--forceExit is needed. Verified against real Postgres+Redis:
- test:e2e (no forceExit, --runInBand) exits 0 in ~18s (was: hung forever)
- --detectOpenHandles exits 0 with no open-handle report
- active handles after app.close(): none
CI timeout-minutes safety nets left untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The #248 store-side empty-guard (onStoreDocument) unconditionally refuses to
overwrite non-empty persisted content with an empty document, because a
momentarily-empty live Y.Doc is indistinguishable from a real clear at the
store layer. That correctly blocks glitches/bad-merges, but also blocks a user
who genuinely wants to empty a page. This re-introduces a WORKING, narrow,
non-spoofable exception (the dead context.intentionalClear hatch #248 removed
never had a real channel).
Definition of an intentional clear (client, IntentionalClear editor extension):
a LOCAL user transaction (docChanged, NOT a remote y-sync change — filtered via
isChangeOrigin) that reduces a non-empty doc to the empty single-paragraph
shape. This is exactly the select-all + Delete/Backspace keystroke path.
Transport (option b — hocuspocus stateless message): on that transition the
client sends a `{type:'intentional-clear'}` stateless message. The server
(PersistenceExtension.onStateless) records a short-lived (TTL 60s > 45s
maxDebounce), single-use "pending clear" flag keyed by the connection's
document. The next debounced onStoreDocument consumes it on the empty-guard
branch to let that one empty write through.
Why this is the right channel and non-spoofable:
- Yjs transaction origin/metadata does not survive to the server store; awareness
is per-connection and racy. A stateless message ties the signal to a specific
clear, survives the debounce, and rides the authenticated connection.
- The document is taken from the connection, never the payload, so a client
cannot target another page.
- The flag is read ONLY on the empty-over-non-empty branch, so the worst a forged
signal can do is clear a page the connection may already edit; it can never
force or alter a non-empty write. Read-only connections cannot arm it. Every
non-empty store drops a pending flag, so "cleared then retyped" leaves nothing
usable; the flag is single-use and TTL-bounded.
NOTE: #248 is not yet on develop, so the empty-guard block is included here as
the foundation this exception extends. If #248 lands first this rebases cleanly
(the guard logic is identical; the #251-unique additions are the exception,
onStateless, the pending-flag state, and the client extension).
Tests:
- Server (real transport path, not a hand-poke): onStateless sets the flag with
the exact client payload, then the debounced onStoreDocument persists the empty
doc; plus single-use consumption, read-only rejection, non-empty-store drops
the flag, and the unchanged #248 guard tests (empty-over-non-empty blocked,
empty-over-empty allowed).
- Client: a real Editor + the actual selectAll+deleteSelection command emits the
signal; typing / non-emptying edits / already-empty docs do not.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
