Compare commits

..

12 Commits

Author SHA1 Message Date
agent_coder 0a53be9e81 docs(converter): correct #515 mark-order comment — imported order depends on extension, not fixed
The inlineToHtml comment falsely claimed import ALWAYS yields the code mark
last; import yields code last for bold/italic/strike but code FIRST for the
==-highlight extension. Reworded to state the real invariant (wrap <code>
innermost regardless of imported order) and de-universalized the parallel
phrase in text-arbitraries.ts. Comment-only, no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 02:44:54 +03:00
agent_coder 95c0d813b0 fix(converter): round-trip code+эмфаза — code внутри, склейка соседних пробегов, HTML-fallback (#515)
Теперь узел может нести [code, bold], и сериализатор PM→Markdown должен это
корректно выгружать в обе стороны (git-sync — data-loss-critical, требуется
байт-стабильность md2===md1).

case "text": убран ранний return для code-рана. Backtick-спан оборачивается
ПЕРВЫМ (самая внутренняя марка), затем прочие марки в том же порядке массива —
`**`code`**`. Для НЕ-code ранов вывод байт-идентичен прежнему (вынесены хелперы
escapeInlineText и applyInlineMark, поведение сохранено).

renderInlineChildren: собирает максимальный пробег подряд идущих text-узлов с
голой-делимитерной эмфазис-маркой (bold/italic/strike/uncolored-highlight),
содержащий хотя бы один code-узел. ОДНОРОДНЫЙ пробег (у всех идентичное
множество не-code марок) и безопасные границы → общие марки выносятся наружу
ОДИН раз: `**`aaa` + `bbb`**`, `**`code4` tail**`. НЕОДНОРОДНЫЙ (`[code,bold]`
рядом с `[italic]`) ИЛИ граница упирается в словесный символ (делимитер `**`
перед backtick не был бы flanking → эмфаза потерялась бы) → весь пробег через
lossless inlineToHtml (схема-HTML). НЕ-code вывод байт-идентичен, кроме
редкого случая голой-делимитерной эмфазы вплотную к code+эмфазе. Узлы, где
code раньше вообще не мог нести эмфазу, — новая территория, существующие
страницы не затрагиваются.

inlineToHtml: `<code>` тоже оборачивается ВНУТРЕННИМ (импорт отдаёт code
последним в массиве — `[emphasis, code]`; порядок-зависимый цикл переворачивал
бы `<strong><code>` в `<code><strong>` на реэкспорте и ломал байт-фикспойнт).

Тесты: перевёрнуты ассерты, фиксировавшие старое (по CommonMark неверное)
поведение (code+bold/strike/link → теперь `**`x`**`/`~~`x`~~`/`[`x`](…)`);
генераторы расширены на code+{bold,italic,strike,highlight} в каноническом
порядке импорта; добавлены явные round-trip пины 5 кейсов репорта (импорт→марки
и md→pm→md идемпотентность) + усиление code-combo свойства (обе марки выживают).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 23:41:07 +03:00
agent_coder 9004de60e3 fix(schema): снять excludes:"_" у марки code — она комбинируется со всеми марками (#515)
По CommonMark `**` рядом с инлайн-кодом дают `<strong><code>` — узел с
составом марок [code, bold]. Марка `code` из tiptap несёт `excludes: "_"`
(исключает все прочие инлайн-марки), и ProseMirror на HTML→PM импорте
(`generateJSON`) выкидывает сосуществующую bold — так `**`--flag`**` терял
жирный. Ставим `excludes: ""` (не исключает ничего) во всех четырёх местах,
где марка конфигурируется независимо:

- Единый источник: новая каноническая марка `Code = TiptapCode.extend({
  excludes: "" })` в @docmost/editor-ext, экспортируется из barrel.
- Живой редактор (extensions.ts): базовая `Code` теперь из editor-ext,
  поверх сохранены client-only addInputRules/addKeyboardShortcuts.
- Collab-сервер + серверный HTML-парс/экспорт (collaboration.util.ts):
  StarterKit code:false + общая `Code`; @docmost/editor-ext объявлен в
  apps/server/package.json (использовался, но не был задекларирован).
- Редактор комментариев (comment-editor.tsx): StarterKit code:false + `Code`.
- Вендор-зеркало docmostExtensions (docmost-schema.ts) — сознательно отдельная
  копия, не тянущая editor-ext в node-рантайм: `excludes:""` объявлен локально
  (StarterKit code:false + `Code.extend({ excludes:"" })`), держится в
  синхроне с editor-ext паритет-тестом.

Паритет-гард: тест в пакете сверяет excludes марки code вендор-зеркала с
канонической editor-ext Code (обе ""); vitest резолвит @docmost/editor-ext на
sibling-исходник, чтобы гард был герметичным.

`excludes:""` делает code «перекрывающейся» маркой в y-prosemirror (как comment):
она пишется в Yjs под хешированным ключом `code--<hash>` и распаковывается
обратно в `code` штатным декодером — правим mcp-тест-хелпер fragmentToJson,
чтобы он снимал хеш ровно как yattr2markname (иначе overlapping-code утёк бы
в сравнение как `code--<hash>`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 23:41:07 +03:00
agent_coder 3a344626db fix(converter): block-escape закрывает setext-подчёркивание -- и одиночный = (#514 ревью)
CRITICAL из ревью: escapeLeadingBlockTrigger не покрывал setext-underline
из ровно двух дефисов (--) и одиночного = → строка-продолжение после
hardBreak, равная -- или =, репарсилась как setext-heading, а текст
предыдущей строки терялся. Тот же класс потери данных, что PR и чинит.

Добавлена setext-рука после тематической: целая строка ^-+[ \t]*$ или
^=+[ \t]*$ экранирует ведущий символ. Заякорено на всю строку → mid-content
-/= не задевается; после тематической руки → ---/---- не двойно-экранируются;
идемпотентно против уже-экранированного \=\= (инлайн-escape отрабатывает
раньше). Пины --/----/=/==== + генеративный hardBreakThenSetextArb, оба
mutation-verified. CHANGELOG.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 23:40:26 +03:00
agent_coder 31f51eaa47 fix(mcp): agent-write НЕ срезает ведущий ---…--- (front-matter strip — только импорт)
Ревью #493 (MEDIUM): вшив normalizeForeignMarkdown первым шагом в
markdownToProseMirrorCanonical, коммит 4 распространил срез YAML front-matter
(YAML_FRONT_MATTER_RE) на КАЖДЫЙ agent-write путь. convertProseMirrorToMarkdown
эмитит `---` для horizontalRule, поэтому страница, начинающаяся с
horizontalRule и содержащая второй `---`, при полном agent-write теряла всё до
второго `---` — молчаливая потеря ранее сохранённого контента.

Правка: разделил нормализацию. normalizeForeignMarkdown (серверный file-import
boundary) по-прежнему срезает front-matter. Новый normalizeAgentMarkdown
(agent-write, markdownToProseMirrorCanonical) делает ТОЛЬКО CRLF-нормализацию +
rewrite GFM reference-сносок (тот самый drift, ради которого коммит 4) и НЕ
трогает ведущий `---…---`. На каноническом сериализованном контенте rewrite —
no-op (он не эмитит `[^id]:`-строк).

Тесты: agent-write horizontalRule-led дока со вторым `---` сохраняет весь
контент (round-trip); file-import с реальным YAML front-matter его по-прежнему
срезает; agent-write всё ещё канонизирует GFM reference-сноски.
Mutation-verify: strip обратно на agent-write → тесты потери контента краснеют.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 18:54:38 +03:00
agent_coder b66929714f fix(converter): block-escape КАЖДОЙ строки параграфа (не только первой)
Ревью #493 (HIGH): escapeLeadingBlockTrigger применялся к всему результату
renderInlineChildren один раз, а регэкспы якорены на ^ без флага m → защищалась
только ПЕРВАЯ строка. hardBreak сериализуется как `  \n`, поэтому триггер на
строке-продолжении не экранировался и на git-sync round-trip re-парсился в
другой блок; для setext/thematic `---` текст строки терялся ЦЕЛИКОМ
([text "a", hardBreak, text "---"] → heading, "---" пропадал).

Правка: параграф теперь бьётся на `\n`-строки и escapeLeadingBlockTrigger
применяется к КАЖДОЙ. Фаззер расширен: hardBreakThenTriggerArb вставляет
триггер ПОСЛЕ hardBreak в inlineContentArb, так что P1/P2/P3 структурно
покрывают подслучай (раньше триггер стоял только первым run'ом). Добавлен
детерминированный пин на continuation-line триггеры, включая `a  \n---`
(текст «---» сохраняется, не setext).

Mutation-verify: со старым single-line escape новый пин + P1/P2 краснеют.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 18:54:38 +03:00
agent_coder a09935aa29 fix(mcp): resolved-якоря переживают полный markdown-write
Read-путь прячет resolved comment-анкоры (#337), поэтому markdown, который
агент шлёт в updatePageMarkdown, их уже не содержит — наивный full-write
стирал ВСЕ resolved comment-марки (потеря данных). Активные комментарии
переживают round-trip сами (read отдаёт их <span data-comment-id>), а
resolved — нет.

Правка: в write-пути (updatePageContentRealtime) пере-прививаем resolved-
марки из ЖИВОГО дока на совпадающие текстовые диапазоны свежеимпортированного
тела, механикой анкоринга comment-anchor. spliceCommentMark обобщён на
произвольную марку; добавлены applyCommentMarkInDoc (сохраняет resolved:true
+ attrs), collectResolvedCommentSpans и regraftResolvedComments (чистая, не
мутирует входы). Спан, чей текст агент изменил/удалил, просто не
переанкорится и отбрасывается (он и так resolved). first-occurrence-семантика
как у остального анкоринга.

Проверено: 6 новых наблюдаемых тестов + весь MCP unit-suite (691) зелёные.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 18:19:10 +03:00
agent_coder 047433595e refactor(mcp): дедуп stripInlineMarkdown — единый источник в каноническом пакете
Локаторная нормализация markdown (stripInlineMarkdown + примитив
stripWrappersAndLinks с WRAPPER_PATTERNS/LINK_IMAGE_RE) была ФОРКНУТА один-в-
один в packages/mcp/src/lib/text-normalize.ts и в каноническом
@docmost/prosemirror-markdown (где ей пользуется node-ops). MCP теперь
импортирует оба примитива из пакета (mcp и так от него зависит — цикла нет) и
держит на них лишь свои тонкие надстройки stripBalancedWrappers/
closestBlockHint. ~60 строк дубля удалено, дрейф закрыт.

Проверено: пакет node-ops (103) + весь MCP unit-suite (685) зелёные.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 18:14:39 +03:00
agent_coder 9e95412695 refactor(converter): normalizeForeignMarkdown -> в пакет, единый import-boundary
Нормализация чужого markdown (GFM reference-сноски [^id] -> инлайн ^[body],
срез ведущего YAML front-matter) жила только в apps/server, поэтому MCP-путь
записи страницы (updatePageMarkdown -> markdownToProseMirrorCanonical) тот же
ввод обрабатывал ИНАЧЕ, чем серверный импорт: front-matter и [^id] утекали
как литеральный текст / битая ссылка.

Перенёс normalizeForeignMarkdown в @docmost/prosemirror-markdown и вызвал его
первым шагом в MCP markdownToProseMirrorCanonical — теперь агентский
updatePageMarkdown нормализуется точно как серверный импорт. Серверные
импортёры (import.service, file-import-task.service, page.service) берут
функцию из пакета. Тест-корпус перенесён в пакет (foreign-markdown.test.ts).

Проверено: пакет (17 тестов corpus) + весь MCP unit-suite (685) зелёные,
включая reference-footnote/fence-кейсы на canonical-пути.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 18:05:47 +03:00
agent_coder 2fa86e2a33 fix(converter): warnings вместо тихой потери незнакомых нод/марок
Незнакомый тип ноды (default-ветка switch) молча схлопывался в свои дети, а
незнакомая марка молча выбрасывалась — тихая потеря данных. Теперь
сериализатор РЕПОРТИТ потерю:
- по умолчанию поведение байт-в-байт прежнее (graceful degrade), но при
  переданном options.warnings в сток кладётся по одному сообщению на
  незамапленный тип (дедуп по типу) — потеря наблюдаема;
- options.strict бросает ConverterLossError на ПЕРВОМ незнакомом типе
  (warning = ошибка).

git-sync (lossless-путь) включает strict в stabilizePageBody: тип без
серизализующей ветки падает громко на записи, а не пишет lossy .md. Валидный
контент не затронут — у всех текущих типов схемы есть ветка.

Покрытие: converter-loss-warnings.test.ts (нода/марка × strict/non-strict,
дедуп, чистый контент) и strict-пин в git-sync stabilize.test.ts — всё через
реальный конвертер.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 17:56:11 +03:00
agent_coder e3eece78c3 test(converter): атрибутный contract-тест схем editor-ext <-> mirror
Name-level контракт ловил пропажу целой ноды/марки, но не дрейф АТРИБУТОВ
внутри вендоренной ноды — класс, из-за которого молча потерялся
subpages.recursive. Добавлен атрибутный контракт: для каждой ноды/марки
@docmost/editor-ext сравниваются её СОБСТВЕННЫЕ объявленные атрибуты (имена
+ дефолты, читаются из config.addAttributes) с spec.attrs собранной схемы
зеркала.

Направление editor-ext -> mirror: зеркало намеренно надмножество (глобальные
id/textAlign/indent, нормализация части дефолтов в null), поэтому обратное
сравнение — ложный дрейф. Значимый провал — атрибут, который зеркало РОНЯЕТ
(имя) или чей дефолт молча меняет. Два blessed вида расхождений вынесены в
обоснованные allowlist'ы (highlight.colorName — нет md-формы;
image.src/link.internal/pdf.width/height — null-нормализация непереносимых
атрибутов), оба со stale-guard, чтобы список не сгнил.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 17:49:03 +03:00
agent_coder e1b8ef5b8b fix(converter): block-escape начала параграфа — закрытие класса потерь данных
Строка параграфа, начинающаяся с блочного триггера (`#`/`-`/`*`/`+`/`>`,
упорядоченного `N.`/`N)`, фенса ```/~~~, таблицы `|` или тематического
разрыва `---`/`***`/`___`), на round-trip doc->markdown->doc молча
превращалась в heading/list/quote/code block/table/horizontalRule. Худший
случай — тематический разрыв: horizontalRule не несёт текста, и строка
теряла его целиком.

Сериализатор параграфа теперь backslash-экранирует ведущий блочный триггер
(escapeLeadingBlockTrigger): экранируется только ПЕРВЫЙ значащий символ,
токенизатор CommonMark декодирует `\` обратно в литерал И снимает блочную
интерпретацию, так что строка round-trip'ится байт-в-байт как параграф.
Emphasis `**x**`, inline-code и обычная проза триггерами не являются и не
трогаются (нет мусорных backslash).

Класс раньше не чинили, а ОБХОДИЛИ; обход убран у обоих потребителей:
- клиентский мост (gitmost-recording.ts) больше не подставляет ZWSP-хак;
- генеративный корпус (text-arbitraries.ts) снял самоцензуру — добавлен
  blockTriggerLeadRunArb, параграф теперь МОЖЕТ открываться триггером, и
  P1/P2/P3 сами доказывают закрытие класса.

Пины на каждый триггер — детерминированные round-trip через реальный
конвертер (gitmost-transcript-neutralization.test.ts). Обновлён
документировавший старую потерю gap-тест (spec 13).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 17:43:24 +03:00
80 changed files with 1951 additions and 3419 deletions
-14
View File
@@ -335,20 +335,6 @@ MCP_DOCMOST_PASSWORD=
# VictoriaMetrics/Prometheus reaching it as <host>:<port>/metrics.
# METRICS_PORT=9464
#
# METRICS_BIND — interface the /metrics listener binds to. DEFAULT 127.0.0.1
# (loopback only), so the unauthenticated endpoint is NOT exposed on all
# interfaces. If the scraper runs in a SEPARATE container and reaches this as
# docmost:9464, set METRICS_BIND=0.0.0.0 — but then also set METRICS_TOKEN
# and/or keep the port on a private network, since /metrics is otherwise open.
# METRICS_BIND=127.0.0.1
#
# METRICS_TOKEN — optional Bearer token guarding /metrics. When set, every
# scrape MUST send `Authorization: Bearer <token>` (others get 401). Configure
# the scraper with the same bearer token (e.g. VictoriaMetrics/vmagent
# `bearer_token`, Prometheus `authorization.credentials`). Leave unset only
# when the endpoint is bound to loopback or an otherwise-trusted network.
# METRICS_TOKEN=
#
# 2) CLIENT_TELEMETRY_ENABLED — the public client perf-telemetry sink.
# OFF by default. When true, the unauthenticated POST /api/telemetry/vitals
# endpoint is registered and browsers collect + send web-vitals / editor
+1 -1
View File
@@ -470,7 +470,7 @@ Vite SPA. Code is organized by feature under `apps/client/src/features/*` (mirro
- **Errors must never be swallowed or shown as generic messages.** Every caught error MUST (1) be logged in full to the console/logger — error name, message, stack, `cause`, and (for HTTP/provider failures) the status code and response body — and (2) be surfaced to the user with a *specific, human-readable explanation of what actually went wrong*, never a bare generic string like "Something went wrong" / "Could not start recording" / "Transcription failed". Include the real reason (the underlying error/provider message) in the user-facing text. On the server, wrap third-party/provider failures with `describeProviderError` (or equivalent) and rethrow as a meaningful HTTP status + message — never let them collapse into an opaque 500. On the client, `console.error(<context>, err)` the raw error AND show the extracted reason (e.g. `err.response?.data?.message`, or the error `name: message`) in the notification.
- The version string shown in the UI comes from `APP_VERSION` (CI/Docker) or `git describe --tags --always` (local), resolved in `vite.config.ts` — not from `package.json`.
- Server TS config is permissive (`noImplicitAny: false`, `strictNullChecks: false`, `no-explicit-any` lint disabled). Follow the existing relaxed style rather than tightening types broadly.
- Dependency versions are heavily pinned via `pnpm.overrides` and `pnpm.patchedDependencies` (`scimmy`, `yjs`, `ai`) in the root `package.json`. Don't bump pinned/patched deps casually; the patches and overrides exist for compatibility/security reasons. The `ai@6.0.134` patch carries TWO independent server fixes, each with its own tripwire test: (1) it disables the SDK's O(n²) cumulative `partialOutput` accumulation when no output strategy is requested (server heap OOM on long agent runs, #184; tripwire: `apps/server/src/integrations/ai/ai-sdk-partial-output.patch.spec.ts`); (2) it fixes `writeToServerResponse`'s drain-hang — the loop awaited only `"drain"` under backpressure, so a mid-write client disconnect parked the pipe forever and leaked the reader/buffers until restart; it now races `"drain"` against `"close"`/`"error"`, cancels the reader on disconnect, and swallows the fire-and-forget read rejection (#486; tripwire: `apps/server/src/integrations/ai/ai-sdk-drain-hang.patch.spec.ts`). Both tripwires assert BOTH installed dist builds carry their patch marker. The patch MUST be re-created via `pnpm patch` when bumping `ai`.
- Dependency versions are heavily pinned via `pnpm.overrides` and `pnpm.patchedDependencies` (`scimmy`, `yjs`, `ai`) in the root `package.json`. Don't bump pinned/patched deps casually; the patches and overrides exist for compatibility/security reasons. The `ai@6.0.134` patch disables the SDK's O(n²) cumulative `partialOutput` accumulation when no output strategy is requested (server heap OOM on long agent runs, #184; tripwire test: `apps/server/src/integrations/ai/ai-sdk-partial-output.patch.spec.ts`) — it MUST be re-created via `pnpm patch` when bumping `ai`.
- **The MCP tool inventory in `SERVER_INSTRUCTIONS` is GENERATED from the registry** (`packages/mcp/src/server-instructions.ts`: `buildToolInventory()` over `SHARED_TOOL_SPECS`) and spliced into the hand-written routing prose (`ROUTING_PROSE`). So adding/renaming/removing a **shared** spec in `packages/mcp/src/tool-specs.ts` auto-updates the `<tool_inventory>` — no manual `SERVER_INSTRUCTIONS` edit needed. Only an **inline** MCP-only tool (those registered via `server.registerTool(...)` in `index.ts`, not through the registry) needs a one-line entry in `INLINE_MCP_INVENTORY`. Enforced by `packages/mcp/test/unit/tool-inventory.test.mjs`, which fails when a registered tool is missing from the generated inventory (there is no `EXCEPTIONS` opt-out anymore — every tool must appear). Update `ROUTING_PROSE` when a tool's *intent guidance* (when-to-use) changes. `packages/mcp/build/` is gitignored and rebuilt in CI/Docker via `pnpm build` (same convention as `git-sync`/`prosemirror-markdown`) — never commit it; rebuild locally after editing to run the tests.
## CI / release
+9 -63
View File
@@ -115,18 +115,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
the old ProseMirror-JSON output. Released together with the `#411`/`#412`
breaking window so external configs break exactly once. (#413)
- **The Prometheus `/metrics` listener now binds to `127.0.0.1` (loopback) by
default instead of `0.0.0.0` (all interfaces).** This closes an unauthenticated
endpoint that was previously reachable on every interface. **DEPLOY MIGRATION —
cross-container scraping breaks silently otherwise:** if your scraper runs in a
SEPARATE container and reaches the app as `docmost:9464` (the exact topology the
old `0.0.0.0` hardcode served), you MUST now set `METRICS_BIND=0.0.0.0` — and,
because that re-exposes the endpoint, also set `METRICS_TOKEN=<secret>` and
configure the scraper with a matching Bearer token. Without `METRICS_BIND`, the
scraper can no longer connect and metrics go dark with no error. See the
`METRICS_BIND` / `METRICS_TOKEN` block in `.env.example` for the migration.
Same-host (loopback) scrapers need no change. (#486)
### Added
- **Place several images side by side in a row.** A new "Inline (side by
@@ -314,6 +302,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- **Markdown round-trips no longer silently drop a line that opens with a block
trigger.** When a document is exported to Markdown and re-imported (git-sync
stabilize, agent writes), a paragraph or continuation line (after a hard break)
that begins with a block marker — an ATX heading `#`, a blockquote/callout `>`,
a list marker (`-`/`*`/`+`/`N.`/`N)`), a code fence, a table `|`, a thematic
break (`---`), or a setext underline (`--`, `----`, or a lone `=`) — is now
backslash-escaped so it round-trips as text instead of being re-parsed into a
heading/list/quote/rule and losing its content. Front-matter stripping is
scoped to the import path only. (#493)
- **The server no longer runs out of heap during long autonomous agent runs.** A
new pnpm patch on `ai@6.0.134` stops the SDK from building a cumulative
snapshot of the ENTIRE turn text on every streamed text-delta when no output
@@ -322,39 +319,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`tee()` branch of the stream result — a ~20-step, ~28k-chunk agent run
retained ~1.7 GB and OOM'd the 2 GB JS heap. Streaming granularity is
unchanged; the patch must be re-created if `ai` is ever bumped. (#184)
- **The server no longer leaks a hung stream pipe on every mid-run client
disconnect.** The same `ai@6.0.134` pnpm patch now also fixes the SDK's
`writeToServerResponse`, which awaited only a `"drain"` event under
backpressure: when a client disconnected mid-write the socket never drained, so
the write loop parked forever, `response.end()` was unreachable, and the stream
reader plus buffered chunks were pinned until process restart (every mid-run
disconnect in autonomous mode leaked one). The patch races `"drain"` against
`"close"`/`"error"`, cancels the reader and ends the response on disconnect, and
swallows the fire-and-forget read rejection instead of crashing on an
unhandledRejection. (#486)
- **A failed autonomous agent-run start no longer becomes an unstoppable ghost
run.** When `beginRun` failed for a transient reason (e.g. a DB-pool blip),
the turn previously continued with NO run row — invisible to `/stop`, not
aborted on disconnect, and able to slip a second run past the one-run-per-chat
gate, leaving an unstoppable run until restart. The turn now fails fast with an
honest `503 A_RUN_BEGIN_FAILED` before the first byte (no orphan state), and the
client shows a "temporary — please try again" message instead of a misleading
"provider not configured". (#486)
- **A pathological draw.io graph can no longer wedge the whole server.** The ELK
auto-layout (`layout:"elk"`) ran elkjs synchronously on the main event loop, so
a graph at the node/edge cap blocked ALL HTTP/SSE/loopback traffic while it
churned — and the old `setTimeout` "timeout" could never fire because the same
thread was blocked. Layout now runs in a worker thread with the timeout enforced
by `worker.terminate()`; the main loop stays responsive. (#486)
- **The `/health` Redis probe no longer leaks a client on every tick while Redis
is down.** It built a new `ioredis` client per probe and disconnected it only on
success, so during an outage each health tick added another forever-reconnecting
client (an unbounded handle leak). A single long-lived probe client is now
reused and closed on shutdown. (#486)
- **Internal links in exported Markdown no longer lose their visible text.** A
link whose target page name had no file extension (e.g. a bare title) was
collapsed to empty text during export, producing an unclickable, label-less
@@ -431,24 +395,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
share); any other value now returns the generic "not found" instead of
serving the page. (#218)
- **Tool and provider error text no longer leaks to anonymous readers in the
public-share AI chat.** A failing tool's raw error (which could carry an
internal page title or a stack fragment) and a provider error (which bundles the
provider `statusCode` and response body — potentially the internal baseUrl or
model name) were streamed verbatim to the anonymous reader over SSE. Errors are
now sanitized at the source: the share toolset collapses any unclassified tool
error to a safe generic string (safe, classified tool messages still pass
through for the model's self-correction), and the anonymous stream `onError`
maps provider failures to a fixed set of neutral strings — the full detail goes
only to the server log. A UI render gate is layered on top. (closes #394)
- **The Prometheus `/metrics` endpoint can now require Bearer authentication and
is loopback-bound by default.** Previously it listened on all interfaces with no
auth. Setting `METRICS_TOKEN` requires every scrape to present
`Authorization: Bearer <token>` (compared in constant time), and the listener
defaults to `127.0.0.1` (see the Breaking Changes entry for the cross-container
migration). (#486)
## [0.94.0] - 2026-06-26
This release makes AI chat durable and fast: assistant turns are persisted to
@@ -203,52 +203,6 @@ describe("ChatThread — send now (#198)", () => {
});
});
// #486: the final onFinish -> flushNext() must be gated on the live-mount flag.
// A clean onFinish can land AFTER the thread unmounts (New-chat / chat-switch
// mid-stream — the async attach/resume settles late); flushing then dequeues and
// re-POSTs a queued message from an abandoned thread (a "ghost" send).
describe("ChatThread — onFinish flush gated on mount (#486)", () => {
beforeEach(resetState);
afterEach(cleanup);
it("a clean onFinish WHILE MOUNTED flushes the queued message (control)", () => {
renderThread();
fireEvent.click(screen.getByTestId("queue-btn")); // enqueue "queued text"
expect(h.state.sendMessage).not.toHaveBeenCalled();
act(() => {
h.state.onFinish?.({
message: { id: "a", role: "assistant", parts: [] },
isAbort: false,
isDisconnect: false,
isError: false,
});
});
// Mounted: the queue flushes normally.
expect(h.state.sendMessage).toHaveBeenCalledWith({ text: "queued text" });
});
it("a clean onFinish AFTER unmount does NOT flush (no ghost send)", () => {
const { unmount } = renderThread();
fireEvent.click(screen.getByTestId("queue-btn")); // enqueue "queued text"
h.state.sendMessage.mockClear();
// Chat switched away mid-stream: the streamer unmounts...
unmount();
// ...and a late, clean onFinish lands on the abandoned thread.
act(() => {
h.state.onFinish?.({
message: { id: "a", role: "assistant", parts: [] },
isAbort: false,
isDisconnect: false,
isError: false,
});
});
// Gated on mountedRef: NOTHING is sent from the dead thread.
expect(h.state.sendMessage).not.toHaveBeenCalled();
});
});
// #396: in autonomous mode a live sendNow must additionally request the
// AUTHORITATIVE server stop of the detached run (a local abort is only a client
// disconnect the server ignores) and arm a bounded 409 retry so the re-POST
@@ -659,13 +659,7 @@ export default function ChatThread({
return;
}
if (isAbort || isDisconnect || isError) return;
// Gate the final flush on the live-mount flag (#486): a clean onFinish can
// land AFTER this thread unmounted (a New-chat / chat-switch mid-stream —
// the async attach/resume settles late). Flushing then dequeues and POSTs a
// queued message from an abandoned thread — a "ghost" send / ghost chat.
// Every other queue side effect already guards on mountedRef; this last one
// was the gap.
if (mountedRef.current) flushNext();
flushNext();
},
// `onError` runs in addition to `onFinish` (which ai@6 also calls on error).
// Log the raw failure here for devtools; the UI shows a friendly classified
@@ -47,13 +47,6 @@ interface MessageItemProps {
* agent's raw query/argument text.
*/
showInput?: boolean;
/**
* Forwarded to ToolCallCard: whether a failed tool card renders its raw
* errorText. Defaults to true (internal chat). The public share passes false so
* internal detail in a tool error is never painted (belt to the server-side
* byte sanitization).
*/
showErrors?: boolean;
/**
* Neutralize internal/relative markdown links in the rendered answer (drop
* their href so they become inert text). Defaults to false (internal chat,
@@ -132,7 +125,6 @@ function MessageItem({
message,
showCitations = true,
showInput = true,
showErrors = true,
neutralizeInternalLinks = false,
assistantName,
turnStreaming = false,
@@ -227,7 +219,6 @@ function MessageItem({
part={part as unknown as ToolUiPart}
showCitations={showCitations}
showInput={showInput}
showErrors={showErrors}
/>
);
}
@@ -293,7 +284,6 @@ export function arePropsEqual(
prev.signature === next.signature &&
prev.showCitations === next.showCitations &&
prev.showInput === next.showInput &&
prev.showErrors === next.showErrors &&
prev.neutralizeInternalLinks === next.neutralizeInternalLinks &&
prev.assistantName === next.assistantName &&
// The turn-end flip re-renders every row once (cheap, terminal event) —
@@ -32,12 +32,6 @@ interface MessageListProps {
* doesn't see the agent's raw query/argument text.
*/
showInput?: boolean;
/**
* Forwarded to MessageItem -> ToolCallCard: whether a failed tool card renders
* its raw errorText. Defaults to true (internal chat). The public share passes
* false so internal detail in a tool error is never painted.
*/
showErrors?: boolean;
/**
* Forwarded to MessageItem: neutralize internal/relative markdown links in
* the rendered answers (drop their href so they render as inert text).
@@ -133,7 +127,6 @@ export default function MessageList({
emptyState,
showCitations = true,
showInput = true,
showErrors = true,
neutralizeInternalLinks = false,
assistantName,
}: MessageListProps) {
@@ -224,7 +217,6 @@ export default function MessageList({
signature={messageSignature(message)}
showCitations={showCitations}
showInput={showInput}
showErrors={showErrors}
neutralizeInternalLinks={neutralizeInternalLinks}
assistantName={assistantName}
// Turn-level liveness, gated to the TAIL row: only the tail message
@@ -30,16 +30,6 @@ interface ToolCallCardProps {
* the extra summary line, leaving the card (the action log) intact.
*/
showInput?: boolean;
/**
* Whether to render the tool's raw errorText on a failed call. Defaults to true
* (the internal chat, where the operator may debug). The public share passes
* false: a tool error string can carry internal detail (an internal page title,
* a stack fragment, a provider message). This is the RENDER gate only — the
* authoritative fix also sanitizes the bytes server-side (see
* PublicShareChatToolsService.forShare), so a share reader never receives raw
* error text over the wire, not just never sees it painted (#394).
*/
showErrors?: boolean;
}
/**
@@ -51,7 +41,6 @@ export default function ToolCallCard({
part,
showCitations = true,
showInput = true,
showErrors = true,
}: ToolCallCardProps) {
const { t } = useTranslation();
const toolName = getToolName(part);
@@ -85,7 +74,7 @@ export default function ToolCallCard({
</Text>
)}
{state === "error" && showErrors && part.errorText && (
{state === "error" && part.errorText && (
<Text size="xs" c="red" mt={2}>
{part.errorText}
</Text>
@@ -23,25 +23,6 @@ describe("describeChatError", () => {
});
});
it("classifies an A_RUN_BEGIN_FAILED 503 as a temporary run-start failure, NOT provider-not-configured (#486)", () => {
// The FULL real body the server writes for a beginRun failure: a
// ServiceUnavailableException(object) whose response is serialized verbatim
// onto the raw socket, self-describing statusCode 503 + the run-start code.
const body =
'{"message":"Could not start the agent run. This is usually temporary — please try again.","code":"A_RUN_BEGIN_FAILED","statusCode":503}';
expect(describeChatError(body, t)).toEqual({
title: "Could not start the run",
detail:
"The agent run could not be started. This is usually temporary — please try again.",
});
// ORDER GUARD: even though the body ALSO carries statusCode 503 (which the
// generic branch matches), the A_RUN_BEGIN_FAILED branch runs first, so it is
// never mislabeled "AI provider not configured".
expect(describeChatError(body, t).title).not.toBe(
"AI provider not configured",
);
});
it("classifies a dropped connection (ECONNRESET) as a lost-connection error", () => {
expect(
describeChatError("Cannot connect to API: read ECONNRESET", t).title,
@@ -24,21 +24,6 @@ export function describeChatError(
): ChatErrorView {
const msg = message ?? "";
// Our own "could not start the run" gate (A_RUN_BEGIN_FAILED, #486): a 503
// whose body carries this code is a TEMPORARY server-side failure while
// starting the run (e.g. a DB-pool blip), NOT an unconfigured provider. It MUST
// be matched STRICTLY BEFORE the generic 503 branch below, which would
// otherwise mislabel it "The AI provider is not configured" and tell the user
// to call an admin instead of just retrying.
if (/"code"\s*:\s*"A_RUN_BEGIN_FAILED"/.test(msg)) {
return {
title: t("Could not start the run"),
detail: t(
"The agent run could not be started. This is usually temporary — please try again.",
),
};
}
if (/"statusCode"\s*:\s*403\b/.test(msg)) {
return {
title: t("AI chat is disabled"),
@@ -1,7 +1,7 @@
import { EditorContent, ReactNodeViewRenderer, useEditor } from "@tiptap/react";
import { Placeholder } from "@tiptap/extension-placeholder";
import { StarterKit } from "@tiptap/starter-kit";
import { Mention, LinkExtension } from "@docmost/editor-ext";
import { Mention, LinkExtension, Code } from "@docmost/editor-ext";
import classes from "./comment.module.css";
import { useFocusWithin } from "@mantine/hooks";
import clsx from "clsx";
@@ -44,7 +44,12 @@ const CommentEditor = forwardRef(
gapcursor: false,
dropcursor: false,
link: false,
// #515: use the shared editor-ext `Code` (excludes: "") instead of
// StarterKit's excluding one, so inline code in a comment can carry
// other marks and does not drop them when the comment is edited.
code: false,
}),
Code,
Placeholder.configure({
placeholder: placeholder || t("Reply..."),
}),
@@ -1,6 +1,5 @@
import { markInputRule } from "@tiptap/core";
import { StarterKit } from "@tiptap/starter-kit";
import { Code } from "@tiptap/extension-code";
import { TextAlign } from "@tiptap/extension-text-align";
import { TaskList, TaskItem } from "@tiptap/extension-list";
import { Placeholder, CharacterCount, UndoRedo } from "@tiptap/extensions";
@@ -67,6 +66,7 @@ import {
FootnoteReference,
FootnotesList,
FootnoteDefinition,
Code,
} from "@docmost/editor-ext";
import {
randomElement,
@@ -153,6 +153,10 @@ export const mainExtensions = [
codeBlock: false,
code: false,
}),
// Base `Code` comes from @docmost/editor-ext, which overrides `excludes: ""`
// (#515) so inline code can co-occur with bold/italic/… — the SINGLE shared
// source also used by the collab server and comment editor. Here we keep the
// existing client-only behavior on top of it:
// Override TipTap's Code extension to fix the inline code input rule.
// The upstream regex /(^|[^`])`([^`]+)`(?!`)$/ captures the character
// before the opening backtick as part of the match, causing markInputRule
@@ -9,7 +9,7 @@ import { Italic } from "@tiptap/extension-italic";
import { Link } from "@tiptap/extension-link";
import { gitmostInsertTranscriptIntoEditor } from "./gitmost-recording.ts";
const ZWSP = "​"; // U+200B, the helper's block-trigger neutralizer
const ZWSP = "​"; // U+200B — asserted ABSENT (the block-escape lives in the serializer now)
/**
* #377 — the web-side bridge must append the native host's transcript below the
@@ -18,8 +18,9 @@ const ZWSP = "​"; // U+200B, the helper's block-trigger neutralizer
* regression would be caught), asserting the resulting document rather than
* mocking the editor: transcript present -> "Transcript" heading + one paragraph
* per non-empty line; content is inserted as LITERAL TEXT (no HTML/markdown
* parsing); col-0 markdown block triggers are neutralized so git-sync keeps them
* paragraphs; absent/empty/non-string -> no-op.
* parsing); col-0 markdown block triggers are stored verbatim (the git-sync
* serializer block-escapes them, so no client-side ZWSP is needed);
* absent/empty/non-string -> no-op.
*/
describe("gitmostInsertTranscriptIntoEditor", () => {
const makeEditor = () =>
@@ -91,19 +92,22 @@ describe("gitmostInsertTranscriptIntoEditor", () => {
editor.destroy();
});
it("neutralizes col-0 markdown block triggers with a leading ZWSP (git-sync safety)", () => {
it("inserts col-0 markdown block triggers as verbatim paragraph text (no ZWSP workaround)", () => {
const editor = makeEditor();
// Trigger lines (some with a leaked indent) + a normal prefixed line.
// Trigger lines (some with a leaked indent) + a normal prefixed line. The
// git-sync serializer now block-escapes a leading trigger itself, so the
// bridge inserts each line's TEXT byte-exact (only the leaked indent is
// trimmed) — no invisible ZWSP is prepended anymore.
const inserted = gitmostInsertTranscriptIntoEditor(
editor,
[
"- dash",
" > quote", // leading indent must be trimmed then neutralized
" > quote", // leading indent is trimmed, text otherwise verbatim
"# hash",
"1. one",
"> [!info] note",
"```js",
"---", // solid thematic break -> horizontalRule (text-losing) if unneutralized
"---",
"***",
"___",
"You: normal line",
@@ -116,20 +120,23 @@ describe("gitmostInsertTranscriptIntoEditor", () => {
.map((n: any) => n.content?.[0]?.text)
.filter((t: any) => typeof t === "string") as string[];
// Every block-trigger line is prefixed with the invisible ZWSP (indent
// trimmed first); the normal `You:` line is left byte-exact.
// Each trigger line is stored as its own byte-exact text (indent trimmed);
// the git-sync round-trip keeps it a paragraph via the serializer's
// block-escape, so no ZWSP is needed here.
expect(texts).toEqual([
ZWSP + "- dash",
ZWSP + "> quote",
ZWSP + "# hash",
ZWSP + "1. one",
ZWSP + "> [!info] note",
ZWSP + "```js",
ZWSP + "---",
ZWSP + "***",
ZWSP + "___",
"- dash",
"> quote",
"# hash",
"1. one",
"> [!info] note",
"```js",
"---",
"***",
"___",
"You: normal line",
]);
// Guard: no invisible ZWSP leaked into any inserted line.
for (const t of texts) expect(t).not.toContain(ZWSP);
editor.destroy();
});
@@ -240,45 +240,22 @@ export async function gitmostUploadFileToEditor(
}
}
// Zero-width space (U+200B). Prepended to a transcript line that begins with a
// markdown BLOCK trigger: it is invisible in the rendered doc but shifts the
// trigger off column 0, so the git-sync doc->markdown->doc round-trip keeps the
// line a plain paragraph (see GITMOST_MD_BLOCK_TRIGGER_RE).
const GITMOST_ZWSP = "​";
// A markdown BLOCK-level construct that, sitting at column 0 of a paragraph
// line, the git-sync markdown serializer (packages/prosemirror-markdown
// markdown-converter.ts, `case "paragraph"`) would re-parse into a NON-paragraph
// block on the doc->markdown->doc cycle. That serializer emits paragraph text
// verbatim with NO block-escape (the pre-existing root cause), so a leading
// `#`/`-`/`*`/`+`/`>`, an ordered-list `N.`/`N)`, a code fence ```/~~~, a table
// `|`, or a `> [!info]` callout opener would silently become a heading / list /
// quote / code block / table / callout. The final alternative matches a WHOLE-
// LINE thematic break — solid `---`/`***`/`___` or spaced `- - -`/`_ _ _` (3+ of
// the same `-`/`*`/`_`) — which round-trips into a `horizontalRule`; because
// that node carries NO text, an un-neutralized separator line would LOSE its
// text entirely (worse than the list/quote case). This matches a TRIMMED line's
// start; the transcript's own `You:` / `Speaker N:` prefix begins with a letter
// and never matches, so prefixed lines are left byte-exact.
const GITMOST_MD_BLOCK_TRIGGER_RE =
/^(?:#{1,6}(?:\s|$)|[-*+](?:\s|$)|>|\d+[.)](?:\s|$)|```|~~~|\||([-*_])(?:\s*\1){2,}\s*$)/;
// Append a transcript block BELOW the recording's audio node in a live editor:
// a "Transcript" heading followed by one paragraph per non-empty transcript
// line. The transcript is plain text, `\n`-separated, each line already
// formatted as `You: ...` / `Speaker N: ...` by the native host — line text is
// inserted as a TEXT node (never HTML/markdown), so there is no injection or
// mark-parsing surface. Each kept line is trimmed (drops an indent that would
// both leak into the display and, at col 0, form a markdown block trigger) and,
// if it still begins with a col-0 markdown block trigger, gets an invisible
// zero-width space prepended so the git-sync round-trip cannot turn it into a
// list/quote/heading/callout/code/table (defensive boundary against the
// serializer's missing block-escape). This is best-effort and meant to run
// AFTER the audio has already been inserted; the caller must guard against a
// throw so a transcript failure never fails the (already successful) recording.
// Returns true when a block was inserted, false when there was nothing to
// insert (transcript undefined/empty/not-a-string). A non-string value is a
// no-op, not an error.
// leak into the display). A line that begins with a col-0 markdown block
// trigger (`#`/`-`/`>`/`1.`/fence/`---`/…) needs no client-side workaround: the
// git-sync serializer (packages/prosemirror-markdown, `case "paragraph"`) now
// block-escapes such a leading trigger, so the doc->markdown->doc round-trip
// keeps the line a paragraph on its own — the former invisible-ZWSP defense is
// gone. This is best-effort and meant to run AFTER the audio has already been
// inserted; the caller must guard against a throw so a transcript failure never
// fails the (already successful) recording. Returns true when a block was
// inserted, false when there was nothing to insert (transcript
// undefined/empty/not-a-string). A non-string value is a no-op, not an error.
export function gitmostInsertTranscriptIntoEditor(
editor: Editor,
transcript: unknown,
@@ -288,13 +265,7 @@ export function gitmostInsertTranscriptIntoEditor(
.split("\n")
// Trim each line and drop blank (whitespace-only) ones.
.map((line) => line.trim())
.filter((line) => line.length > 0)
// Neutralize a col-0 markdown block trigger with an invisible ZWSP so the
// git-sync round-trip keeps the line a paragraph. Host lines (`You:` /
// `Speaker N:`) never match and stay byte-exact.
.map((line) =>
GITMOST_MD_BLOCK_TRIGGER_RE.test(line) ? GITMOST_ZWSP + line : line,
);
.filter((line) => line.length > 0);
if (lines.length === 0) return false;
const content = [
@@ -168,10 +168,6 @@ export default function ShareAiWidget({
// Anonymous reader: suppress the tool-argument summary line so the
// agent's raw query/argument text isn't shown on the public share.
showInput={false}
// Anonymous reader: never paint a tool's raw errorText (it can carry
// internal detail). This is the render gate; the bytes are also
// sanitized server-side in PublicShareChatToolsService.forShare (#394).
showErrors={false}
// Anonymous reader: neutralize internal/relative links in the
// assistant's markdown so internal UUIDs/auth-gated routes don't
// leak as clickable links (external http(s) links are kept).
+1
View File
@@ -41,6 +41,7 @@
"@aws-sdk/s3-request-presigner": "3.1050.0",
"@azure/storage-blob": "12.31.0",
"@clickhouse/client": "^1.18.2",
"@docmost/editor-ext": "workspace:*",
"@docmost/mcp": "workspace:*",
"@docmost/pdf-inspector": "1.9.6",
"@docmost/prosemirror-markdown": "workspace:*",
@@ -49,6 +49,7 @@ import {
FootnotesList,
FootnoteDefinition,
PageEmbed,
Code,
} from '@docmost/editor-ext';
import { convertProseMirrorToMarkdown } from '@docmost/prosemirror-markdown';
import { generateText, getSchema, JSONContent } from '@tiptap/core';
@@ -67,7 +68,12 @@ export const tiptapExtensions = [
link: false,
trailingNode: false,
heading: false,
// #515: replace StarterKit's bundled inline `code` (which inherits tiptap's
// `excludes: "_"`) with the shared editor-ext `Code` below, so the server's
// HTML->PM parse/export keeps code co-occurring with other marks.
code: false,
}),
Code,
Heading,
UniqueID.configure({
types: ['heading', 'paragraph', 'transclusionSource'],
@@ -1,109 +0,0 @@
import {
ConflictException,
Logger,
ServiceUnavailableException,
} from '@nestjs/common';
import { AiChatService } from './ai-chat.service';
import { RunAlreadyActiveError } from './ai-chat-run.service';
/**
* Fail-fast guard for beginRun failures (#486, commit 4).
*
* When runHooks.begin() rejects for a reason OTHER than RunAlreadyActiveError
* (e.g. a DB-pool blip), the turn must NOT continue untracked. The old code
* logged and streamed anyway, leaving a run with NO run-row: in autonomous mode
* nobody could abort it (/stop can't see it, disconnect doesn't abort it, and the
* one-run gate would admit a SECOND run) — an unstoppable invisible run until
* restart. The fix throws A_RUN_BEGIN_FAILED (503) BEFORE the first byte and
* before the user row is persisted.
*
* We drive `stream()` directly on a prototype instance wired with only the
* collaborators it touches before the throw, so the assertion is on the REAL
* control flow, not a mock of it.
*/
describe('AiChatService beginRun failure (#486)', () => {
function makeService(insertSpy: jest.Mock): AiChatService {
// Bypass the (heavy) DI constructor: exercise the real stream() method on a
// bare prototype instance with just the fields reached before the throw.
// `any` because the private `logger` field makes a typed intersection collapse.
const svc = Object.create(AiChatService.prototype);
svc.aiChatRepo = {
// Existing chat -> no insert path; chatId is kept as-is.
findById: jest.fn().mockResolvedValue({ id: 'chat1' }),
};
svc.aiChatMessageRepo = { insert: insertSpy };
svc.logger = new Logger('test');
return svc as AiChatService;
}
const baseArgs = () => {
const write = jest.fn();
const res = {
raw: { write, writableEnded: false, headersSent: false },
};
return {
user: { id: 'u1' } as never,
workspace: { id: 'w1' } as never,
sessionId: 's1',
// openPage undefined -> resolveOpenPageContext returns null without any DB
// call; chatId present -> the existing-chat path.
body: { chatId: 'chat1', messages: [] } as never,
res: res as never,
signal: new AbortController().signal,
model: {} as never,
role: null,
write,
};
};
it('throws A_RUN_BEGIN_FAILED (503) before the first byte and before persisting the user turn', async () => {
const insertSpy = jest.fn();
const svc = makeService(insertSpy);
const { write, ...args } = baseArgs();
const runHooks = {
begin: jest.fn().mockRejectedValue(new Error('DB pool exhausted')),
} as never;
let caught: unknown;
try {
await svc.stream({ ...args, runHooks });
} catch (e) {
caught = e;
}
expect(caught).toBeInstanceOf(ServiceUnavailableException);
const http = caught as ServiceUnavailableException;
expect(http.getStatus()).toBe(503);
expect(http.getResponse()).toMatchObject({ code: 'A_RUN_BEGIN_FAILED' });
// Fail-fast: nothing was written to the socket and NO user message row was
// persisted, so the turn left no orphan state to clean up.
expect(write).not.toHaveBeenCalled();
expect(insertSpy).not.toHaveBeenCalled();
});
it('still maps a lost-the-race RunAlreadyActiveError to a 409, not A_RUN_BEGIN_FAILED', async () => {
const insertSpy = jest.fn();
const svc = makeService(insertSpy);
const { write, ...args } = baseArgs();
const runHooks = {
begin: jest.fn().mockRejectedValue(new RunAlreadyActiveError('chat1')),
} as never;
let caught: unknown;
try {
await svc.stream({ ...args, runHooks });
} catch (e) {
caught = e;
}
expect(caught).toBeInstanceOf(ConflictException);
expect((caught as ConflictException).getResponse()).toMatchObject({
code: 'A_RUN_ALREADY_ACTIVE',
});
expect(write).not.toHaveBeenCalled();
expect(insertSpy).not.toHaveBeenCalled();
});
});
@@ -1,8 +1,4 @@
import {
ConflictException,
Logger,
ServiceUnavailableException,
} from '@nestjs/common';
import { ConflictException, Logger } from '@nestjs/common';
// Mock the AI SDK so we can PROVE no provider call is made for the turn we are
// about to reject. The race rejection happens at runHooks.begin(), long before
@@ -364,22 +360,22 @@ describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
});
/**
* F14 — the begin-failure branch (the `else` of the run-race guard).
* F14 — the begin-failure RESILIENCE branch (the `else` of the run-race guard).
*
* stream() wraps runHooks.begin in try/catch with TWO branches:
* - RunAlreadyActiveError -> 409 ConflictException (pinned above).
* - ANY OTHER begin failure -> throw ServiceUnavailableException(A_RUN_BEGIN_FAILED)
* BEFORE the first byte (#486, commit 4).
* - ANY OTHER begin failure -> SWALLOW + continue UNTRACKED on the socket signal
* (legacy fallback): it logs "...streaming without run tracking", leaves
* `effectiveSignal = signal` (runId undefined) and serves the turn anyway.
*
* POLICY CHANGE (#486): the OLD contract here was "SWALLOW + stream the turn
* UNTRACKED on the socket signal". That was reversed: an untracked run is
* invisible to /stop, is not aborted on disconnect, and slips past the one-run
* gate — an unstoppable ghost run in autonomous mode. Now a plain begin failure
* FAILS the turn fast with a 503 A_RUN_BEGIN_FAILED, before any user row is
* persisted and before streamText runs. This case is INVERTED (not deleted) so
* the "plain begin failure" path stays explicitly pinned under the new policy.
* The contract: a transient beginRun failure (e.g. a non-unique DB error inserting
* the run row) must STILL serve the user's turn — it must NOT re-throw and must NOT
* be misclassified as a 409. A regression that re-threw here would break EVERY turn
* on a begin failure with nothing to catch it. This branch is otherwise undriven by
* any spec, so it is pinned here SEPARATELY from the 409 path: a plain begin error
* proceeds to streamText with the SOCKET signal and still persists the user turn.
*/
describe('AiChatService.stream — begin-failure fails the turn (#184 F14 / #486)', () => {
describe('AiChatService.stream — begin-failure resilience / legacy fallback (#184 F14)', () => {
const streamTextMock = streamText as unknown as jest.Mock;
function makeStreamResult() {
@@ -459,7 +455,7 @@ describe('AiChatService.stream — begin-failure fails the turn (#184 F14 / #486
afterEach(() => jest.restoreAllMocks());
it('a PLAIN begin() failure (NOT RunAlreadyActiveError) FAILS the turn with a 503 A_RUN_BEGIN_FAILED before the first byte — NO untracked stream (#486)', async () => {
it('a PLAIN begin() failure (NOT RunAlreadyActiveError) does NOT 409 — it swallows, logs, and streams the turn UNTRACKED on the socket signal', async () => {
const errorSpy = jest
.spyOn(Logger.prototype, 'error')
.mockImplementation(() => undefined as never);
@@ -491,26 +487,28 @@ describe('AiChatService.stream — begin-failure fails the turn (#184 F14 / #486
} as never,
});
// NEW POLICY: the turn is REJECTED with a 503 A_RUN_BEGIN_FAILED (not a 409,
// and NOT swallowed into an untracked stream).
await expect(promise).rejects.toBeInstanceOf(ServiceUnavailableException);
const err = (await promise.catch(
(e) => e,
)) as ServiceUnavailableException;
expect(err.getStatus()).toBe(503);
expect(err.getResponse()).toMatchObject({ code: 'A_RUN_BEGIN_FAILED' });
// The turn proceeds: NO throw at all (in particular NOT a 409).
await expect(promise).resolves.toBeUndefined();
expect(begin).toHaveBeenCalledTimes(1);
// It logged the fail-the-turn line.
// The resilience branch logged the legacy-fallback warning.
expect(errorSpy).toHaveBeenCalledWith(
expect.stringContaining('failing the turn'),
expect.stringContaining('streaming without run tracking'),
expect.anything(),
);
// Fail-fast: the turn NEVER streamed — no user row persisted, no streamText
// call, so no orphan/untracked run was left behind.
expect(aiChatMessageRepo.insert).not.toHaveBeenCalled();
expect(streamTextMock).not.toHaveBeenCalled();
// The turn really streamed: the user message was persisted and streamText ran.
expect(aiChatMessageRepo.insert).toHaveBeenCalled();
expect(streamTextMock).toHaveBeenCalledTimes(1);
// The decisive wiring: with no run handle, the fallback uses the SOCKET signal
// (effectiveSignal = signal, runId undefined) — not a run-bound signal. #444:
// the signal is unioned with the degeneration controller via AbortSignal.any,
// so assert the socket abort still reaches the turn rather than identity.
const passed = streamTextMock.mock.calls[0][0].abortSignal as AbortSignal;
expect(passed.aborted).toBe(false);
socketController.abort();
expect(passed.aborted).toBe(true);
});
});
@@ -4,7 +4,6 @@ import {
Injectable,
Logger,
OnModuleInit,
ServiceUnavailableException,
} from '@nestjs/common';
import { FastifyReply } from 'fastify';
import {
@@ -56,7 +55,6 @@ import {
import {
isDegenerateOutput,
truncateDegeneratedTail,
shouldCheckDegeneration,
} from './output-degeneration';
// Max agent steps per turn. One step = one model generation; a step that calls
@@ -758,13 +756,6 @@ export class AiChatService implements OnModuleInit {
// or violate the page_id FK on insert (this runs after res.hijack(), so a
// DB error would break the stream).
const originPageId: string | null = openPageContext?.id ?? null;
// ORPHAN-ON-BEGIN-FAILURE tradeoff (#486, B3): the chat row is inserted
// HERE, before runHooks.begin below. If begin fails (e.g. a 503 / run-slot
// rejection) the turn aborts before the client is told this new chatId, so
// an empty chat is left behind and a retry mints ANOTHER one. We accept this
// over reordering: begin needs a chatId to bind the run to, and inserting
// the chat first keeps the id stable + the FK/history-join invariants above
// intact. Orphan empty chats are cheap and swept by normal chat cleanup.
const chat = await this.aiChatRepo.insert({
creatorId: user.id,
workspaceId: workspace.id,
@@ -806,32 +797,12 @@ export class AiChatService implements OnModuleInit {
code: 'A_RUN_ALREADY_ACTIVE',
});
}
// Any OTHER run-start failure (e.g. a DB-pool blip) must FAIL THE TURN,
// not silently stream without a run-row. The old fallback let the turn
// continue untracked: in autonomous mode nobody could then abort it —
// /stop can't see a run that doesn't exist, a client disconnect doesn't
// abort it, and the one-run-per-chat gate would let a SECOND run in. That
// is an unstoppable, invisible run until process restart. Reject NOW,
// BEFORE the first byte (nothing is written yet, no user row inserted, no
// MCP lease taken), so the controller's post-hijack catch turns this
// HttpException into an honest 503 on the raw socket. Same policy for BOTH
// modes — #487 inherits it (no mode-branching here).
// Any OTHER run-start failure must not break the turn — fall back to the
// socket signal (legacy behavior) and stream anyway.
this.logger.error(
`Failed to begin agent run (chat ${chatId}); failing the turn`,
`Failed to begin agent run (chat ${chatId}); streaming without run tracking`,
err as Error,
);
throw new ServiceUnavailableException({
message:
'Could not start the agent run. This is usually temporary — please try again.',
code: 'A_RUN_BEGIN_FAILED',
// Self-describe the status in the body: the controller's post-hijack
// catch writes getResponse() verbatim onto the raw socket, and an
// object-arg HttpException does NOT inject statusCode. Without it the
// client's 503 classifier (which reads the body JSON) could not see the
// status. With it present, the client's A_RUN_BEGIN_FAILED branch (which
// runs strictly before the generic-503 branch) shows "temporary, retry".
statusCode: 503,
});
}
}
@@ -1109,6 +1080,7 @@ export class AiChatService implements OnModuleInit {
const degenerationController = new AbortController();
let degenerationDetected = false;
let lastDegenerationCheckLen = 0;
const DEGENERATION_CHECK_STEP = 2000;
// Step-granular durability (#183): create the assistant row UPFRONT in the
// 'streaming' state (before any token), then UPDATE it as each step finishes
@@ -1282,10 +1254,8 @@ export class AiChatService implements OnModuleInit {
// trigger, abort the run ONCE with a distinguishable reason.
if (
!degenerationDetected &&
shouldCheckDegeneration(
inProgressText.length,
lastDegenerationCheckLen,
)
inProgressText.length - lastDegenerationCheckLen >=
DEGENERATION_CHECK_STEP
) {
lastDegenerationCheckLen = inProgressText.length;
if (isDegenerateOutput(inProgressText)) {
@@ -1305,13 +1275,6 @@ export class AiChatService implements OnModuleInit {
// the in-progress accumulator for the next step.
capturedSteps.push(step as StepLike);
inProgressText = '';
// Reset the degeneration-check watermark too (#486): it tracks a byte
// offset INTO inProgressText, so once that resets to '' a stale (large)
// mark makes `inProgressText.length - lastDegenerationCheckLen` go
// negative and the throttled detector stays silent until a later step's
// text re-grows past the old offset — a whole degenerate step could slip
// through undetected. Zeroing it re-arms the check from the next byte.
lastDegenerationCheckLen = 0;
// Step-granular durability (#183): persist this finished step (its text +
// tool calls + tool RESULTS) the moment it ends, so a process death after
// this point still recovers the step. Not awaited here (never block the
@@ -1,24 +1,11 @@
import { Logger } from '@nestjs/common';
import { streamText } from 'ai';
import {
hasRepeatedLineRun,
hasPeriodicTail,
isDegenerateOutput,
truncateDegeneratedTail,
shouldCheckDegeneration,
DEGENERATION_CHECK_STEP,
REPEATED_LINES_THRESHOLD,
MIN_PERIOD_REPEATS,
} from './output-degeneration';
import { AiChatService } from './ai-chat.service';
// Mock ONLY streamText so we can capture the onChunk/onStepFinish callbacks the
// service registers and drive them by hand; every other `ai` export the service
// uses (convertToModelMessages, stepCountIs, …) stays real.
jest.mock('ai', () => {
const actual = jest.requireActual('ai');
return { ...actual, streamText: jest.fn() };
});
/**
* Unit tests for the token-degeneration detector (#444) — the sole anti-babble
@@ -193,188 +180,3 @@ describe('truncateDegeneratedTail', () => {
expect(truncateDegeneratedTail(text)).toBe(text);
});
});
/**
* Throttle + step-boundary reset (#486). The stream keeps a watermark
* (`lastDegenerationCheckLen`) that is an OFFSET into the accumulated step text.
* On a step boundary the accumulator resets to '', so the watermark MUST reset to
* 0 too — otherwise the throttle goes silent for the whole next step. These tests
* pin the pure decision AND the reset property that ai-chat.service.onStepFinish
* now enforces.
*/
describe('shouldCheckDegeneration (throttle) + step-boundary reset (#486)', () => {
it('fires once the text grows a full DEGENERATION_CHECK_STEP past the mark', () => {
expect(shouldCheckDegeneration(DEGENERATION_CHECK_STEP, 0)).toBe(true);
expect(shouldCheckDegeneration(DEGENERATION_CHECK_STEP - 1, 0)).toBe(false);
expect(shouldCheckDegeneration(5000, 3000)).toBe(true); // grew 2000 since mark
expect(shouldCheckDegeneration(4000, 3000)).toBe(false); // grew only 1000
});
it('BUG (no reset): a stale large watermark silences the next step', () => {
// End of a long step: the watermark sits at 5000. The step ends and the
// accumulator resets to '' — but if the watermark is NOT reset, a fresh short
// degenerate burst (length 2000) never triggers a check: 2000 - 5000 < STEP.
const staleWatermark = 5000;
const nextStepLen = DEGENERATION_CHECK_STEP; // a fresh 2KB burst
expect(shouldCheckDegeneration(nextStepLen, staleWatermark)).toBe(false);
});
it('FIX (reset to 0): the same short degenerate burst IS checked and detected', () => {
// onStepFinish now zeroes the watermark, so the fresh burst re-arms the check.
const resetWatermark = 0;
const degenerateBurst = 'loadTools.\n'.repeat(300); // real degeneration
expect(degenerateBurst.length).toBeGreaterThanOrEqual(DEGENERATION_CHECK_STEP);
// The throttle now fires...
expect(
shouldCheckDegeneration(degenerateBurst.length, resetWatermark),
).toBe(true);
// ...and the detector catches the loop that would otherwise stream unchecked.
expect(isDegenerateOutput(degenerateBurst)).toBe(true);
});
});
/**
* BEHAVIOR guard for the ACTUAL fix (#486, ai-chat.service.onStepFinish resets
* lastDegenerationCheckLen to 0). The pure tests above use a hard-coded
* resetWatermark, so a REVERT of the real `lastDegenerationCheckLen = 0` line
* would not redden any of them. This drives the REAL onChunk/onStepFinish
* closures from stream() end to end and asserts the run is aborted when a fresh
* degenerate burst arrives in the step AFTER a long clean step — which only
* happens if the watermark was actually zeroed on the step boundary.
*/
describe('AiChatService: onStepFinish re-arms the degeneration watermark (#486)', () => {
const streamTextMock = streamText as unknown as jest.Mock;
function makeRes() {
return {
raw: {
writeHead: jest.fn(),
write: jest.fn(),
once: jest.fn(),
on: jest.fn(),
flushHeaders: jest.fn(),
writableEnded: false,
destroyed: false,
},
};
}
function makeService() {
const aiChatRepo = {
findById: jest.fn(async () => ({ id: 'chat-1', workspaceId: 'ws-1' })),
insert: jest.fn(),
};
const aiChatMessageRepo = {
insert: jest.fn(async () => ({ id: 'msg-1' })),
findAllByChat: jest.fn(async () => []),
update: jest.fn(async () => ({ id: 'msg-1' })),
};
const aiSettings = { resolve: jest.fn(async () => ({})) };
const tools = { forUser: jest.fn(async () => ({})) };
const mcpClients = {
toolsFor: jest.fn(async () => ({
tools: {},
clients: [],
outcomes: [],
instructions: [],
})),
};
return new AiChatService(
{} as never, // ai
aiChatRepo as never,
aiChatMessageRepo as never,
{} as never, // aiChatPageSnapshotRepo
aiSettings as never,
tools as never,
mcpClients as never,
{} as never, // aiAgentRoleRepo
{} as never, // pageRepo
{} as never, // pageAccess
{
isAiChatDeferredToolsEnabled: () => false,
// Lockdown OFF -> the degeneration guard is the active anti-babble path.
isAiChatFinalStepLockdownEnabled: () => false,
} as never, // environment
);
}
beforeEach(() => {
streamTextMock.mockReset();
jest.spyOn(Logger.prototype, 'log').mockImplementation(() => undefined as never);
jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined as never);
});
afterEach(() => jest.restoreAllMocks());
it('aborts on a fresh degenerate burst in the NEXT step (reverting the reset line reddens this)', async () => {
let captured:
| {
onChunk?: (e: { chunk: { type: string; text: string } }) => void;
onStepFinish?: (step: unknown) => void;
abortSignal?: AbortSignal;
}
| undefined;
streamTextMock.mockImplementation((opts: never) => {
captured = opts;
return {
consumeStream: jest.fn(),
pipeUIMessageStreamToResponse: jest.fn(),
};
});
const svc = makeService();
await svc.stream({
user: { id: 'user-1' } as never,
workspace: { id: 'ws-1' } as never,
sessionId: 'sess-1',
body: {
chatId: 'chat-1',
messages: [
{ id: 'm1', role: 'user', parts: [{ type: 'text', text: 'hi' }] },
],
} as never,
res: makeRes() as never,
signal: new AbortController().signal,
model: {} as never,
role: null,
// No runHooks -> legacy path (socket signal), degeneration guard active.
});
expect(streamTextMock).toHaveBeenCalledTimes(1);
const onChunk = captured!.onChunk!;
const onStepFinish = captured!.onStepFinish!;
const abortSignal = captured!.abortSignal!;
expect(abortSignal.aborted).toBe(false);
// STEP 1: a LONG, non-degenerate first step. Distinct lines never trip the
// detector, but they advance the throttle watermark far past the burst size
// that follows (to ~5x the step). This is the stale watermark that, WITHOUT
// the reset, would silence step 2.
let counter = 0;
let accumulated = 0;
while (accumulated < DEGENERATION_CHECK_STEP * 5) {
const line = `unique clean line number ${counter++} with distinct words\n`;
accumulated += line.length;
onChunk({ chunk: { type: 'text-delta', text: line } });
}
expect(abortSignal.aborted).toBe(false); // clean step must not abort
// STEP BOUNDARY: the real onStepFinish resets inProgressText AND (the fix)
// zeroes lastDegenerationCheckLen.
onStepFinish({ text: 'a clean first step', toolCalls: [], toolResults: [] });
// STEP 2: a FRESH, short degenerate burst (~3.3KB). Its length is far below
// the step-1 stale watermark (~10KB), so WITHOUT the reset the throttle stays
// silent and this streams unchecked. WITH the reset (watermark 0) it re-arms,
// the detector fires, and the run aborts.
const burst = 'loadTools.\n'.repeat(300);
expect(burst.length).toBeGreaterThanOrEqual(DEGENERATION_CHECK_STEP);
expect(burst.length).toBeLessThan(DEGENERATION_CHECK_STEP * 5);
onChunk({ chunk: { type: 'text-delta', text: burst } });
// The decisive assertion: the composed abortSignal (unioned with the
// degeneration controller) is now aborted. Reverting `lastDegenerationCheckLen
// = 0` in onStepFinish makes this stay false.
expect(abortSignal.aborted).toBe(true);
});
});
@@ -131,32 +131,6 @@ export function isDegenerateOutput(text: string): boolean {
return hasRepeatedLineRun(text) || hasPeriodicTail(text);
}
/**
* How many bytes the in-progress text must grow before the (amortized) tail
* heuristics are re-run. Shared with ai-chat.service so the throttle the stream
* applies is the SAME one the unit test drives.
*/
export const DEGENERATION_CHECK_STEP = 2000;
/**
* Throttle decision for the degeneration guard (#444/#486). Returns true when
* the accumulated text has grown at least DEGENERATION_CHECK_STEP bytes past the
* last-checked offset, so the pure rules only fire every ~2KB. Pure; the caller
* updates its watermark to `textLen` when this returns true.
*
* The watermark is an offset INTO the accumulator, so when the accumulator is
* reset to '' on a step boundary the caller MUST reset the watermark to 0 too
* (#486). Otherwise `textLen - lastCheckLen` goes negative after the reset and
* this returns false until a later step re-grows past the stale offset — a whole
* degenerate step could stream unchecked.
*/
export function shouldCheckDegeneration(
textLen: number,
lastCheckLen: number,
): boolean {
return textLen - lastCheckLen >= DEGENERATION_CHECK_STEP;
}
/**
* Truncate a degenerated tail before persist so hundreds of KB of garbage never
* reach the DB / replay (#444). Keeps everything up to and including the FIRST
@@ -1,241 +0,0 @@
// Break the editor-ext import chain (share.service -> collaboration.util ->
// @docmost/editor-ext -> @tiptap/core) that is unresolvable in this jest env and
// pre-existingly breaks these specs. jsonToMarkdown is never reached in these
// tests (the tools fail before rendering markdown).
jest.mock('../../collaboration/collaboration.util', () => ({
jsonToMarkdown: () => '',
}));
import { Logger } from '@nestjs/common';
import { MockLanguageModelV3, simulateReadableStream } from 'ai/test';
import { PublicShareChatService } from './public-share-chat.service';
import { PublicShareChatToolsService } from './tools/public-share-chat-tools.service';
/**
* SECURITY integration guard for #394 (commit 5): a tool's or the provider's raw
* error text must NOT leak to an anonymous public-share reader.
*
* The render gate (ToolCallCard showErrors=false) hides the text in the DOM but
* NOT on the wire, so this test asserts on the RAW SSE BYTES the server writes —
* exactly the channel the render gate masks. We drive the real
* PublicShareChatService.stream() with a real share toolset (its underlying
* services mocked to fail) and a mock model, then inspect every byte piped to the
* fake socket.
*/
// A minimal ServerResponse stand-in that records every written chunk.
class FakeSocket {
chunks: string[] = [];
statusCode = 200;
writableEnded = false;
destroyed = false;
headersSent = false;
writeHead(): this {
this.headersSent = true;
return this;
}
setHeader(): void {}
removeHeader(): void {}
getHeader(): undefined {
return undefined;
}
flushHeaders(): void {}
write(chunk: unknown): boolean {
this.chunks.push(
typeof chunk === 'string' ? chunk : Buffer.from(chunk as never).toString('utf8'),
);
return true;
}
end(chunk?: unknown): void {
if (chunk) this.write(chunk);
this.writableEnded = true;
}
on(): this {
return this;
}
once(): this {
return this;
}
get body(): string {
return this.chunks.join('');
}
}
/** Mock model that issues one getSharePage tool call, then finishes with text. */
function toolCallingModel(): MockLanguageModelV3 {
let call = 0;
return new MockLanguageModelV3({
doStream: async () => {
call++;
if (call === 1) {
return {
stream: simulateReadableStream({
chunks: [
{ type: 'stream-start' as const, warnings: [] },
{ type: 'tool-input-start' as const, id: 't1', toolName: 'getSharePage' },
{ type: 'tool-input-end' as const, id: 't1' },
{
type: 'tool-call' as const,
toolCallId: 't1',
toolName: 'getSharePage',
input: '{"pageId":"secret-page"}',
},
{
type: 'finish' as const,
finishReason: { unified: 'tool-calls' as const, raw: 'tool_calls' },
usage: {
inputTokens: { total: 1, noCache: undefined, cacheRead: undefined, cacheWrite: undefined },
outputTokens: { total: 1, text: 1, reasoning: undefined },
},
},
],
}),
};
}
return {
stream: simulateReadableStream({
chunks: [
{ type: 'stream-start' as const, warnings: [] },
{ type: 'text-start' as const, id: '1' },
{ type: 'text-delta' as const, id: '1', delta: 'Sorry.' },
{ type: 'text-end' as const, id: '1' },
{
type: 'finish' as const,
finishReason: { unified: 'stop' as const, raw: 'stop' },
usage: {
inputTokens: { total: 1, noCache: undefined, cacheRead: undefined, cacheWrite: undefined },
outputTokens: { total: 1, text: 1, reasoning: undefined },
},
},
],
}),
};
},
});
}
/** Mock model whose stream emits a provider error carrying an internal secret. */
function providerErrorModel(secret: string): MockLanguageModelV3 {
return new MockLanguageModelV3({
doStream: async () => ({
stream: simulateReadableStream({
chunks: [
{ type: 'stream-start' as const, warnings: [] },
{
type: 'error' as const,
error: {
statusCode: 503,
message: 'Service Unavailable',
responseBody: `upstream ${secret} model=internal-gpt`,
},
},
],
}),
}),
});
}
function makeService(toolsService: PublicShareChatToolsService): {
svc: PublicShareChatService;
logSpy: jest.SpyInstance;
} {
const svc = Object.create(PublicShareChatService.prototype);
const logger = new Logger('test');
const logSpy = jest.spyOn(logger, 'error').mockImplementation(() => undefined);
jest.spyOn(logger, 'warn').mockImplementation(() => undefined);
svc.tools = toolsService;
svc.logger = logger;
svc.tokenBudget = { record: jest.fn().mockResolvedValue(undefined) };
return { svc, logSpy };
}
async function runStream(
svc: PublicShareChatService,
model: MockLanguageModelV3,
): Promise<FakeSocket> {
const socket = new FakeSocket();
await svc.stream({
workspaceId: 'ws1',
shareId: 'share1',
share: { id: 'share1', pageId: 'p1', sharedPage: { id: 'p1', title: 'Docs' } },
openedPage: null,
messages: [
{ id: 'm1', role: 'user', parts: [{ type: 'text', text: 'read the page' }] } as never,
],
res: { raw: socket } as never,
signal: new AbortController().signal,
model: model as never,
role: null,
});
// Let the piped stream drain fully.
await new Promise((r) => setTimeout(r, 300));
return socket;
}
describe('public share chat error leak (#394)', () => {
afterEach(() => jest.restoreAllMocks());
it('does NOT leak a tool\'s raw internal error to the SSE bytes (generic classified string instead)', async () => {
const SECRET = 'INTERNAL_baseUrl_http://provider.internal:8080/v1';
const shareService = {
// The canonical boundary throws a RAW internal error (with a secret).
resolveReadableSharePage: jest
.fn()
.mockRejectedValue(new Error(`db failed at ${SECRET} stack@line42`)),
};
const tools = new PublicShareChatToolsService(
shareService as never,
{} as never,
{} as never,
);
const { svc } = makeService(tools);
const socket = await runStream(svc, toolCallingModel());
// The tool-output-error frame is present on the wire...
expect(socket.body).toContain('tool-output-error');
// ...but it carries ONLY the generic classified string — never the secret,
// the raw driver message, or a stack fragment.
expect(socket.body).toContain('The tool could not complete the request.');
expect(socket.body).not.toContain(SECRET);
expect(socket.body).not.toContain('stack@line42');
expect(socket.body).not.toContain('db failed');
});
it('passes a SAFE ShareToolError message (page not available) through to the bytes', async () => {
const shareService = {
// Not found in this share -> the tool throws the classified SAFE message.
resolveReadableSharePage: jest.fn().mockResolvedValue(null),
};
const tools = new PublicShareChatToolsService(
shareService as never,
{} as never,
{} as never,
);
const { svc } = makeService(tools);
const socket = await runStream(svc, toolCallingModel());
expect(socket.body).toContain('tool-output-error');
expect(socket.body).toContain('not available in this share');
});
it('does NOT leak a provider error (statusCode + response body) to the SSE bytes', async () => {
const SECRET = 'http://provider.internal:8080';
const tools = new PublicShareChatToolsService(
{} as never,
{} as never,
{} as never,
);
const { svc, logSpy } = makeService(tools);
const socket = await runStream(svc, providerErrorModel(SECRET));
// The anon sees a fixed classified string, not the provider body/baseUrl/model.
expect(socket.body).toContain('temporarily unavailable');
expect(socket.body).not.toContain(SECRET);
expect(socket.body).not.toContain('internal-gpt');
// The FULL provider detail is logged server-side only.
const logged = logSpy.mock.calls.map((c) => String(c[0])).join('\n');
expect(logged).toContain(SECRET);
});
});
@@ -12,10 +12,7 @@ import { AiAgentRoleRepo } from '@docmost/db/repos/ai-agent-roles/ai-agent-roles
import { AiAgentRole } from '@docmost/db/types/entity.types';
import { AiService } from '../../integrations/ai/ai.service';
import { AiSettingsService } from '../../integrations/ai/ai-settings.service';
import {
PublicShareChatToolsService,
ShareToolError,
} from './tools/public-share-chat-tools.service';
import { PublicShareChatToolsService } from './tools/public-share-chat-tools.service';
import { buildShareSystemPrompt } from './public-share-chat.prompt';
import { roleModelOverride } from './roles/role-model-config';
import {
@@ -105,30 +102,6 @@ export function filterShareTranscript(messages: UIMessage[]): UIMessage[] {
);
}
/**
* Fixed, classified strings an ANONYMOUS share reader may see when the assistant
* stream fails (#394). These reveal NOTHING about the internal provider, its
* baseUrl, the model name, or the raw response body — unlike describeProviderError
* (which is for the server log / the authenticated operator only). We classify by
* HTTP status where available so the reader still gets a useful hint (retry vs.
* give up) without any internal detail.
*/
export function classifyAnonStreamError(error: unknown): string {
const status =
typeof error === 'object' && error !== null
? (error as { statusCode?: number }).statusCode
: undefined;
if (status === 429) {
return 'The assistant is receiving too many requests right now. Please try again shortly.';
}
if (typeof status === 'number' && status >= 500) {
return 'The assistant is temporarily unavailable. Please try again.';
}
// Any other failure (including a bare connection error with no status): a
// single neutral line. No provider identity, no config, no response body.
return 'The assistant could not complete your request. Please try again.';
}
/**
* Anonymous, read-only AI assistant for a single PUBLIC share tree.
*
@@ -345,28 +318,11 @@ export class PublicShareChatService {
result.pipeUIMessageStreamToResponse(res.raw, {
headers: { 'X-Accel-Buffering': 'no' },
onError: (error: unknown) => {
// SECURITY (#394): the string this returns is written verbatim into the
// SSE error frame delivered to an ANONYMOUS reader (for a tool failure
// it becomes the atomic `tool-output-error` frame's errorText; for a
// stream/provider failure, the terminal error frame).
//
// A ShareToolError is already a classified, safe tool message (see
// PublicShareChatToolsService.wrapToolErrors) — pass it through so the
// reader still gets the useful "page not available in this share" hint.
if (error instanceof ShareToolError) {
return error.message;
}
// Anything else is a provider/stream error. describeProviderError
// bundles the provider statusCode AND response body, which can carry the
// internal baseUrl or model name — NEVER expose that to the public. Log
// the full detail server-side only and return a fixed classified string.
this.logger.error(
`Public share chat pipe error: ${describeProviderError(
error,
'AI stream error',
)}`,
);
return classifyAnonStreamError(error);
// Reuse the shared formatter so provider error formatting stays
// unified between the log line and the streamed error message — a
// share reader sees 402/429/503 causes consistently with the
// authenticated path.
return describeProviderError(error, 'AI stream error');
},
});
@@ -808,7 +808,7 @@ describe('PublicShareChatToolsService share scoping', () => {
};
await expect(getSharePage.execute({ pageId: 'p-outside' })).rejects.toThrow(
/not available in this share/i,
/not part of this published share/i,
);
// The tool delegated the resolve to the canonical boundary with the
// forShare-scoped shareId, and returned NO content for a non-resolving page.
@@ -841,7 +841,7 @@ describe('PublicShareChatToolsService share scoping', () => {
await expect(
getSharePage.execute({ pageId: 'p-restricted' }),
).rejects.toThrow(/not available in this share/i);
).rejects.toThrow(/not part of this published share/i);
// No content was ever sanitized/returned for the blocked page.
expect(shareService.updatePublicAttachments).not.toHaveBeenCalled();
});
@@ -1003,7 +1003,7 @@ describe('public-share assistant boundary locks (red-team regression guards)', (
};
await expect(
getSharePage.execute({ pageId: 'p-elsewhere' }),
).rejects.toThrow(/not available in this share/i);
).rejects.toThrow(/not part of this published share/i);
// The forged share id is the scope the boundary re-derivation rejects against.
expect(shareService.resolveReadableSharePage).toHaveBeenCalledWith(
'FORGED-SHARE',
@@ -1,15 +1,7 @@
import { createHash } from 'node:crypto';
import {
mkdtempSync,
mkdirSync,
writeFileSync,
rmSync,
readdirSync,
statSync,
readFileSync,
} from 'node:fs';
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { dirname, join, relative, sep } from 'node:path';
import { join } from 'node:path';
import { computeSrcRegistryStamp } from './docmost-client.loader';
@@ -38,14 +30,10 @@ function assertStaleGuard(
}
}
// Build a throwaway `<pkg>/build/index.js` + optional `<pkg>/src/` tree so
// `computeSrcRegistryStamp(<pkg>/build/index.js)` resolves src the same way the
// loader does (dirname(dirname(entry))/src). Since #486 the stamp hashes the WHOLE
// src tree, so a fixture is a { relPath: content } map. A bare string is sugar for
// a single `tool-specs.ts`; `null` means "no src tree" (the prod no-op path).
function makeFakePackage(
src: string | Record<string, string> | null,
): {
// Build a throwaway `<pkg>/build/index.js` + optional `<pkg>/src/tool-specs.ts`
// layout so `computeSrcRegistryStamp(<pkg>/build/index.js)` resolves src the same
// way the loader does (dirname(dirname(entry))/src/tool-specs.ts).
function makeFakePackage(toolSpecsSource: string | null): {
entry: string;
cleanup: () => void;
} {
@@ -54,15 +42,10 @@ function makeFakePackage(
mkdirSync(buildDir, { recursive: true });
const entry = join(buildDir, 'index.js');
writeFileSync(entry, '// fake @docmost/mcp build entry\n', 'utf8');
if (src !== null) {
const files =
typeof src === 'string' ? { 'tool-specs.ts': src } : src;
if (toolSpecsSource !== null) {
const srcDir = join(root, 'src');
for (const [rel, content] of Object.entries(files)) {
const full = join(srcDir, rel);
mkdirSync(dirname(full), { recursive: true });
writeFileSync(full, content, 'utf8');
}
mkdirSync(srcDir, { recursive: true });
writeFileSync(join(srcDir, 'tool-specs.ts'), toolSpecsSource, 'utf8');
}
return { entry, cleanup: () => rmSync(root, { recursive: true, force: true }) };
}
@@ -110,109 +93,34 @@ describe('computeSrcRegistryStamp (#447 stale-build guard)', () => {
}
});
// #486 CORE (negative): an edit to a NON-tool-specs src file (client.ts) with a
// rebuild NOT run must move the src stamp away from the built REGISTRY_STAMP, so
// the loader's stale-check refuses. Under the old tool-specs.ts-only hash this
// edit was invisible and a stale build/ served the old client.ts silently.
it('a client.ts edit (no rebuild) moves the src stamp -> loader refuses (#486)', () => {
// "Built" state: the package as it was compiled.
const built = makeFakePackage({
'tool-specs.ts': 'export const SPECS = 1;\n',
'client.ts': "export const impl = 'v1';\n",
});
// "Dev edited src, forgot to rebuild": client.ts changed, tool-specs.ts not.
const edited = makeFakePackage({
'tool-specs.ts': 'export const SPECS = 1;\n',
'client.ts': "export const impl = 'v2';\n",
});
try {
const builtStamp = computeSrcRegistryStamp(built.entry);
const editedStamp = computeSrcRegistryStamp(edited.entry);
expect(builtStamp).not.toBeNull();
expect(editedStamp).not.toBe(builtStamp);
// build/ still carries builtStamp; src now hashes to editedStamp -> refuse.
expect(() => assertStaleGuard(editedStamp, builtStamp as string)).toThrow(
STALE_BUILD_MESSAGE,
);
} finally {
built.cleanup();
edited.cleanup();
}
});
// *.generated.ts is excluded (the codegen's own output — a fixed-point cycle
// otherwise): its presence/content must not move the stamp.
it('excludes *.generated.ts from the stamp', () => {
const without = makeFakePackage({ 'tool-specs.ts': 'x\n' });
const withGen = makeFakePackage({
'tool-specs.ts': 'x\n',
'registry-stamp.generated.ts': 'export const REGISTRY_STAMP = "abc";\n',
});
try {
expect(computeSrcRegistryStamp(withGen.entry)).toBe(
computeSrcRegistryStamp(without.entry),
);
} finally {
without.cleanup();
withGen.cleanup();
}
});
// CROSS-IMPL EQUALITY (covers reviewer suggestion 2). The SAME fixed tree and
// CROSS-IMPL EQUALITY (covers reviewer suggestion 2). The SAME fixed input and
// EXPECTED hash are asserted in the mcp-side node test
// (packages/mcp/test/unit/registry-stamp.test.mjs) against the codegen's
// `computeRegistryStamp`. Asserting the SAME pair here against the loader's
// `computeSrcRegistryStamp` proves both implementations enumerate+normalize+hash
// `computeSrcRegistryStamp` proves both implementations normalize+hash
// identically; a divergence in EITHER side reddens one of the two tests.
const CROSS_IMPL_TREE = {
'tool-specs.ts': 'line1\r\nline2\n',
'client/read.ts': 'export const R = 1;\n',
'registry-stamp.generated.ts': 'export const REGISTRY_STAMP="ignored";\n',
};
const CROSS_IMPL_EXPECTED =
'131c1b9e4e2f5a7d6cef91ca8df619822b442f52bc45ebd09474a4c1d6728616';
it('matches the documented cross-impl hash for a fixed tree', () => {
const { entry, cleanup } = makeFakePackage(CROSS_IMPL_TREE);
it('matches the documented cross-impl hash for a fixed input', () => {
const FIXED_INPUT = 'line1\r\nline2\n';
const EXPECTED =
'683376e290829b482c2655745caffa7a1dccfa10afaa62dac2b42dd6c68d0f83';
const { entry, cleanup } = makeFakePackage(FIXED_INPUT);
try {
expect(computeSrcRegistryStamp(entry)).toBe(CROSS_IMPL_EXPECTED);
expect(computeSrcRegistryStamp(entry)).toBe(EXPECTED);
} finally {
cleanup();
}
});
it('the documented EXPECTED is the enumerate+normalize+sha256 of the tree', () => {
// Proves EXPECTED is not a magic constant but the documented computation — a
// local re-implementation of the loader's tree walk.
const { entry, cleanup } = makeFakePackage(CROSS_IMPL_TREE);
it('the documented EXPECTED is the normalize+sha256 of the fixed input', () => {
// Proves EXPECTED is not a magic constant but the documented computation.
const FIXED_INPUT = 'line1\r\nline2\n';
const normalized = FIXED_INPUT.replace(/\r\n/g, '\n').replace(/\n$/, '');
const expected = createHash('sha256')
.update(normalized, 'utf8')
.digest('hex');
const { entry, cleanup } = makeFakePackage(FIXED_INPUT);
try {
const srcDir = join(dirname(dirname(entry)), 'src');
const collect = (dir: string): string[] => {
const out: string[] = [];
for (const e of readdirSync(dir)) {
const f = join(dir, e);
if (statSync(f).isDirectory()) out.push(...collect(f));
else if (e.endsWith('.ts') && !e.endsWith('.generated.ts'))
out.push(f);
}
return out;
};
const files = collect(srcDir)
.map((abs) => ({ rel: relative(srcDir, abs).split(sep).join('/'), abs }))
.sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0));
const h = createHash('sha256');
for (const { rel, abs } of files) {
const n = readFileSync(abs, 'utf8')
.replace(/\r\n/g, '\n')
.replace(/\n$/, '');
h.update(rel, 'utf8');
h.update('\0', 'utf8');
h.update(n, 'utf8');
h.update('\0', 'utf8');
}
const localHash = h.digest('hex');
expect(computeSrcRegistryStamp(entry)).toBe(localHash);
expect(localHash).toBe(CROSS_IMPL_EXPECTED);
expect(computeSrcRegistryStamp(entry)).toBe(expected);
} finally {
cleanup();
}
@@ -1,6 +1,6 @@
import { createHash } from 'node:crypto';
import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs';
import { dirname, join, relative, sep } from 'node:path';
import { existsSync, readFileSync } from 'node:fs';
import { dirname, join } from 'node:path';
import { pathToFileURL } from 'node:url';
import type { DocmostClient, SharedToolSpec } from '@docmost/mcp';
@@ -191,52 +191,33 @@ interface DocmostMcpModule {
* present. Returns the stamp string, or `null` when the source is absent (a prod
* image ships only build/, no src/). MUST stay byte-for-byte identical to
* packages/mcp/scripts/gen-registry-stamp.mjs's `computeRegistryStamp` so the
* build-time and src-time hashes agree: same file set (every src/**\/*.ts except
* *.generated.ts), same POSIX-relative sort, same per-file normalization (CRLF ->
* LF, strip a single trailing newline) with the same path+content framing, same
* sha256. Hashing the WHOLE src tree (not just tool-specs.ts) is #486: an edit to
* client.ts / a client/* module / comment-signal / drawio-* without a rebuild
* must also be caught, otherwise build/ silently serves the old code.
* build-time and src-time hashes agree: same input file (src/tool-specs.ts), same
* normalization (CRLF -> LF, strip a single trailing newline), same sha256.
*
* DEV vs PROD detection is by FILE EXISTENCE, not NODE_ENV: we resolve the
* package's own directory from `require.resolve('@docmost/mcp')` (which points at
* build/index.js) and look for ../src next to it. In a dev/test worktree that
* directory exists; in a prod image (build/ only, src/ stripped) it does not, so
* this returns null and the caller skips the check. Any error (ENOENT, a bad
* resolve) is swallowed to null — the stale-check must NEVER break startup.
* build/index.js) and look for ../src/tool-specs.ts next to it. In a dev/test
* worktree that file exists; in a prod image (build/ only, src/ stripped) it does
* not, so this returns null and the caller skips the check. Any error (ENOENT, a
* bad resolve) is swallowed to null — the stale-check must NEVER break startup.
*
* Exported for unit testing (docmost-client.loader.spec.ts): the export keyword
* is behaviourally a no-op — the module-internal caller `loadDocmostMcp` is
* unaffected. The test drives the null (no-src) path and asserts this
* enumerate+normalize+sha256 stays identical to the codegen's
* `computeRegistryStamp`.
* normalize+sha256 stays identical to the codegen's `computeRegistryStamp`.
*/
export function computeSrcRegistryStamp(packageEntry: string): string | null {
try {
// packageEntry is <pkg>/build/index.js; the source lives at <pkg>/src/.
const srcDir = join(dirname(dirname(packageEntry)), 'src');
if (!existsSync(srcDir)) return null; // prod: no src tree -> skip.
// Enumerate every src/**\/*.ts except the codegen's own *.generated.ts
// output (including it would be a fixed-point cycle). Sort by POSIX-relative
// path so ordering is platform-independent, then fold each file's relative
// path + normalized content into one hash — identical to the codegen.
const files = collectStampFiles(srcDir)
.map((abs) => ({
rel: relative(srcDir, abs).split(sep).join('/'),
abs,
}))
.sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0));
const hash = createHash('sha256');
for (const { rel, abs } of files) {
const normalized = readFileSync(abs, 'utf8')
.replace(/\r\n/g, '\n')
.replace(/\n$/, '');
hash.update(rel, 'utf8');
hash.update('\0', 'utf8');
hash.update(normalized, 'utf8');
hash.update('\0', 'utf8');
}
return hash.digest('hex');
const toolSpecsPath = join(
dirname(dirname(packageEntry)),
'src',
'tool-specs.ts',
);
if (!existsSync(toolSpecsPath)) return null; // prod: no src tree -> skip.
const source = readFileSync(toolSpecsPath, 'utf8');
const normalized = source.replace(/\r\n/g, '\n').replace(/\n$/, '');
return createHash('sha256').update(normalized, 'utf8').digest('hex');
} catch {
// Never let a resolution/read hiccup break server startup — treat as "no
// src available" and skip the check (identical to the prod no-op path).
@@ -244,24 +225,6 @@ export function computeSrcRegistryStamp(packageEntry: string): string | null {
}
}
/**
* Recursively enumerate every `*.ts` under `dir`, EXCLUDING `*.generated.ts`.
* Mirror of the codegen's `collectStampFiles` (packages/mcp/scripts/
* gen-registry-stamp.mjs) — keep the two walk/filter rules identical.
*/
function collectStampFiles(dir: string): string[] {
const out: string[] = [];
for (const entry of readdirSync(dir)) {
const full = join(dir, entry);
if (statSync(full).isDirectory()) {
out.push(...collectStampFiles(full));
} else if (entry.endsWith('.ts') && !entry.endsWith('.generated.ts')) {
out.push(full);
}
}
return out;
}
// TS with module:commonjs downlevels a literal `import()` to `require()`, which
// cannot load the ESM-only `@docmost/mcp` package. Indirect through Function so
// the real dynamic `import()` survives compilation and can load ESM from
@@ -224,7 +224,7 @@ describe('PublicShareChatToolsService.forShare', () => {
(tools.getSharePage as unknown as ToolExec).execute({
pageId: 'page-1',
}),
).rejects.toThrow('The requested page is not available in this share.');
).rejects.toThrow('That page is not part of this published share.');
// No content is ever fetched/returned for a non-resolving page.
expect(shareService.updatePublicAttachments).not.toHaveBeenCalled();
@@ -7,22 +7,6 @@ import { PageRepo } from '@docmost/db/repos/page/page.repo';
import { jsonToMarkdown } from '../../../collaboration/collaboration.util';
import { modelFriendlyInput } from './model-friendly-input';
/**
* A tool error whose message is DELIBERATELY safe to expose to an anonymous
* share reader (and to the model, for self-correction). Every OTHER thrown error
* is treated as internal and replaced with a generic string by `wrapToolErrors`,
* so a raw exception message an internal page title, a DB/stack fragment, a
* driver detail never rides the public UI stream (#394).
*/
export class ShareToolError extends Error {}
// The only two classified strings an anonymous reader may ever see from a tool
// failure. The specific one keeps the model's self-correction useful ("try a
// different page"); the generic one reveals nothing about the internal fault.
const SHARE_TOOL_ERROR_NOT_AVAILABLE =
'The requested page is not available in this share.';
const SHARE_TOOL_ERROR_GENERIC = 'The tool could not complete the request.';
/**
* Isolated, READ-ONLY toolset for the ANONYMOUS public-share assistant.
*
@@ -60,7 +44,7 @@ export class PublicShareChatToolsService {
* are NO write tools, NO comments/history, NO cross-space or external tools.
*/
forShare(shareId: string, workspaceId: string): Record<string, Tool> {
return this.wrapToolErrors({
return {
searchSharePages: tool({
description:
'Search the pages of THIS published documentation share for a ' +
@@ -112,7 +96,7 @@ export class PublicShareChatToolsService {
execute: async ({ pageId }) => {
const id = (pageId ?? '').trim();
if (!id) {
throw new ShareToolError('A pageId is required.');
throw new Error('A pageId is required.');
}
// Resolve via the SINGLE canonical share-access boundary: confirms the
// page resolves to THIS share (recursive CTE up the tree, honouring
@@ -128,7 +112,7 @@ export class PublicShareChatToolsService {
workspaceId,
);
if (!resolved) {
throw new ShareToolError(SHARE_TOOL_ERROR_NOT_AVAILABLE);
throw new Error('That page is not part of this published share.');
}
const { page } = resolved;
@@ -209,57 +193,6 @@ export class PublicShareChatToolsService {
}
},
}),
});
}
/**
* Wrap every tool's `execute` so a THROWN error is sanitized in ONE place
* closing the byte leak, the render, and the model context at once (#394).
*
* The AI SDK surfaces a tool-execution throw as an atomic `tool-output-error`
* frame on the v6 UI stream whose `errorText` is the thrown message; on the
* public share that frame goes straight to an anonymous reader. Unwrapped, a
* raw exception (an internal page title, a DB/stack fragment, a driver detail)
* would ride that frame verbatim. Here we catch it, LOG the full detail
* server-side only, and re-throw a CLASSIFIED, safe error: the tool's own
* intentional ShareToolError messages pass through (they keep the model's
* self-correction useful), everything else collapses to a generic string.
*/
private wrapToolErrors(
tools: Record<string, Tool>,
): Record<string, Tool> {
const wrapped: Record<string, Tool> = {};
for (const [name, t] of Object.entries(tools)) {
const original = t.execute;
if (typeof original !== 'function') {
wrapped[name] = t;
continue;
}
wrapped[name] = {
...t,
execute: async (args: unknown, options: unknown) => {
try {
return await (
original as (a: unknown, o: unknown) => Promise<unknown>
)(args, options);
} catch (err) {
const safe =
err instanceof ShareToolError
? err.message
: SHARE_TOOL_ERROR_GENERIC;
// Full detail to the server log ONLY — never to the anon.
this.logger.warn(
`Public share tool "${name}" failed: ${
err instanceof Error ? err.message : String(err)
}`,
);
// This safe string is ALL that rides the tool-output-error frame,
// becomes model context, and could be rendered — one choke point.
throw new ShareToolError(safe);
}
},
} as Tool;
}
return wrapped;
};
}
}
@@ -120,102 +120,3 @@ describe('JwtStrategy — provenance derivation', () => {
expect(req.raw.actor).toBeUndefined();
});
});
/**
* Provenance derivation on the API-KEY path (jwt.strategy.validateApiKey, #486).
*
* The access-token path stamped provenance; the API-key path returned early
* WITHOUT it, so an is_agent API key's REST writes recorded no 'agent' marker.
* The API-key payload carries no signed claim, so provenance is resolved from the
* SERVER-SIDE user returned by ApiKeyService.validateApiKey: isAgent -> 'agent',
* otherwise 'user'; aiChatId is always null (an API key has no ai_chats row).
*
* The enterprise ApiKeyService is not bundled in the OSS build, so the strategy
* loads it through an overridable `resolveApiKeyService` seam that we stub here.
*/
describe('JwtStrategy — API-key provenance derivation (#486)', () => {
function makeApiKeyStrategy(validateApiKeyImpl: (p: any) => Promise<any>) {
const userRepo: any = { findById: jest.fn() };
const workspaceRepo: any = { findById: jest.fn() };
const userSessionRepo: any = { findActiveById: jest.fn() };
const sessionActivityService: any = { trackActivity: jest.fn() };
const environmentService: any = { getAppSecret: () => 'test-secret' };
const moduleRef: any = {};
const strategy = new JwtStrategy(
userRepo,
workspaceRepo,
userSessionRepo,
sessionActivityService,
environmentService,
moduleRef,
);
// Stub the EE ApiKeyService seam (the real module is not in the OSS build).
const validateApiKey = jest.fn(validateApiKeyImpl);
jest
.spyOn(strategy as any, 'resolveApiKeyService')
.mockReturnValue({ validateApiKey });
return { strategy, validateApiKey };
}
const makeReq = () => ({ raw: {} as Record<string, any> });
const apiKeyPayload = () => ({
sub: 'svc-1',
workspaceId: 'ws-1',
apiKeyId: 'key-1',
type: JwtType.API_KEY,
});
it("stamps actor='agent' for an is_agent API key (from the validated user)", async () => {
const validated = {
user: { id: 'svc-1', isAgent: true },
workspace: { id: 'ws-1' },
};
const { strategy, validateApiKey } = makeApiKeyStrategy(
async () => validated,
);
const req = makeReq();
const result = await strategy.validate(req, apiKeyPayload() as any);
expect(validateApiKey).toHaveBeenCalledTimes(1);
expect(req.raw.actor).toBe('agent');
// API keys carry no internal ai_chats row -> null.
expect(req.raw.aiChatId).toBeNull();
// The validated auth object is returned unchanged (req.user shape preserved).
expect(result).toBe(validated);
});
it("stamps actor='user' for an ordinary (non-agent) API key", async () => {
const { strategy } = makeApiKeyStrategy(async () => ({
user: { id: 'u-1', isAgent: false },
workspace: { id: 'ws-1' },
}));
const req = makeReq();
await strategy.validate(req, apiKeyPayload() as any);
expect(req.raw.actor).toBe('user');
expect(req.raw.aiChatId).toBeNull();
});
it('throws Unauthorized (and stamps nothing) when the EE module is missing', async () => {
const userRepo: any = { findById: jest.fn() };
const strategy = new JwtStrategy(
userRepo,
{ findById: jest.fn() } as any,
{ findActiveById: jest.fn() } as any,
{ trackActivity: jest.fn() } as any,
{ getAppSecret: () => 'test-secret' } as any,
{} as any,
);
// EE not bundled: the seam returns null.
jest.spyOn(strategy as any, 'resolveApiKeyService').mockReturnValue(null);
const req = makeReq();
await expect(
strategy.validate(req, apiKeyPayload() as any),
).rejects.toThrow(UnauthorizedException);
expect(req.raw.actor).toBeUndefined();
});
});
@@ -102,49 +102,28 @@ export class JwtStrategy extends PassportStrategy(Strategy, 'jwt') {
}
private async validateApiKey(req: any, payload: JwtApiKeyPayload) {
const apiKeyService = this.resolveApiKeyService();
if (!apiKeyService) {
throw new UnauthorizedException('Enterprise API Key module missing');
}
let ApiKeyModule: any;
let isApiKeyModuleReady = false;
const result = await apiKeyService.validateApiKey(payload);
// Stamp the agent-edit provenance for the API-KEY path too (#486). Unlike the
// access-token path above, it CANNOT be resolved before this point: the
// API-key payload carries no signed actor/aiChatId claim, and the user (with
// its isAgent flag) is unknown until the key is validated. Claim semantics for
// API keys: an is_agent API key (an agent service account) stamps 'agent' on
// every REST write; an ordinary API key resolves to 'user'. An API key has no
// internal ai_chats row, so aiChatId is always null. Derived from the
// SERVER-SIDE user (never a client field), so an 'agent' badge is unspoofable
// — mirroring the access-token path. Passing `null` for the claim means the
// actor is decided solely by user.isAgent.
const provenance = resolveProvenance((result as any)?.user, null);
req.raw.actor = provenance.actor;
req.raw.aiChatId = provenance.aiChatId;
return result;
}
/**
* Resolve the enterprise ApiKeyService, or `null` when the EE module is not
* bundled in this build (community build). Extracted as an overridable seam so
* the API-key provenance stamping can be unit-tested without the EE package
* present (docmost is OSS + a separate EE bundle; `require` of the EE path
* throws here). Any load/resolve error is treated as "module missing".
*/
protected resolveApiKeyService(): {
validateApiKey: (payload: JwtApiKeyPayload) => Promise<unknown>;
} | null {
try {
// eslint-disable-next-line @typescript-eslint/no-require-imports
const ApiKeyModule = require('./../../../ee/api-key/api-key.service');
return this.moduleRef.get(ApiKeyModule.ApiKeyService, { strict: false });
ApiKeyModule = require('./../../../ee/api-key/api-key.service');
isApiKeyModuleReady = true;
} catch (err) {
this.logger.debug(
'API Key module requested but enterprise module not bundled in this build',
);
return null;
isApiKeyModuleReady = false;
}
if (isApiKeyModuleReady) {
const ApiKeyService = this.moduleRef.get(ApiKeyModule.ApiKeyService, {
strict: false,
});
return ApiKeyService.validateApiKey(payload);
}
throw new UnauthorizedException('Enterprise API Key module missing');
}
}
@@ -53,8 +53,10 @@ import {
extractPageSlugId,
} from '../../../integrations/export/utils';
import { canonicalizeFootnotes } from '@docmost/editor-ext';
import { markdownToProseMirror } from '@docmost/prosemirror-markdown';
import { normalizeForeignMarkdown } from '../../../integrations/import/utils/foreign-markdown';
import {
markdownToProseMirror,
normalizeForeignMarkdown,
} from '@docmost/prosemirror-markdown';
import { WatcherService } from '../../watcher/watcher.service';
import { sql } from 'kysely';
import { TransclusionService } from '../transclusion/transclusion.service';
@@ -1,133 +0,0 @@
import { readFileSync } from 'fs';
import { EventEmitter } from 'node:events';
import { streamText } from 'ai';
import { MockLanguageModelV3, simulateReadableStream } from 'ai/test';
/**
* Regression tests for the writeToServerResponse drain-hang fix in
* patches/ai@6.0.134.patch (#486, commit 6).
*
* Unpatched ai@6.0.134's writeToServerResponse awaits ONLY `once("drain")` when
* response.write() returns false (backpressure). If the client disconnects
* mid-write the socket never drains, so that await never resolves: the read loop
* parks FOREVER, its `finally { response.end() }` is unreachable, and the stream
* reader + buffered chunks are pinned until process restart. In autonomous mode
* the run keeps producing output after the disconnect, so EVERY mid-run
* disconnect leaks a hung pipe. The patch races drain against close/error, and on
* a terminal socket event cancels the reader and breaks so `finally` always runs.
*
* This drives the REAL patched writeToServerResponse through the public
* pipeUIMessageStreamToResponse API with a response that never drains and closes
* mid-write exactly the leak scenario.
*/
/** A ServerResponse-like emitter whose first write() stalls (returns false) and
* then "closes" like a disconnecting client never firing 'drain'. */
class DisconnectingResponse extends EventEmitter {
ended = false;
writeCount = 0;
statusCode = 200;
writableEnded = false;
destroyed = false;
writeHead(): this {
return this;
}
setHeader(): void {}
flushHeaders(): void {}
write(): boolean {
this.writeCount++;
if (this.writeCount === 1) {
// Simulate the client vanishing mid-write: backpressure (false) and then a
// 'close' on the next tick, and CRUCIALLY never a 'drain'. Unpatched, the
// loop would await drain forever here.
setImmediate(() => this.emit('close'));
return false;
}
return true;
}
end(): void {
this.ended = true;
this.writableEnded = true;
this.emit('finish');
}
}
function makeModel() {
return new MockLanguageModelV3({
doStream: async () => ({
stream: simulateReadableStream({
chunks: [
{ type: 'stream-start' as const, warnings: [] },
{ type: 'text-start' as const, id: '1' },
{ type: 'text-delta' as const, id: '1', delta: 'hello ' },
{ type: 'text-delta' as const, id: '1', delta: 'world' },
{ type: 'text-end' as const, id: '1' },
{
type: 'finish' as const,
finishReason: { unified: 'stop' as const, raw: 'stop' },
usage: {
inputTokens: { total: 1, noCache: undefined, cacheRead: undefined, cacheWrite: undefined },
outputTokens: { total: 1, text: 1, reasoning: undefined },
},
},
],
}),
}),
});
}
describe('ai@6.0.134 pnpm patch: writeToServerResponse drain-hang (#486)', () => {
it('ends the response (does NOT hang) when the socket closes mid-write without draining', async () => {
const result = streamText({ model: makeModel(), prompt: 'hi' });
const res = new DisconnectingResponse();
// Drain the SDK stream independently, like the production detached path.
void result.consumeStream({ onError: () => undefined });
result.pipeUIMessageStreamToResponse(res as never);
// TRIPWIRE: the patched loop exits on 'close' and runs finally -> end().
// Unpatched, it awaits 'drain' forever and this never becomes true.
await new Promise<void>((resolve, reject) => {
const started = Date.now();
const poll = setInterval(() => {
if (res.ended) {
clearInterval(poll);
resolve();
} else if (Date.now() - started > 3000) {
clearInterval(poll);
reject(new Error('writeToServerResponse hung: response never ended'));
}
}, 20);
});
expect(res.ended).toBe(true);
});
it('does not emit an unhandledRejection when the fire-and-forget read() throws', async () => {
// The patch swallows read()'s rejection (fire-and-forget) with a log instead
// of letting it surface as a process-killing unhandledRejection.
const rejections: unknown[] = [];
const onUnhandled = (e: unknown) => rejections.push(e);
process.on('unhandledRejection', onUnhandled);
// Silence the patch's diagnostic console.error for the throwing read().
const errSpy = jest.spyOn(console, 'error').mockImplementation(() => undefined);
try {
const result = streamText({ model: makeModel(), prompt: 'hi' });
const res = new DisconnectingResponse();
void result.consumeStream({ onError: () => undefined });
result.pipeUIMessageStreamToResponse(res as never);
await new Promise((r) => setTimeout(r, 300));
} finally {
process.off('unhandledRejection', onUnhandled);
errSpy.mockRestore();
}
expect(rejections).toEqual([]);
});
it('both installed dist builds (CJS and ESM) carry the #486 patch marker', () => {
const cjsPath = require.resolve('ai');
const mjsPath = cjsPath.replace(/index\.js$/, 'index.mjs');
expect(cjsPath).toMatch(/index\.js$/);
expect(readFileSync(cjsPath, 'utf8')).toContain('PATCH(docmost #486)');
expect(readFileSync(mjsPath, 'utf8')).toContain('PATCH(docmost #486)');
});
});
@@ -1,211 +0,0 @@
import type { HealthIndicatorService } from '@nestjs/terminus';
import type { EnvironmentService } from '../environment/environment.service';
/**
* Integration guard for the /health Redis-probe handle leak (#486, commit 2).
*
* The bug: `pingCheck` built `new Redis(...)` per call and only disconnected on
* the SUCCESS path, so when Redis is DOWN every probe tick added ANOTHER
* forever-reconnecting client an unbounded handle/client leak for the duration
* of the outage. The fix reuses ONE long-lived probe client.
*
* This is an OBSERVABLE-property test, not an assertion on a mocked return value:
* we point the indicator at a REAL, refused TCP endpoint (a dead port) so ioredis
* genuinely fails to connect, run many probes, and assert the number of live
* Redis CLIENTS created stays at exactly ONE. `ioredis` is delegated to its real
* implementation (requireActual) only the constructor is wrapped to COUNT the
* real clients it creates, which is precisely the leaking resource.
*/
import type { Redis } from 'ioredis';
const mockLiveClients: Redis[] = [];
/**
* Fully tear a REAL ioredis client down so NO timer survives jest's 1s exit
* window (this suite must exit cleanly WITHOUT forceExit; see #382).
*
* `connector.disconnect()` arms a ~12s "force-destroy the stream" `setTimeout`
* that is cleared ONLY by the stream's 'close' event but only when the
* connector still holds a stream. Two problem cases:
* - a LIVE/connecting socket: disconnect arms the timer and 'close' may lag
* past jest's window, so we destroy the socket to make 'close' fire NOW;
* - a client BETWEEN reconnect attempts to a dead port: the held socket is
* ALREADY destroyed (its 'close' fired long ago), so disconnect would arm a
* timer whose clearing 'close' can never come again. We drop that dead stream
* reference BEFORE disconnect so the doomed timer is never armed.
* `disconnect()` itself also clears ioredis' own reconnect backoff timer.
*/
type DrainableStream = { destroyed?: boolean; destroy?: () => void } | null;
type DrainableClient = {
removeAllListeners: (event: string) => void;
disconnect: () => void;
stream?: DrainableStream;
connector?: { stream?: DrainableStream };
};
async function drainClient(client: Redis): Promise<void> {
if (!client || client.status === 'end') return;
const c = client as unknown as DrainableClient;
c.removeAllListeners('error');
// Drop an already-dead held socket so disconnect() can't arm a timer whose
// clearing 'close' will never fire again.
if (c.connector?.stream && c.connector.stream.destroyed) {
c.connector.stream = null;
}
if (c.stream && c.stream.destroyed) {
c.stream = null;
}
await new Promise<void>((resolve) => {
let done = false;
const finish = () => {
if (done) return;
done = true;
resolve();
};
client.once('end', finish);
// reconnect=false (the default): stop the retry loop and close the socket.
client.disconnect();
// Force any still-live socket closed NOW so the connector's stream-destroy
// timer clears inside jest's window instead of lagging behind a real 'close'.
if (c.stream && !c.stream.destroyed) {
c.stream.destroy?.();
}
// Fallback for a client with no live stream to emit 'end' (unref'd so it
// can never itself hold the loop open).
const fallback = setTimeout(finish, 500);
(fallback as { unref?: () => void }).unref?.();
});
}
async function drainAll(): Promise<void> {
await Promise.all(mockLiveClients.map((c) => drainClient(c)));
}
jest.mock('ioredis', () => {
const actual = jest.requireActual('ioredis');
const RealRedis = actual.Redis ?? actual.default ?? actual;
class CountingRedis extends RealRedis {
constructor(...args: unknown[]) {
super(...(args as []));
mockLiveClients.push(this as never);
}
}
return { ...actual, Redis: CountingRedis, default: CountingRedis };
});
// Import AFTER the mock is registered so the class picks up the counting client.
import { RedisHealthIndicator } from './redis.health';
describe('RedisHealthIndicator handle leak (#486)', () => {
const indicatorService = {
check: (key: string) => ({
up: () => ({ [key]: { status: 'up' } }),
down: (message: string) => ({ [key]: { status: 'down', message } }),
}),
} as unknown as HealthIndicatorService;
// A port with (almost certainly) nothing listening -> connection refused fast.
const environmentService = {
getRedisUrl: () => 'redis://127.0.0.1:6399/0',
} as unknown as EnvironmentService;
let indicator: RedisHealthIndicator;
beforeEach(() => {
mockLiveClients.length = 0;
indicator = new RedisHealthIndicator(indicatorService, environmentService);
});
afterEach(async () => {
// Drain (destroy socket + AWAIT 'end') every client the test created FIRST,
// so each is fully 'end' before onModuleDestroy's disconnect runs — that way
// no ioredis reconnect / stream-destroy timer outlives jest's exit window.
await drainAll();
indicator.onModuleDestroy();
});
it('creates exactly ONE Redis client across many probes while Redis is DOWN', async () => {
const N = 8;
for (let i = 0; i < N; i++) {
const result = await indicator.pingCheck('redis');
// Down endpoint -> every probe reports "down" (not an unhandled crash).
expect(result.redis.status).toBe('down');
}
// THE OBSERVABLE LEAK: on the buggy code this is N (a fresh, never-cleaned
// reconnecting client per probe). The fix reuses one shared client.
expect(mockLiveClients).toHaveLength(1);
});
it('onModuleDestroy releases the probe client (a later probe builds a fresh one)', async () => {
await indicator.pingCheck('redis');
expect(mockLiveClients).toHaveLength(1);
indicator.onModuleDestroy();
// A second destroy is a safe no-op (probeClient was nulled).
indicator.onModuleDestroy();
// After shutdown the indicator lazily builds a NEW client on the next probe,
// proving the old one was truly released rather than reused.
await indicator.pingCheck('redis');
expect(mockLiveClients).toHaveLength(2);
});
});
/**
* Happy-path regression guard (#486, B2): the FIRST probe against a LIVE Redis
* must report UP.
*
* With `lazyConnect: true` + `enableOfflineQueue: false`, a freshly-built client
* is in the `wait` state and the socket opens lazily. If the very first `ping()`
* is issued before an explicit `connect()`, ioredis rejects it instantly with
* "Stream isn't writeable and enableOfflineQueue options is false" a FALSE
* DOWN even though Redis is alive. The fix opens the socket before the first
* ping. This exercises a REAL ioredis client against a REAL TCP redis server
* (not a mock), so a regression genuinely reddens it.
*/
describe('RedisHealthIndicator live Redis first-probe (#486, B2)', () => {
const indicatorService = {
check: (key: string) => ({
up: () => ({ [key]: { status: 'up' } }),
down: (message: string) => ({ [key]: { status: 'down', message } }),
}),
} as unknown as HealthIndicatorService;
// A REAL running redis (see the neighboring harness / CI env).
const environmentService = {
getRedisUrl: () => 'redis://127.0.0.1:6379/0',
} as unknown as EnvironmentService;
let indicator: RedisHealthIndicator;
beforeEach(() => {
mockLiveClients.length = 0;
indicator = new RedisHealthIndicator(indicatorService, environmentService);
});
afterEach(async () => {
// Await full socket close of every live client (see drainClient) BEFORE
// onModuleDestroy: a real, connected ioredis client MUST be drained to 'end'
// or its stream-destroy timer keeps the jest worker alive past the 1s window.
await drainAll();
indicator.onModuleDestroy();
});
it('reports UP on the FIRST probe against a live Redis', async () => {
// The VERY FIRST probe — no warm-up ping — must be UP.
const result = await indicator.pingCheck('redis');
expect(result.redis.status).toBe('up');
});
it('stays UP on a probe AFTER onModuleDestroy re-creates the client', async () => {
await indicator.pingCheck('redis');
indicator.onModuleDestroy();
// The re-created client is again in `wait`; the first ping on it must still
// open the socket (the false-DOWN also recurs on the post-destroy path).
const result = await indicator.pingCheck('redis');
expect(result.redis.status).toBe('up');
});
});
@@ -2,173 +2,33 @@ import {
HealthIndicatorResult,
HealthIndicatorService,
} from '@nestjs/terminus';
import { Injectable, Logger, OnModuleDestroy } from '@nestjs/common';
import { Injectable, Logger } from '@nestjs/common';
import { EnvironmentService } from '../environment/environment.service';
import { Redis } from 'ioredis';
@Injectable()
export class RedisHealthIndicator implements OnModuleDestroy {
export class RedisHealthIndicator {
private readonly logger = new Logger(RedisHealthIndicator.name);
/**
* ONE long-lived probe connection, reused across every /health tick. The old
* code built `new Redis(...)` per call and only `disconnect()`d on the SUCCESS
* path, so while Redis was DOWN every probe added a fresh, forever-reconnecting
* client a handle leak that grew without bound for as long as the outage (and
* the health checker keeps polling) lasted. A single shared client keeps at most
* ONE background reconnect loop regardless of how many probes run.
*/
private probeClient: Redis | null = null;
/**
* How long the first-ping `connect()` may take before a probe gives up and
* reports DOWN. A `connect()` against a truly-down Redis never settles on its
* own (ioredis retries the socket indefinitely per its retryStrategy), so the
* probe MUST bound it or the /health handler would hang. Kept short so a real
* outage is reported fast; localhost/live Redis connects well within it.
*/
private static readonly CONNECT_TIMEOUT_MS = 2000;
/**
* The single in-flight first-`connect()`, memoized so CONCURRENT probes share
* it. k8s liveness+readiness hit /health in parallel on startup: without this,
* probe A drives `connect()` (the client leaves the `wait` state) and probe B,
* seeing a not-`wait`/not-`ready` client, would skip connect and fire `ping()`
* at a still-opening socket an instant FALSE DOWN. With the memo, B awaits
* the SAME connect. Cleared once it settles so a later disconnect / re-create
* starts a fresh connect.
*/
private connectingPromise: Promise<void> | null = null;
constructor(
private readonly healthIndicatorService: HealthIndicatorService,
private environmentService: EnvironmentService,
) {}
private getProbeClient(): Redis {
if (!this.probeClient) {
this.probeClient = new Redis(this.environmentService.getRedisUrl(), {
// Constructing must never throw or eagerly connect; the first ping opens
// the socket. This lets us build the client once and reuse it.
lazyConnect: true,
// A health probe must fail FAST, not queue behind a stuck reconnect: one
// retry per request, and no offline queue so a ping while disconnected
// rejects immediately instead of buffering commands that pile up in RAM.
maxRetriesPerRequest: 1,
enableOfflineQueue: false,
});
// ioredis emits 'error' on every failed (re)connect; with no listener that
// surfaces as an unhandled 'error' event and can crash the process. Swallow
// it here — pingCheck already reports health — and log at debug so a Redis
// outage does not flood the logs.
this.probeClient.on('error', (err) => {
this.logger.debug(
`Redis probe connection error: ${
err instanceof Error ? err.message : String(err)
}`,
);
});
}
return this.probeClient;
}
/**
* Open the probe socket BEFORE the first ping. `lazyConnect: true` leaves a
* freshly-built (or post-destroy re-built) client in the `wait` state: the
* socket is NOT open yet, so with `enableOfflineQueue: false` the very first
* `ping()` rejects instantly with "Stream isn't writeable and
* enableOfflineQueue options is false" even when Redis is perfectly alive a
* false DOWN on the happy path. We drive `connect()` ONLY from `wait`; once
* the client is connected, ioredis owns its own (re)connect loop and a ping
* issued while it reconnects still fast-fails to a correct DOWN (offline queue
* stays off). A failed/timed-out connect rejects reported DOWN, which is the
* right signal for a truly-down Redis.
*/
private ensureConnected(client: Redis): Promise<void> {
// Already open — steady state, nothing to do.
if (client.status === 'ready') return Promise.resolve();
// A first-connect is already in flight (possibly started by a CONCURRENT
// probe): await the SAME one instead of racing a second connect() (ioredis
// throws "already connecting") or firing ping() at a not-yet-open socket.
if (this.connectingPromise) return this.connectingPromise;
// Only DRIVE connect() from the initial `wait` state (fresh / post-destroy
// re-created client). In any other non-ready state ioredis already owns its
// (re)connect loop; a ping there fast-fails to a correct DOWN, so we must not
// start a competing connect.
if (client.status !== 'wait') return Promise.resolve();
const promise = this.connectWithTimeout(client).finally(() => {
// Clear only if still ours, so a later disconnect / re-create can connect
// again. Whether it resolved or rejected, the memo has served its window.
if (this.connectingPromise === promise) {
this.connectingPromise = null;
}
});
this.connectingPromise = promise;
return promise;
}
private connectWithTimeout(client: Redis): Promise<void> {
return new Promise<void>((resolve, reject) => {
let settled = false;
const timer = setTimeout(() => {
if (settled) return;
settled = true;
reject(new Error('Redis probe connect timed out'));
}, RedisHealthIndicator.CONNECT_TIMEOUT_MS);
// Never let THIS timer alone keep the event loop (or a jest worker) alive;
// it is cleared on settle anyway, this is belt-and-braces.
timer.unref?.();
// `.catch` is always attached, so a connect() that rejects AFTER we have
// already timed out is handled here (guarded by `settled`) and never
// surfaces as an unhandled rejection.
client
.connect()
.then(() => {
if (settled) return;
settled = true;
clearTimeout(timer);
resolve();
})
.catch((err) => {
if (settled) return;
settled = true;
clearTimeout(timer);
reject(err);
});
});
}
async pingCheck(key: string): Promise<HealthIndicatorResult> {
const indicator = this.healthIndicatorService.check(key);
try {
const redis = this.getProbeClient();
// Open the socket before the first ping (see ensureConnected); without
// this the first probe after (re)creation falsely reports DOWN on a live
// Redis because lazyConnect defers the connect past the first ping.
await this.ensureConnected(redis);
const redis = new Redis(this.environmentService.getRedisUrl(), {
maxRetriesPerRequest: 15,
});
await redis.ping();
redis.disconnect();
return indicator.up();
} catch (e) {
this.logger.error(e);
return indicator.down(`${key} is not available`);
}
}
onModuleDestroy(): void {
if (this.probeClient) {
// disconnect() (not quit()) tears the socket + reconnect loop down
// immediately without waiting on a round-trip to a possibly-down server.
// Do NOT removeAllListeners() with no event name — that would also strip
// ioredis' OWN internal listeners and break its teardown; our 'error'
// listener is harmless and dies with the dropped client reference.
this.probeClient.disconnect();
this.probeClient = null;
}
// Drop any in-flight first-connect memo so the NEXT client (lazily rebuilt on
// the next probe) starts a fresh connect rather than awaiting a promise tied
// to the client we just tore down.
this.connectingPromise = null;
}
}
@@ -22,10 +22,12 @@ import { v7 } from 'uuid';
import { generateJitteredKeyBetween } from 'fractional-indexing-jittered';
import { FileTask, InsertablePage } from '@docmost/db/types/entity.types';
import { canonicalizeFootnotes } from '@docmost/editor-ext';
import { markdownToProseMirror } from '@docmost/prosemirror-markdown';
import {
markdownToProseMirror,
normalizeForeignMarkdown,
} from '@docmost/prosemirror-markdown';
import { getProsemirrorContent } from '../../../common/helpers/prosemirror/utils';
import { formatImportHtml } from '../utils/import-formatter';
import { normalizeForeignMarkdown } from '../utils/foreign-markdown';
import {
buildAttachmentCandidates,
collectMarkdownAndHtmlFiles,
@@ -18,8 +18,10 @@ import { generateJitteredKeyBetween } from 'fractional-indexing-jittered';
import { TiptapTransformer } from '@hocuspocus/transformer';
import * as Y from 'yjs';
import { canonicalizeFootnotes } from '@docmost/editor-ext';
import { markdownToProseMirror } from '@docmost/prosemirror-markdown';
import { normalizeForeignMarkdown } from '../utils/foreign-markdown';
import {
markdownToProseMirror,
normalizeForeignMarkdown,
} from '@docmost/prosemirror-markdown';
import {
FileTaskStatus,
FileTaskType,
@@ -16,7 +16,6 @@ import {
} from './mcp-auth.helpers';
import { JwtType } from '../../core/auth/dto/jwt-payload';
import { CREDENTIALS_MISMATCH_MESSAGE } from '../../core/auth/auth.constants';
import { McpService } from './mcp.service';
// The /mcp per-user auth decision logic is tested through the framework-free
// `resolveMcpSessionConfig` helper that McpService delegates to. McpService
@@ -1180,46 +1179,3 @@ describe('mapAuthResultToResponse (handle status/body mapping, refactor R2)', ()
});
});
});
// #486: onModuleDestroy must ALSO tear down the live loopback CollabSessions, not
// just clear the sweep timer — otherwise the embedded MCP's collab sockets keep
// docs pinned open on the collab server past process exit. The teardown goes
// through an overridable seam (destroyAllMcpSessions) so it can be spied without
// loading the ESM-only @docmost/mcp package.
describe('McpService.onModuleDestroy — CollabSession teardown (#486)', () => {
function makeService(): McpService {
// The constructor only stores its deps and starts the (unref'd) sweep timer,
// so bare stubs suffice. onModuleDestroy clears that timer, so no leak.
return new McpService(
{} as any,
{} as any,
{} as any,
{} as any,
{} as any,
{} as any,
{} as any,
);
}
it('destroys all sessions AND clears the sweep timer on shutdown', async () => {
const svc = makeService();
const destroy = jest.fn().mockResolvedValue(undefined);
(svc as any).destroyAllMcpSessions = destroy;
const clearSpy = jest.spyOn(global, 'clearInterval');
await svc.onModuleDestroy();
expect(destroy).toHaveBeenCalledTimes(1);
expect(clearSpy).toHaveBeenCalledWith((svc as any).sweepTimer);
clearSpy.mockRestore();
});
it('swallows a teardown failure so shutdown never throws', async () => {
const svc = makeService();
(svc as any).destroyAllMcpSessions = jest
.fn()
.mockRejectedValue(new Error('collab teardown boom'));
await expect(svc.onModuleDestroy()).resolves.toBeUndefined();
});
});
@@ -119,42 +119,10 @@ export class McpService implements OnModuleDestroy {
this.sweepTimer.unref?.();
}
async onModuleDestroy(): Promise<void> {
onModuleDestroy(): void {
clearInterval(this.sweepTimer);
// Tear down any live loopback CollabSession providers at shutdown (#486). The
// embedded MCP (and the in-app AI agent) open Hocuspocus collab sockets against
// THIS process; without an explicit teardown those sessions keep their docs
// "open" on the collab server and hold providers/buffers until they idle out,
// so a restart can race a doc still pinned by the dying worker. Best-effort:
// any failure is logged, never allowed to break shutdown.
try {
await this.destroyAllMcpSessions();
} catch (err) {
this.logger.error(
'MCP CollabSession teardown on shutdown failed',
err as Error,
);
}
}
/**
* Resolve @docmost/mcp's `destroyAllSessions` and invoke it (#486). The live
* CollabSession registry is a module-level singleton in the ESM package, shared
* by every entry (`.`/`./http`), so this tears down ALL sessions regardless of
* which surface opened them. The module is already loaded whenever MCP was used;
* if it was never loaded (or is absent) the import + no-op is harmless.
*
* Held as an overridable field so a unit test can spy the teardown without
* loading the ESM-only package or standing up the DI graph.
*/
private destroyAllMcpSessions: () => Promise<void> = async () => {
const entry = require.resolve('@docmost/mcp');
const mod = (await esmImport(pathToFileURL(entry).href)) as {
destroyAllSessions?: () => void;
};
mod.destroyAllSessions?.();
};
// Service account the embedded MCP uses to talk back to this Docmost
// instance over loopback REST + the collaboration WebSocket. Now OPTIONAL:
// it is only a fallback when no per-user Basic/Bearer credentials are sent.
@@ -1,148 +0,0 @@
import { get as httpGet } from 'node:http';
import { AddressInfo } from 'node:net';
import { createServer } from 'node:http';
// Drive the metrics HTTP server without the load-time METRICS_PORT gate: mock the
// registry so isMetricsEnabled()/getMetricsRegistry() are always satisfied. What
// we assert is observed over a REAL socket (bind address, status codes), not on
// the mock.
jest.mock('./metrics.registry', () => ({
isMetricsEnabled: () => true,
getMetricsRegistry: () => ({
metrics: async () => '# HELP up test\nup 1\n',
contentType: 'text/plain; version=0.0.4',
}),
}));
import {
startMetricsServer,
closeMetricsServer,
resolveMetricsBind,
resolveMetricsToken,
} from './metrics.server';
/** Find a free TCP port (the metrics server requires METRICS_PORT > 0). */
function freePort(): Promise<number> {
return new Promise((resolve, reject) => {
const s = createServer();
s.once('error', reject);
s.listen(0, '127.0.0.1', () => {
const p = (s.address() as AddressInfo).port;
s.close(() => resolve(p));
});
});
}
/** Minimal GET against 127.0.0.1:port with optional Authorization header. */
function req(
port: number,
headers: Record<string, string> = {},
): Promise<{ status: number; body: string }> {
return new Promise((resolve, reject) => {
const r = httpGet(
{ host: '127.0.0.1', port, path: '/metrics', headers },
(res) => {
let body = '';
res.on('data', (c) => (body += c));
res.on('end', () =>
resolve({ status: res.statusCode ?? 0, body }),
);
},
);
r.on('error', reject);
});
}
describe('metrics server bind + auth (#486)', () => {
const saved = {
bind: process.env.METRICS_BIND,
token: process.env.METRICS_TOKEN,
port: process.env.METRICS_PORT,
};
afterEach(async () => {
await closeMetricsServer();
process.env.METRICS_BIND = saved.bind;
process.env.METRICS_TOKEN = saved.token;
process.env.METRICS_PORT = saved.port;
delete process.env.METRICS_BIND;
delete process.env.METRICS_TOKEN;
});
describe('resolveMetricsBind', () => {
it('defaults to loopback 127.0.0.1', () => {
delete process.env.METRICS_BIND;
expect(resolveMetricsBind()).toBe('127.0.0.1');
});
it('honours the METRICS_BIND override', () => {
process.env.METRICS_BIND = '0.0.0.0';
expect(resolveMetricsBind()).toBe('0.0.0.0');
});
it('treats a blank override as unset (loopback)', () => {
process.env.METRICS_BIND = ' ';
expect(resolveMetricsBind()).toBe('127.0.0.1');
});
});
describe('resolveMetricsToken', () => {
it('is null when unset', () => {
delete process.env.METRICS_TOKEN;
expect(resolveMetricsToken()).toBeNull();
});
it('returns the trimmed token when set', () => {
process.env.METRICS_TOKEN = ' s3cret ';
expect(resolveMetricsToken()).toBe('s3cret');
});
});
it('binds to loopback by default and serves /metrics without auth when no token', async () => {
delete process.env.METRICS_BIND;
delete process.env.METRICS_TOKEN;
const port = await freePort();
process.env.METRICS_PORT = String(port);
const server = startMetricsServer();
expect(server).not.toBeNull();
await new Promise<void>((resolve) => {
if (server!.listening) resolve();
else server!.once('listening', () => resolve());
});
// OBSERVABLE: the listener bound to loopback, not 0.0.0.0.
expect((server!.address() as AddressInfo).address).toBe('127.0.0.1');
const res = await req(port);
expect(res.status).toBe(200);
expect(res.body).toContain('up 1');
});
it('rejects unauthenticated scrapes with 401 and accepts the exact Bearer token', async () => {
delete process.env.METRICS_BIND;
process.env.METRICS_TOKEN = 'topsecret';
const port = await freePort();
process.env.METRICS_PORT = String(port);
const server = startMetricsServer();
expect(server).not.toBeNull();
// No auth -> 401.
const noAuth = await req(port);
expect(noAuth.status).toBe(401);
// Wrong token, DIFFERENT length -> 401 (short-circuits on the length guard).
const wrong = await req(port, { authorization: 'Bearer nope' });
expect(wrong.status).toBe(401);
// Wrong token, SAME length -> 401. This drives the timingSafeEqual compare
// itself (the length guard passes: 'Bearer topsecreX' has the same length as
// 'Bearer topsecret'). Pins the constant-time compare: a regression that made
// it return true would let this equal-length wrong token through — the
// different-length case above would NOT catch that.
const sameLen = await req(port, { authorization: 'Bearer topsecreX' });
expect(sameLen.status).toBe(401);
// Correct token -> 200 with the metrics body.
const ok = await req(port, { authorization: 'Bearer topsecret' });
expect(ok.status).toBe(200);
expect(ok.body).toContain('up 1');
});
});
@@ -1,27 +1,7 @@
import { createServer, Server } from 'node:http';
import { timingSafeEqual } from 'node:crypto';
import { Logger } from '@nestjs/common';
import { getMetricsRegistry, isMetricsEnabled } from './metrics.registry';
/**
* Constant-time compare of the presented Authorization header against the
* expected `Bearer <token>`. This is the ONLY auth layer for the metrics
* endpoint, so a naive `!==` would leak the token byte-by-byte via timing.
* timingSafeEqual requires equal-length buffers, so a length mismatch short-
* circuits to "not equal" (its own length is not itself a useful oracle: the
* expected string length is fixed by config, not secret-derived).
*/
function bearerMatches(
presented: string | undefined,
expected: string,
): boolean {
if (typeof presented !== 'string') return false;
const a = Buffer.from(presented);
const b = Buffer.from(expected);
if (a.length !== b.length) return false;
return timingSafeEqual(a, b);
}
/**
* Start the Prometheus scrape endpoint on a SEPARATE port, taken from
* `METRICS_PORT`. There is NO default port: when `METRICS_PORT` is unset the
@@ -36,30 +16,6 @@ function bearerMatches(
*/
let metricsServer: Server | null = null;
/**
* Interface the metrics endpoint binds to. Defaults to LOOPBACK (127.0.0.1) so
* the unauthenticated `/metrics` surface is NOT exposed on all interfaces by
* default the old `0.0.0.0` bind put an auth-less endpoint on every interface.
* Deployments where the scraper runs in a SEPARATE container (and reaches this as
* `docmost:9464`) set `METRICS_BIND=0.0.0.0`, ideally together with METRICS_TOKEN
* and/or a private network so the port is not world-readable.
*/
export function resolveMetricsBind(): string {
const raw = (process.env.METRICS_BIND ?? '').trim();
return raw.length > 0 ? raw : '127.0.0.1';
}
/**
* Optional Bearer token guarding `/metrics`. When `METRICS_TOKEN` is set, every
* scrape must present `Authorization: Bearer <token>`; unset (default) leaves the
* endpoint open (safe when bound to loopback / a trusted network). Returns the
* trimmed token or null when unset/blank.
*/
export function resolveMetricsToken(): string | null {
const raw = (process.env.METRICS_TOKEN ?? '').trim();
return raw.length > 0 ? raw : null;
}
export function startMetricsServer(): Server | null {
if (!isMetricsEnabled()) return null;
@@ -75,22 +31,8 @@ export function startMetricsServer(): Server | null {
return null;
}
const bind = resolveMetricsBind();
const token = resolveMetricsToken();
const server = createServer(async (req, res) => {
if (req.method === 'GET' && req.url === '/metrics') {
// Optional Bearer auth: reject scrapes without the exact token when one is
// configured. This is the auth layer the old all-interfaces bind lacked.
if (token) {
const auth = req.headers['authorization'];
if (!bearerMatches(auth, `Bearer ${token}`)) {
res.statusCode = 401;
res.setHeader('WWW-Authenticate', 'Bearer');
res.end();
return;
}
}
try {
const body = await register.metrics();
res.setHeader('Content-Type', register.contentType);
@@ -106,14 +48,10 @@ export function startMetricsServer(): Server | null {
res.end();
});
// Bind to loopback by default so the auth-less endpoint is not exposed on all
// interfaces. Set METRICS_BIND=0.0.0.0 (ideally with METRICS_TOKEN) when the
// scraper runs in a separate container and reaches this as docmost:9464.
server.listen(port, bind, () => {
logger.log(
`Metrics endpoint listening on ${bind}:${port}/metrics` +
(token ? ' (Bearer auth required)' : ''),
);
// Bind on all interfaces: the scraper (VictoriaMetrics) reaches this from
// another container as docmost:9464. The port is not published to the host.
server.listen(port, '0.0.0.0', () => {
logger.log(`Metrics endpoint listening on :${port}/metrics`);
});
server.on('error', (err) => {
@@ -31,6 +31,9 @@ export enum QueueJob {
IMPORT_TASK = 'import-task',
EXPORT_TASK = 'export-task',
SEARCH_REMOVE_PAGE = 'search-remove-page',
SEARCH_REMOVE_ASSET = 'search-remove-attachment',
SEARCH_REMOVE_FACE = 'search-remove-comment',
TYPESENSE_FLUSH = 'typesense-flush',
PAGE_CREATED = 'page-created',
+1
View File
@@ -1,4 +1,5 @@
export * from "./lib/trailing-node";
export * from "./lib/code";
export * from "./lib/comment/comment";
export * from "./lib/utils";
export * from "./lib/math";
+20
View File
@@ -0,0 +1,20 @@
import { Code as TiptapCode } from "@tiptap/extension-code";
// #515: canonical inline `code` mark for Docmost.
//
// Tiptap's stock Code mark (via StarterKit) declares `excludes: "_"`, which
// makes it exclude EVERY other inline mark: applying `code` drops any co-
// occurring bold/italic/… on both the HTML->PM import and editor transactions.
// That silently stripped emphasis adjacent to inline code (`` **`--flag`** ``
// lost its bold on markdown import). CommonMark nests them (`<strong><code>`),
// so Docmost lets `code` combine with all marks by overriding `excludes` to the
// empty string (excludes nothing).
//
// This is the SINGLE shared source imported by the live editor, the collab
// server and the comment editor schemas. The markdown-import mirror in
// @docmost/prosemirror-markdown re-declares the same override locally (it must
// not pull this React-aware package into its node runtime) and a parity test
// keeps the two in lockstep.
export const Code = TiptapCode.extend({
excludes: "",
});
+8 -2
View File
@@ -72,7 +72,13 @@ export async function stabilizePageFile(
* keeps re-pulls of an unchanged page byte-identical (no churn, loop-guard).
*/
export async function stabilizePageBody(content: unknown): Promise<string> {
const md1 = convertProseMirrorToMarkdown(content);
// git-sync is the LOSSLESS mirror path, so run the serializer in `strict`
// mode: a node/mark type the converter has no case for (e.g. one added to the
// schema without a matching serializer arm) throws a ConverterLossError here
// rather than silently degrading — surfacing the loss loudly at write time
// instead of committing a lossy file. Valid content (every current schema type
// has a case) is unaffected.
const md1 = convertProseMirrorToMarkdown(content, { strict: true });
const doc2 = await markdownToProseMirror(md1);
return convertProseMirrorToMarkdown(doc2);
return convertProseMirrorToMarkdown(doc2, { strict: true });
}
+18
View File
@@ -4,6 +4,7 @@ import { stabilizePageFile, type PageMeta } from '../src/engine/stabilize.js';
// global DOM via jsdom at module load time (required for @tiptap/html under Node).
import { markdownToProseMirror } from '@docmost/prosemirror-markdown';
import { parseDocmostMarkdown } from '@docmost/prosemirror-markdown';
import { ConverterLossError } from '@docmost/prosemirror-markdown';
// stabilize.ts (SPEC §11 normalize-on-write) was 0% covered (only the gated e2e
// touched it). stabilizePageFile is import-testable: build a small ProseMirror
@@ -66,6 +67,23 @@ describe('stabilizePageFile — normalize-on-write fixpoint (SPEC §11)', () =>
expect(body1).toContain('data-src="/d.drawio"');
});
it('runs the serializer in STRICT mode — an unmappable node throws, not a lossy write (#493)', async () => {
// git-sync is the lossless mirror path: a node type the converter has no
// case for (here a fabricated one, standing in for a schema type added
// without a matching serializer arm) must surface loudly at write time
// rather than being silently flattened into a lossy .md file.
const content = {
type: 'doc',
content: [
{ type: 'paragraph', content: [{ type: 'text', text: 'ok' }] },
{ type: 'quantumWidget', content: [{ type: 'text', text: 'lost?' }] },
],
};
await expect(stabilizePageFile(content, meta)).rejects.toBeInstanceOf(
ConverterLossError,
);
});
it('already-stable content is unchanged by the pass (idempotent)', async () => {
// Plain prose is already a fixpoint; stabilizing it once and twice agree.
const content = {
+37 -77
View File
@@ -1,100 +1,60 @@
// Codegen: emit src/registry-stamp.generated.ts with a REGISTRY_STAMP hash of
// the ENTIRE src/ tree, so a build/ vs src/ skew (issue #447) is detectable at
// runtime for ANY source file — not just tool-specs.ts.
// the tool-specs REGISTRY CONTENT, so a build/ vs src/ skew (issue #447) is
// detectable at runtime.
//
// WHY hash the whole src tree (not just tool-specs.ts): the runtime tools are
// assembled from far more than the spec registry — client.ts, the client/*
// domain modules, comment-signal.ts and the drawio-* helpers all ship in build/
// and are loaded by the in-app server. Hashing ONLY tool-specs.ts meant an edit
// to any of those (e.g. a behavioural fix in client.ts) left the stamp unchanged,
// so a stale build/ served the OLD code silently (issue #486). Hashing every
// src/**/*.ts closes that gap: any source edit changes the stamp.
// WHY hash the raw source text (not extracted structured data):
// SHARED_TOOL_SPECS carries `buildShape` functions (the input SCHEMAS) which are
// NOT serializable. The input schema is exactly one of the things that MUST stay
// in sync between build/ and src/, so we cannot drop it from the hash. Rather
// than probe zod with a fragile shim to reconstruct the schema shape, we hash the
// STABLE, deterministic source TEXT of tool-specs.ts. That text fully captures
// every field that must stay in sync — mcpName, inAppKey, description, tier,
// catalogLine AND the buildShape bodies (input schemas) — with zero probing
// fragility. Any edit to a spec (a renamed tool, a reworded description, a
// changed schema field) changes the text and therefore the stamp.
//
// WHY hash the raw source text (not extracted structured data): the tool input
// SCHEMAS live as `buildShape` functions which are NOT serializable, so we cannot
// reduce them to structured data without a fragile zod shim. Hashing the STABLE,
// deterministic source TEXT captures every field that must stay in sync with zero
// probing fragility. Any edit to any source file changes the text → the stamp.
//
// DETERMINISM: files are enumerated recursively, filtered to *.ts EXCLUDING
// *.generated.ts (the codegen's OWN output — including it would create a
// fixed-point cycle), and sorted by their POSIX-normalized path relative to src/
// so the order is platform-independent. Each file contributes its relative path
// AND its content with line endings normalized to LF and a single trailing
// newline stripped, so a CRLF checkout or an editor's trailing-newline habit
// cannot make build/ and src/ disagree. No Date.now / randomness. The loader's
// dev-only stale-check (docmost-client.loader.ts) re-runs THIS SAME enumeration +
// normalization + sha256 and compares to the built REGISTRY_STAMP; the two must
// compute identically.
// DETERMINISM: the hash is computed over the file bytes with line endings
// normalized to LF and a single trailing newline stripped, so a CRLF checkout or
// an editor's trailing-newline habit cannot make build/ and src/ disagree. No
// Date.now / randomness. The loader's dev-only stale-check (docmost-client.loader.ts)
// re-runs THIS SAME normalization + sha256 over src/tool-specs.ts and compares to
// the built REGISTRY_STAMP; the two must compute identically.
//
// This script runs from the `build` and `pretest` npm scripts BEFORE tsc, so
// build/ always carries a stamp derived from the src/ tree that was compiled.
// build/ always carries a stamp derived from the tool-specs.ts that was compiled.
import { createHash } from 'node:crypto';
import { readdirSync, readFileSync, statSync, writeFileSync } from 'node:fs';
import { readFileSync, writeFileSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { dirname, join, relative, sep } from 'node:path';
import { dirname, join } from 'node:path';
const __dirname = dirname(fileURLToPath(import.meta.url));
const SRC_DIR = join(__dirname, '..', 'src');
const TOOL_SPECS_PATH = join(SRC_DIR, 'tool-specs.ts');
const OUT_PATH = join(SRC_DIR, 'registry-stamp.generated.ts');
/**
* Recursively enumerate every `*.ts` file under `dir`, EXCLUDING the codegen's
* own `*.generated.ts` output (a self-referential cycle otherwise). Returns
* absolute paths, unsorted (the caller sorts by relative path for determinism).
* Kept as a plain exported function so the algorithm has a single home; the
* loader duplicates it because it lives in the CJS server build and cannot import
* this ESM script. If you change the walk/filter here, mirror it in
* apps/server/src/core/ai-chat/tools/docmost-client.loader.ts.
* Deterministic stamp of the tool-specs registry content. Kept as a plain
* function (exported) so the algorithm has a single home; the loader duplicates
* only the tiny normalize+sha256 steps because it lives in the CJS server build
* and cannot import this ESM script. If you change the normalization here, mirror
* it in apps/server/src/core/ai-chat/tools/docmost-client.loader.ts.
*/
export function collectStampFiles(dir) {
const out = [];
for (const entry of readdirSync(dir)) {
const full = join(dir, entry);
if (statSync(full).isDirectory()) {
out.push(...collectStampFiles(full));
} else if (entry.endsWith('.ts') && !entry.endsWith('.generated.ts')) {
out.push(full);
}
}
return out;
}
/**
* Deterministic stamp of the whole src/ tree. Enumerate + sort by POSIX-relative
* path, then fold each file's relative path AND normalized content into one
* sha256. MUST stay byte-for-byte identical to the loader's recompute.
*/
export function computeRegistryStamp(srcDir) {
const files = collectStampFiles(srcDir)
.map((abs) => ({
rel: relative(srcDir, abs).split(sep).join('/'),
abs,
}))
.sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0));
const hash = createHash('sha256');
for (const { rel, abs } of files) {
const normalized = readFileSync(abs, 'utf8')
.replace(/\r\n/g, '\n')
.replace(/\n$/, '');
hash.update(rel, 'utf8');
hash.update('\0', 'utf8');
hash.update(normalized, 'utf8');
hash.update('\0', 'utf8');
}
return hash.digest('hex');
export function computeRegistryStamp(toolSpecsSource) {
const normalized = toolSpecsSource.replace(/\r\n/g, '\n').replace(/\n$/, '');
return createHash('sha256').update(normalized, 'utf8').digest('hex');
}
function main() {
const stamp = computeRegistryStamp(SRC_DIR);
const source = readFileSync(TOOL_SPECS_PATH, 'utf8');
const stamp = computeRegistryStamp(source);
const out =
'// AUTO-GENERATED by scripts/gen-registry-stamp.mjs — DO NOT EDIT BY HAND.\n' +
'// A deterministic hash of the whole src/ tree (every src/**/*.ts except\n' +
'// *.generated.ts). Regenerated on every build/pretest so build/ always\n' +
'// matches the compiled src. The in-app loader recomputes this from src and\n' +
'// refuses to run on a mismatch (issue #447/#486). This file is gitignored\n' +
'// and produced by the build — see .gitignore.\n' +
'// A deterministic hash of src/tool-specs.ts content (tool names, descriptions,\n' +
'// tiers, catalog lines and input schemas). Regenerated on every build/pretest\n' +
'// so build/ always matches the compiled src. The in-app loader recomputes this\n' +
'// from src and refuses to run on a mismatch (issue #447). This file is\n' +
'// gitignored and produced by the build — see .gitignore.\n' +
`export const REGISTRY_STAMP = ${JSON.stringify(stamp)};\n`;
writeFileSync(OUT_PATH, out, 'utf8');
// eslint-disable-next-line no-console
+15 -62
View File
@@ -19,10 +19,7 @@ import {
assertYjsEncodable,
MutationResult,
} from "../lib/collaboration.js";
import {
acquireCollabSession,
isCollabAuthFailedError,
} from "../lib/collab-session.js";
import { acquireCollabSession } from "../lib/collab-session.js";
import { withPageLock, isUuid } from "../lib/page-lock.js";
import { getCollabToken, performLogin } from "../lib/auth-utils.js";
import { formatDocmostAxiosError } from "./errors.js";
@@ -495,37 +492,6 @@ export abstract class DocmostClientContext {
);
}
/**
* Run a collab write and, on a Hocuspocus HANDSHAKE auth failure, self-heal
* once (#486). Symmetric to the HTTP-401 path in getCollabTokenWithReauth: the
* REST interceptor and login() already drop the cached collab token on a 401/
* 403, but a rejected WEBSOCKET handshake left the stale token in the cache, so
* every subsequent mutation kept re-presenting the same bad token for up to the
* collab-token TTL (minutes) with no self-heal. Here, when the write rejects
* with the tagged collab-auth error, we invalidate the cached token and retry
* the write EXACTLY once with a force-refreshed token. Not a loop: a second
* failure (or any non-auth error) propagates unchanged.
*
* `write` receives the token to use, so the retry can hand it a genuinely fresh
* one rather than re-running with the same stale string.
*/
protected async writeWithCollabAuthRetry<T>(
collabToken: string,
write: (token: string) => Promise<T>,
): Promise<T> {
try {
return await write(collabToken);
} catch (e) {
if (!isCollabAuthFailedError(e)) throw e;
// The WS handshake rejected our token: drop it from the cache so it can't
// be reused for the rest of the TTL, mint a fresh one (forceRefresh bypasses
// the cache and re-invokes the provider/login), and retry the write once.
this.collabTokenCache = null;
const fresh = await this.getCollabTokenWithReauth(true);
return await write(fresh);
}
}
/**
* Connect to the collaboration websocket, read the live doc, apply
* `transform`, write the result, and wait for the server to persist it
@@ -560,24 +526,19 @@ export abstract class DocmostClientContext {
// unsyncedChanges/connectionLost ack logic live in CollabSession.mutate,
// preserved verbatim from the old inline machine (incl. the #152 structural
// diff that keeps a live editor's cursor anchored).
// Wrap in the collab-auth self-heal (#486): a rejected WS handshake drops the
// cached collab token and retries once with a fresh one (the retry passes the
// refreshed token down to acquireCollabSession via `token`).
return this.writeWithCollabAuthRetry(collabToken, async (token) => {
const session = await acquireCollabSession(pageId, token, this.apiUrl, {
// Only the actual 25s collab connect timeout emits this — the connect-vs-
// unload signal; the other failure paths must NOT emit it.
onConnectTimeout: () =>
this.onMetricFn?.("collab_connect_timeouts_total", 1),
});
try {
return await session.mutate(transform);
} catch (e) {
// Drop the session on any failure so the next call reconnects fresh.
session.destroy("mutate failed");
throw e;
}
const session = await acquireCollabSession(pageId, collabToken, this.apiUrl, {
// Only the actual 25s collab connect timeout emits this — the connect-vs-
// unload signal; the other failure paths must NOT emit it.
onConnectTimeout: () =>
this.onMetricFn?.("collab_connect_timeouts_total", 1),
});
try {
return await session.mutate(transform);
} catch (e) {
// Drop the session on any failure so the next call reconnects fresh.
session.destroy("mutate failed");
throw e;
}
}
/**
@@ -706,11 +667,7 @@ export abstract class DocmostClientContext {
transform: (doc: any) => any,
): Promise<{ doc?: any; verify?: any }> {
const pageUuid = await this.resolvePageId(pageId);
// #486: on a rejected collab-WS handshake, invalidate + refresh the token and
// retry the write once (symmetric to the HTTP-401 reauth path).
return this.writeWithCollabAuthRetry(collabToken, (token) =>
mutatePageContent(pageUuid, token, apiUrl, transform),
);
return mutatePageContent(pageUuid, collabToken, apiUrl, transform);
}
/**
@@ -730,11 +687,7 @@ export abstract class DocmostClientContext {
apiUrl: string,
): Promise<{ doc?: any; verify?: any }> {
const pageUuid = await this.resolvePageId(pageId);
// #486: on a rejected collab-WS handshake, invalidate + refresh the token and
// retry the write once (symmetric to the HTTP-401 reauth path).
return this.writeWithCollabAuthRetry(collabToken, (token) =>
replacePageContent(pageUuid, doc, token, apiUrl),
);
return replacePageContent(pageUuid, doc, collabToken, apiUrl);
}
/**
+2 -7
View File
@@ -127,13 +127,8 @@ export function ReadMixin<TBase extends GConstructor<DocmostClientContext>>(Base
"listPages: tree mode requires a spaceId (a page tree is scoped to one space). Pass spaceId, or omit tree to get the recent-pages list.",
);
}
// #486: propagate `truncated` (same pattern as check_new_comments). The old
// code dropped it, so a caller handed an INCOMPLETE tree (the stdio-fallback
// BFS hit its node cap) had no way to know pages were missing. Return the
// tree alongside the flag; the primary /pages/tree path is uncapped so this
// is false there.
const { pages, truncated } = await this.enumerateSpacePages(spaceId);
return { tree: buildPageTree(pages), truncated };
const { pages } = await this.enumerateSpacePages(spaceId);
return buildPageTree(pages);
}
const clampedLimit = Math.max(1, Math.min(100, limit));
+1
View File
@@ -4,6 +4,7 @@ import { readFileSync } from "fs";
import { fileURLToPath } from "url";
import { dirname, join } from "path";
import { DocmostClient, DocmostMcpConfig } from "./client.js";
import { parseNodeArg } from "@docmost/prosemirror-markdown";
import { searchShapes } from "./lib/drawio-shapes.js";
import { getGuideSection } from "./lib/drawio-guide.js";
import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js";
+4 -26
View File
@@ -37,25 +37,6 @@ const CONNECT_TIMEOUT_MS = 25000;
/** Time we wait for the server to acknowledge our write before giving up. */
const PERSIST_TIMEOUT_MS = 20000;
/**
* Marker property set on the Error thrown when the Hocuspocus handshake REJECTS
* our collab token (onAuthenticationFailed). The client wraps content writes so
* that on this specific failure it invalidates its cached collab token and
* retries once with a fresh one symmetric to the HTTP-401 reauth path (#486).
* A plain message-match would be brittle; a tagged property is unambiguous and
* survives teardown (which rejects pending ops with this SAME error object).
*/
const COLLAB_AUTH_FAILED_MARKER = "collabAuthFailed";
/** True when `e` is the tagged collab-WS auth-failure error (see marker above). */
export function isCollabAuthFailedError(e: unknown): boolean {
return !!(
e &&
typeof e === "object" &&
(e as Record<string, unknown>)[COLLAB_AUTH_FAILED_MARKER] === true
);
}
/**
* Tunables, read fresh from the environment on every acquire so tests (and a
* live rollback) can change them without reloading the module. Mirrors how
@@ -321,13 +302,10 @@ export class CollabSession {
this.openResolve?.();
},
onAuthenticationFailed: () => {
// Tag the error so the client can tell a REJECTED collab token apart
// from a generic disconnect and invalidate + refresh it (#486).
const err = new Error(
"Authentication failed for collaboration connection",
) as Error & { [COLLAB_AUTH_FAILED_MARKER]?: boolean };
err[COLLAB_AUTH_FAILED_MARKER] = true;
this.teardown(err, true);
this.teardown(
new Error("Authentication failed for collaboration connection"),
true,
);
},
});
});
+24 -3
View File
@@ -10,7 +10,10 @@ import { JSDOM } from "jsdom";
// handled there). MCP consumes it directly instead of maintaining its own
// drifted marked pipeline; only the collab/yjs write glue and the footnote
// canonicalization wrapper stay mcp-side.
import { markdownToProseMirror } from "@docmost/prosemirror-markdown";
import {
markdownToProseMirror,
normalizeAgentMarkdown,
} from "@docmost/prosemirror-markdown";
import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
import { withPageLock } from "./page-lock.js";
import {
@@ -20,6 +23,7 @@ import {
} from "@docmost/prosemirror-markdown";
import { canonicalizeFootnotes } from "./footnote-canonicalize.js";
import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js";
import { regraftResolvedComments } from "./comment-anchor.js";
import { VerifyReport } from "./diff.js";
import { acquireCollabSession } from "./collab-session.js";
@@ -97,6 +101,15 @@ global.WebSocket = WebSocket;
* plain `markdownToProseMirror` (no canonicalization) safe now because inline
* `^[body]` footnotes carry their body at the reference point, so a comment can
* no longer produce a reference-less footnote definition to be dropped.
*
* #493: `normalizeAgentMarkdown` runs FIRST, so an agent's `updatePageMarkdown`
* body gets the SAME GFM `[^id]` reference-footnote -> inline `^[body]` rewrite as
* the server import path (instead of the reference leaking as literal text / a
* bogus link). It DELIBERATELY does NOT strip a leading YAML front-matter block:
* a full-body agent rewrite that opens with a `---…---` is (almost) always a
* horizontalRule the serializer emitted, and stripping it would silently drop the
* page's leading content (#493 review). The front-matter strip stays on the
* server FILE-import boundary only (`normalizeForeignMarkdown`).
*/
export async function markdownToProseMirrorCanonical(
markdownContent: string,
@@ -105,7 +118,9 @@ export async function markdownToProseMirrorCanonical(
// canonicalizing, so the canonicalizer re-hangs references and drops the
// now-orphaned duplicate definitions.
return canonicalizeFootnotes(
normalizeAndMergeFootnotes(await markdownToProseMirror(markdownContent)),
normalizeAndMergeFootnotes(
await markdownToProseMirror(normalizeAgentMarkdown(markdownContent)),
),
);
}
@@ -328,6 +343,12 @@ export async function updatePageContentRealtime(
pageId,
collabToken,
baseUrl,
() => tiptapJson,
// #493: an agent read HIDES resolved-comment anchors (#337), so the markdown
// it sends here no longer carries them — a naive full rewrite would erase
// every resolved comment mark. Re-graft the resolved marks from the LIVE doc
// onto the matching text in the freshly-imported body. Active comments are
// untouched (they ride through the markdown themselves); a resolved span whose
// text the agent changed simply does not re-anchor and is dropped.
(liveDoc) => regraftResolvedComments(liveDoc, tiptapJson),
);
}
+112 -3
View File
@@ -312,10 +312,9 @@ export function canAnchorInDoc(doc: any, selection: string): boolean {
function spliceCommentMark(
blockContent: any[],
match: AnchorMatch,
commentId: string,
commentMark: any,
): void {
const { startChild, startOffset, endChild, endOffset } = match;
const commentMark = makeCommentMark(commentId);
const fragments: any[] = [];
for (let k = startChild; k <= endChild; k++) {
@@ -451,6 +450,22 @@ export function applyAnchorInDoc(
doc: any,
selection: string,
commentId: string,
): boolean {
return applyCommentMarkInDoc(doc, selection, makeCommentMark(commentId));
}
/**
* Core of {@link applyAnchorInDoc}, but splices an ARBITRARY comment mark object
* (not just a fresh `{ commentId, resolved:false }`) across the first matching
* range. This lets a caller re-apply a mark that carries `resolved:true` and any
* other stored attrs. Depth-first (same order as canAnchorInDoc); mutates in
* place on the first matching block and returns true, else returns false without
* mutating.
*/
export function applyCommentMarkInDoc(
doc: any,
selection: string,
commentMark: any,
): boolean {
const { selection: effective, found } = resolveAnchorSelection(doc, selection);
if (!found) return false;
@@ -459,7 +474,7 @@ export function applyAnchorInDoc(
if (!Array.isArray(node.content)) return false;
const match = findAnchorInBlock(node.content, effective);
if (match) {
spliceCommentMark(node.content, match, commentId);
spliceCommentMark(node.content, match, commentMark);
return true;
}
for (const child of node.content) {
@@ -471,3 +486,97 @@ export function applyAnchorInDoc(
};
return visit(doc, 0);
}
/** A resolved inline-comment span lifted from a doc: its mark + anchored text. */
export interface ResolvedCommentSpan {
commentId: string;
/** The full comment mark (carrying `resolved:true` + any stored attrs). */
mark: any;
/** The concatenated raw text the mark spans — used as the re-anchor selection. */
text: string;
}
/** True when a text node carries a RESOLVED comment mark; returns that mark. */
function resolvedCommentMarkOf(node: any): any | null {
if (!node || node.type !== "text" || !Array.isArray(node.marks)) return null;
return (
node.marks.find(
(m: any) =>
m && m.type === "comment" && m.attrs?.resolved === true && m.attrs?.commentId,
) || null
);
}
/**
* Collect every RESOLVED inline-comment span in `doc`, in document order. Within
* each block's direct content, a maximal run of consecutive text nodes sharing
* the same resolved `commentId` is ONE span; its concatenated raw text is the
* selection used to re-anchor it elsewhere. Active (unresolved) comment marks are
* ignored they survive a markdown round-trip on their own (a page read emits
* their `<span data-comment-id>` wrapper), whereas resolved anchors are hidden
* from agent reads (#337) and would be erased by a full-body markdown rewrite.
*/
export function collectResolvedCommentSpans(doc: any): ResolvedCommentSpan[] {
const spans: ResolvedCommentSpan[] = [];
const visit = (node: any, depth: number): void => {
if (depth > MAX_DEPTH || !node || typeof node !== "object") return;
if (!Array.isArray(node.content)) return;
const content = node.content;
let i = 0;
while (i < content.length) {
const mark = resolvedCommentMarkOf(content[i]);
if (mark) {
const commentId = mark.attrs.commentId;
let text = "";
let j = i;
while (j < content.length) {
const mj = resolvedCommentMarkOf(content[j]);
if (!mj || mj.attrs.commentId !== commentId) break;
text += typeof content[j].text === "string" ? content[j].text : "";
j++;
}
if (text.length > 0) spans.push({ commentId, mark, text });
i = j > i ? j : i + 1;
} else {
i++;
}
}
for (const child of content) {
if (child && typeof child === "object" && Array.isArray(child.content)) {
visit(child, depth + 1);
}
}
};
visit(doc, 0);
return spans;
}
/**
* Re-graft RESOLVED comment marks from `oldDoc` onto matching text ranges in
* `newDoc`, returning a NEW doc (never mutates the inputs).
*
* WHY (#493): an agent read hides resolved-comment anchors (#337), so the
* markdown it sends to a FULL-body rewrite (`updatePageMarkdown`) no longer
* carries them a naive full write would erase every resolved comment mark.
* This restores them: each resolved span from the previous document is re-anchored
* onto the SAME text in the newly-imported body (first occurrence, using the
* shared anchoring / markdown-strip fallback), preserving `resolved:true` and the
* stored attrs. A span whose text the agent changed or deleted simply does not
* re-anchor and is dropped (its anchor is gone; it was already resolved). Active
* comments are untouched they ride through the markdown themselves.
*/
export function regraftResolvedComments<T = any>(oldDoc: any, newDoc: T): T {
if (!newDoc || typeof newDoc !== "object") return newDoc;
const spans = collectResolvedCommentSpans(oldDoc);
if (spans.length === 0) return newDoc;
const out =
typeof structuredClone === "function"
? structuredClone(newDoc)
: (JSON.parse(JSON.stringify(newDoc)) as T);
for (const span of spans) {
// Clone the mark so the new document never shares a mark object with oldDoc.
const markClone = { type: "comment", attrs: { ...span.mark.attrs } };
applyCommentMarkInDoc(out, span.text, markClone);
}
return out;
}
+31 -74
View File
@@ -10,7 +10,7 @@
// exactly mxGraph's convention for a child of a container, so they map across
// directly. Container sizes are computed by ELK; leaf sizes are preserved.
import { Worker } from "node:worker_threads";
import ELK from "elkjs/lib/elk.bundled.js";
import { JSDOM } from "jsdom";
import { normalizeInput, parseCells, type DrawioCell } from "./drawio-xml.js";
@@ -18,33 +18,22 @@ import { normalizeInput, parseCells, type DrawioCell } from "./drawio-xml.js";
const DEFAULT_W = 140;
const DEFAULT_H = 60;
// DoS bounds for the ELK layout. The mxGraph XML is LLM-supplied (layout:"elk"
// in drawioCreate/drawioUpdate). elkjs' layout() returns a Promise but runs the
// crossing-minimisation SYNCHRONOUSLY — it blocks whatever thread it runs on for
// the whole pass. A ~1MB XML (well under the stage-1 16MB cap) can carry
// thousands of nodes. We (a) cap the graph size before ever calling ELK and
// (b) run the layout in a WORKER THREAD so the main event loop stays free, with
// the wall-clock timeout enforced by terminating that worker. On either bound we
// fall back to the ORIGINAL model, the same best-effort contract the catch honours.
// DoS bounds for the in-process ELK layout. The mxGraph XML is LLM-supplied
// (layout:"elk" in drawioCreate/drawioUpdate) and elkjs runs synchronously on
// the MCP server's event loop, so an unbounded graph would block it for
// seconds-to-minutes. A ~1MB XML (well under the stage-1 16MB cap) can carry
// thousands of nodes. We cap the graph size and race the layout against a
// wall-clock timeout; on either bound we fall back to the ORIGINAL model, the
// same best-effort contract the catch already honours.
// - 500 nodes lays out in well under a second; beyond that ELK cost climbs
// steeply, so refuse and leave the (already-valid) model untouched.
// - Edges dominate the layered-crossing cost, so allow a bit more headroom
// (1000) than nodes but still bound them.
// - The timeout is a HARD kill of the worker thread — the only way to interrupt
// synchronous JS. The in-process setTimeout race we used before was an
// illusion: the timer could never fire while the SAME thread was blocked
// inside elkjs, so it "protected" nothing. Now the timer runs on the main
// thread while ELK runs on the worker, so it can actually fire and terminate.
// - 5s is generous for any graph within the caps yet short enough that a
// pathological input can never wedge the server.
const ELK_MAX_NODES = 500;
const ELK_MAX_EDGES = 1000;
// Wall-clock ceiling for a single layout pass. Overridable for tests (a tiny
// value forces the terminate-on-timeout path deterministically); a non-positive
// or unparseable override falls back to the default.
const ELK_TIMEOUT_DEFAULT_MS = 5000;
function resolveElkTimeoutMs(): number {
const raw = Number(process.env.DRAWIO_ELK_TIMEOUT_MS);
return Number.isFinite(raw) && raw > 0 ? Math.floor(raw) : ELK_TIMEOUT_DEFAULT_MS;
}
const ELK_TIMEOUT_MS = 5000;
// Spacing is set >=150px on purpose so an ELK layout never trips the linter's
// "gap between adjacent shapes < 150px" quality warning (acceptance #3).
@@ -89,57 +78,13 @@ interface ElkGraph extends ElkNode {
edges?: ElkEdge[];
}
/**
* Run one ELK layered layout on a worker thread and resolve with the laid-out
* graph. The timeout is enforced by `worker.terminate()` a HARD kill, which is
* the only way to interrupt elkjs' synchronous crossing-minimisation once it has
* started. Rejects on timeout, worker error, or an early exit; the caller treats
* any rejection as "keep the original model" (best-effort layout).
*/
function layoutInWorker(graph: ElkGraph, timeoutMs: number): Promise<ElkGraph> {
return new Promise((resolve, reject) => {
const worker = new Worker(
new URL("./drawio-layout.worker.js", import.meta.url),
{ workerData: { graph } },
);
let settled = false;
const finish = (fn: () => void) => {
if (settled) return;
settled = true;
clearTimeout(timer);
// Always tear the worker down: on the happy path so it does not linger,
// on timeout so the blocked synchronous ELK run is actually interrupted.
void worker.terminate();
fn();
};
const timer = setTimeout(
() => finish(() => reject(new Error("ELK layout timed out"))),
timeoutMs,
);
worker.once("message", (msg: { ok?: boolean; laid?: ElkGraph; error?: string }) => {
finish(() =>
msg?.ok
? resolve(msg.laid as ElkGraph)
: reject(new Error(msg?.error ?? "ELK layout failed")),
);
});
worker.once("error", (err) => finish(() => reject(err)));
worker.once("exit", (code) => {
// A clean exit after we already settled is normal (terminate()); only an
// unexpected early exit while still pending is a failure.
if (settled) return;
finish(() => reject(new Error(`ELK worker exited early (code ${code})`)));
});
});
}
/**
* Apply an ELK layered layout to a drawio input and return a full mxGraphModel
* string with rewritten geometry. Accepts the same three input forms as
* drawioCreate (a bare model, an <mxfile>, or a <mxCell> list). Async because
* the layout runs on a worker thread. On any layout failure (including a
* terminate-on-timeout) the ORIGINAL (normalized) model is returned unchanged
* layout is best-effort polish, never a reason to fail the write.
* elkjs' layout() is promise-based. On any layout failure the ORIGINAL
* (normalized) model is returned unchanged layout is best-effort polish, never
* a reason to fail the write.
*/
export async function applyElkLayout(inputXml: string): Promise<string> {
const modelXml = normalizeInput(inputXml);
@@ -205,14 +150,26 @@ export async function applyElkLayout(inputXml: string): Promise<string> {
};
let laid: ElkGraph;
let timer: ReturnType<typeof setTimeout> | undefined;
try {
// Run the (synchronous-under-the-hood) ELK pass on a worker thread so the
// main event loop is never blocked, and enforce the wall-clock ceiling by
// terminating that worker on timeout. A graph under the node/edge caps but
// still pathologically slow is hard-killed instead of wedging anything.
laid = await layoutInWorker(graph, resolveElkTimeoutMs());
// elkjs ships a CJS default export whose interop shape varies across
// module systems; resolve the real constructor at runtime, then cast (the
// runtime call is verified — see the layout unit test).
const Ctor: any = (ELK as any).default ?? ELK;
const elk = new Ctor();
// Race the layout against a wall-clock timeout so a graph that is under the
// node/edge caps but still pathologically slow can never wedge the server.
const timeout = new Promise<never>((_, reject) => {
timer = setTimeout(
() => reject(new Error("ELK layout timed out")),
ELK_TIMEOUT_MS,
);
});
laid = (await Promise.race([elk.layout(graph as any), timeout])) as ElkGraph;
} catch {
return modelXml; // best-effort: keep the model as-is on timeout or ELK failure
} finally {
if (timer) clearTimeout(timer);
}
// Collect computed geometry per node id (coords are parent-relative already).
@@ -1,36 +0,0 @@
// Worker-thread entry for the ELK layered layout (issue #486, commit 1).
//
// elkjs' layout() returns a Promise but runs the actual crossing-minimisation
// SYNCHRONOUSLY — it blocks whatever thread it runs on for the whole pass. On
// the in-app MCP host that thread used to be the main NestJS event loop, so a
// pathological graph at the node/edge cap could wedge ALL HTTP/SSE/loopback
// traffic while it churned. Running it HERE, on a dedicated worker thread, keeps
// the main loop free; the parent enforces the wall-clock timeout by calling
// `worker.terminate()` — the only way to interrupt synchronous JS — since the
// in-process `setTimeout` race the parent used before could never fire while the
// same thread was blocked inside elkjs.
import { parentPort, workerData } from "node:worker_threads";
import ELK from "elkjs/lib/elk.bundled.js";
interface WorkerInput {
graph: unknown;
}
const { graph } = (workerData ?? {}) as WorkerInput;
// elkjs ships a CJS default export whose interop shape varies across module
// systems; resolve the real constructor at runtime (same as the parent did).
const Ctor: any = (ELK as any).default ?? ELK;
const elk = new Ctor();
elk
.layout(graph as any)
.then((laid: unknown) => {
parentPort?.postMessage({ ok: true, laid });
})
.catch((err: unknown) => {
parentPort?.postMessage({
ok: false,
error: err instanceof Error ? err.message : String(err),
});
});
+21 -93
View File
@@ -1,64 +1,30 @@
/**
* Locator normalization: strip inline markdown wrappers and trailing
* decoration from a LOCATOR string so a find/anchor that the model wrote with
* markdown (or a stray emoji) can still match the document's plain text.
* Locator normalization helpers for mcp. The two PRIMITIVES
* `stripInlineMarkdown` (lenient locator normalizer) and `stripWrappersAndLinks`
* (strict balanced-wrapper/link collapse) live in the canonical package
* `@docmost/prosemirror-markdown` (#493 dedup: they used to be forked verbatim
* here). This module now only re-exports `stripInlineMarkdown` and adds the two
* mcp-only helpers built on top: `stripBalancedWrappers` and `closestBlockHint`.
*
* This is used ONLY as a fallback for LOCATING (after an exact match fails);
* it is never applied to replacement text or inserted node content, so no
* formatting is ever lost.
* They are used ONLY as a fallback for LOCATING (after an exact match fails) and
* for formatting-vs-plain intent detection; never applied to replacement text or
* inserted node content, so no formatting is ever lost.
*/
import {
stripInlineMarkdown,
stripWrappersAndLinks,
} from "@docmost/prosemirror-markdown";
/** Maximum unwrap passes, so pathological/nested input cannot loop forever. */
const MAX_PASSES = 8;
// Re-export the canonical locator normalizer so mcp call sites keep importing it
// from `./text-normalize.js` unchanged.
export { stripInlineMarkdown };
/**
* Inline emphasis/code/strikethrough wrappers, strong BEFORE emphasis so
* `**x**` collapses to `x` rather than leaving a stray `*x*`. Each pattern is
* non-greedy and capture group 1 is the inner text. Applied repeatedly until
* the string stops changing (nested wrappers like `**_x_**`).
*/
const WRAPPER_PATTERNS: RegExp[] = [
/\*\*([^*]+?)\*\*/g, // **x**
/__([^_]+?)__/g, // __x__
/~~([^~]+?)~~/g, // ~~x~~
/\*([^*]+?)\*/g, // *x*
/_([^_]+?)_/g, // _x_
/``([^`]+?)``/g, // ``x``
/`([^`]+?)`/g, // `x`
];
/** Links/images -> their visible text. `!?` covers both `[t](u)` and `![a](s)`. */
const LINK_IMAGE_RE = /!?\[([^\]]*)\]\([^)]*\)/g;
/**
* Apply ONLY the two balanced/link passes shared by both normalizers: first
* collapse links/images to their visible text, then collapse balanced inline
* wrappers repeatedly until stable. Does NOT trim decoration, does NOT guard
* against an empty result it returns exactly the transformed string.
*/
function stripWrappersAndLinks(s: string): string {
// 1. Links/images -> their visible text.
let out = s.replace(LINK_IMAGE_RE, "$1");
// 2. Strip balanced wrappers, repeating until the string is stable so nested
// wrappers (`**_x_**`) and adjacent runs both collapse.
for (let pass = 0; pass < MAX_PASSES; pass++) {
const before = out;
for (const re of WRAPPER_PATTERNS) {
out = out.replace(re, "$1");
}
if (out === before) break;
}
return out;
}
/**
* STRICT formatting detector distinct from the lenient locator
* normalization below. It strips ONLY what unambiguously is markdown markup:
* 1. links/images `[text](url)` -> `text`, `![alt](src)` -> `alt`, and
* 2. balanced inline `**`/`__`/`~~`/`*`/`_`/`` ` `` wrappers (repeat-until-stable),
* and DELIBERATELY does NOT trim leading/trailing whitespace, emoji, or lone
* marker chars (the lenient extras `stripInlineMarkdown` does in its step 3).
* STRICT formatting detector distinct from the lenient locator normalization.
* It strips ONLY what unambiguously is markdown markup (links/images to visible
* text, and balanced inline `**`/`__`/`~~`/`*`/`_`/`` ` `` wrappers) and
* DELIBERATELY does NOT trim leading/trailing whitespace, emoji, or lone marker
* chars (the lenient extras `stripInlineMarkdown` does).
*
* It exists ONLY to recognize formatting-vs-plain INTENT in `applyTextEdits`
* (deciding whether find/replace differ purely by markdown markers). Because it
@@ -77,44 +43,6 @@ export function stripBalancedWrappers(s: string): string {
return stripWrappersAndLinks(s);
}
/**
* Conservatively strip inline markdown from a locator string.
*
* Deterministic, order-fixed steps:
* 1. Links/images: `[text](url)` -> `text`, `![alt](src)` -> `alt`.
* 2. Balanced inline wrappers (strong before emphasis, code, strikethrough),
* applied repeatedly until stable for nested cases.
* 3. Trim leading/trailing decoration only: whitespace, leftover marker chars
* (`* _ ~ \``) and emoji. Letters/digits and sentence punctuation (`.`/`,`
* etc.) are NEVER trimmed.
*
* If the result is empty (e.g. the input was only markers like `***`), the
* ORIGINAL string is returned so a locator can never normalize down to "" and
* match everything.
*/
export function stripInlineMarkdown(s: string): string {
if (typeof s !== "string" || s.length === 0) return s;
// 1 + 2. Shared link/image and balanced-wrapper passes.
let out = stripWrappersAndLinks(s);
// 3. Trim leading/trailing decoration: whitespace, leftover markdown markers,
// and emoji (Extended_Pictographic plus the VS16 / ZWJ joiners, plus the
// regional-indicator range U+1F1E6–U+1F1FF for flag emoji, which are NOT
// Extended_Pictographic). The `u` flag enables the Unicode property escape.
// Anchored runs only — interior text and sentence punctuation are untouched.
const DECORATION =
"[\\s*_~\\x60\\p{Extended_Pictographic}\\u{1F1E6}-\\u{1F1FF}\\u{FE0F}\\u{200D}]+";
out = out
.replace(new RegExp("^" + DECORATION, "u"), "")
.replace(new RegExp(DECORATION + "$", "u"), "");
// 4. Never normalize a locator down to nothing.
if (out.length === 0) return s;
return out;
}
/**
* Build a bounded "closest text" hint for an anchor/find MISS, shared by
* editPageText (json-edit) and createComment (client) so both surface the
+3 -4
View File
@@ -923,10 +923,9 @@ export const SHARED_TOOL_SPECS = {
'List the most recent pages (ordered by updatedAt, descending), ' +
'optionally scoped to a single space. Returns a bounded list (default ' +
'50, max 100) — use search for lookups in large spaces. tree:true (with ' +
"spaceId) returns { tree, truncated } — the space's full page hierarchy " +
'as a nested tree, plus a `truncated` flag that is true when the tree was ' +
'capped and is INCOMPLETE — but is DEPRECATED, use getTree instead ' +
'(leaner nodes, plus rootPageId / maxDepth).',
"spaceId) returns the space's full page hierarchy as a nested tree, but " +
'is DEPRECATED — use getTree instead (leaner nodes, plus rootPageId / ' +
'maxDepth).',
tier: 'core',
catalogLine:
"listPages — list recent pages (tree:true is deprecated; use getTree for the hierarchy).",
@@ -1,137 +0,0 @@
// Unit tests for the collab-token reset on a Hocuspocus WS auth failure (#486).
//
// Before this fix the cached collab token (#435) was dropped ONLY on an HTTP
// 401/403 (the REST interceptor + login()); a rejected collab-WEBSOCKET handshake
// left the stale token in the cache, so every subsequent mutation re-presented
// the SAME bad token for up to the collab-token TTL (minutes) with no self-heal.
//
// The fix wraps collab writes in `writeWithCollabAuthRetry`: when the write
// rejects with the tagged collab-auth error (collab-session.ts's
// onAuthenticationFailed), it invalidates the cached token and retries the write
// ONCE with a force-refreshed token — symmetric to the HTTP-401 path.
//
// writeWithCollabAuthRetry / getCollabTokenWithReauth are protected in TS but
// plain methods on the compiled build, so the tests call them directly (same
// convention as collab-token-cache.test.mjs).
import { test, afterEach } from "node:test";
import assert from "node:assert/strict";
import { DocmostClient } from "../../build/client.js";
const ENV_KEY = "MCP_COLLAB_TOKEN_TTL_MS";
afterEach(() => {
delete process.env[ENV_KEY];
});
// A counting provider that returns a distinct token each call so a cached
// (reused) token is visibly the SAME string while a fresh mint is different.
function countingProvider() {
let n = 0;
const fn = async () => {
n++;
return `provider-token-${n}`;
};
return {
fn,
get calls() {
return n;
},
};
}
// The tagged error collab-session.ts throws on a rejected WS handshake.
function collabAuthError() {
const err = new Error("Authentication failed for collaboration connection");
err.collabAuthFailed = true;
return err;
}
test("a WS auth failure clears the cached token and retries the write with a FRESH one (#486)", async () => {
process.env[ENV_KEY] = "300000"; // 5 min: the cache is warm across the burst.
const p = countingProvider();
const client = new DocmostClient({
apiUrl: "http://127.0.0.1:1/api",
getToken: async () => "access",
getCollabToken: p.fn,
});
// Warm the cache the way a real write would (mints provider-token-1).
const initial = await client.getCollabTokenWithReauth();
assert.equal(initial, "provider-token-1");
assert.equal(p.calls, 1);
const tokensSeen = [];
const write = async (token) => {
tokensSeen.push(token);
// The FIRST attempt (with the stale cached token) fails the WS handshake;
// the retry (with a fresh token) succeeds.
if (tokensSeen.length === 1) throw collabAuthError();
return `written-with:${token}`;
};
const result = await client.writeWithCollabAuthRetry(initial, write);
assert.equal(tokensSeen.length, 2, "write attempted exactly twice (one retry)");
assert.equal(tokensSeen[0], "provider-token-1", "first attempt used the stale token");
assert.equal(
tokensSeen[1],
"provider-token-2",
"retry used a FRESH force-refreshed token, not the stale cached one",
);
assert.equal(result, "written-with:provider-token-2", "the retry's result wins");
assert.equal(p.calls, 2, "exactly one extra mint for the retry — no loop");
// The cache now holds the fresh token, so a subsequent op reuses it (proving
// the stale token was evicted and the fresh one cached, not re-minted).
const next = await client.getCollabTokenWithReauth();
assert.equal(next, "provider-token-2", "the fresh token replaced the stale cache");
assert.equal(p.calls, 2, "served from cache — provider not re-invoked");
});
test("a successful write is NOT retried and mints nothing extra", async () => {
process.env[ENV_KEY] = "300000";
const p = countingProvider();
const client = new DocmostClient({
apiUrl: "http://127.0.0.1:1/api",
getToken: async () => "access",
getCollabToken: p.fn,
});
const initial = await client.getCollabTokenWithReauth(); // provider-token-1
let attempts = 0;
const result = await client.writeWithCollabAuthRetry(initial, async (token) => {
attempts++;
return `ok:${token}`;
});
assert.equal(attempts, 1, "no retry on success");
assert.equal(result, "ok:provider-token-1");
assert.equal(p.calls, 1, "no extra mint");
});
test("a NON-auth write error propagates unchanged (no reset, no retry)", async () => {
process.env[ENV_KEY] = "300000";
const p = countingProvider();
const client = new DocmostClient({
apiUrl: "http://127.0.0.1:1/api",
getToken: async () => "access",
getCollabToken: p.fn,
});
const initial = await client.getCollabTokenWithReauth(); // provider-token-1
let attempts = 0;
await assert.rejects(
client.writeWithCollabAuthRetry(initial, async () => {
attempts++;
throw new Error("collab connection closed before persist"); // NOT tagged.
}),
/closed before persist/,
);
assert.equal(attempts, 1, "a non-auth error is not retried");
assert.equal(p.calls, 1, "the cache is untouched -> no fresh mint");
// Cache still holds the original token (was never invalidated).
const still = await client.getCollabTokenWithReauth();
assert.equal(still, "provider-token-1");
assert.equal(p.calls, 1);
});
@@ -1,55 +0,0 @@
// Unit test: listPages tree mode must propagate the `truncated` flag (#486).
//
// enumerateSpacePages returns { pages, truncated } — truncated is true ONLY when
// the stdio-fallback BFS hit its node cap (the primary /pages/tree path is
// uncapped). The old tree-mode listPages destructured only `pages` and returned a
// bare tree, dropping `truncated`, so a caller handed an INCOMPLETE tree had no
// way to know pages were missing. The fix returns { tree, truncated } (same
// pattern check_new_comments uses).
//
// Reaching the real cap (MAX_NODES = 10000) in a mock is impractical, so we stub
// enumerateSpacePages directly to assert the flag is threaded through verbatim.
import { test } from "node:test";
import assert from "node:assert/strict";
import { DocmostClient } from "../../build/client.js";
function stubClient() {
const client = new DocmostClient({
apiUrl: "http://127.0.0.1:1/api",
getToken: async () => "access",
});
// No network: the tree path only calls ensureAuthenticated + enumerateSpacePages.
client.ensureAuthenticated = async () => {};
return client;
}
const onePage = [{ id: "r1", title: "Root", parentPageId: null }];
test("tree mode carries truncated:true when the enumeration truncated (#486)", async () => {
const client = stubClient();
client.enumerateSpacePages = async () => ({ pages: onePage, truncated: true });
const res = await client.listPages("space-1", 50, true);
assert.equal(res.truncated, true, "the truncated flag is threaded through");
assert.ok(Array.isArray(res.tree), "the built tree rides alongside the flag");
assert.equal(res.tree[0].id, "r1");
});
test("tree mode carries truncated:false for a complete enumeration", async () => {
const client = stubClient();
client.enumerateSpacePages = async () => ({ pages: onePage, truncated: false });
const res = await client.listPages("space-1", 50, true);
assert.equal(res.truncated, false);
assert.equal(res.tree[0].id, "r1");
});
test("tree mode still requires a spaceId", async () => {
const client = stubClient();
await assert.rejects(
client.listPages(undefined, 50, true),
/tree mode requires a spaceId/,
);
});
@@ -108,6 +108,17 @@ async function spawnCollabStack(seedDoc) {
return { state, baseURL };
}
// y-prosemirror stores an OVERLAPPING mark (one whose type does not exclude
// itself — e.g. `comment`, and since #515 `code` with `excludes: ""`) under a
// HASHED Yjs attribute key `name--<8-char hash>` so several may coexist on a
// range. The real read path (yDocToProsemirrorJSON) strips that suffix back to
// the bare mark name via this exact regex; mirror it here so this minimal decoder
// reports the same mark names Docmost actually returns (without it an overlapping
// `code` would leak as `code--<hash>`).
const hashedMarkNameRegex = /(.*)(--[a-zA-Z0-9+/=]{8})$/;
const yattr2markname = (attrName) =>
hashedMarkNameRegex.exec(attrName)?.[1] ?? attrName;
// Minimal XmlFragment -> ProseMirror JSON decode, mirroring the shape Docmost
// stores. Reads element name as node type, attributes as attrs, and recurses into
// children; text nodes carry their string.
@@ -121,8 +132,8 @@ function fragmentToJson(frag) {
if (d.attributes && Object.keys(d.attributes).length) {
node.marks = Object.entries(d.attributes).map(([type, attrs]) =>
attrs && typeof attrs === "object" && Object.keys(attrs).length
? { type, attrs }
: { type },
? { type: yattr2markname(type), attrs }
: { type: yattr2markname(type) },
);
}
return node;
@@ -184,14 +184,12 @@ test("enumerateSpacePages (via listPages tree) uses one /pages/tree request", as
});
const client = new DocmostClient(baseURL, "user@example.com", "pw");
// listPages tree:true -> enumerateSpacePages(spaceId) -> { tree, truncated }.
const { tree, truncated } = await client.listPages("space-1", 50, true);
// listPages tree:true -> enumerateSpacePages(spaceId) -> buildPageTree.
const tree = await client.listPages("space-1", 50, true);
assert.equal(treeRequests, 1, "exactly one /pages/tree request for the space");
assert.equal(sidebarRequests, 0, "no per-node sidebar BFS requests");
assert.deepEqual(treeBody, { spaceId: "space-1" }, "space scope posts spaceId only");
// The uncapped /pages/tree path is never truncated (#486).
assert.equal(truncated, false, "primary /pages/tree path is not truncated");
// buildPageTree nests c1 under r1; two roots at the top level.
assert.equal(tree.length, 2, "two root nodes");
const r1 = tree.find((n) => n.id === "r1");
@@ -251,7 +249,7 @@ test("enumerateSpacePages falls back to the cursor BFS on /pages/tree 404", asyn
});
const client = new DocmostClient(baseURL, "user@example.com", "pw");
const { tree, truncated } = await client.listPages("space-1", 50, true);
const tree = await client.listPages("space-1", 50, true);
assert.ok(treeRequests >= 1, "the tree endpoint was attempted first");
assert.deepEqual(
@@ -259,8 +257,6 @@ test("enumerateSpacePages falls back to the cursor BFS on /pages/tree 404", asyn
["<root>", "r1"],
"fell back to the sidebar BFS: roots then the root's children",
);
// Small fallback walk well under the node cap -> not truncated (#486).
assert.equal(truncated, false, "fallback BFS below the cap is not truncated");
assert.equal(tree.length, 1, "one root in the built tree");
assert.equal(tree[0].children[0].id, "c1", "leaf nested via the BFS");
});
@@ -101,88 +101,6 @@ test("DoS guard: a graph over the node cap is returned unchanged, quickly", asyn
assert.ok(dt < 2000, `cap path should be fast, took ${dt}ms`);
});
/** Build a layered DAG near the caps: `n` vertices, up to ~2 edges each into the
* next layer of `layerSize`. Used as a real worst-case graph for the benchmark. */
function layeredGraph(n, layerSize) {
let cells = "";
for (let i = 2; i < 2 + n; i++) {
cells +=
`<mxCell id="${i}" value="N${i}" style="rounded=1;html=1;" vertex="1" parent="1">` +
`<mxGeometry x="10" y="10" width="120" height="60" as="geometry"/></mxCell>`;
}
let ei = 0;
for (let i = 2; i < 2 + n; i++) {
for (const off of [layerSize, layerSize + 1]) {
const t = i + off;
if (t < 2 + n) cells += `<mxCell id="e${ei++}" edge="1" parent="1" source="${i}" target="${t}"><mxGeometry relative="1" as="geometry"/></mxCell>`;
}
}
return (
'<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/>' +
cells +
"</root></mxGraphModel>"
);
}
test("terminate-on-timeout: a layout that exceeds the wall-clock ceiling is hard-killed and the original model is returned (#486)", async () => {
// A 1ms ceiling fires before the worker can even finish loading elkjs, so the
// parent must terminate() the worker and fall back to the ORIGINAL model. On
// the OLD in-process race this timer could never fire while the SAME thread was
// blocked inside elkjs — the fallback path was unreachable; here it works.
const prev = process.env.DRAWIO_ELK_TIMEOUT_MS;
process.env.DRAWIO_ELK_TIMEOUT_MS = "1";
try {
const model = layeredGraph(400, 20);
const t0 = Date.now();
const laid = await applyElkLayout(model);
const dt = Date.now() - t0;
// Original geometry is preserved verbatim: every vertex is still stacked at
// (10,10), proving NO ELK coordinates were applied (the pass was killed).
const verts = parseCells(laid).filter((c) => c.vertex);
assert.equal(verts.length, 400, "all vertices survived the fallback");
for (const v of verts) {
assert.equal(v.geometry.x, 10, "x untouched -> layout was terminated");
assert.equal(v.geometry.y, 10, "y untouched -> layout was terminated");
}
// The kill is prompt: terminate() returns the call well under the natural
// layout time for a 400-node graph.
assert.ok(dt < 2000, `terminate path should be prompt, took ${dt}ms`);
} finally {
if (prev === undefined) delete process.env.DRAWIO_ELK_TIMEOUT_MS;
else process.env.DRAWIO_ELK_TIMEOUT_MS = prev;
}
});
test("benchmark guard: a worst-case graph AT the cap lays out without wedging the main event loop (#486)", async () => {
// ~500 nodes / ~1000 edges — a real worst case at the node/edge caps. The
// layout runs on a WORKER thread, so the MAIN event loop must stay responsive
// throughout: a timer scheduled on the main thread keeps firing while ELK
// churns. On the OLD synchronous-on-main-thread code this counter would be
// pinned at 0 for the whole layout (event loop wedged) — exactly the prod fire.
const model = layeredGraph(500, 20);
let mainLoopTicks = 0;
const iv = setInterval(() => {
mainLoopTicks++;
}, 2);
const t0 = Date.now();
const laid = await applyElkLayout(model);
const dt = Date.now() - t0;
clearInterval(iv);
assert.ok(
mainLoopTicks > 0,
"main event loop must stay responsive while ELK runs on the worker",
);
// Benchmark guard: the worst-case graph actually LAYS OUT within the default
// ceiling (it did not fall back). At least one vertex moved off the stack.
const verts = parseCells(laid).filter((c) => c.vertex);
assert.equal(verts.length, 500, "all vertices survived");
const moved = verts.some((v) => v.geometry.x !== 10 || v.geometry.y !== 10);
assert.ok(moved, "layout was applied (did not time out / fall back)");
// Sanity ceiling well under the 5s wall-clock timeout.
assert.ok(dt < 5000, `worst-case layout should be under the ceiling, took ${dt}ms`);
});
test("layout is best-effort: an empty/degenerate model is returned intact", async () => {
const model =
'<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/></root></mxGraphModel>';
+70 -189
View File
@@ -1,220 +1,101 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import {
mkdtempSync,
mkdirSync,
writeFileSync,
rmSync,
readdirSync,
statSync,
readFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import { fileURLToPath } from "node:url";
import { dirname, join, relative, sep } from "node:path";
import { createHash } from "node:crypto";
import { readFileSync } from "node:fs";
import { fileURLToPath } from "node:url";
import { dirname, join } from "node:path";
import { computeRegistryStamp } from "../../scripts/gen-registry-stamp.mjs";
import { REGISTRY_STAMP } from "../../build/index.js";
// Guard tests for the build/src-skew stamp (issues #447/#486). The codegen script
// exports `computeRegistryStamp(srcDir)` — a sha256 over the WHOLE src/ tree
// (every src/**/*.ts EXCEPT *.generated.ts), each file folded in as its
// POSIX-relative path + its normalized content (CRLF->LF, single trailing newline
// stripped). Hashing the whole tree (not just tool-specs.ts) is #486: an edit to
// client.ts / a client/* module without a rebuild must ALSO redden. The in-app
// loader (apps/server/.../docmost-client.loader.ts) DUPLICATES this enumerate+
// normalize+sha256 to refuse a stale build. These tests pin the algorithm and
// assert the built stamp matches the current src.
// Guard tests for the build/src-skew stamp (issue #447). The codegen script
// exports `computeRegistryStamp(sourceText)` — a sha256 over normalized source
// text (CRLF->LF, single trailing newline stripped). The in-app loader
// (apps/server/.../docmost-client.loader.ts) DUPLICATES that normalize+sha256 to
// recompute the stamp from src and refuse a stale build. These tests pin the
// algorithm's behaviour AND assert the built stamp matches the current src, so a
// stale generated file OR a normalize divergence reddens.
const __dirname = dirname(fileURLToPath(import.meta.url));
const SRC_DIR = join(__dirname, "..", "..", "src");
const TOOL_SPECS_PATH = join(__dirname, "..", "..", "src", "tool-specs.ts");
// Build a throwaway src/ tree from a { relPath: content } map and return its dir.
function makeSrcTree(files) {
const root = mkdtempSync(join(tmpdir(), "mcp-stamp-tree-"));
const src = join(root, "src");
for (const [rel, content] of Object.entries(files)) {
const full = join(src, rel);
mkdirSync(dirname(full), { recursive: true });
writeFileSync(full, content, "utf8");
}
return { src, cleanup: () => rmSync(root, { recursive: true, force: true }) };
}
test("computeRegistryStamp is deterministic: same tree -> same hash", () => {
const a = makeSrcTree({ "tool-specs.ts": "export const X = 1;\n" });
const b = makeSrcTree({ "tool-specs.ts": "export const X = 1;\n" });
try {
assert.equal(computeRegistryStamp(a.src), computeRegistryStamp(b.src));
} finally {
a.cleanup();
b.cleanup();
}
test("computeRegistryStamp is deterministic: same input -> same hash", () => {
const input = "export const X = 1;\nexport const Y = 2;\n";
assert.equal(computeRegistryStamp(input), computeRegistryStamp(input));
});
test("computeRegistryStamp returns a 64-char lowercase hex sha256", () => {
const t = makeSrcTree({ "tool-specs.ts": "anything\n" });
try {
assert.match(computeRegistryStamp(t.src), /^[0-9a-f]{64}$/);
} finally {
t.cleanup();
}
const stamp = computeRegistryStamp("anything");
assert.match(stamp, /^[0-9a-f]{64}$/);
});
// #486 CORE: an edit to a NON-tool-specs source file (client.ts) must change the
// stamp. Under the old single-file (tool-specs.ts only) hash this edit was
// invisible and a stale build/ served the old client.ts silently.
test("editing client.ts (not tool-specs.ts) changes the stamp (#486)", () => {
const before = makeSrcTree({
"tool-specs.ts": "export const SPECS = 1;\n",
"client.ts": "export const impl = 'v1';\n",
});
const after = makeSrcTree({
"tool-specs.ts": "export const SPECS = 1;\n",
"client.ts": "export const impl = 'v2';\n",
});
try {
assert.notEqual(
computeRegistryStamp(before.src),
computeRegistryStamp(after.src),
"a client.ts edit with an unchanged tool-specs.ts must move the stamp",
);
} finally {
before.cleanup();
after.cleanup();
}
test("normalizes CRLF vs LF: the same content hashes equal", () => {
const lf = "line1\nline2\nline3";
const crlf = "line1\r\nline2\r\nline3";
assert.equal(computeRegistryStamp(crlf), computeRegistryStamp(lf));
});
test("editing a nested client/* module changes the stamp", () => {
const before = makeSrcTree({
"tool-specs.ts": "x\n",
"client/read.ts": "export const READ = 1;\n",
});
const after = makeSrcTree({
"tool-specs.ts": "x\n",
"client/read.ts": "export const READ = 2;\n",
});
try {
assert.notEqual(
computeRegistryStamp(before.src),
computeRegistryStamp(after.src),
);
} finally {
before.cleanup();
after.cleanup();
}
test("normalizes a trailing newline: with/without a final \\n hashes equal", () => {
const noTrailing = "alpha\nbeta";
const trailing = "alpha\nbeta\n";
assert.equal(computeRegistryStamp(trailing), computeRegistryStamp(noTrailing));
});
// *.generated.ts is EXCLUDED (else the codegen's own output is a fixed-point
// cycle): adding/removing/changing it must not move the stamp.
test("*.generated.ts is excluded from the stamp", () => {
const without = makeSrcTree({ "tool-specs.ts": "x\n" });
const withGen = makeSrcTree({
"tool-specs.ts": "x\n",
"registry-stamp.generated.ts": 'export const REGISTRY_STAMP = "abc";\n',
});
try {
assert.equal(
computeRegistryStamp(without.src),
computeRegistryStamp(withGen.src),
"a *.generated.ts file must not affect the stamp",
);
} finally {
without.cleanup();
withGen.cleanup();
}
test("a CRLF checkout WITH a trailing CRLF still hashes equal to bare LF", () => {
// A worst-case Windows checkout: CRLF line endings + a trailing CRLF. Both the
// \r\n->\n replace and the trailing-newline strip must apply for parity.
const bare = "alpha\nbeta";
const crlfTrailing = "alpha\r\nbeta\r\n";
assert.equal(
computeRegistryStamp(crlfTrailing),
computeRegistryStamp(bare),
);
});
test("a CRLF checkout WITH trailing CRLF hashes equal to bare LF", () => {
const bare = makeSrcTree({ "tool-specs.ts": "alpha\nbeta" });
const crlfTrailing = makeSrcTree({ "tool-specs.ts": "alpha\r\nbeta\r\n" });
try {
assert.equal(
computeRegistryStamp(crlfTrailing.src),
computeRegistryStamp(bare.src),
);
} finally {
bare.cleanup();
crlfTrailing.cleanup();
}
test("a real content change hashes differently", () => {
const before = "export const description = 'search a page';\n";
const after = "export const description = 'search a PAGE';\n";
assert.notEqual(computeRegistryStamp(before), computeRegistryStamp(after));
});
// Only a SINGLE trailing newline is stripped — a second blank line is content.
// Only a SINGLE trailing newline is stripped — a second blank line is content and
// must change the hash. This pins the exact `/\n$/` semantics the loader mirrors.
test("only ONE trailing newline is stripped (two differ from one)", () => {
const one = makeSrcTree({ "tool-specs.ts": "x\n" });
const two = makeSrcTree({ "tool-specs.ts": "x\n\n" });
try {
assert.notEqual(
computeRegistryStamp(one.src),
computeRegistryStamp(two.src),
);
} finally {
one.cleanup();
two.cleanup();
}
assert.notEqual(
computeRegistryStamp("x\n"),
computeRegistryStamp("x\n\n"),
);
});
// Cross-impl equality against a fixed, documented tree. The SAME literal tree and
// expected hash are asserted in the server-side jest test
// (docmost-client.loader.spec.ts). If either side's enumerate+normalize+sha256
// ever diverges, one of the two tests reddens. The tree exercises: a nested file,
// BOTH normalize steps (tool-specs.ts uses CRLF + trailing \n) and the
// *.generated.ts exclusion.
const CROSS_IMPL_TREE = {
"tool-specs.ts": "line1\r\nline2\n",
"client/read.ts": "export const R = 1;\n",
"registry-stamp.generated.ts": 'export const REGISTRY_STAMP="ignored";\n',
};
const CROSS_IMPL_EXPECTED =
"131c1b9e4e2f5a7d6cef91ca8df619822b442f52bc45ebd09474a4c1d6728616";
test("fixed-tree hash matches the documented cross-impl value", () => {
const t = makeSrcTree(CROSS_IMPL_TREE);
try {
assert.equal(computeRegistryStamp(t.src), CROSS_IMPL_EXPECTED);
} finally {
t.cleanup();
}
// Cross-impl equality against a fixed, documented input. The SAME literal input
// and expected hash are asserted in the server-side jest test
// (docmost-client.loader.spec.ts). If either side's normalize+sha256 ever
// diverges, one of the two tests reddens. Input exercises BOTH normalize steps.
test("fixed-input hash matches the documented cross-impl value", () => {
const FIXED_INPUT = "line1\r\nline2\n";
const EXPECTED =
"683376e290829b482c2655745caffa7a1dccfa10afaa62dac2b42dd6c68d0f83";
assert.equal(computeRegistryStamp(FIXED_INPUT), EXPECTED);
});
// Sanity: the EXPECTED constant is not a magic value but the documented
// enumerate+normalize+sha256 of CROSS_IMPL_TREE (a local re-implementation).
test("the documented EXPECTED is the enumerate+normalize+sha256 of the tree", () => {
const t = makeSrcTree(CROSS_IMPL_TREE);
try {
const collect = (dir) => {
const out = [];
for (const e of readdirSync(dir)) {
const f = join(dir, e);
if (statSync(f).isDirectory()) out.push(...collect(f));
else if (e.endsWith(".ts") && !e.endsWith(".generated.ts")) out.push(f);
}
return out;
};
const files = collect(t.src)
.map((abs) => ({ rel: relative(t.src, abs).split(sep).join("/"), abs }))
.sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0));
const h = createHash("sha256");
for (const { rel, abs } of files) {
const n = readFileSync(abs, "utf8")
.replace(/\r\n/g, "\n")
.replace(/\n$/, "");
h.update(rel, "utf8");
h.update("\0", "utf8");
h.update(n, "utf8");
h.update("\0", "utf8");
}
assert.equal(h.digest("hex"), CROSS_IMPL_EXPECTED);
} finally {
t.cleanup();
}
// DESYNC GUARD (covers reviewer suggestion 2). Recompute the stamp from the
// actual src/tool-specs.ts and assert it equals the REGISTRY_STAMP baked into the
// freshly-built build/index.js. This reddens if the generated file is stale OR if
// the codegen normalize ever diverges from what produced the built stamp.
test("built REGISTRY_STAMP equals the stamp recomputed from src/tool-specs.ts", () => {
const source = readFileSync(TOOL_SPECS_PATH, "utf8");
assert.equal(computeRegistryStamp(source), REGISTRY_STAMP);
});
// DESYNC GUARD. Recompute the stamp from the REAL src/ tree and assert it equals
// the REGISTRY_STAMP baked into the freshly-built build/index.js. This reddens if
// the generated file is stale OR if the codegen ever diverges from what produced
// the built stamp.
test("built REGISTRY_STAMP equals the stamp recomputed from src/", () => {
assert.equal(computeRegistryStamp(SRC_DIR), REGISTRY_STAMP);
// Sanity: the fixed-input helper computes the SAME way the codegen does, proving
// the EXPECTED constant above is not an arbitrary magic value but the documented
// normalize+sha256 of FIXED_INPUT. Belt-and-braces so a bad EXPECTED can't hide a
// real regression.
test("the documented EXPECTED constant is the normalize+sha256 of FIXED_INPUT", () => {
const FIXED_INPUT = "line1\r\nline2\n";
const normalized = FIXED_INPUT.replace(/\r\n/g, "\n").replace(/\n$/, "");
const expected = createHash("sha256")
.update(normalized, "utf8")
.digest("hex");
assert.equal(computeRegistryStamp(FIXED_INPUT), expected);
});
@@ -0,0 +1,107 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import {
collectResolvedCommentSpans,
regraftResolvedComments,
applyCommentMarkInDoc,
} from "../../build/lib/comment-anchor.js";
/**
* #493 commit 6 resolved-comment anchors must survive a full markdown rewrite
* (updatePageMarkdown). An agent read HIDES resolved anchors (#337), so its
* markdown drops them; a naive full write would erase the resolved comment marks.
* `regraftResolvedComments(oldDoc, newDoc)` re-anchors them onto the matching
* text. These exercise the real anchoring (no mock).
*/
const doc = (...content) => ({ type: "doc", content });
const para = (...content) => ({ type: "paragraph", content });
const text = (t, marks) => (marks ? { type: "text", text: t, marks } : { type: "text", text: t });
const resolvedComment = (commentId) => ({ type: "comment", attrs: { commentId, resolved: true } });
const activeComment = (commentId) => ({ type: "comment", attrs: { commentId, resolved: false } });
/** The comment mark on a text node, or null. */
function commentMarkOf(node) {
const marks = Array.isArray(node?.marks) ? node.marks : [];
return marks.find((m) => m && m.type === "comment") || null;
}
/** Flatten every text node in a doc (deep). */
function textNodes(node, out = []) {
if (!node || typeof node !== "object") return out;
if (node.type === "text") out.push(node);
if (Array.isArray(node.content)) for (const c of node.content) textNodes(c, out);
return out;
}
test("collectResolvedCommentSpans: only resolved marks, concatenated across a run", () => {
const old = doc(
para(
text("keep "),
text("resolved bit", [resolvedComment("r1")]),
text(" and "),
text("active bit", [activeComment("a1")]),
),
);
const spans = collectResolvedCommentSpans(old);
assert.equal(spans.length, 1);
assert.equal(spans[0].commentId, "r1");
assert.equal(spans[0].text, "resolved bit");
assert.equal(spans[0].mark.attrs.resolved, true);
});
test("regraft restores a resolved mark the agent's markdown dropped", () => {
// OLD doc has a resolved comment on "important note".
const old = doc(para(text("An "), text("important note", [resolvedComment("r1")]), text(" here.")));
// NEW doc (re-imported from the agent's markdown) has the SAME text but NO
// comment mark — the resolved anchor was hidden on read.
const fresh = doc(para(text("An important note here.")));
const out = regraftResolvedComments(old, fresh);
// Inputs are not mutated.
assert.equal(commentMarkOf(textNodes(fresh)[0]), null);
// The resolved mark is back on exactly "important note".
const marked = textNodes(out).filter((n) => commentMarkOf(n));
assert.equal(marked.length, 1);
assert.equal(marked[0].text, "important note");
assert.equal(commentMarkOf(marked[0]).attrs.commentId, "r1");
assert.equal(commentMarkOf(marked[0]).attrs.resolved, true);
});
test("a resolved span whose text the agent changed is dropped (no re-anchor)", () => {
const old = doc(para(text("stale text", [resolvedComment("r1")])));
const fresh = doc(para(text("completely rewritten body")));
const out = regraftResolvedComments(old, fresh);
assert.equal(textNodes(out).filter((n) => commentMarkOf(n)).length, 0);
});
test("regraft is a no-op when the old doc has no resolved comments", () => {
const old = doc(para(text("plain "), text("active", [activeComment("a1")])));
const fresh = doc(para(text("plain active")));
const out = regraftResolvedComments(old, fresh);
assert.equal(textNodes(out).filter((n) => commentMarkOf(n)).length, 0);
});
test("multiple distinct resolved comments are all restored", () => {
const old = doc(
para(text("first", [resolvedComment("r1")]), text(" middle "), text("second", [resolvedComment("r2")])),
);
const fresh = doc(para(text("first middle second")));
const out = regraftResolvedComments(old, fresh);
const byId = Object.fromEntries(
textNodes(out)
.filter((n) => commentMarkOf(n))
.map((n) => [commentMarkOf(n).attrs.commentId, n.text]),
);
assert.equal(byId["r1"], "first");
assert.equal(byId["r2"], "second");
});
test("applyCommentMarkInDoc preserves an arbitrary mark's attrs (resolved:true)", () => {
const d = doc(para(text("anchor me somewhere")));
const ok = applyCommentMarkInDoc(d, "anchor me", { type: "comment", attrs: { commentId: "x9", resolved: true } });
assert.equal(ok, true);
const marked = textNodes(d).filter((n) => commentMarkOf(n));
assert.equal(marked[0].text, "anchor me");
assert.equal(commentMarkOf(marked[0]).attrs.resolved, true);
});
@@ -16,6 +16,7 @@
* `@docmost/editor-ext` before updating the snapshot.
*/
import StarterKit from "@tiptap/starter-kit";
import { Code } from "@tiptap/extension-code";
import Image from "@tiptap/extension-image";
import TaskList from "@tiptap/extension-task-list";
import TaskItem from "@tiptap/extension-task-item";
@@ -1481,7 +1482,20 @@ export const docmostExtensions = [
codeBlock: {},
heading: {},
link: { openOnClick: false },
// #515: disable StarterKit's bundled inline `code` mark so it can be replaced
// by the local override below. StarterKit's `code` inherits tiptap's
// `excludes: "_"`, which strips every co-occurring mark on HTML->PM import
// (`generateJSON`) — so `` **`--flag`** `` lost its bold. This mirror is a
// DELIBERATE standalone copy (it must not pull @docmost/editor-ext into the
// node import runtime — that would drag in React/node-views; see #293), so
// the `excludes: ""` override is declared LOCALLY here and kept in lockstep
// with the canonical `Code` in @docmost/editor-ext by a parity test.
code: false,
}),
// #515: inline code that COMBINES with other marks (CommonMark-consistent).
// `excludes: ""` means the mark excludes nothing, so bold/italic/strike/… may
// co-occur with `code` and survive import.
Code.extend({ excludes: "" }),
// Preserve image width/height as the AUTHORED string. Without an explicit
// parseHTML the stock Image node attribute falls back to tiptap core's
// `fromString`, which coerces a numeric width like "320" into the number 320
@@ -1,7 +1,14 @@
/**
* Foreign-markdown normalizer an input-liberal / output-canonical adapter that
* runs at the IMPORT boundary, BEFORE the canonical parser
* (`markdownToProseMirror` from `@docmost/prosemirror-markdown`).
* (`markdownToProseMirror`, this package).
*
* OWNED BY THIS PACKAGE (#493): the normalizer used to live only in
* apps/server's import path, so the MCP page-write path (`updatePageMarkdown` ->
* `markdownToProseMirrorCanonical`) handled the SAME foreign input differently
* (no front-matter strip, no `[^id]` reference-footnote rewrite) than the server
* importer. Moving it here and calling it from `markdownToProseMirrorCanonical`
* makes every canonical import boundary treat foreign markdown identically.
*
* The canonical parser is deliberately STRICT: it only understands Docmost's
* canonical markdown surface (Obsidian-style `> [!type]` callouts, Pandoc/Obsidian
@@ -247,11 +254,18 @@ function convertReferenceFootnotes(markdown: string): string {
const YAML_FRONT_MATTER_RE = /^\uFEFF?---\n[\s\S]*?\n---\n?/;
/**
* Normalize a foreign markdown string into Docmost's canonical markdown surface
* so the strict canonical parser accepts it losslessly: normalize line endings,
* strip a leading YAML front-matter block, then rewrite GFM reference footnotes
* into inline footnotes. Add further fixture-driven foreign-surface cases here as
* they are found.
* Normalize a foreign markdown string from a FILE IMPORT into Docmost's canonical
* markdown surface so the strict canonical parser accepts it losslessly: normalize
* line endings, strip a leading YAML front-matter block, then rewrite GFM reference
* footnotes into inline footnotes. Add further fixture-driven foreign-surface cases
* here as they are found.
*
* FRONT-MATTER STRIP IS IMPORT-ONLY (#493 review): use this ONLY at the server
* file-import boundary, where a `.md` file really can open with an Obsidian/Hugo
* YAML header. Do NOT use it on the canonical AGENT-WRITE path see
* {@link normalizeAgentMarkdown} for why a full-body agent rewrite must NOT strip
* a leading `---…---` (it is normally a horizontalRule the serializer emitted, and
* stripping it would silently drop the page's leading content).
*/
export function normalizeForeignMarkdown(markdown: string): string {
if (!markdown) return markdown;
@@ -264,3 +278,26 @@ export function normalizeForeignMarkdown(markdown: string): string {
const withoutFrontMatter = src.replace(YAML_FRONT_MATTER_RE, '').trimStart();
return convertReferenceFootnotes(withoutFrontMatter);
}
/**
* Canonical AGENT-WRITE normalization: normalize line endings and rewrite GFM
* `[^id]` reference footnotes to inline `^[body]` but DELIBERATELY NOT strip a
* leading YAML front-matter block.
*
* WHY the split (#493 review): the reference-footnote rewrite is the drift the
* MCP page-write path (`updatePageMarkdown` -> `markdownToProseMirrorCanonical`)
* needed unified with the server import (an agent may paste GFM footnotes). The
* front-matter strip, however, is a FILE-import concern: on a full-body agent
* rewrite a leading `---…---` is (almost) always a `horizontalRule` the
* serializer emitted plus a later rule/heading NOT a foreign YAML header so
* `YAML_FRONT_MATTER_RE` would match it and SILENTLY DELETE the page's leading
* content (a page that starts with a horizontal rule and contains a second `---`
* lost everything up to it). Agent writes must never lose already-stored content,
* so this variant skips the strip. It IS a no-op on canonical serialized content
* (which never emits `[^id]:` reference-definition lines).
*/
export function normalizeAgentMarkdown(markdown: string): string {
if (!markdown) return markdown;
const src = markdown.replace(/\r\n/g, '\n');
return convertReferenceFootnotes(src);
}
+28 -1
View File
@@ -15,7 +15,10 @@ export {
} from "./markdown-document.js";
export type { DocmostMdMeta } from "./markdown-document.js";
export { convertProseMirrorToMarkdown } from "./markdown-converter.js";
export {
convertProseMirrorToMarkdown,
ConverterLossError,
} from "./markdown-converter.js";
export type { ConvertProseMirrorToMarkdownOptions } from "./markdown-converter.js";
export {
@@ -23,6 +26,19 @@ export {
markdownToProseMirrorSync,
} from "./markdown-to-prosemirror.js";
// Foreign-markdown normalizer (#493): the input-liberal pre-pass that rewrites
// GFM `[^id]` reference footnotes to canonical inline `^[body]`. Two variants:
// `normalizeForeignMarkdown` (server FILE-import boundary) ALSO strips a leading
// YAML front-matter block; `normalizeAgentMarkdown` (canonical AGENT-WRITE path,
// mcp `markdownToProseMirrorCanonical`) does NOT — a full-body agent rewrite must
// not lose a leading `---…---` horizontalRule to the front-matter strip (#493
// review). The reference-footnote rewrite is shared so agent + import stay unified
// where it matters, without the content-losing strip on the write path.
export {
normalizeForeignMarkdown,
normalizeAgentMarkdown,
} from "./foreign-markdown.js";
// The Docmost tiptap schema mirror. Exposed so consumers (and the sync
// engine's schema-validity regression tests) can build the exact ProseMirror
// schema the converter targets.
@@ -76,6 +92,17 @@ export type { OutlineEntry } from "./node-ops.js";
// string (#414: single copy shared by mcp and the CommonJS server app).
export { parseNodeArg } from "./parse-node-arg.js";
// Locator markdown-stripping (#493 dedup): the single canonical copy of the
// markdown-tolerant anchor-normalization primitives, imported by mcp's
// text-normalize.ts instead of a forked duplicate. `stripInlineMarkdown` is the
// lenient locator normalizer (trims stray decoration); `stripWrappersAndLinks`
// is the strict balanced-wrapper/link primitive mcp builds `stripBalancedWrappers`
// on top of.
export {
stripInlineMarkdown,
stripWrappersAndLinks,
} from "./text-normalize.js";
// Inline-footnote authoring convention (#414: single copy, formerly the mcp
// `footnote-authoring.ts` fork), shared with the importer's `assembleFootnotes`.
export {
@@ -33,6 +33,26 @@ import {
*/
const MAX_NODE_DEPTH = 400;
/**
* Thrown by {@link convertProseMirrorToMarkdown} in `strict` mode when it hits a
* node or mark type it has no lossless markdown form for (the serializer would
* otherwise silently degrade it drop an unknown mark, flatten an unknown node
* to its children). Carries the offending kind/name so a caller (git-sync) can
* surface exactly what would have been lost.
*/
export class ConverterLossError extends Error {
readonly kind: "node" | "mark";
readonly typeName: string;
constructor(kind: "node" | "mark", typeName: string) {
super(
`convertProseMirrorToMarkdown: unknown ${kind} type "${typeName}" has no lossless markdown representation (strict mode)`,
);
this.name = "ConverterLossError";
this.kind = kind;
this.typeName = typeName;
}
}
/**
* Options for {@link convertProseMirrorToMarkdown}.
*/
@@ -46,6 +66,23 @@ export interface ConvertProseMirrorToMarkdownOptions {
* path where resolved anchors MUST be preserved for round-tripping.
*/
dropResolvedCommentAnchors?: boolean;
/**
* Optional sink for LOSS warnings. When the serializer reaches a node or mark
* type it has no dedicated case for, it degrades gracefully (flattens an
* unknown node to its children, drops an unknown mark) historically a SILENT
* data loss. When this array is provided, one human-readable message per such
* event is pushed here so the caller can observe (and log) what was degraded.
* Not provided by default -> behavior is byte-identical to before for existing
* callers.
*/
warnings?: string[];
/**
* When true, THROW a {@link ConverterLossError} on the FIRST unknown node/mark
* instead of degrading silently a warning becomes a hard error. Used by the
* lossless git-sync export path and the converter tests, where an unmapped
* type is a bug to surface, not data to quietly drop.
*/
strict?: boolean;
}
/**
@@ -63,6 +100,70 @@ export interface ConvertProseMirrorToMarkdownOptions {
* separator is emitted for any other join, so non-list output is unchanged.
*/
const LIST_MARKER_SEPARATOR = "<!-- -->";
/**
* Backslash-escape a leading markdown BLOCK trigger so a serialized paragraph
* line re-parses as a PARAGRAPH, not another block. Without this, a paragraph
* whose text begins at column 0 with an ATX heading `#`, a blockquote/callout
* `>`, a bullet marker `-`/`*`/`+`, an ordered marker `N.`/`N)`, a code fence
* (```` ``` ````/`~~~`), a table `|`, or a thematic break (`---`/`***`/`___`,
* solid or spaced) silently becomes a heading/list/quote/code block/table/rule
* on the next markdown -> ProseMirror import a known data-loss class (the
* thematic-break case drops the text entirely, since a horizontalRule carries
* none). CommonMark's escape tokenizer decodes the inserted `\` back to the
* literal character on import AND stops the block interpretation, so the line
* round-trips byte-exact as paragraph text. Only the FIRST offending character
* is escaped (the minimum needed to break block recognition); a line that does
* NOT open a block emphasis `**x**`, an inline code span, ordinary prose is
* returned verbatim, so there is no backslash churn for the common case.
*
* Applied ONLY to paragraph text, once per `\n`-separated LINE (the paragraph
* case splits on `\n` each hardBreak emits ` \n` so a trigger on a
* continuation line is escaped too): headings/lists/blockquotes legitimately
* open with these markers and render them from their own cases. This is the
* single, canonical fix for the class the client bridge worked around with a
* ZWSP (`gitmost-recording.ts`) and the generative suite self-censored around
* (`text-arbitraries.ts`) both now removed.
*/
function escapeLeadingBlockTrigger(line: string): string {
// ATX heading: 1..6 `#` then whitespace/EOL.
if (/^#{1,6}(?:\s|$)/.test(line)) return "\\" + line;
// Blockquote / Docmost callout opener (`>` or `> [!info]`).
if (line.startsWith(">")) return "\\" + line;
// Bullet list marker then whitespace/EOL. Emphasis (`*x*`, `**x**`) has no
// space after the leading marker and is intentionally left verbatim.
if (/^[-*+](?:\s|$)/.test(line)) return "\\" + line;
// Ordered list marker `N.` / `N)`: escape the DELIMITER so the digits stay
// literal (`1. x` -> `1\. x`, which imports back as the text `1. x`).
const ordered = line.match(/^(\d+)[.)](?:\s|$)/);
if (ordered) {
const digits = ordered[1].length;
return line.slice(0, digits) + "\\" + line.slice(digits);
}
// Fenced code block: 3+ backticks or tildes. A single/double backtick is an
// inline code span and is left verbatim.
if (/^(?:`{3,}|~{3,})/.test(line)) return "\\" + line;
// Thematic break: a WHOLE line of 3+ identical `-`/`*`/`_`, optionally spaced.
if (/^([-*_])(?:\s*\1){2,}\s*$/.test(line)) return "\\" + line;
// Setext underline: a continuation line (after a hardBreak) that is ONLY `-`
// or ONLY `=` (any count, trailing spaces allowed). Under a paragraph line
// such a line re-parses as a SETEXT HEADING and SILENTLY DROPS its own text
// (`a\n--` -> heading "a", the `--` is LOST; `a\n=` -> heading "a", `=` LOST).
// The bullet arm above catches a lone `-` (via its `$`) and the thematic arm
// catches 3+ dashes, but exactly TWO dashes (`--`) fall through both; and no
// arm covers a lone `=` at all (a `==` pair is neutralized earlier by the
// inline `==`->`\=\=` escape, so only a single `=` line reaches here). Escaping
// the leading char (`\--`, `\=`) breaks the setext interpretation so the line
// round-trips as paragraph text. The WHOLE line must be the marker (anchored
// `^-+`/`^=+` to EOL), so a mid-content `-`/`=` is never spuriously escaped;
// and a `---`/`----` already handled by the thematic arm never reaches here,
// so there is no double-escape.
if (/^-+[ \t]*$/.test(line) || /^=+[ \t]*$/.test(line)) return "\\" + line;
// GFM table row opener.
if (line.startsWith("|")) return "\\" + line;
return line;
}
function listMarkerFamily(type: string | undefined): "ul" | "ol" | null {
if (type === "bulletList" || type === "taskList") return "ul";
if (type === "orderedList") return "ol";
@@ -109,6 +210,26 @@ export function convertProseMirrorToMarkdown(
// callers (mcp getPage / in-app AI chat) pass it true.
const dropResolvedCommentAnchors = options.dropResolvedCommentAnchors === true;
// Loss reporting for node/mark types with no dedicated serializer case. In
// `strict` mode the FIRST such type throws (git-sync, tests); otherwise the
// serializer degrades gracefully (as it always has) but records one warning
// per unmapped type into the optional sink so the loss is observable, not
// silent. Deduped per type so a document with many unknown nodes of one type
// produces one message.
const strict = options.strict === true;
const warningsSink = options.warnings;
const seenLossTypes = new Set<string>();
const warnLoss = (kind: "node" | "mark", typeName: string): void => {
if (strict) throw new ConverterLossError(kind, typeName);
if (!warningsSink) return;
const key = `${kind}:${typeName}`;
if (seenLossTypes.has(key)) return;
seenLossTypes.add(key);
warningsSink.push(
`Unknown ${kind} type "${typeName}" has no lossless markdown form; it was degraded on export.`,
);
};
// Escape a value interpolated into an HTML double-quoted attribute value
// (textAlign, colors, image src, math `text`, all data-* attrs, etc.). In the
// ATTRIBUTE context only the quote that delimits the value and the ampersand
@@ -362,6 +483,99 @@ export function convertProseMirrorToMarkdown(
return `<table><tbody>${htmlRows}</tbody></table>`;
};
// Layer the intentional inline escapes onto a NON-code text run BEFORE its
// marks are applied. Extracted so both `case "text"` and the #515 code-emphasis
// run factoring (renderInlineChildren) escape the inner text identically. NEVER
// called on code content (a code span is literal — see the gating in the text
// case and the run helper). Order is load-bearing: the footnote raw-backslash
// doubling MUST precede the `==`/`$`/`^[` escapes (see inFootnoteBody).
const escapeInlineText = (text: string): string => {
let t = text;
if (inFootnoteBody) t = t.replace(/\\/g, "\\\\");
t = t.replace(/==/g, "\\=\\=");
t = escapeProseMath(t);
t = t.replace(/\^\[/g, "^\\[");
return t;
};
// Wrap `text` with the markdown/HTML form of a SINGLE inline mark. Extracted
// from `case "text"` so the same per-mark emission is reused when the #515
// run factoring layers a shared outer mark over a code-emphasis run. `code` is
// handled by the callers (wrapped innermost, before this runs), so this branch
// is defensive only. For any non-code mark the output is byte-identical to the
// pre-#515 inline switch.
const applyInlineMark = (text: string, mark: any): string => {
switch (mark.type) {
case "bold":
return `**${text}**`;
case "italic":
return `*${text}*`;
case "code":
// Callers wrap the code span innermost themselves; reached only if a
// mark list is applied through here directly. Emit the backtick span.
return `\`${text}\``;
case "link": {
const href = mark.attrs?.href || "";
const title = mark.attrs?.title;
if (title) {
// Emit the optional markdown link title; escape an embedded double-
// quote so it cannot terminate the title string early.
const safeTitle = String(title).replace(/"/g, '\\"');
return `[${text}](${href} "${safeTitle}")`;
}
return `[${text}](${href})`;
}
case "strike":
return `~~${text}~~`;
case "underline":
return `<u>${text}</u>`;
case "subscript":
return `<sub>${text}</sub>`;
case "superscript":
return `<sup>${text}</sup>`;
case "highlight": {
// #293 canon #7: a highlight WITHOUT a color serializes as the
// Obsidian/GFM `==text==` syntax; a colored highlight keeps the `<mark
// style>` HTML form. The inner text already had any literal `==`
// backslash-escaped upstream.
const color = mark.attrs?.color;
return color
? `<mark style="background-color: ${escapeAttr(color)}">${text}</mark>`
: `==${text}==`;
}
case "textStyle":
if (mark.attrs?.color) {
return `<span style="color: ${escapeAttr(mark.attrs.color)}">${text}</span>`;
}
return text;
case "spoiler":
// Markdown has no native spoiler syntax, so emit the same raw inline HTML
// the editor-ext/MCP stack uses (span[data-spoiler] round-trips).
return `<span data-spoiler="true">${text}</span>`;
case "comment": {
// Inline comment anchor (span[data-comment-id]); resolved anchors are
// optionally dropped for agent reads, keeping only the bare text.
const cid = mark.attrs?.commentId;
if (cid) {
if (mark.attrs?.resolved && dropResolvedCommentAnchors) {
return text;
}
const resolvedAttr = mark.attrs?.resolved
? ` data-resolved="true"`
: "";
return `<span data-comment-id="${escapeAttr(cid)}"${resolvedAttr}>${text}</span>`;
}
return text;
}
default:
// Unknown mark: no dedicated case, so it has no markdown form and is
// dropped from the run. Report the loss (throws in strict mode) then
// leave the text unwrapped — the historical behavior.
warnLoss("mark", String(mark.type));
return text;
}
};
const processNode = (node: any): string => {
if (nodeDepth >= MAX_NODE_DEPTH) {
// Bail out of deeper recursion without throwing. A text node still has
@@ -412,7 +626,17 @@ export function convertProseMirrorToMarkdown(
}
case "paragraph": {
const text = renderInlineChildren(nodeContent);
// Escape a leading block trigger on EVERY line of the paragraph, not
// just the first: a hardBreak serializes as ` \n`, so a `#`/`-`/`>`/
// `1.`/`|`/fence/`---` at the start of a CONTINUATION line would also
// re-parse into another block on the next import (a heading/list/table/
// setext-`---`), and for the text-less thematic/setext case would LOSE
// that line's text entirely. Escaping each `\n`-separated line closes
// the class for multi-line paragraphs too.
const text = renderInlineChildren(nodeContent)
.split("\n")
.map(escapeLeadingBlockTrigger)
.join("\n");
const align = node.attrs?.textAlign;
// Non-default alignment round-trips as an ATTACHED HTML comment at the
// END of the block line (#293 canon #9):
@@ -451,154 +675,38 @@ export function convertProseMirrorToMarkdown(
return headingLine;
}
case "text":
case "text": {
let textContent = node.text || "";
// #293 canon #7: `==` is now a LIVE inline highlight syntax on import (a
// marked inline extension turns `==text==` into a color-less highlight
// mark). A LITERAL `==` in a text run would therefore be misparsed as a
// highlight on the next import, so backslash-escape each `=` of a `==`
// pair; marked's escape tokenizer decodes `\=` back to a literal `=`, so
// a literal `==` round-trips as text (never materializes a phantom mark).
// This runs for BOTH unmarked text and marked non-code runs, but NOT for
// an inline code span (a run carrying the `code` mark returns a backtick
// span below with `==` verbatim, matching `` `a == b` `` staying code).
// A highlight run's own `==` delimiters are appended AFTER this in the
// marks loop, so they are never escaped; only the run's inner text is.
if (!(node.marks || []).some((m: any) => m.type === "code")) {
// #293 canon #2 (F2): inside a footnote body, DOUBLE every RAW user
// backslash FIRST, so it survives `^[…]` (the import tokenizer treats
// `\<char>` as an escape when balancing brackets, and `parseInline`
// decodes escapes). Doing it before the intentional escapes below keeps
// the serializer's own single escapes (`\=` `\$` `^\[`, and the `\[`/
// `\]` balanceBrackets adds) single; only genuine user backslashes are
// doubled. Skipped for code runs (a code span's content is NOT decoded
// by parseInline, so its backslashes must stay verbatim).
if (inFootnoteBody) {
textContent = textContent.replace(/\\/g, "\\\\");
}
textContent = textContent.replace(/==/g, "\\=\\=");
// #293 canon #6: escape a would-be inline-math `$…$` span so it stays
// literal text on re-import (currency `$5` is left clean — see
// escapeProseMath). Runs on the SAME non-code runs as the `==` escape
// above; an inline `code` run returns verbatim below, matching the
// codeBlock path (a `$…$` inside code must stay code, never math).
textContent = escapeProseMath(textContent);
// #293 canon #2: `^[` opens a LIVE inline-footnote span on import
// (`^[text]` -> a footnote reference). A LITERAL `^[` in prose text
// would therefore materialize a phantom footnote on the next import, so
// backslash-escape the bracket (`^[` -> `^\[`); marked's escape
// tokenizer decodes `\[` back to `[`, so a literal `^[…]` round-trips
// as text and never opens a footnote. Only the OPENING `^[` needs
// breaking (the tokenizer requires it), so this is a minimal, idempotent
// escape. A real footnoteReference node emits `^[body]` from its own
// case, never through here.
textContent = textContent.replace(/\^\[/g, "^\\[");
// #515: `code` is no longer exclusive (`excludes: ""`), so a run may
// carry `code` TOGETHER with other marks. The inner escapes below apply
// ONLY to a NON-code run (a code span's content is literal — `==`, `$…$`,
// `^[` must stay verbatim, matching `` `a == b` `` staying code). See
// #293 canon #2/#6/#7 for why each escape exists (extracted into
// escapeInlineText). A code run's `==`/`$`/`^[` are protected by the
// backticks, so they are never misparsed on re-import.
const hasCode = (node.marks || []).some((m: any) => m.type === "code");
if (!hasCode) {
textContent = escapeInlineText(textContent);
}
// Apply marks (bold, italic, code, etc.)
if (node.marks) {
// The schema's `code` mark declares `excludes: "_"` — it excludes every
// other inline mark — so the editor can NEVER produce a text run that
// carries `code` together with another mark, and on import any
// co-occurring mark is always dropped (the run comes back as code-only).
// The lossless, byte-stable behavior is therefore: when a run has the
// `code` mark, emit ONLY the backtick code span and ignore every other
// mark, so md1 is already code-only and md2 === md1. Runs WITHOUT a code
// mark are rendered exactly as before.
const markTypes = node.marks.map((m: any) => m.type);
const hasCode = markTypes.includes("code");
// #515: wrap the backtick code span FIRST (innermost mark), then layer
// the REMAINING marks in array order. For a run WITHOUT a code mark the
// loop applies every mark exactly as the pre-#515 switch did, so the
// output is byte-identical. For a code+emphasis run the code span sits
// inside the emphasis delimiters (`` **`code`** ``), matching CommonMark.
// The shared-mark grouping across ADJACENT nodes (`` **`a` + `b`** ``)
// lives in renderInlineChildren; this direct path handles a lone run
// and the table/`default` callers that invoke processNode per node.
if (hasCode) {
textContent = `\`${textContent}\``;
return textContent;
}
for (const mark of node.marks) {
switch (mark.type) {
case "bold":
textContent = `**${textContent}**`;
break;
case "italic":
textContent = `*${textContent}*`;
break;
case "code":
// A `code` run already returned above (hasCode early return), so
// this branch is only reached for a non-code run that somehow
// still lists `code`; emit the plain backtick span.
textContent = `\`${textContent}\``;
break;
case "link": {
const href = mark.attrs?.href || "";
const title = mark.attrs?.title;
if (title) {
// Emit the optional markdown link title; escape an embedded
// double-quote so it cannot terminate the title string early.
const safeTitle = String(title).replace(/"/g, '\\"');
textContent = `[${textContent}](${href} "${safeTitle}")`;
} else {
textContent = `[${textContent}](${href})`;
}
break;
}
case "strike":
textContent = `~~${textContent}~~`;
break;
case "underline":
textContent = `<u>${textContent}</u>`;
break;
case "subscript":
textContent = `<sub>${textContent}</sub>`;
break;
case "superscript":
textContent = `<sup>${textContent}</sup>`;
break;
case "highlight": {
// #293 canon #7: a highlight WITHOUT a color serializes as the
// Obsidian/GFM `==text==` syntax (the importer's marked inline
// `==` extension parses it back to a color-less highlight mark).
// A highlight WITH a color keeps the `<mark style="background-
// color: …">` HTML form (the condition is deterministic on the
// `color` attr), so a colored highlight is not flattened. The
// inner textContent already had any literal `==` backslash-
// escaped above, so a highlight over text containing `==` still
// round-trips.
const color = mark.attrs?.color;
textContent = color
? `<mark style="background-color: ${escapeAttr(color)}">${textContent}</mark>`
: `==${textContent}==`;
break;
}
case "textStyle":
if (mark.attrs?.color) {
textContent = `<span style="color: ${escapeAttr(mark.attrs.color)}">${textContent}</span>`;
}
break;
case "spoiler":
// Markdown has no native spoiler syntax, so emit the same raw
// inline HTML the editor-ext/MCP stack uses. The schema's Spoiler
// mark parses span[data-spoiler] back on import, so the mark
// survives the PM -> MD -> PM round-trip.
textContent = `<span data-spoiler="true">${textContent}</span>`;
break;
case "comment": {
// Emit the inline comment anchor so highlights round-trip. The
// schema's Comment mark parses span[data-comment-id] (attrs
// commentId/resolved).
const cid = mark.attrs?.commentId;
if (cid) {
// Hide resolved anchors from agent reads: drop the wrapper and
// keep only the bare text. Active anchors keep their wrapper.
if (mark.attrs?.resolved && dropResolvedCommentAnchors) {
break;
}
const resolvedAttr = mark.attrs?.resolved
? ` data-resolved="true"`
: "";
textContent = `<span data-comment-id="${escapeAttr(cid)}"${resolvedAttr}>${textContent}</span>`;
}
break;
}
}
if (mark.type === "code") continue; // wrapped innermost above
textContent = applyInlineMark(textContent, mark);
}
}
return textContent;
}
case "codeBlock":
const language = node.attrs?.language || "";
@@ -1173,7 +1281,11 @@ export function convertProseMirrorToMarkdown(
}
default:
// Fallback: process children
// Unknown node type: no dedicated case, so the node's identity + attrs
// have no lossless markdown form. Report the loss (throws in strict
// mode) then degrade by flattening to its children — the historical
// graceful fallback.
warnLoss("node", String(type));
return nodeContent.map(processNode).join("");
}
};
@@ -1186,18 +1298,165 @@ export function convertProseMirrorToMarkdown(
// For that node ONLY we fall back to the lossless schema-HTML `<span>` form.
// Every other inline node is rendered exactly as processNode would, so output
// is unchanged whenever no math sits directly before a digit.
// #515: a "bare-delimiter" emphasis mark is one that serializes as a naked
// markdown delimiter run (`**` `*` `~~` `==`) — bold / italic / strike /
// UNCOLORED highlight. These delimiters COLLIDE with the backtick-flanking
// delimiters emitted around a code+emphasis run: rendering `[code,bold]` next
// to `[italic]` node-by-node would produce `` **`a`***b* `` (a `***` run that
// re-imports wrong). Every OTHER mark (underline/sub/sup/spoiler/comment/
// textStyle/colored-highlight/link) emits an HTML/bracket form whose boundaries
// do NOT collapse, so those neighbors never join a run.
const isBareEmphasisMark = (mark: any): boolean => {
switch (mark?.type) {
case "bold":
case "italic":
case "strike":
return true;
case "highlight":
return !mark.attrs?.color; // colored highlight emits <mark>, not `==`
default:
return false;
}
};
// A text node participates in a code-emphasis run iff it carries at least one
// bare-delimiter emphasis mark. A code-ONLY node (no emphasis) does NOT — so a
// plain `` `code` `` next to `**bold**` keeps its clean, byte-identical
// markdown (they share no colliding delimiter). Existing pages, where a code
// node could never carry emphasis, therefore serialize exactly as before.
const isEmphasisMember = (node: any): boolean =>
node?.type === "text" &&
(node.marks || []).some((m: any) => isBareEmphasisMark(m));
// The run's non-code marks (order preserved) — the candidate marks to factor.
const nonCodeMarks = (node: any): any[] =>
(node.marks || []).filter((m: any) => m.type !== "code");
// Deep structural equality of two marks (type + full attrs). Two `link` marks
// are equal only when EVERY attr matches (class/href/internal/rel/target/title
// — not just href), so a homogeneous run never merges links that differ.
const marksEqual = (a: any, b: any): boolean =>
a.type === b.type &&
JSON.stringify(a.attrs ?? null) === JSON.stringify(b.attrs ?? null);
// Two non-code mark lists are equal AS SETS (a run is homogeneous when every
// node shares the identical non-code mark set — order-independent).
const markSetsEqual = (a: any[], b: any[]): boolean =>
a.length === b.length &&
a.every((ma) => b.some((mb) => marksEqual(ma, mb))) &&
b.every((mb) => a.some((ma) => marksEqual(mb, ma)));
// Serialize one node's INNER form for a homogeneous run: the factored marks are
// applied by the caller, so here a code node emits only its literal backtick
// span and a non-code node emits only its (escaped) text.
const renderRunInner = (node: any): string => {
const text = node.text || "";
if ((node.marks || []).some((m: any) => m.type === "code")) {
return `\`${text}\``; // code content is literal
}
return escapeInlineText(text);
};
// A markdown emphasis delimiter (`**`/`*`/`~~`/`==`) wrapping a code span opens
// with the delimiter immediately followed by a backtick and closes immediately
// preceded by one. A backtick is CommonMark punctuation, so such a delimiter is
// only left/right-flanking — able to open/close emphasis — when the character
// on its OUTER side is start/end, whitespace or punctuation. If a run boundary
// abuts a word character, the delimiter would NOT flank (`a**` `code` `**`
// never opens) and the emphasis silently degrades on re-import. This checks the
// outer boundary char conservatively: ASCII whitespace or ASCII punctuation (or
// the string edge) is safe; anything else (a letter/number, unicode letter or
// emoji) is treated as unsafe so the run takes the lossless HTML fallback.
const SAFE_BOUNDARY = /[\s!-/:-@[-`{-~]/;
const isSafeBoundary = (c: string): boolean => c === "" || SAFE_BOUNDARY.test(c);
// Serialize a maximal run of adjacent emphasis-member text nodes that contains
// at least one `code` node (#515). HOMOGENEOUS (all share the identical
// non-code mark set) AND flank-safe on both boundaries: factor the common marks
// ONCE around the concatenated inner spans — `` **`aaa` + `bbb`** ``, code
// innermost. Otherwise — HETEROGENEOUS (non-code sets differ, e.g. `[code,bold]`
// next to `[italic]`) OR a boundary abuts a word char — emit the whole run as
// schema-HTML via the lossless inlineToHtml fallback, avoiding a colliding
// `***` delimiter run or a non-flanking `a**` that would drop the emphasis.
const renderCodeEmphasisRun = (
run: any[],
prevChar: string,
nextChar: string,
): string => {
const firstNonCode = nonCodeMarks(run[0]);
const homogeneous = run.every((n) =>
markSetsEqual(nonCodeMarks(n), firstNonCode),
);
if (!homogeneous || !isSafeBoundary(prevChar) || !isSafeBoundary(nextChar)) {
return inlineToHtml(run);
}
let out = run.map(renderRunInner).join("");
// Apply the common non-code marks in the FIRST node's array order (code is
// already innermost inside each span).
for (const mark of firstNonCode) out = applyInlineMark(out, mark);
return out;
};
const renderInlineChildren = (nodes: any[]): string => {
const parts = nodes.map(processNode);
for (let i = 0; i < nodes.length - 1; i++) {
if (
nodes[i]?.type === "mathInline" &&
parts[i].startsWith("$") &&
/^[0-9]/.test(parts[i + 1] || "")
) {
parts[i] = mathInlineHtml(nodes[i].attrs?.text || "");
// Pass 1: segment the nodes. Each segment is either an already-rendered
// non-run node / pure-emphasis node (byte-identical to the pre-#515 output),
// or a DEFERRED code-emphasis run (a maximal block of consecutive
// emphasis-member text nodes containing a code node) — its markdown-vs-HTML
// choice needs the neighbor boundary chars, resolved in pass 2.
type Seg = { firstNode: any; text?: string; run?: any[] };
const segs: Seg[] = [];
let i = 0;
while (i < nodes.length) {
const node = nodes[i];
if (isEmphasisMember(node)) {
let j = i;
while (j < nodes.length && isEmphasisMember(nodes[j])) j++;
const run = nodes.slice(i, j);
const hasCode = run.some((n: any) =>
(n.marks || []).some((m: any) => m.type === "code"),
);
if (hasCode) {
segs.push({ firstNode: run[0], run });
} else {
// Pure-emphasis run (no code): render each node as before.
for (const n of run) segs.push({ firstNode: n, text: processNode(n) });
}
i = j;
} else {
segs.push({ firstNode: node, text: processNode(node) });
i++;
}
}
return parts.join("");
// A deferred run always emits either a delimiter/backtick (markdown) or `<`
// (HTML) first — both punctuation — so a following run counts as a safe
// boundary for the current one without resolving it first.
const firstCharOf = (seg: Seg): string =>
seg.text !== undefined ? seg.text[0] || "" : "*";
// Pass 2: resolve deferred runs left-to-right, tracking the previous emitted
// char (for the opening boundary) and peeking the next segment (for closing).
let prevChar = "";
for (let k = 0; k < segs.length; k++) {
const seg = segs[k];
if (seg.text === undefined) {
const nextChar = k + 1 < segs.length ? firstCharOf(segs[k + 1]) : "";
seg.text = renderCodeEmphasisRun(seg.run!, prevChar, nextChar);
}
if (seg.text.length > 0) prevChar = seg.text[seg.text.length - 1];
}
// Preserve the mathInline-before-digit guard: a `$…$` immediately followed by
// a digit-leading segment would re-tokenize as a longer math span, so emit
// that math node as HTML instead. A code-emphasis run never starts with a
// digit (it opens with a delimiter or `<`), so segment granularity is safe.
for (let k = 0; k < segs.length - 1; k++) {
if (
segs[k].firstNode?.type === "mathInline" &&
(segs[k].text || "").startsWith("$") &&
/^[0-9]/.test(segs[k + 1].text || "")
) {
segs[k].text = mathInlineHtml(segs[k].firstNode.attrs?.text || "");
}
}
return segs.map((s) => s.text).join("");
};
// Render inline content (text runs + their marks) to HTML. Used by the raw
@@ -1232,7 +1491,22 @@ export function convertProseMirrorToMarkdown(
return processNode(n);
}
let t = escapeHtmlText(n.text || "");
// #515: wrap `<code>` INNERMOST first (before the array-order mark loop),
// then skip `code` in the loop. The imported mark order is NOT fixed — it
// DEPENDS on the emphasis extension: import (`generateJSON`) yields code
// LAST for bold/italic/strike (`[emphasis, code]`) but code FIRST for the
// `==`-highlight extension (`[code, highlight]`). So we cannot rely on a
// fixed array position; the invariant is instead "wrap `<code>` innermost
// regardless of the imported order". That keeps `<code>` nested inside the
// emphasis tag both directions (preserving the byte fixpoint — an order-
// sensitive loop would flip `<strong><code>`↔`<code><strong>` depending on
// which order it happened to see) and matches the markdown path (case
// "text" / run factoring).
if ((n.marks || []).some((m: any) => m.type === "code")) {
t = `<code>${t}</code>`;
}
for (const mark of n.marks || []) {
if (mark.type === "code") continue; // wrapped innermost above
switch (mark.type) {
case "bold":
t = `<strong>${t}</strong>`;
@@ -1297,6 +1571,12 @@ export function convertProseMirrorToMarkdown(
t = `<span data-comment-id="${escapeAttr(mark.attrs.commentId)}"${r}>${t}</span>`;
}
break;
default:
// Unknown mark on the raw-HTML path: dropped (no HTML form). Report
// the loss (throws in strict mode) — same policy as the markdown
// path's marks loop above.
warnLoss("mark", String(mark.type));
break;
}
}
return t;
@@ -7,13 +7,12 @@
* it is never applied to replacement text or inserted node content, so no
* formatting is ever lost.
*
* Scope note (#414): this package-local copy exists so `node-ops.ts` which
* lives here now (the single canonical copy) can resolve its markdown-tolerant
* anchor fallback without a circular dependency back on `@docmost/mcp`. It
* intentionally carries ONLY `stripInlineMarkdown` (the primitive `node-ops`
* needs); the mcp-side `text-normalize.ts` (which additionally serves
* `json-edit.ts` via `stripBalancedWrappers`) is the subject of a separate
* dedup task and is left untouched here.
* CANONICAL HOME (#414/#493): this is the single source of truth for locator
* markdown-stripping. `node-ops.ts` (which lives here) uses it directly, and the
* mcp-side `text-normalize.ts` now IMPORTS `stripInlineMarkdown` and the shared
* `stripWrappersAndLinks` primitive from here (via `@docmost/prosemirror-markdown`)
* instead of keeping a drifting copy mcp only adds its own thin
* `stripBalancedWrappers`/`closestBlockHint` on top.
*/
/** Maximum unwrap passes, so pathological/nested input cannot loop forever. */
@@ -44,7 +43,7 @@ const LINK_IMAGE_RE = /!?\[([^\]]*)\]\([^)]*\)/g;
* Does NOT trim decoration, does NOT guard against an empty result it returns
* exactly the transformed string.
*/
function stripWrappersAndLinks(s: string): string {
export function stripWrappersAndLinks(s: string): string {
// 1. Links/images -> their visible text.
let out = s.replace(LINK_IMAGE_RE, "$1");
@@ -0,0 +1,112 @@
import { describe, expect, it } from "vitest";
import {
convertProseMirrorToMarkdown,
ConverterLossError,
} from "../src/lib/markdown-converter.js";
/**
* #493 commit 3 a node/mark type the serializer has no dedicated case for used
* to be degraded SILENTLY (an unknown node flattened to its children, an unknown
* mark dropped from the run). The serializer now REPORTS the loss:
* - default (non-strict): unchanged graceful degradation, but one warning per
* unmapped type is pushed into an optional `warnings` sink so callers can
* observe it;
* - strict: the FIRST unmapped type throws a ConverterLossError (git-sync +
* tests), turning a silent loss into a hard, surfaced error.
*
* Exercised through the REAL converter (no mock): the observable properties are
* the emitted markdown, the warnings collected, and the thrown error.
*/
const doc = (...nodes: any[]) => ({ type: "doc", content: nodes });
describe("converter loss reporting — unknown node types", () => {
const unknownNode = doc({
type: "quantumWidget",
content: [{ type: "text", text: "inner text" }],
});
it("degrades to children AND records a warning (non-strict, sink provided)", () => {
const warnings: string[] = [];
const md = convertProseMirrorToMarkdown(unknownNode, { warnings });
// Graceful degrade: the child text still survives (historical behavior).
expect(md).toContain("inner text");
// The loss is now observable.
expect(warnings).toHaveLength(1);
expect(warnings[0]).toContain("quantumWidget");
expect(warnings[0]).toContain("node");
});
it("stays byte-identical for callers that pass no sink (zero behavior change)", () => {
const withSink: string[] = [];
const a = convertProseMirrorToMarkdown(unknownNode, { warnings: withSink });
const b = convertProseMirrorToMarkdown(unknownNode);
expect(b).toBe(a); // the sink does not alter the produced markdown
});
it("throws ConverterLossError in strict mode", () => {
try {
convertProseMirrorToMarkdown(unknownNode, { strict: true });
expect.unreachable("strict mode must throw on an unknown node");
} catch (e) {
expect(e).toBeInstanceOf(ConverterLossError);
expect((e as ConverterLossError).kind).toBe("node");
expect((e as ConverterLossError).typeName).toBe("quantumWidget");
}
});
it("dedupes the warning per type (many unknown nodes -> one message)", () => {
const warnings: string[] = [];
convertProseMirrorToMarkdown(
doc(
{ type: "quantumWidget", content: [{ type: "text", text: "a" }] },
{ type: "quantumWidget", content: [{ type: "text", text: "b" }] },
),
{ warnings },
);
expect(warnings).toHaveLength(1);
});
});
describe("converter loss reporting — unknown mark types", () => {
const unknownMark = doc({
type: "paragraph",
content: [{ type: "text", text: "glowing", marks: [{ type: "glow" }] }],
});
it("drops the mark but keeps the text AND records a warning (non-strict)", () => {
const warnings: string[] = [];
const md = convertProseMirrorToMarkdown(unknownMark, { warnings });
expect(md).toBe("glowing"); // text survives, mark silently had no form
expect(warnings).toHaveLength(1);
expect(warnings[0]).toContain("glow");
expect(warnings[0]).toContain("mark");
});
it("throws ConverterLossError in strict mode", () => {
expect(() =>
convertProseMirrorToMarkdown(unknownMark, { strict: true }),
).toThrow(ConverterLossError);
});
});
describe("converter loss reporting — known content is never flagged", () => {
it("a fully-mapped document produces no warnings and does not throw in strict mode", () => {
const d = doc(
{ type: "heading", attrs: { level: 2 }, content: [{ type: "text", text: "Title" }] },
{
type: "paragraph",
content: [
{ type: "text", text: "bold", marks: [{ type: "bold" }] },
{ type: "text", text: " and " },
{ type: "text", text: "link", marks: [{ type: "link", attrs: { href: "https://x.y" } }] },
],
},
{ type: "bulletList", content: [{ type: "listItem", content: [{ type: "paragraph", content: [{ type: "text", text: "item" }] }] }] },
);
const warnings: string[] = [];
const md = convertProseMirrorToMarkdown(d, { warnings, strict: true });
expect(warnings).toEqual([]);
expect(md).toContain("## Title");
});
});
@@ -1,12 +1,15 @@
import { describe, it, expect } from 'vitest';
import { convertProseMirrorToMarkdown } from '../src/lib/markdown-converter.js';
import { markdownToProseMirror } from '../src/lib/markdown-to-prosemirror.js';
import {
convertProseMirrorToMarkdown,
markdownToProseMirror,
} from '@docmost/prosemirror-markdown';
import { normalizeForeignMarkdown } from './foreign-markdown';
normalizeForeignMarkdown,
normalizeAgentMarkdown,
} from '../src/lib/foreign-markdown.js';
/**
* STEP 2 goldens for issue #345: the foreign-markdown normalizer that runs at the
* import boundary BEFORE the strict canonical parser (`markdownToProseMirror`).
* STEP 2 goldens for issue #345 (moved into the package with the normalizer in
* #493): the foreign-markdown normalizer that runs at the import boundary BEFORE
* the strict canonical parser (`markdownToProseMirror`).
*
* Two layers:
* 1. PURE stringstring cases pinning the normalizer's own behavior (GFM
@@ -216,3 +219,53 @@ describe('foreign markdown import acceptance (normalizer + canonical parser)', (
).toHaveLength(1);
});
});
describe('normalizeAgentMarkdown vs normalizeForeignMarkdown — front-matter strip is IMPORT-only (#493 review)', () => {
// A page that OPENS with a horizontalRule and contains a later `---` serializes
// to a `---…---`-shaped body. On a full-body AGENT rewrite this must NOT be
// mistaken for YAML front-matter and stripped — that silently dropped the
// page's leading content.
const rulePage = '---\n\nIntro\n\nMore\n\n---\n\nRest';
it('normalizeAgentMarkdown does NOT strip a leading ---…--- (no content loss)', () => {
expect(normalizeAgentMarkdown(rulePage)).toBe(rulePage);
});
it('normalizeForeignMarkdown (file import) STILL strips a real leading YAML front-matter block', () => {
const withYaml = '---\ntitle: My Page\ntags: [a, b]\n---\n\nBody here.';
const out = normalizeForeignMarkdown(withYaml);
expect(out).toBe('Body here.');
// And the horizontalRule-shaped body IS stripped on the import path (its
// documented file-import behavior) — the two variants differ ONLY here.
expect(normalizeForeignMarkdown(rulePage)).not.toContain('Intro');
});
it('agent-write round-trip keeps a horizontalRule-led doc with a second rule intact', async () => {
// Simulate the serializer output for [horizontalRule, para, para, horizontalRule, para].
const doc = {
type: 'doc',
content: [
{ type: 'horizontalRule' },
{ type: 'paragraph', content: [{ type: 'text', text: 'Intro' }] },
{ type: 'paragraph', content: [{ type: 'text', text: 'More' }] },
{ type: 'horizontalRule' },
{ type: 'paragraph', content: [{ type: 'text', text: 'Rest' }] },
],
};
const body = convertProseMirrorToMarkdown(doc);
// The agent-write normalization must NOT eat the head; re-import keeps every
// paragraph's text.
const back = await markdownToProseMirror(normalizeAgentMarkdown(body));
const texts = JSON.stringify(back);
for (const t of ['Intro', 'More', 'Rest']) expect(texts).toContain(t);
// Both horizontal rules survive.
expect(back.content.filter((n: any) => n.type === 'horizontalRule')).toHaveLength(2);
});
it('agent-write STILL rewrites GFM reference footnotes (the shared drift-fix)', () => {
const gfm = 'See[^1].\n\n[^1]: the note.';
const out = normalizeAgentMarkdown(gfm);
expect(out).toContain('^[the note.]');
expect(out).not.toMatch(/\[\^1\]:/);
});
});
@@ -11,9 +11,11 @@
*
* The corpus deliberately spans the CommonMark / canon hostile alphabet
* (`* _ [ ] ( ) { } | < > & # ! ~ = + -`), unicode / emoji / RTL, and the legal
* mark combinations on runs (including the `code` mark, which the schema's
* `excludes: "_"` makes suppress every co-occurring mark so it is never
* combined with another mark in the byte-stable space).
* mark combinations on runs. As of #515 the `code` mark no longer excludes other
* marks (`excludes: ""`), so the corpus ALSO combines `code` with bold / italic /
* strike / highlight exercising both the HOMOGENEOUS run factoring (adjacent
* code+bold spans -> `` **`a` `b`** ``) and the HETEROGENEOUS anti-collision
* fallback (`[code,bold]` next to `[italic]` -> schema-HTML, never `` `a`***b* ``).
*/
import fc from 'fast-check';
@@ -106,16 +108,16 @@ export const urlArb: fc.Arbitrary<string> = fc
/**
* A text run with an OPTIONAL single non-code formatting mark (bold/italic/
* strike/underline/superscript/subscript/spoiler), or a SOLE `code` mark, or a
* link, or an inline comment anchor. `code` is NEVER combined with another mark
* in the byte-stable space (that combination is a documented converter
* limitation the schema's `code` mark declares `excludes: "_"`). Marks wrap
* `safeTextArb`, which stays stable even when it contains isolated specials.
* `code` mark COMBINED with a bare-delimiter emphasis mark (#515), or a link, or
* an inline comment anchor. Marks wrap `safeTextArb`, which stays stable even
* when it contains isolated specials.
*
* The mark set here is broadened past the sibling test's {bold,italic,strike}
* to also cover underline / superscript / subscript / spoiler / textStyle /
* highlight (all single, non-code marks), so the marks-on-text generator
* exercises every mark the schema declares except the deliberately-excluded
* `code`+other combination.
* The mark set here is broadened past the sibling test's {bold,italic,strike} to
* also cover underline / superscript / subscript / spoiler / textStyle /
* highlight (all single, non-code marks). As of #515 it ALSO emits `code`
* combined with bold/italic/strike, so the assembled inline content exercises the
* converter's code-emphasis run detection (adjacent combos -> homogeneous
* factoring or heterogeneous HTML fallback, both lossless).
*/
export const markedTextRunArb: fc.Arbitrary<any> = fc.oneof(
// Plain text.
@@ -138,6 +140,25 @@ export const markedTextRunArb: fc.Arbitrary<any> = fc.oneof(
// Sole code mark (backtick span). safeTextArb is backtick-free, so the span
// content cannot contain an inner backtick.
safeTextArb.map((t) => ({ type: 'text', text: t, marks: [{ type: 'code' }] })),
// #515: code COMBINED with a bare-delimiter emphasis mark. The converter nests
// the backtick span inside the emphasis delimiters (`` **`x`** ``) and, when
// such runs sit adjacent, factors a shared mark or falls back to schema-HTML.
// Mark order here is `[emphasis, code]` — the order the HTML->PM import yields
// for bold/italic/strike specifically (code last). This is NOT universal: the
// `==`-highlight case below imports code FIRST — so match each case to its own
// imported order for the order-exact P1 round-trip (do not assume a fixed order).
fc
.tuple(safeTextArb, fc.constantFrom('bold', 'italic', 'strike'))
.map(([t, m]) => ({ type: 'text', text: t, marks: [{ type: m }, { type: 'code' }] })),
// #515: code combined with an UNCOLORED highlight (also a bare-delimiter mark,
// `==…==`), so the highlight+code delimiter interaction is covered too. Import
// yields `[code, highlight]` here (the `==` inline extension nests code first),
// so the generator matches that order for the order-exact P1 round-trip.
safeTextArb.map((t) => ({
type: 'text',
text: t,
marks: [{ type: 'code' }, { type: 'highlight' }],
})),
// Link with safe text, a paren/space-free href, optionally a letter-bearing
// title (a purely numeric title is coerced to a number and dropped).
fc
@@ -212,25 +233,93 @@ export function normalizeInline(nodes: any[]): any[] {
return out;
}
/**
* #493 commit 1: a plain-text run whose text DELIBERATELY OPENS with a markdown
* BLOCK trigger ATX heading `#`, bullet `-`/`*`/`+`, blockquote `>`, ordered
* `N.`/`N)`, or a table `|` followed by safe text. Pre-#493 the corpus
* self-censored these away (safeTextArb's leading-word guarantee); the paragraph
* serializer now BLOCK-ESCAPES a leading trigger, so the generative round-trip
* itself proves the data-loss class is closed rather than avoiding it.
*
* DELIBERATELY excludes the code-fence (backtick) trigger the backtick is a
* code-span delimiter that re-pairs globally (see specialCharArb's note), an
* instability UNRELATED to block-escape and the whole-line thematic break
* (`---`), which only triggers when the line is ONLY dashes; both are covered by
* the deterministic pin (gitmost-transcript-neutralization.test.ts). Each still
* ENDS in a word (safeTextArb) so adjacent-run concatenation stays safe.
*/
export const blockTriggerLeadRunArb: fc.Arbitrary<any> = fc
.tuple(
fc.constantFrom('# ', '## ', '- ', '* ', '+ ', '> ', '1. ', '1) ', '| '),
safeTextArb,
)
.map(([trigger, rest]) => ({ type: 'text', text: trigger + rest }));
/**
* A hardBreak IMMEDIATELY followed by a block-trigger-leading run a two-node
* segment. Because a hardBreak serializes as ` \n`, the trigger then sits at
* the START of a CONTINUATION line, exercising the serializer's PER-LINE block
* escape (not just the first line). #493 review: without this the fuzzer never
* placed a trigger after a hardBreak, so a single-line-only escape passed P1P3.
*/
export const hardBreakThenTriggerArb: fc.Arbitrary<any[]> = fc
.tuple(hardBreakArb, blockTriggerLeadRunArb)
.map(([hb, trigger]) => [hb, trigger]);
/**
* #493 (setext data-loss): a WHOLE-LINE setext underline landing on a
* continuation line. A setext underline is a line of ONLY `-` (any count) or
* ONLY `=` (any count) that FOLLOWS a paragraph line; on re-parse it turns the
* preceding line into a heading and DROPS its own text. The block-escape must
* neutralize it. Unlike blockTriggerLeadRunArb, the underline must occupy the
* whole line, so we sandwich it between two hardBreaks (underline on its own
* line, preceded by earlier paragraph content, followed by a trailing word so
* the closing hardBreak is not dropped by normalizeInline). Covers underlines
* of every length: `--` (the two-dash case the bullet/thematic arms miss), a
* lone `=`, `==`/`====` (neutralized by the inline `==` escape), and `---`/
* `----` (regression for the existing thematic case).
*/
export const hardBreakThenSetextArb: fc.Arbitrary<any[]> = fc
.tuple(
fc.constantFrom('--', '=', '==', '====', '---', '----'),
safeTextArb,
)
.map(([underline, rest]) => [
{ type: 'hardBreak' },
{ type: 'text', text: underline },
{ type: 'hardBreak' },
{ type: 'text', text: rest },
]);
/**
* Inline content for a paragraph: at least one marked text run, optionally with
* inline atoms (math/mention) and hard breaks interspersed. Always starts with a
* text run so the paragraph never opens with a block trigger. (Ported.)
* inline atoms (math/mention) and hard breaks interspersed. The FIRST run is
* usually an ordinary marked run, but sometimes a block-trigger-leading run
* (blockTriggerLeadRunArb) so the paragraph OPENS with a markdown block trigger;
* and a `hardBreak + trigger` segment can appear anywhere in the rest, so a
* trigger also lands at the start of a CONTINUATION line both exercising the
* serializer's per-line block-escape end-to-end. (Ported, with the #493
* leading-trigger + post-hardBreak dimensions added.)
*/
export const inlineContentArb: fc.Arbitrary<any[]> = fc
.tuple(
markedTextRunArb,
fc.oneof(
{ weight: 5, arbitrary: markedTextRunArb },
{ weight: 1, arbitrary: blockTriggerLeadRunArb },
),
fc.array(
fc.oneof(
{ weight: 5, arbitrary: markedTextRunArb },
{ weight: 1, arbitrary: mathInlineArb },
{ weight: 1, arbitrary: mentionArb },
{ weight: 1, arbitrary: hardBreakArb },
{ weight: 5, arbitrary: markedTextRunArb.map((n) => [n]) },
{ weight: 1, arbitrary: mathInlineArb.map((n) => [n]) },
{ weight: 1, arbitrary: mentionArb.map((n) => [n]) },
{ weight: 1, arbitrary: hardBreakArb.map((n) => [n]) },
{ weight: 2, arbitrary: hardBreakThenTriggerArb },
{ weight: 2, arbitrary: hardBreakThenSetextArb },
),
{ minLength: 0, maxLength: 4 },
),
)
.map(([first, rest]) => normalizeInline([first, ...rest]));
.map(([first, rest]) => normalizeInline([first, ...rest.flat()]));
/**
* Inline content for a HEADING identical to a paragraph's, but WITHOUT hard
@@ -5,32 +5,21 @@ import { convertProseMirrorToMarkdown } from "../src/lib/markdown-converter.js";
import { markdownToProseMirror } from "../src/lib/markdown-to-prosemirror.js";
/**
* gitmost #377 (round-1 review, finding #1) proof, against the REAL
* converter, that the transcript-insert boundary defense survives git-sync.
* #493 commit 1 the paragraph serializer's leading-block-escape closes the
* data-loss class where a paragraph whose text opens at column 0 with a markdown
* block trigger (`#`/`-`/`*`/`+`/`>`, an ordered `N.`/`N)`, a code fence, a
* table `|`, a callout opener, or a thematic break) silently re-parsed into a
* heading / list / quote / code block / table / horizontalRule on the git-sync
* doc -> markdown -> doc cycle. The thematic-break case was the worst: a
* horizontalRule carries NO text, so the line's text was lost entirely.
*
* The web bridge (apps/client .../gitmost/gitmost-recording.ts,
* `gitmostInsertTranscriptIntoEditor`) appends each transcript line as a
* PARAGRAPH text node. The paragraph serializer here (`case "paragraph"`) emits
* that text VERBATIM with no block-escape, so a line whose text begins with a
* col-0 markdown block trigger would, on the doc -> markdown -> doc git-sync
* cycle, silently re-parse into a heading / list / quote / callout / code block.
* That missing block-escape is the pre-existing root cause; the bridge's
* boundary defense prepends an invisible zero-width space (U+200B) to a line
* that begins with such a trigger, shifting it off column 0.
*
* This test keeps a COPY of the bridge's trigger regex (the bridge is in a
* different package and can't be imported here) and asserts:
* 1. bare trigger lines DO corrupt (documents the root cause), and
* 2. the ZWSP-neutralized form round-trips as a single PARAGRAPH with the
* text byte-preserved.
* This is the deterministic PIN, one assertion per trigger, exercised through
* the REAL converter round-trip (not a mock): each bare trigger line now
* round-trips as a SINGLE paragraph with its text byte-preserved proving the
* class is closed WITHOUT the former client-side ZWSP workaround (removed) or
* the generative suite's leading-word self-censorship (removed).
*/
const ZWSP = "​"; // U+200B
// MUST stay in sync with GITMOST_MD_BLOCK_TRIGGER_RE in the client bridge.
const MD_BLOCK_TRIGGER_RE =
/^(?:#{1,6}(?:\s|$)|[-*+](?:\s|$)|>|\d+[.)](?:\s|$)|```|~~~|\||([-*_])(?:\s*\1){2,}\s*$)/;
const doc = (...nodes: any[]) => ({ type: "doc", content: nodes });
const para = (t: string) => ({
type: "paragraph",
@@ -43,78 +32,117 @@ const roundtrip = async (text: string) => {
return back.content as any[];
};
describe("gitmost transcript neutralization (git-sync round-trip)", () => {
// Lines that, at column 0, the serializer's missing block-escape would let
// git-sync re-parse into a non-paragraph block.
describe("paragraph block-escape (git-sync round-trip)", () => {
// Every line here, at column 0, WOULD (pre-fix) re-parse into a non-paragraph
// block. Each is now block-escaped by the serializer and round-trips clean.
const triggerLines = [
"- dash",
"* star",
"+ plus",
"> quote",
"# hash",
"## two hash",
"###### six hash",
"1. one",
"1) one",
"> [!info] note",
"```js",
"~~~",
// Solid + spaced thematic breaks — these re-parse into a `horizontalRule`,
// which carries NO text, so a bare separator line LOSES its text entirely
// (round-2 finding). `_` also only forms a block via this construct.
"| a | b |",
// Solid + spaced thematic breaks — the text-LOSING case pre-fix.
"---",
"***",
"___",
"- - -", // spaced dash break (solid form is caught by [-*+]\s too, but this is the break)
"- - -",
"_ _ _",
];
it("BARE trigger lines corrupt into non-paragraph blocks (root cause)", async () => {
it("every bare trigger line round-trips as a single paragraph, text byte-preserved", async () => {
for (const line of triggerLines) {
const blocks = await roundtrip(line);
// At least one produced block is NOT a paragraph — i.e. corruption.
const allParagraphs = blocks.every((b) => b.type === "paragraph");
expect(
allParagraphs,
`expected "${line}" to corrupt when inserted bare`,
).toBe(false);
}
});
it("BARE solid thematic breaks corrupt into a text-LOSING horizontalRule", async () => {
// The severe case: no text node survives. Documents why neutralization
// matters more here than for list/quote (where the text survived).
for (const line of ["---", "***", "___"]) {
const blocks = await roundtrip(line);
expect(blocks.map((b) => b.type)).toContain("horizontalRule");
// No block carries the original text anywhere.
const flat = JSON.stringify(blocks);
expect(flat).not.toContain(line);
}
});
it("ZWSP-neutralized trigger lines round-trip as a single paragraph, text preserved", async () => {
for (const line of triggerLines) {
// The regex must actually classify each as a trigger.
expect(MD_BLOCK_TRIGGER_RE.test(line), `regex missed "${line}"`).toBe(
true,
expect(blocks, `"${line}" should be one block`).toHaveLength(1);
expect(blocks[0].type, `"${line}" should stay a paragraph`).toBe(
"paragraph",
);
const neutralized = ZWSP + line;
const blocks = await roundtrip(neutralized);
expect(blocks).toHaveLength(1);
expect(blocks[0].type).toBe("paragraph");
// Text is byte-preserved (ZWSP + original line), so the display is the
// original line with only an invisible leading character.
expect(blocks[0].content[0].text).toBe(neutralized);
expect(
blocks[0].content?.[0]?.text,
`"${line}" text should survive byte-exact`,
).toBe(line);
}
});
it("normal host-prefixed lines never match the trigger regex and round-trip byte-exact", async () => {
it("emphasis / inline-code paragraphs are NOT escaped (no backslash churn)", async () => {
// These open with `*`/`` ` `` but are NOT block triggers; the serialized
// markdown must not gain a stray leading backslash, and they round-trip.
for (const [text, mark] of [
["bold", "bold"],
["italic", "italic"],
["code", "code"],
] as const) {
const node = doc({
type: "paragraph",
content: [{ type: "text", text, marks: [{ type: mark }] }],
});
const md = convertProseMirrorToMarkdown(node);
expect(md.startsWith("\\"), `${mark} must not be block-escaped`).toBe(
false,
);
const back = await markdownToProseMirror(md);
expect(back.content[0].type).toBe("paragraph");
expect(back.content[0].content[0].text).toBe(text);
expect(back.content[0].content[0].marks?.[0]?.type).toBe(mark);
}
});
it("a block trigger on a CONTINUATION line (after a hardBreak) is escaped too", async () => {
// A hardBreak serializes as ` \n`, so a trigger on the second line would,
// without a per-line escape, re-parse into another block. The worst case is
// `---`: a setext underline would turn the first line into a heading and LOSE
// the `---` text entirely. Each pair round-trips as ONE paragraph with the
// hardBreak and both texts preserved.
for (const [first, second] of [
["a", "# b"],
["a", "- b"],
["a", "> b"],
["a", "1. b"],
["a", "| b |"],
["a", "---"], // setext / thematic (3 dashes) — the text-losing case
["a", "--"], // setext underline, EXACTLY two dashes (bullet/thematic miss it)
["a", "----"], // setext / thematic (4 dashes)
["a", "="], // setext H1 underline, a lone `=` (no other arm covers it)
["a", "===="], // setext H1 underline, run of `=`
]) {
const d = doc({
type: "paragraph",
content: [
{ type: "text", text: first },
{ type: "hardBreak" },
{ type: "text", text: second },
],
});
const back = await markdownToProseMirror(convertProseMirrorToMarkdown(d));
expect(back.content, `"${first}${second}" should be one block`).toHaveLength(1);
expect(back.content[0].type).toBe("paragraph");
const texts = (back.content[0].content as any[])
.filter((n) => n.type === "text")
.map((n) => n.text);
const hasBreak = (back.content[0].content as any[]).some(
(n) => n.type === "hardBreak",
);
expect(hasBreak, `"${first}${second}" should keep the hardBreak`).toBe(true);
expect(texts, `"${first}${second}" should preserve both line texts`).toEqual([
first,
second,
]);
}
});
it("normal host-prefixed lines round-trip byte-exact (unaffected)", async () => {
for (const line of [
"You: hello there",
"Speaker 1: - and then a dash mid-line",
"Speaker 2: 1. not a list",
]) {
expect(MD_BLOCK_TRIGGER_RE.test(line)).toBe(false);
const blocks = await roundtrip(line);
expect(blocks).toHaveLength(1);
expect(blocks[0].type).toBe("paragraph");
@@ -294,10 +294,11 @@ describe('converter gap coverage — emission branches (specs 1–11)', () => {
);
});
// 5. code + link co-occur: the schema's `code` mark excludes all other marks
// (including link), so the link cannot survive import. The lossless,
// byte-stable behavior is to emit ONLY the backtick code span (code wins).
it('a code+link run emits the backtick code form (code wins, link dropped)', () => {
// 5. code + link co-occur (#515): `code` no longer excludes other marks, so a
// link can wrap inline code. The code span is emitted innermost and the link
// wraps it — CommonMark allows inline code inside link text, so it survives
// the round trip.
it('a code+link run nests the backtick span inside the link (#515)', () => {
const out = convertProseMirrorToMarkdown(
doc(
para({
@@ -310,7 +311,7 @@ describe('converter gap coverage — emission branches (specs 1–11)', () => {
}),
),
);
expect(out).toBe('`x`');
expect(out).toBe('[`x`](http://a?b&c"d)');
});
// 6. hardBreak inside a heading: prefix applied once, " \n" between a and b.
@@ -430,7 +431,7 @@ describe('converter gap coverage — emission branches (specs 1–11)', () => {
});
});
describe('converter gap coverage — documented round-trip data loss (specs 12–14)', () => {
describe('converter gap coverage — formerly-lossy round-trips, now closed (specs 12–14)', () => {
// 12. A 3-backtick fence inside a codeBlock body is now lengthened: the outer
// fence widens to (longest inner run + 1) backticks per CommonMark, so the
// inner ``` is treated as content and the block survives as ONE node.
@@ -460,25 +461,24 @@ describe('converter gap coverage — documented round-trip data loss (specs 12
expect(docsCanonicallyEqual(d, doc2)).toBe(false);
});
// 13. A leading ordered-list marker in paragraph text is NOT escaped, so a
// plain paragraph silently becomes an orderedList on re-import.
it('a paragraph starting with "1. " is promoted to an orderedList on re-import', async () => {
// 13. #493 commit 1: a leading ordered-list marker in paragraph text is now
// BLOCK-ESCAPED, so the paragraph round-trips as a paragraph instead of
// silently becoming an orderedList (was documented data loss, now closed).
it('a paragraph starting with "1. " is block-escaped and stays a paragraph', async () => {
const d = doc({
type: 'paragraph',
content: [{ type: 'text', text: '1. not a list' }],
});
const md1 = convertProseMirrorToMarkdown(d);
expect(md1).toBe('1. not a list'); // no backslash escape
expect(md1).toBe('1\\. not a list'); // the ordered-list delimiter is escaped
const doc2 = await markdownToProseMirror(md1);
expect(doc2.content?.[0]?.type).toBe('orderedList');
const li = doc2.content[0].content?.[0];
expect(li?.type).toBe('listItem');
expect(li.content?.[0]?.content?.[0]).toMatchObject({
expect(doc2.content?.[0]?.type).toBe('paragraph');
expect(doc2.content[0].content?.[0]).toMatchObject({
type: 'text',
text: 'not a list', // the "1. " was consumed as a list marker
text: '1. not a list', // the escape decodes back to the literal text
});
expect(docsCanonicallyEqual(d, doc2)).toBe(false);
expect(docsCanonicallyEqual(d, doc2)).toBe(true);
});
// 14. #293 canon #4: the image title now round-trips via the attached
@@ -59,22 +59,21 @@ describe('convertProseMirrorToMarkdown', () => {
).toBe('`x`');
});
it('code + another mark emits the backtick code form (code wins)', () => {
// The schema's `code` mark excludes all other marks, so the editor can
// never produce code+bold on one run and import always drops the co-mark.
// The lossless, byte-stable behavior is to emit ONLY the backtick code
// span and ignore the co-occurring mark.
it('code + bold nests the backtick span inside the emphasis (#515)', () => {
// #515: the `code` mark no longer excludes other marks (`excludes: ""`), so
// a run can carry code+bold. CommonMark nests them (`<strong><code>`), so
// the code span is emitted innermost and the bold delimiters wrap it.
const out = convertProseMirrorToMarkdown(
doc(para(text('x', [{ type: 'bold' }, { type: 'code' }]))),
);
expect(out).toBe('`x`');
expect(out).toBe('**`x`**');
});
it('code + strike combo emits the backtick code form (code wins)', () => {
it('code + strike nests the backtick span inside the emphasis (#515)', () => {
const out = convertProseMirrorToMarkdown(
doc(para(text('x', [{ type: 'strike' }, { type: 'code' }]))),
);
expect(out).toBe('`x`');
expect(out).toBe('~~`x`~~');
});
});
@@ -80,13 +80,7 @@ import { stripBlockIds } from './roundtrip-helpers.js';
// `it.fails` blocks below (so the suite stays green only because they are marked
// expected-to-fail, never by hiding them):
//
// 1. The `code` mark COMBINED with any other mark. The converter emits nested
// HTML (`<strong><code>x</code></strong>`), but the schema's `code` mark
// declares `excludes: "_"`, so on import every co-occurring mark is dropped
// and the run comes back as `code` only -> md2 == "`x`". Acknowledged in
// markdown-converter.ts (the long comment above the marks switch);
// impossible to round-trip both while `code` excludes them.
// 2. A BLOCK-level `image` placed BETWEEN other blocks. The Docmost image node
// 1. A BLOCK-level `image` placed BETWEEN other blocks. The Docmost image node
// is block-level but `![](url)` is inline; marked wraps it in a <p>, the
// schema hoists the <img> out and leaves an empty paragraph sibling, which
// injects an extra blank gap on the second export. An image IS byte-stable
@@ -625,7 +619,7 @@ describe('markdown <-> ProseMirror round-trip (property-based)', () => {
});
// -------------------------------------------------------------------------
// KNOWN, DOCUMENTED non-roundtrip bug #2 (kept honest as it.fails).
// KNOWN, DOCUMENTED non-roundtrip bug #1 (kept honest as it.fails).
//
// BUG: a block-level `image` placed BETWEEN other blocks is not byte-stable.
// The Docmost image node is BLOCK-level but its markdown form `![](url)` is
@@ -655,23 +649,18 @@ describe('markdown <-> ProseMirror round-trip (property-based)', () => {
});
// -------------------------------------------------------------------------
// KNOWN, DOCUMENTED non-roundtrip bug #1 (kept honest as it.fails).
// #515 ROUND-TRIP PIN: `code` combined with another mark.
//
// BUG: the `code` mark combined with ANY other mark does NOT round-trip.
// The converter emits nested HTML so the output is well-formed, e.g.
// marks [code, bold] -> md1 = "<strong><code>x</code></strong>"
// but the schema's `code` mark declares `excludes: "_"`, so on import the
// co-occurring mark is dropped and the run comes back as code-only:
// md2 = "`x`" (=> md2 !== md1).
// Minimal repro doc:
// { type:'doc', content:[ { type:'paragraph', content:[
// { type:'text', text:'x', marks:[{type:'code'},{type:'bold'}] } ] } ] }
// This is acknowledged in markdown-converter.ts (the long comment above the
// marks switch): preserving both marks is impossible while `code` excludes
// them. Documented here, not "fixed", because the source must not change.
// Before #515 the `code` mark declared `excludes: "_"`, dropping every co-
// occurring mark on import so `` **`x`** `` came back as code-only. Now
// `excludes: ""` lets code combine with all marks (CommonMark nests them,
// `<strong><code>x</code></strong>`), so the run BOTH round-trips byte-stably
// AND preserves the co-occurring mark. This asserts the observable property in
// both directions: md2 === md1 (idempotent export) and the imported doc still
// carries [code, other].
// -------------------------------------------------------------------------
it(
'code mark combined with another mark is byte-stable',
'code combined with another mark round-trips and keeps both marks (#515)',
async () => {
const codeComboArb = fc
.tuple(safeTextArb, fc.constantFrom('bold', 'italic', 'strike'))
@@ -688,11 +677,90 @@ describe('markdown <-> ProseMirror round-trip (property-based)', () => {
}));
await fc.assert(
fc.asyncProperty(codeComboArb, async (doc) => {
const { md1, md2 } = await roundTrip(doc);
const { md1, md2, doc2 } = await roundTrip(doc);
expect(md2).toBe(md1);
// The re-imported run carries BOTH code and the co-occurring mark.
const run = doc2?.content?.[0]?.content?.[0];
const markTypes = (run?.marks || []).map((m: any) => m.type).sort();
expect(markTypes).toContain('code');
expect(markTypes.length).toBe(2);
}),
{ numRuns: 20, seed: SEED },
);
},
);
// -------------------------------------------------------------------------
// #515 REPRO CASES: the five markdown inputs from the issue must import to a
// code+bold node (import correctness) AND re-export byte-stably with no
// dangling `**` (export correctness). Import direction is checked against the
// real markdown->PM bridge; export direction via the md->pm->md fixpoint.
// -------------------------------------------------------------------------
it('the five #515 repro cases import to [code,bold] and round-trip clean', async () => {
// Collect every inline text run in a doc with its mark type set.
const runs = (node: any): { text: string; marks: string[] }[] => {
if (node?.type === 'text') {
return [{ text: node.text || '', marks: (node.marks || []).map((m: any) => m.type) }];
}
return (node?.content || []).flatMap(runs);
};
const findRun = (doc: any, text: string) =>
runs(doc).find((r) => r.text === text);
// Case 1: **`code1`** -> code1 = [code, bold].
{
const md = '**`code1`**';
const pm = await markdownToProseMirror(md);
const r = findRun(pm, 'code1');
expect(r?.marks.sort()).toEqual(['bold', 'code']);
const md2 = convertProseMirrorToMarkdown(pm);
expect(md2).toBe('**`code1`**');
// md -> pm -> md fixpoint.
expect(convertProseMirrorToMarkdown(await markdownToProseMirror(md2))).toBe(md2);
}
// Case 2: **`aaa` + `bbb`** -> aaa,bbb = [code,bold], "+" carries bold; no
// dangling `**` on export.
{
const md = '**`aaa` + `bbb`**';
const pm = await markdownToProseMirror(md);
expect(findRun(pm, 'aaa')?.marks.sort()).toEqual(['bold', 'code']);
expect(findRun(pm, 'bbb')?.marks.sort()).toEqual(['bold', 'code']);
const md2 = convertProseMirrorToMarkdown(pm);
expect(md2).toBe('**`aaa` + `bbb`**');
// NOT the old broken export with the bold delimiters split onto each span.
expect(md2).not.toBe('`aaa`** + **`bbb`');
expect(convertProseMirrorToMarkdown(await markdownToProseMirror(md2))).toBe(md2);
}
// Case 3 (control): **bold3** and `code3` -> bold and code stay SEPARATE.
{
const md = '**bold3** and `code3`';
const pm = await markdownToProseMirror(md);
expect(findRun(pm, 'bold3')?.marks).toEqual(['bold']);
expect(findRun(pm, 'code3')?.marks).toEqual(['code']);
const md2 = convertProseMirrorToMarkdown(pm);
expect(md2).toBe('**bold3** and `code3`');
}
// Case 4: **`code4` tail** -> code4 = [code,bold], " tail" = [bold].
{
const md = '**`code4` tail**';
const pm = await markdownToProseMirror(md);
expect(findRun(pm, 'code4')?.marks.sort()).toEqual(['bold', 'code']);
const md2 = convertProseMirrorToMarkdown(pm);
expect(md2).toBe('**`code4` tail**');
expect(convertProseMirrorToMarkdown(await markdownToProseMirror(md2))).toBe(md2);
}
// Case 5: pre **`code5`** post -> code5 = [code,bold], surroundings plain.
{
const md = 'pre **`code5`** post';
const pm = await markdownToProseMirror(md);
expect(findRun(pm, 'code5')?.marks.sort()).toEqual(['bold', 'code']);
const md2 = convertProseMirrorToMarkdown(pm);
expect(md2).toBe('pre **`code5`** post');
expect(convertProseMirrorToMarkdown(await markdownToProseMirror(md2))).toBe(md2);
}
});
});
@@ -16,14 +16,16 @@ import * as editorExt from "@docmost/editor-ext";
// or mark added upstream that the mirror forgets to vendor fails CI loudly
// (otherwise it is silently dropped on the markdown <-> ProseMirror round-trip).
//
// LIMITATION (intentional, see schema-surface-snapshot.test.ts): this is a
// NAME-LEVEL contract only, not a full attribute-level structural compare.
// editor-ext's Tiptap representation (node views, commands, suggestion plugins,
// addGlobalAttributes spread across separate extensions) differs from this
// minimal mirror, so a mechanical attribute-by-attribute equality would be
// fragile and produce false drift. Attribute parity is guarded by the inline
// surface snapshot (reviewed in every diff); this test guards that no canonical
// node/mark TYPE goes unmirrored. StarterKit-provided types (paragraph, bold,
// This file now holds TWO contracts (see the two describe blocks): the original
// NAME-LEVEL type contract (no canonical node/mark TYPE goes unmirrored) AND, as
// of #493, an ATTRIBUTE-LEVEL contract that compares each editor-ext node/mark's
// OWN declared attributes (names + defaults) against the mirror's built schema.
// A full mechanical attribute-by-attribute EQUALITY would be fragile (the mirror
// is a deliberate superset: it injects the global id/textAlign/indent attrs and
// normalizes some editor-ext defaults to null), so the attribute contract is
// asymmetric — editor-ext -> mirror — with a small, reasoned, stale-guarded
// allowlist for the two blessed divergence kinds (non-round-trippable omissions
// and null-normalized defaults). StarterKit-provided types (paragraph, bold,
// heading, …) are contributed by @tiptap/starter-kit in the mirror rather than
// by editor-ext, so they are naturally covered by the mirror's superset.
//
@@ -85,3 +87,224 @@ describe("docmost schema vs @docmost/editor-ext (name-level contract)", () => {
expect(missing).toEqual([]);
});
});
// ── #515 CODE-MARK `excludes` PARITY (data-loss-sensitive) ──────────────────
//
// The `code` mark's `excludes` field decides whether inline code can co-occur
// with other marks. #515 sets it to "" (excludes nothing) in the canonical
// `Code` exported by @docmost/editor-ext AND, because the vendored markdown
// mirror must NOT pull that React-aware package into its node runtime, RE-DECLARES
// the same override locally in docmost-schema.ts. If the two drift, markdown
// import would silently strip bold/italic adjacent to inline code again. Guard it
// mechanically: the mirror's built `code` mark and the canonical editor-ext
// `Code` must agree on `excludes` (both ""). getSchema surfaces the resolved
// value on the mark spec.
describe("docmost schema vs @docmost/editor-ext (#515 code excludes parity)", () => {
it("keeps the vendored `code` mark's excludes in lockstep with editor-ext Code", () => {
// Mirror side: the value the mirror's BUILT schema resolves for `code`.
const mirrorExcludes = getSchema(docmostExtensions as never).marks.code.spec
.excludes;
// Canonical side: the `excludes` DECLARED on the editor-ext `Code` extension
// (read from its config — getSchema needs a full node set, so a lone mark
// can't be built into a schema here).
const canonicalCode = (
editorExt as unknown as { Code?: { config?: { excludes?: unknown } } }
).Code;
const canonicalExcludes = canonicalCode?.config?.excludes;
// Both must be the empty string: `code` excludes NOTHING, so bold/italic/…
// survive alongside inline code (#515). A drift here would silently strip
// marks adjacent to code on markdown import again.
expect(canonicalCode).toBeDefined();
expect(mirrorExcludes).toBe("");
expect(canonicalExcludes).toBe("");
expect(mirrorExcludes).toBe(canonicalExcludes);
});
});
// ── ATTRIBUTE-LEVEL CONTRACT (#493 commit 2) ────────────────────────────────
//
// The name-level contract above catches a WHOLE node/mark type going unmirrored,
// but not ATTRIBUTE drift within a vendored type — the exact class that silently
// dropped `subpages.recursive`: editor-ext grew an attribute the hand-synced
// mirror forgot, so documents using it lost that attribute on a git-sync
// round-trip while CI stayed green. This closes that gap by comparing each
// editor-ext node/mark's OWN declared attributes (names + defaults) against the
// mirror's built ProseMirror schema `spec.attrs`.
//
// DIRECTION: editor-ext -> mirror. The mirror is deliberately a SUPERSET (it
// injects the global `id`/`textAlign`/`indent` attributes and normalizes some
// editor-ext "required" attrs to a `null` default), so a reverse compare would
// be pure false drift; the meaningful failure is an editor-ext attribute the
// mirror DROPS (name) or whose DEFAULT it silently changes. Both directions of
// staleness are guarded so the allowlists cannot rot.
/**
* The attributes an editor-ext Tiptap Node/Mark DECLARES itself, read from its
* `config.addAttributes()`. Global attributes injected by separate extensions
* (unique-id, indent, textAlign) are NOT included here they are the mirror's
* superset and are not part of a per-type declaration so this isolates each
* type's own contribution. A declared attribute with no explicit `default` is a
* required attr (Tiptap default `undefined`); we surface that as-is so the
* default compare can skip it (the mirror makes such attrs optional/`null`).
*/
function editorExtOwnAttrs(): Map<
string,
{ kind: "node" | "mark"; attrs: Record<string, unknown> }
> {
const out = new Map<
string,
{ kind: "node" | "mark"; attrs: Record<string, unknown> }
>();
for (const value of Object.values(editorExt)) {
if (!isTiptapNodeOrMark(value)) continue;
const ext = value as unknown as {
name: string;
type: "node" | "mark";
options?: unknown;
storage?: unknown;
config?: { addAttributes?: () => Record<string, { default?: unknown }> };
};
const fn = ext.config?.addAttributes;
// addAttributes reads `this.options`/`this.name`; bind a minimal context
// (verified sufficient for every editor-ext extension — none reach for
// `this.editor` here). A type with no addAttributes contributes no attrs.
const declared =
typeof fn === "function"
? fn.call({
options: ext.options ?? {},
name: ext.name,
parent: undefined,
storage: ext.storage ?? {},
} as never)
: {};
const attrs: Record<string, unknown> = {};
for (const [attr, spec] of Object.entries(declared || {})) {
// `undefined` marks a required (no-default) attr; keep it so the default
// compare can distinguish "no default declared" from "default is null".
attrs[attr] = (spec as { default?: unknown })?.default;
}
out.set(ext.name, { kind: ext.type, attrs });
}
return out;
}
/** The mirror's built-schema `spec.attrs` for a type: attr name -> default. */
function mirrorAttrs(
name: string,
kind: "node" | "mark",
): Record<string, unknown> | null {
const schema = getSchema(docmostExtensions as never);
const spec = kind === "node" ? schema.nodes[name]?.spec : schema.marks[name]?.spec;
if (!spec) return null;
const out: Record<string, unknown> = {};
for (const [attr, def] of Object.entries(spec.attrs || {})) {
out[attr] = (def as { default?: unknown }).default;
}
return out;
}
// An editor-ext attribute the mirror deliberately does NOT vendor because it has
// NO markdown round-trip representation — dropping it loses nothing on the
// git-sync cycle (the same rationale the flat-roundtrip property suite uses to
// allowlist e.g. `tableCell.backgroundColorName`). Blessed by the hand-curated
// surface snapshot (schema-surface-snapshot.test.ts), reviewed in every diff.
const ACCEPTED_ATTR_OMISSIONS = new Set<string>([
"highlight.colorName", // only `highlight.color` round-trips (==text==); the
// secondary palette-name is presentational and has no markdown form.
]);
// An editor-ext attribute the mirror vendors but with a DIFFERENT default: the
// mirror normalizes an "absent" value to `null` (its uniform optional-attr
// convention) rather than editor-ext's UI-oriented default. None of these attrs
// is emitted on the markdown surface (the converter round-trips only the
// serializable ones), so the default never round-trips and the divergence is
// inert — but pinned here so a NEW default change on either side forces review.
const ACCEPTED_DEFAULT_DIVERGENCE = new Set<string>([
"image.src", // mirror null vs editor "" (an image is never emitted src-less)
"link.internal", // mirror null vs editor false (routing attr, not in md link)
"pdf.width", // mirror null vs editor 800 (presentational sizing, not in md)
"pdf.height", // mirror null vs editor 600 (presentational sizing, not in md)
]);
describe("docmost schema vs @docmost/editor-ext (attribute-level contract)", () => {
it("vendors every editor-ext attribute (name) of every shared type — no silently-dropped attrs", () => {
const dropped: string[] = [];
for (const [name, { kind, attrs }] of editorExtOwnAttrs()) {
const mirror = mirrorAttrs(name, kind);
if (!mirror) continue; // whole-type omission is the name-level test's job
for (const attr of Object.keys(attrs)) {
const key = `${name}.${attr}`;
if (!(attr in mirror) && !ACCEPTED_ATTR_OMISSIONS.has(key)) {
dropped.push(key);
}
}
}
// Any entry here exists on the editor-ext node/mark but NOT in the mirror
// (and is not a blessed non-round-trippable omission): documents using it
// lose that attribute on a git-sync round-trip — the subpages.recursive
// class. Re-sync src/lib/docmost-schema.ts (and the surface snapshot) or add
// a reasoned ACCEPTED_ATTR_OMISSIONS entry before clearing.
expect(dropped.sort()).toEqual([]);
});
it("keeps every editor-ext attribute DEFAULT in sync — no silent default drift", () => {
const drift: string[] = [];
for (const [name, { kind, attrs }] of editorExtOwnAttrs()) {
const mirror = mirrorAttrs(name, kind);
if (!mirror) continue;
for (const [attr, extDefault] of Object.entries(attrs)) {
const key = `${name}.${attr}`;
// Skip attrs editor-ext declares WITHOUT a default (required attrs):
// the mirror deliberately makes them optional (`null`), a safe superset.
if (extDefault === undefined) continue;
if (!(attr in mirror)) continue; // a drop, reported by the name test
if (
JSON.stringify(mirror[attr]) !== JSON.stringify(extDefault) &&
!ACCEPTED_DEFAULT_DIVERGENCE.has(key)
) {
drift.push(
`${key}: mirror=${JSON.stringify(mirror[attr])} editor-ext=${JSON.stringify(extDefault)}`,
);
}
}
}
expect(drift.sort()).toEqual([]);
});
it("the attribute allowlists have no stale rows (each is really omitted / divergent)", () => {
const ext = editorExtOwnAttrs();
const staleOmission: string[] = [];
for (const key of ACCEPTED_ATTR_OMISSIONS) {
const [name, attr] = key.split(".");
const entry = ext.get(name);
const mirror = entry ? mirrorAttrs(name, entry.kind) : null;
// Stale if editor-ext no longer declares it, or the mirror now DOES vendor
// it (so it should be removed from the omission allowlist).
if (!entry || !(attr in entry.attrs) || (mirror && attr in mirror)) {
staleOmission.push(key);
}
}
expect(staleOmission, "stale ACCEPTED_ATTR_OMISSIONS rows").toEqual([]);
const staleDivergence: string[] = [];
for (const key of ACCEPTED_DEFAULT_DIVERGENCE) {
const [name, attr] = key.split(".");
const entry = ext.get(name);
const mirror = entry ? mirrorAttrs(name, entry.kind) : null;
const extDefault = entry?.attrs[attr];
// Stale if the divergence no longer exists (attr gone, or defaults now
// agree) — the row should be dropped so the allowlist stays honest.
if (
!entry ||
!mirror ||
!(attr in entry.attrs) ||
!(attr in mirror) ||
extDefault === undefined ||
JSON.stringify(mirror[attr]) === JSON.stringify(extDefault)
) {
staleDivergence.push(key);
}
}
expect(staleDivergence, "stale ACCEPTED_DEFAULT_DIVERGENCE rows").toEqual([]);
});
});
@@ -9,11 +9,18 @@ import { defineConfig } from 'vitest/config';
// envelope, markdownToProseMirror) is re-exported there.
const here = path.dirname(fileURLToPath(import.meta.url));
const libBarrel = path.resolve(here, 'src/lib/index.ts');
// Resolve the cross-package `@docmost/editor-ext` specifier to the SIBLING
// workspace SOURCE. In a normal checkout this is what pnpm's workspace link +
// the package's `module` field already yield; pinning it here makes the schema
// contract tests (incl. the #515 code-excludes parity) hermetic and independent
// of node_modules layout (e.g. a shared/hoisted store in a git worktree).
const editorExtBarrel = path.resolve(here, '../editor-ext/src/index.ts');
export default defineConfig({
resolve: {
alias: {
'docmost-client': libBarrel,
'@docmost/editor-ext': editorExtBarrel,
},
},
test: {
+6 -114
View File
@@ -1,62 +1,8 @@
diff --git a/dist/index.js b/dist/index.js
index ae447a12f7823ec0a00837ee9f0eb809a610d5f8..210b5a9009e5cf1537cb2007c524007c1a01253c 100644
index ae447a12f7823ec0a00837ee9f0eb809a610d5f8..a3402b2c2d021ef432cfa76e35d370073d525135 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -5036,9 +5036,40 @@ function writeToServerResponse({
break;
const canContinue = response.write(value);
if (!canContinue) {
- await new Promise((resolve3) => {
- response.once("drain", resolve3);
+ // PATCH(docmost #486): race "drain" against "close"/"error". The
+ // original awaited ONLY "drain", so a client that disconnected mid-write
+ // (the socket never drains) parked this loop FOREVER: the finally never
+ // ran, response.end() was unreachable, and the reader + buffered chunks
+ // were held until process restart. On close/error we cancel the reader
+ // and break so the finally always runs (safe for detached runs:
+ // consumeStream drains the SDK stream independently). Listener hygiene:
+ // all three once-listeners are removed on the first settle, so they
+ // cannot pile up one-per-stall.
+ const closed = await new Promise((resolve3) => {
+ function finish(isClosed) {
+ response.removeListener("drain", onDrain);
+ response.removeListener("close", onClose);
+ response.removeListener("error", onError);
+ resolve3(isClosed);
+ }
+ function onDrain() {
+ finish(false);
+ }
+ function onClose() {
+ finish(true);
+ }
+ function onError() {
+ finish(true);
+ }
+ response.once("drain", onDrain);
+ response.once("close", onClose);
+ response.once("error", onError);
});
+ if (closed) {
+ await reader.cancel().catch(() => {
+ });
+ break;
+ }
}
}
} catch (error) {
@@ -5047,7 +5078,9 @@ function writeToServerResponse({
response.end();
}
};
- read();
+ read().catch((error) => {
+ console.error("ai writeToServerResponse read() failed:", error);
+ });
}
// src/text-stream/pipe-text-stream-to-response.ts
@@ -6578,9 +6611,19 @@ function createOutputTransformStream(output) {
@@ -6578,9 +6578,19 @@ function createOutputTransformStream(output) {
controller.enqueue({ part: chunk, partialOutput: void 0 });
return;
}
@@ -77,7 +23,7 @@ index ae447a12f7823ec0a00837ee9f0eb809a610d5f8..210b5a9009e5cf1537cb2007c524007c
const result = await output.parsePartialOutput({ text: text2 });
if (result !== void 0) {
const currentJson = JSON.stringify(result.partial);
@@ -6959,7 +7002,7 @@ var DefaultStreamTextResult = class {
@@ -6959,7 +6969,7 @@ var DefaultStreamTextResult = class {
})
);
}
@@ -87,64 +33,10 @@ index ae447a12f7823ec0a00837ee9f0eb809a610d5f8..210b5a9009e5cf1537cb2007c524007c
maxRetries: maxRetriesArg,
abortSignal
diff --git a/dist/index.mjs b/dist/index.mjs
index 663875332e3f9a9bd167c25583c515876f42951b..a51514a390d22811a407edd8b703e21793586cc8 100644
index 663875332e3f9a9bd167c25583c515876f42951b..b840b0502c9894df983e0154805abb80e70e6331 100644
--- a/dist/index.mjs
+++ b/dist/index.mjs
@@ -4957,9 +4957,40 @@ function writeToServerResponse({
break;
const canContinue = response.write(value);
if (!canContinue) {
- await new Promise((resolve3) => {
- response.once("drain", resolve3);
+ // PATCH(docmost #486): race "drain" against "close"/"error". The
+ // original awaited ONLY "drain", so a client that disconnected mid-write
+ // (the socket never drains) parked this loop FOREVER: the finally never
+ // ran, response.end() was unreachable, and the reader + buffered chunks
+ // were held until process restart. On close/error we cancel the reader
+ // and break so the finally always runs (safe for detached runs:
+ // consumeStream drains the SDK stream independently). Listener hygiene:
+ // all three once-listeners are removed on the first settle, so they
+ // cannot pile up one-per-stall.
+ const closed = await new Promise((resolve3) => {
+ function finish(isClosed) {
+ response.removeListener("drain", onDrain);
+ response.removeListener("close", onClose);
+ response.removeListener("error", onError);
+ resolve3(isClosed);
+ }
+ function onDrain() {
+ finish(false);
+ }
+ function onClose() {
+ finish(true);
+ }
+ function onError() {
+ finish(true);
+ }
+ response.once("drain", onDrain);
+ response.once("close", onClose);
+ response.once("error", onError);
});
+ if (closed) {
+ await reader.cancel().catch(() => {
+ });
+ break;
+ }
}
}
} catch (error) {
@@ -4968,7 +4999,9 @@ function writeToServerResponse({
response.end();
}
};
- read();
+ read().catch((error) => {
+ console.error("ai writeToServerResponse read() failed:", error);
+ });
}
// src/text-stream/pipe-text-stream-to-response.ts
@@ -6501,9 +6534,19 @@ function createOutputTransformStream(output) {
@@ -6501,9 +6501,19 @@ function createOutputTransformStream(output) {
controller.enqueue({ part: chunk, partialOutput: void 0 });
return;
}
@@ -165,7 +57,7 @@ index 663875332e3f9a9bd167c25583c515876f42951b..a51514a390d22811a407edd8b703e217
const result = await output.parsePartialOutput({ text: text2 });
if (result !== void 0) {
const currentJson = JSON.stringify(result.partial);
@@ -6882,7 +6925,7 @@ var DefaultStreamTextResult = class {
@@ -6882,7 +6892,7 @@ var DefaultStreamTextResult = class {
})
);
}
+6 -6
View File
@@ -48,7 +48,7 @@ patchedDependencies:
hash: d8dc66e5ec3b9d23a876b979f493b6aa901fd2d965be54729495da5136296a42
path: patches/@hocuspocus__server@3.4.4.patch
ai@6.0.134:
hash: e8c599b3963eb01b9ed1481683b7b795ce94137aa1a0c951917c20c8b870299b
hash: f60bfc3357e01e1f3978c6c40fdd65aeb33fefaad7179cde8676465b6c5ff4d9
path: patches/ai@6.0.134.patch
scimmy@1.3.5:
hash: 775d80f86830b2c5dd1a250c9802c10f8fc3da3c7898373de5aa0c23993d1673
@@ -644,10 +644,10 @@ importers:
version: 8.3.0(socket.io-adapter@2.5.4)
ai:
specifier: ^6.0.134
version: 6.0.134(patch_hash=e8c599b3963eb01b9ed1481683b7b795ce94137aa1a0c951917c20c8b870299b)(zod@4.3.6)
version: 6.0.134(patch_hash=f60bfc3357e01e1f3978c6c40fdd65aeb33fefaad7179cde8676465b6c5ff4d9)(zod@4.3.6)
ai-sdk-ollama:
specifier: ^3.8.1
version: 3.8.1(ai@6.0.134(patch_hash=e8c599b3963eb01b9ed1481683b7b795ce94137aa1a0c951917c20c8b870299b)(zod@4.3.6))(zod@4.3.6)
version: 3.8.1(ai@6.0.134(patch_hash=f60bfc3357e01e1f3978c6c40fdd65aeb33fefaad7179cde8676465b6c5ff4d9)(zod@4.3.6))(zod@4.3.6)
bcrypt:
specifier: ^6.0.0
version: 6.0.0
@@ -16455,17 +16455,17 @@ snapshots:
agent-base@7.1.4: {}
ai-sdk-ollama@3.8.1(ai@6.0.134(patch_hash=e8c599b3963eb01b9ed1481683b7b795ce94137aa1a0c951917c20c8b870299b)(zod@4.3.6))(zod@4.3.6):
ai-sdk-ollama@3.8.1(ai@6.0.134(patch_hash=f60bfc3357e01e1f3978c6c40fdd65aeb33fefaad7179cde8676465b6c5ff4d9)(zod@4.3.6))(zod@4.3.6):
dependencies:
'@ai-sdk/provider': 3.0.8
'@ai-sdk/provider-utils': 4.0.21(zod@4.3.6)
ai: 6.0.134(patch_hash=e8c599b3963eb01b9ed1481683b7b795ce94137aa1a0c951917c20c8b870299b)(zod@4.3.6)
ai: 6.0.134(patch_hash=f60bfc3357e01e1f3978c6c40fdd65aeb33fefaad7179cde8676465b6c5ff4d9)(zod@4.3.6)
jsonrepair: 3.13.3
ollama: 0.6.3
transitivePeerDependencies:
- zod
ai@6.0.134(patch_hash=e8c599b3963eb01b9ed1481683b7b795ce94137aa1a0c951917c20c8b870299b)(zod@4.3.6):
ai@6.0.134(patch_hash=f60bfc3357e01e1f3978c6c40fdd65aeb33fefaad7179cde8676465b6c5ff4d9)(zod@4.3.6):
dependencies:
'@ai-sdk/gateway': 3.0.77(zod@4.3.6)
'@ai-sdk/provider': 3.0.8