Append `(pageId <id>; transient — retry once; persistent failures mean the
collab server is unreachable/overloaded)` to the connect-timeout, persist-timeout
and connection-closed error texts in CollabSession, so the agent can self-correct
instead of blind-looping. The Yjs-encode error is left untouched (it already names
the offending attribute).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase A: add ONE response interceptor on DocmostClient's axios instance,
registered AFTER the re-login interceptor, that reformats a failed request's
error.message IN PLACE (never a custom Error subclass, so the live
axios.isAxiosError / error.response?.status / config._retry checks keep working)
into `<METHOD> <path> failed (<status> <statusText>): <serverMessage>`, or the
no-response variant. serverMessage is built ONLY from the whitelisted
message/error fields or statusText — raw string/HTML bodies, headers and config
never appear; an arraybuffer body is size-capped JSON.parsed; the full body goes
to stderr only under DEBUG (parity with downloadImage). A _docmostFormatted flag
guards against double-processing.
Phase B (#436): assertFullUuid throws an actionable error BEFORE any network
call at all five commentId sites (resolve/update/delete/get_comment and
create_comment's parentCommentId when provided), so a truncated id can no longer
loop as an opaque 400/404.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #434 (drawio stage 1) added drawioGet/drawioCreate/drawioUpdate to the
shared tool-spec registry with in-app metadata (inAppKey, deferred tier,
catalogLine) but wired them only in the standalone MCP server, breaking the
contract-parity and phantom-catalog unit tests on develop CI.
- expose drawioGet/drawioCreate/drawioUpdate in forUser() via sharedTool(),
mirroring the MCP transport's argument mapping (format ?? 'xml' default,
flat schema regrouped into the client's `where` object, positional
baseHash pass-through)
- extend the DocmostClientLike hand-mirror with the three client methods
- append the three names to the HOST_CONTRACT_METHODS drift-guard whitelist
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mutate трекает in-flight одним полем inflightReject; наложенный второй вызов
перезаписал бы рехджектор первого -> при disconnect отклонился бы только второй,
первый бы висел до PERSIST_TIMEOUT_MS (20с). В проде безопасно (оба call-site
сериализуют через per-page withPageLock), но это футган на разделяемом примитиве.
Гард в начале mutate (после ready-проверки, до касания inflightReject): наличие
in-flight -> reject нового вызова без порчи состояния первого. Docstring CONCURRENCY.
Последовательные mutate не задеты (localFinish синхронно чистит inflightReject до
резолва). +2 теста: конкурентный второй реджектится, первый цел; последовательные
оба успешны.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Инцидент: агент заполнял большую таблицу десятками table_update_cell, каждый —
полный цикл connect/auth/load/initial-sync/store/unload; часть падала с 25s connect-
timeout, event loop lag до 1.7с. Вариант B: кэшировать живой HocuspocusProvider
per page на серию правок — пока провайдер жив (connections>0), сервер не входит в
store->unload->reload, дебаунс реально коалесцирует записи (N ячеек -> 1-2 store).
Новый модуль collab-session.ts: класс CollabSession (connecting->ready->dead) +
реестр (ключ wsUrl+pageId+token) с idle-TTL/max-age/LRU-evict. mutatePageContent
(collaboration.ts) и mutateLiveContentUnlocked (client.ts, replaceImage) переведены
на acquireCollabSession; one-shot Promise-машина (~360 строк дублирования) удалена.
5 инвариантов (подтверждены внутренним ревью): (1) read->write атомарна — между
fromYdoc и applyDocToFragment нет await; (2) per-edit ack сохранён дословно (гард
connectionLost от false-success при реконнекте); (3) disconnect=смерть сессии (без
авто-реконнекта, in-flight реджектится теми же текстами ошибок); (4) изоляция
identity (токен в ключе); (5) валидация при reuse. replaceImage работает под
внешним page-локом без дедлока (acquire лок не берёт). Тексты ошибок == develop.
Env: MCP_COLLAB_SESSION_IDLE_MS (0=выкл кэш, точное легаси), _MAX_AGE_MS, _MAX_ENTRIES.
Teardown: destroyAllSessions обвязан в stdio (exit/SIGINT/SIGTERM) + реэкспорт из
index для встраивающего хоста.
closes#400
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- text-normalize.ts: closestBlockHint был вставлен между docstring'ом
stripInlineMarkdown и его определением -> docstring осиротел. closestBlockHint
перенесён ПОСЛЕ stripInlineMarkdown, каждый docstring снова примыкает к своей
функции. Поведение не менялось (только порядок объявлений).
- comment-anchor.ts: header-коммент завышал маршрутизацию — countAnchorMatches НЕ
зовёт resolveAnchorSelection, у него своя параллельная реализация exact-wins.
Коммент уточнён: can/get/apply идут через resolveAnchorSelection, count держит
свой счётчик-примитив, синхронный с ним; обе реализации exact-wins должны
держаться в синхроне при правках.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
createComment — топ-хотспот ошибок агента (промахи по якорю, слепые ретраи).
Портированы аффордансы самокоррекции из editPageText:
- Closest-block hint: общий хелпер closestBlockHint вынесен в text-normalize.ts
(json-edit.ts теперь тоже его зовёт), подключён во все 3 throw-а createComment.
- Markdown-strip fallback в comment-anchor.ts согласованно по всем 4 функциям
(can/count/apply/get) через единый resolveAnchorSelection: exact-verbatim wins
глобально, stripped — только если raw не якорится нигде; soft warning как в
editPageText. Инвариант уникальности suggestion (0/1/>=2) сохранён: raw-unique
никогда не запускает fallback -> не может стать ambiguous. Хранимый selection
остаётся СЫРОЙ подстрокой документа (strip только для поиска).
- Multi-block detection: явное сообщение 'selection spans multiple blocks' когда
per-block поиск провалился, но выделение есть в объединённом тексте блоков.
- tool-spec createComment: копировать selection дословно из getPage/searchInPage.
Известное мелкое ограничение (нит внутреннего ревью): детектор multi-block
использует raw selection, поэтому markdown-стилизованное выделение через границу
блоков получит generic-подсказку вместо spans-multiple-blocks (редко, guidance
всё равно корректный).
closes#408
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Promoting insertFootnote/insertImage/replaceImage into the in-app
DocmostClientLike interface requires their mirror in the drift-guard's
HOST_CONTRACT_METHODS list (the bidirectional deepEqual — 40 vs 43 — was
red, exactly what the guard exists to catch). Added the three names and
corrected the header comment: they were MCP-only but are now in-app-consumed
and tracked; only deleteComment/updateComment remain untracked MCP-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Researcher role wrote 40 literal `^[...]` and zero real footnotes: its
incremental write path (insertNode/editPageText) doesn't parse markdown, and
the footnote-capable tool was MCP-only. Promote three tools from inline
MCP-only to the shared registry so the in-app agent gets them too.
- tool-specs.ts: insertFootnote/insertImage/replaceImage added to
SHARED_TOOL_SPECS (mcpName/schema/description moved VERBATIM from the inline
registrations — MCP names + behaviour unchanged for external clients).
- index.ts: the 3 inline registerTool calls become registerShared; drop the
"MCP-only by design" comments.
- ai-chat-tools.service.ts: register the 3 in-app via sharedTool ->
client.insertFootnote/insertImage/replaceImage (imageUrl->url,
attachmentId->oldAttachmentId mapping).
- tool-tiers.ts: insertFootnote -> core (else the original asymmetry recurs —
footnote tool hidden while editPageText is core); images -> deferred.
- research/{en,ru}.yaml FOOTNOTES: `^[...]` parses ONLY on a whole-markdown
write (create/update/import); for a pinpoint citation to existing text use
insertFootnote; via editPageText/insertNode it stays literal.
- json-edit.ts guardrail: an edit_page_text `replace` containing a `^[...]`
token is refused into failed[] with an insert_footnote hint, mirroring the
existing formatting-marker refusal. (Slightly broader net than that mirror —
a literal `^[a-z]` regex class in a replace is also refused; accepted
defense-in-depth, has a no-false-positive test.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DO-1: add an integration test (test/mock/tool-timing-server.test.mjs) that
constructs a real createDocmostMcpServer with a spy onMetric, links a Client
over InMemoryTransport, invokes get_workspace (no input schema, so the wrapped
handler always runs) and asserts onMetric fired with
("mcp_tool_duration_seconds", <number>, { tool: "get_workspace" }). This locks
that the monkeypatch wraps every tool AND labels with the registration name —
which the isolated timeToolHandler unit test does not. Mutation-verified
(mislabel -> test fails). The tool's expected backend failure (ECONNREFUSED)
is tolerated: the wrapper times in a finally on throw too, so the metric fires.
DO-2: reword the mcp.service.ts "zero overhead when disabled" comment to
"negligible overhead" — the registerTool wrapper still runs performance.now()
+ an async try/finally per tool call when onMetric is undefined (the
`onMetric?.()` short-circuits the label build; cost is immaterial at
tool-call rate), so "zero" was literally inaccurate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
packages/mcp stays free of prom-client/server: it only calls an optional
DocmostMcpConfig.onMetric(name, value, labels?) sink the host provides.
- client.ts: onMetric on the config; fires collab_connect_timeouts_total
once, only inside the 25s connect-timeout callback (the connect-vs-unload
signal). cleanup() clears the timer on every other finish path, so no
double-count.
- index.ts: createDocmostMcpServer monkeypatches server.registerTool (before
registerShared + inline tools are registered) to wrap every handler with
timeToolHandler — times in a finally on success AND throw, re-throws
unchanged, emits mcp_tool_duration_seconds{tool=<registered name>} (bounded
cardinality). Single choke point catches all tools.
- mcp.service.ts: the per-request config resolver injects onMetric ONLY when
isMetricsEnabled(), routing mcp_tool_duration_seconds -> observeMcpTool and
collab_connect_timeouts_total -> incConnectTimeout. Disabled / standalone
(stdio, no onMetric) -> undefined -> zero-overhead no-op.
New node:test unit (tool-timing.test.mjs) covers the wrapper's value/throw
preservation and the standalone no-op. packages/mcp/build/ is gitignored,
not committed (CI rebuilds it).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DO-1 (regression): the orderedList start hardening `Number(x)||1` let a
negative/fractional start through into the marker ("-3.", "2.5.") — marked
can't tokenize it, so the whole list re-imported as a paragraph (structure
corruption); start=0 churned. Both the markdown and raw-HTML export paths
now use `Number.isInteger(raw) && raw > 1 ? raw : 1`, so any degenerate
start collapses to the default "1." markers / bare <ol> (list always valid).
New ordered-list-start-normalization test pins {0,-3,2.5} → valid start=1
list, byte-stable; the round-trip fuzz keeps to integers >=2 (num 2,3,5,42).
DO-2 (nightly was non-functional): NUM_RUNS=5000 OOM'd the worker and the
crash was misreported as a "counterexample" issue whose prefix-dedup then
locked out all future issues. Reworked to shard 8 fresh vitest processes
(600 runs each, distinct seeds, --max-old-space-size) so deep fuzzing
never OOMs; a failing shard's output is preserved, and issue creation
discriminates a real fast-check counterexample from an infra/OOM failure
(distinct titles + scoped dedup). The two issue steps use `always() &&`
so they actually run on the failure path.
DO-3: envInt extracted to test/generative/env-int.ts + unit-tested.
DO-4: nightly dispatch inputs go through env: (no ${{ }} in run:).
DO-5: attr-arbitraries.ts docblock synced (column.width/orderedList.start
are fixed+fuzzed, not pinned it.fails).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Items 1 & 2 of the #351 remainder.
Item 1 — the flat/nested generative property tests now read SEED and
NUM_RUNS from PROPERTY_SEED / PROPERTY_NUM_RUNS (via an envInt helper that
honors an explicit 0 and falls back to the current defaults —
20250705/300 flat, /100 nested — on unset/empty/non-numeric). New
.github/workflows/nightly-property.yml runs the generative suite daily
(and on workflow_dispatch) with a random seed and NUM_RUNS≈5000; on
failure it files a Gitea issue containing fast-check's shrunk
counterexample (jq-escaped, dedup'd by title). No build step — the suite
imports the converter from src/, so a tsc error can't masquerade as a
property failure.
Item 2 — new packages/prosemirror-markdown/README.md documents the
counterexample process (surface -> shrink -> permanent fixture in
test/fixtures/counterexamples/ + counterexamples.test.ts -> fix the
converter, never weaken a property; maintainer-approved ACCEPTED/allowlist
entries carry a reason) plus the two golden layers, the coverage
allowlist, and how to run with the env knobs. Linked from AGENTS.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Item 4 of the #351 remainder — implement value-fuzz for the media
dimension/family attributes previously parked in the flat property
suite's ATTR_VALUE_FUZZ_ALLOWLIST as "round-trip candidates deferred to
a later PR": width/height/align/aspectRatio/size/caption/title/alt across
image, video, youtube, pdf, drawio, excalidraw, embed.
Each gets a non-default-value arbitrary in attr-arbitraries.ts and is
removed from the allowlist; the P1 (semantic) + P2 (byte-stability)
property gate now exercises them. All round-trip green — the comment-JSON
serialization (stable key order, String()-stringified) carries every one
faithfully, so no new pins or accepted-limitations were needed.
embed.width/height are fuzzed as numeric STRINGS (not numbers): a
non-default embed dimension round-trips through the comment JSON as a
string (the numeric 800/600 default is still omitted/re-materialized),
so authoring a number diverged under P1.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Item 3 of the #351 remainder — resolve the two pinned converter
counterexamples, fixtures-first.
orderedList.start (genuine P1 loss): the converter always emitted "1."
and dropped attrs.start. Now the markdown path emits `${start + index}.`
(Number-coerced, guards a stray non-numeric start) and the raw-HTML path
emits `<ol start="N">` when start>1; a default (start=1) list is
byte-unchanged. tiptap StarterKit reads both forms back. Un-pinned as a
passing regression test; orderedList.start is now value-fuzzed.
column.width: the "50% churn" counterexample rested on a false premise —
the canonical editor (editor-ext column.ts) stores width as a unitless
flex-grow NUMBER (parseFloat, `flex:${width}`), never a "%" string, and
docmost-schema.ts is a vendored mirror that MUST match it. The original
parseFloat was correct parity and already byte-stable for numeric widths.
Reverted the mirror to parseFloat, deleted the fabricated counterexample
fixture, and now value-fuzz column.width as a number (real type). No src
behaviour change for column.width — parity with editor-ext preserved.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The #351 converter fix (79a461f7) made multi-block list items emit a blank-line
separator between block children (loose list) to stop silent content merging on
re-parse. The prosemirror-markdown goldens were updated in that PR, but the two
mcp unit tests asserting the old tight output were missed — packages/mcp
re-exports the shared converter, so they broke the develop CI run (test job,
`pnpm -r test`, 2/480 failures).
- expect "- Parent\n\n - A..." instead of "- Parent\n - A..."
- expect "1. Parent\n\n - Child" instead of "1. Parent\n - Child"
Full mcp suite: 480/480 green; editor-ext / prosemirror-markdown / client /
git-sync suites green as well.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
stabilize.test.ts used `data-align="center"` as proof the convergence pass
materialized the drawio node. Since this PR correctly stops emitting the
schema-default center align in the media builders, that marker is no longer a
reliable convergence proof. Assert on the stable canonical markers instead —
`data-type="drawio"` + `data-src="/d.drawio"` — which are always materialized
regardless of the align default. The fixpoint assertion (file2 === file1) is
unchanged; round-trip stays byte-stable.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Финальная ступень #351: генератор ЦЕЛЫХ вложенных документов (random walk
по ContentMatch схемы, min-depth fixpoint для терминирования, глубина/размер
ограничены) + инвариант P4 (фазз парсера: для ЛЮБОЙ строки markdownToProseMirror
не бросает и результат валиден по схеме). Инварианты P1 (семантический
round-trip), P2 (байтовый fixpoint со 2-го прохода), P3 (тотальность) — строгие.
Генератор сразу нашёл классы реальных багов конвертера; все починены (инварианты
НЕ ослаблялись — чинился конвертер):
- loose (много-блочные) контейнеры (listItem/taskItem/callout/detailsContent)
склеивали блоки при реимпорте — ТИХАЯ ПОТЕРЯ ДАННЫХ; теперь blank-line
разделитель между блок-детьми (по образцу blockquote).
- соседние sibling-списки одного marker-family (task+bullet и т.п.) сливались
в один список с ПОТЕРЕЙ чекбокса — теперь между ними эмитится инертный
`<!-- -->` разделитель (byte-stable round-trip).
- paragraph textAlign терялся во вложенных li/td/th.
- вложенный codeBlock терял хвостовой перевод строки.
- pageBreak/pageEmbed/subpages/transclusion дропались во вложении
(blockquote/callout/details/li).
- медиа в columns: number→string ширины/высоты и лишний data-align (churn).
- callout `> [!type]`, вложенный в список/цитату, парсился неверно
(prefix-aware regex).
Golden-обновления (6) — прямые следствия loose-container фикса, каждое
round-trip'ится. 2100+ сгенерированных документов (3 seed) — 0 падений P1/P2.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-1 нейтрализация ловила только spaced-разделители (`- - -`),
но пропускала сплошные `---`/`***`/`___`: на git-sync round-trip такая
строка становится horizontalRule, а он не несёт текста — строка терялась
целиком (хуже list/quote-порчи). Символ `_` вообще отсутствовал в regexе.
GITMOST_MD_BLOCK_TRIGGER_RE дополнен альтернативой на целую строку-
thematic-break `([-*_])(?:\s*\1){2,}\s*$` (3+ одинаковых `-`/`*`/`_`,
solid или через пробел); прежние группы переведены в non-capturing,
чтобы `\1` ссылался на единственную capture-группу. Обе копии regexа
(bridge и pm-markdown тест) синхронны.
Тесты: bare `---`/`***`/`___` документированы как text-losing
horizontalRule; ZWSP-нейтрализованная форма round-trip'ится параграфом
с сохранённым текстом. Кейсы падают на старом regexе.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-1 review found two issues:
- [regressions] Inserting a transcript line VERBATIM as a paragraph is unsafe on the
git-sync doc->markdown->doc round-trip: the paragraph serializer emits text with no
block-escape, so a line starting at col 0 with `- `/`> `/`# `/`1. `/```` ``` ````/`|`/
`> [!info]` silently re-parses into a list/quote/heading/code/table/callout (the last
hits the #359 callout machinery). Safe under the host contract (every line prefixed
`You:`/`Speaker N:`, which starts with a letter) but the helper didn't enforce it, and
leading-whitespace / stray lines exist. Fix: trim each kept line (drops the indent leak),
and if it STILL begins with a col-0 markdown block trigger, prepend an invisible
zero-width space (U+200B) so the trigger isn't at col 0 — the round-trip keeps it a
paragraph. (Backslash-escape was rejected: `marked` consumes the `\` on re-import, so it
would render visibly then vanish. ZWSP is invisible and never markdown-escaped.) The
serializer's missing block-escape is the pre-existing root cause; this is the boundary
defense. Prefixed transcript lines never match the trigger regex → left byte-exact.
- [test-coverage] The "inserted as TEXT, not HTML/markdown" contract wasn't locked (all
test strings were plain alphabet). Added a test that inserts `<b>bold</b>
<script>alert(1)</script> and *stars* and [link](x)` (with Bold/Italic/Link marks
registered so an insertContent(html) regression would parse them) and asserts one
verbatim text node, no marks, and getHTML() has no live tags.
Verified: apps/client tsc --noEmit 0 errors; gitmost bridge tests 4 passed; a NEW
prosemirror-markdown round-trip test (real convertProseMirrorToMarkdown ->
markdownToProseMirror) proves bare triggers corrupt while the ZWSP-neutralized form stays
a single byte-preserved paragraph and normal You:/Speaker N: lines round-trip byte-exact —
3 passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F1 [WARNING] The 'no invisible coverage hole' guard enumerated only
schema.nodes, so MARK attributes silently escaped the value-fuzz completeness
check — link.internal/target/rel/class are never fuzzed and nothing flagged it,
and a new attributed mark would slip through. Added allSchemaMarkAttrKeys() plus a
MARK_ATTR_FUZZED / MARK_ATTR_ALLOWLIST registry and two tests: every schema mark
attr must be in exactly one set (a new one turns it red), and neither set may hold
a stale row.
F2 [WARNING] The ACCEPTED annotation misclassified table colspan/rowspan as
having 'no md representation'. They DO round-trip — a spanned cell makes the
converter emit the whole table as a raw <table> with colspan/rowspan, which the
tiptap parser reads back. They are frozen only because generating a
geometrically-valid spanned table is deferred PR-2 structural work (the flat
generator hardcodes span = 1), not a markdown limit. Reclassified them as
DEFERRED-BUG (distinct from ACCEPTED) so a maintainer does not read them as an
inherent limitation; colwidth / backgroundColor(Name) stay ACCEPTED (the
raw-<table> fallback drops them).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Schema-derived, property-based (fast-check) round-trip tests over flat
single-node ProseMirror documents. One test PR — src/ is untouched; the two
real bugs found are pinned as loud it.fails counterexamples, not fixed here.
- attr-arbitraries.ts: per-attribute four-state arbitraries (absent/default/
nonDefault/degenerate), attribute list sourced from schema.nodes[t].spec.attrs;
a documented override table supplies legal domains for constrained attrs and
distinguishes two frozen classes explicitly — ACCEPTED limitations (no md
representation) vs PINNED bugs (representable but dropped, tracked as
counterexamples).
- text-arbitraries.ts: hostile text corpus (ported from the existing property
test's supported-space guarantees).
- node-generators.ts: flat single-node generators + a completeness contract —
every one of the schema's 45 nodes / 12 marks is either generated or listed in
KNOWN_UNCOVERED with a reason.
- flat-roundtrip.property.test.ts: P1 (semantic round-trip via
docsCanonicallyEqual), P2 (second-pass byte fixpoint — anti GS-EDIT-REVERT),
P3 (totality), generator validity via schema.check(), and an explicit
attribute-value-coverage snapshot so the not-fuzzed set can never grow silently.
- counterexamples: column.width (% dropped on parseFloat -> P2 churn) and
orderedList.start (non-1 start renders as '1.' -> P1 loss) pinned as it.fails.
SEED=20250705, NUM_RUNS=300 per property; ~17s, no OOM (union arbitraries).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- F1: added in-app execute tests for the two wirings that ACTUALLY changed in the
migration (the contract-parity test only checks advertised schema keys, not
execute bodies): movePage forwards the newly-added optional `position` to
client.movePage (and passes undefined position + null parent when omitted); the
table trio (insert/delete/updateCell) forwards the unified `table` param
positionally. A field destructured under the wrong name would have silently
passed undefined to the client (execute is any-cast, tsc won't catch it).
- F2: rewrote the three migrated descriptions that hardcoded snake_case sibling
tool names (which the in-app camelCase layer exposes under different ids,
violating the registry's own transport-neutral-prose convention) into neutral
prose: getPage "use get_page_json" -> "use the lossless page-JSON read tool";
updatePageJson "get_page_json -> ... -> update_page_json" -> "read the page-JSON
view -> modify -> write it back", "prefer rename_page" -> "prefer the rename-page
tool"; exportPageMarkdown "import_page_markdown round-trip" -> "page-Markdown
import round-trip" (the last was a direct regress — the in-app base said the
camelCase importPageMarkdown). (stashPage's pre-existing get_page_json mention is
out of scope, per the reviewer.)
Gate: mcp build 0; ai-chat-tools.service + tool-tiers (catalog-partition) pass,
incl. the 5 new execute-wiring tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrates share_page / sharePage into the transport-agnostic spec registry
(schema + description declared once; each transport keeps only its execute/auth):
- sharePage (deferred) -> SHARED_TOOL_SPECS; index.ts uses registerShared(),
ai-chat uses sharedTool(); removed from INLINE_TOOL_TIERS.
Drift reconciled (documented inline): both inline copies already carried the
"only share when the user explicitly asked" security framing, so the old
"per-transport divergence" note in BOTH layers was STALE — there was no real
behavioral divergence, only wording drift. The canonical description merges the
MCP copy's URL-format + idempotency detail with the in-app copy's reversibility
note and keeps the shared security framing. pageId keeps the MCP copy's stricter
.min(1). The MCP execute keeps its own `searchIndexing ?? true` default
(per-layer, not part of the shared schema).
Intentionally NOT migrated (kept inline — genuinely divergent, as their existing
notes state):
- search / searchPages: the in-app tool is a semantic+keyword hybrid (RRF) with
in-process access control and a tuned schema (limit 1-20); the MCP `search` is
a plain REST full-text search (limit up to 100). Different behavior AND schema.
- docmost_transform / transformPage: the in-app tool deliberately omits the
`deleteComments` schema field (a comment-deletion guardrail) and carries a
shorter description. Different schema.
Gate: mcp build 0 + node --test 458/458 (page-search excluded — hangs only under
the local re2->RegExp type-shim, its source untouched), server jest 775 incl.
tool-tiers catalog-partition + shared-spec contract parity, server tsc 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The media tools — insert_image, replace_image, insert_footnote — are MCP-only
by design: the in-app AI-chat agent exposes no image or footnote tools, so there
is no second layer to unify into SHARED_TOOL_SPECS. A registry spec's
tier/catalogLine are in-app metadata and the catalog-partition test forbids a
spec without a live in-app tool, so forcing them into the registry would break
the invariant. They stay per-transport (inline in index.ts).
No behavior change — documentation only (adds the rationale above each tool so a
future migrator does not re-investigate why these are not shared).
Gate: mcp tsc 0 (comment-only change).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrates the three-layer page tools into the transport-agnostic spec registry
(schema + description declared once; each transport keeps only its execute/auth):
- getPage, listPages (core), createPage, movePage, renamePage, deletePage,
updatePageJson, exportPageMarkdown (deferred) -> SHARED_TOOL_SPECS; index.ts
uses registerShared(), ai-chat uses sharedTool(); removed from
INLINE_TOOL_TIERS. Tiers preserved from CORE_TOOL_KEYS (getPage/listPages =
core, the rest deferred).
delete_page is genuinely three-layer (in-app deletePage exists), so it IS
migrated — not MCP-only. Its H4 guardrail is preserved: the shared schema
exposes ONLY pageId, so no permanentlyDelete/forceDelete flag can reach the
client (still asserted by ai-chat-tools.service.spec.ts).
Descriptions merged (documented inline): each canonical text takes the MCP
copy's richer structural notes plus the in-app copy's reversibility framing.
Schema DRIFT reconciled (documented inline):
- createPage.content: MCP pinned .min(1) but the in-app copy left it unbounded
and DOCUMENTS an empty body as valid ("may be empty" — creating an empty page
to fill later is a real use). Kept the looser no-min form: create_page now also
accepts an empty body (harmless) and no previously-valid in-app input is
rejected. title/spaceId keep the MCP .min(1) (empty is never valid).
- movePage: MCP exposed an optional `position` (fractional-index) field the
in-app copy lacked. Unified by KEEPING position — the in-app client already
accepts an optional position arg, so the in-app execute now forwards it;
optional, so no previously-valid call is rejected. `parentPageId` is nullable
on both (real JSON null -> root); the MCP execute keeps its 'null'/'' string
coercion as a per-layer robustness fallback.
- getPage/renamePage/updatePageJson/exportPageMarkdown/listPages: kept the MCP
copy's stricter .min(1) on ids where the in-app copy was unbounded.
Per-transport execute logic preserved: getPage's {title,markdown} projection,
updatePageJson's JSON-string normalization, list_pages' default limit/tree, and
move_page's cycle guard + positive-confirmation check all stay in their execute
bodies.
Intentionally NOT touched: updatePageContent (Markdown-based body update; no MCP
equivalent) and getTable (name-convention divergence, see tables family) stay
inline.
Gate: mcp build 0 + node --test 458/458 (page-search excluded — hangs only under
the local re2->RegExp type-shim, its source untouched), server jest 770 incl.
tool-tiers catalog-partition + shared-spec contract parity + deletePage H4
guardrail, server tsc 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrates the three-layer table WRITE tools into the transport-agnostic spec
registry (schema + description declared once; each transport keeps only its
execute/auth):
- tableInsertRow, tableDeleteRow, tableUpdateCell -> SHARED_TOOL_SPECS;
index.ts uses registerShared(), ai-chat uses sharedTool(); removed from
INLINE_TOOL_TIERS (all three are deferred; not in CORE_TOOL_KEYS).
Drift reconciled (documented inline): the four table tools previously carried a
"NOT shared" note in both layers over a single parameter-NAME drift — the MCP
layer named the table reference `table`, the in-app layer `tableRef`. Unified on
the MCP name `table` (renaming the public MCP parameter would break external MCP
clients; the in-app parameter is model-facing/prompt-only and safe to rename).
The in-app execute bodies now destructure `table`. Descriptions took the MCP
copy's richer wording (documents `#<index>`, padding, header-row behavior) plus
the in-app copy's "Reversible via page history" note; both fields keep the MCP
copy's stricter .min(1) (in-app left them unbounded); sibling tool references
phrased transport-neutrally.
Intentionally NOT migrated (kept inline): table_get / getTable. Its MCP tool
name is noun-first (`table_get`) while the in-app key is verb-first (`getTable`),
which breaks the snake_case(inAppKey) naming convention the registry enforces
(shared-tool-specs.contract.spec.ts). Renaming the public MCP tool would break
external clients, so it stays per-transport — but its in-app reference param was
still aligned to `table` (was `tableRef`) for consistency with the migrated trio.
Gate: mcp tsc 0 + node --test 458/458 (page-search excluded — hangs only under
the local re2->RegExp type-shim, its source is untouched), server jest 730 incl.
tool-tiers catalog-partition + shared-spec contract parity, server tsc 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrates the three-layer comment tools into the single transport-agnostic spec
registry (schema + model-facing description declared once; each transport keeps
only its execute/auth):
- createComment, listComments, resolveComment, checkNewComments — moved to
SHARED_TOOL_SPECS; index.ts uses registerShared(), ai-chat uses sharedTool();
removed from INLINE_TOOL_TIERS (tier/catalogLine now on the spec). Tiers
preserved from CORE_TOOL_KEYS (create/list/resolve = core, check = deferred).
Intentionally NOT migrated (kept MCP-inline): update_comment / delete_comment —
they are MCP-only by design; the in-app AI-chat layer deliberately has no
updateComment/deleteComment (comment edits are irreversible / not
version-tracked), asserted by ai-chat-tools.service.spec.ts. A registry spec's
tier/catalogLine are in-app metadata and the catalog-partition test forbids a
deferred spec without a live in-app tool, so these stay per-transport.
Drift reconciled (documented inline): createComment/listComments/checkNewComments
took the more-maintained/superset description + stricter .min(1) guards.
resolveComment: `resolved` drifted (MCP optional+default(true) vs in-app
required) — kept the MCP superset, so in-app resolveComment now accepts an
omitted `resolved` (defaults to resolve) — a deliberate, backward-compatible
unification (never rejects a previously-valid input).
Gate: mcp build 0 + node --test 480/480, ai-chat 654, tool-tiers (incl. F3
catalog-partition) 16/16, server tsc 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #333 deliberately changed the canonical markdown export of callout
nodes to the Obsidian-native format ('> [!type]' + blockquote body,
pinned by packages/prosemirror-markdown unit tests); the importer still
parses both ':::type' fences and '> [!type]'. The get_page e2e assertion
was missed in that switch and still expected ':::info', failing the
e2e-mcp job on develop since 4369bbc5.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reviewer addition to the round-trip stability matrix: besides "attr absent" and
"attr has a real value", a string attr in the empty-string class has a third,
degenerate state — a LITERAL "" (a user types alt/title/name in the editor then
deletes it, and Tiptap persists `attr: ""`, distinct from never-set). The fix's
`getAttribute(...) || null` coercion normalizes such a stored "" to the default
on the FIRST round-trip (a one-time "" -> null diff) and is byte-stable from the
SECOND round-trip on.
Adds a convergence contract to the reusable matrix helper (emptyStringClass flag
+ runConvergenceCase): pass 1 must converge the attr to its schema default (NOT
asserted byte-stable vs the "" input — that is the intended one-time
normalization); pass 2 must deep-equal pass 1 (idempotent thereafter). Driven for
every empty-string-class attr across image + the media family (image/drawio
alt+title, video alt via aria-label, pdf/attachment name, attachment mime).
Documents the one-time normalization so a future sync/QA diff does not flag the
single "" -> null change as converter corruption.
Gate: package suite 33 files / 682 tests passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A stored image authored without `alt` gained a phantom `alt: ""` on every
round-trip (`markdownToProseMirror(convertProseMirrorToMarkdown(doc))`): `marked`
renders `` as `<img alt="">`, and the stock tiptap Image `alt` parseHTML
(`getAttribute("alt")`) materialized the empty string where the original had no
attribute. That false diff is a real GS-EDIT-REVERT churn source — an agent /
git-sync touch of a page with an image mutates the stored JSON (`absent -> ""`),
producing phantom diffs that can overwrite live edits.
Fix is PARSE-SIDE ("" ≡ absent), so the RAW round-trip is idempotent — not only
the canonical form (history / stored JSON diff on the raw shape; masking it only
in canonicalize would leave that noise). `image.alt`/`title` parseHTML now coerce
`getAttribute(...) || null`, plus defense-in-depth `|| null` across the at-risk
empty-string class (video aria-label, drawio/excalidraw title+alt, pdf name,
attachment name+mime) matching the existing `image.caption || null` precedent.
NOTE — image `align` is NOT changed: it round-trips correctly (center via the
schema default "center", left/right via the `<!--img {...}-->` comment). Its
`toBeUndefined()` in the git-sync gate is canonical-form normalization, not a loss.
Intentional divergence from editor-ext: editor-ext's literal `alt` parseHTML
returns "" verbatim, but this coercion CONVERGES on editor-ext's real STORED
shape (an image inserted without alt has no `alt` attribute -> re-parses absent,
never ""), so the round-trip is idempotent and matches real documents.
Adds a reusable, node-agnostic round-trip-stability matrix helper
(test/roundtrip-stability.helper.ts) — given a node + attr spec it enumerates
default/non-default combos and asserts byte-stability of BOTH the raw and the
canonical round-trip (the documented numeric width/height→string coercion encoded
as an explicit allowed normalization) — driven over image + the whole media
family (video/audio/pdf/attachment/embed/drawio/excalidraw). The only raw
empty-string instability it found was image.alt; the family was already stable.
Gate: package suite 33 files / 672 tests passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The in-app AI agent shipped all ~41 tool schemas on every model step. This
adds a two-tier catalog: core tools (frequent or one-line) stay always-active;
the rest are advertised as a compact catalog and their full schema is fetched
on demand via the loadTools meta-tool, wired through ai@6 prepareStep's
per-step activeTools.
- tools/tool-tiers.ts: CORE_TOOL_KEYS, INLINE_TOOL_TIERS, applyLoadTools,
catalog builders (+ tool-tiers.spec.ts, 13 cases).
- ai-chat.service.ts prepareAgentStep: returns activeTools =
[...CORE_TOOL_KEYS, loadTools, ...activatedTools]; per-turn activated Set.
- ai-chat.prompt.ts: buildToolCatalogBlock renders the deferred catalog.
- mcp/tool-specs.ts: tier + catalogLine metadata (external snake_case /mcp
transport unchanged).
- EnvironmentService.isAiChatDeferredToolsEnabled(): AI_CHAT_DEFERRED_TOOLS,
default ON per issue intent (kill-switch =false restores old behavior).
Gate: server ai-chat 631/631, tool-tiers 13/13, mcp 472/472, tsc clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The RE2 swap narrowed the contract: regex:true rejects lookaround ((?=…)/(?<=…))
and backreferences (\1). The internal JSDoc was updated, but the AGENT-VISIBLE
tool-spec (the only text the agent reads at call time, single-sourced to both
transports) still said 'a JS regular expression' — so an agent would write a
lookahead/backref and hit an error. Updated the .description and the regex flag
.describe() to name RE2 (linear-time, ReDoS-safe), list that char classes / word
boundaries / anchors / quantifiers work while lookaround and backreferences do
NOT, and keep the 'invalid/unsupported regex -> clear error' note.
mcp: tsc clean; tool-specs / server-instructions / contract tests green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Maintainer decision on the escalated ReDoS fork: use re2. The regex path
compiled agent-supplied patterns with `new RegExp` and ran them synchronously in
the shared event-loop; a catastrophic-backtracking pattern (e.g. `(a+)+$`) hung
the whole Node backend for all users (the tool is in both transports incl. the
in-app apps/server agent), and size caps do NOT bound backtracking.
Switch the regex engine to re2 (Google RE2, linear-time, no backtracking):
- `new RE2(query, caseSensitive?'g':'gi')`. RE2 extends RegExp, so eachMatch and
the zero-length-match lastIndex guard are unchanged.
- Unsupported patterns are now a CLEAN error, not a hang: RE2 throws on invalid
syntax AND on the backtracking-only features it can't do (lookaround
(?=…)/(?<=…), backreferences \1) — caught at compile and returned as a clear
tool error telling the agent to rewrite without them.
- Removed MAX_CONTAINER_TEXT + the per-container slice (re2 is linear, so it's no
longer a ReDoS defense, and truncating risked silently dropping real matches in
a long container); kept MAX_PATTERN_LENGTH as a cheap query sanity cap.
- Verified: `(a+)+$` over 50k `a` completes in ~4ms; lookaround/backref throw.
- Added re2 (^1.21.0) to packages/mcp; lockfile updated.
Reviewer DO items:
- F1 [doc]: removed the false "pass nodeId as a comment anchor" claim
(create_comment has no nodeId param — it needs a text `selection`). Fixed in
tool-specs.ts + page-search.ts (module + SearchMatch JSDoc) + client.ts; the ref
is for get_node/patch_node, and for a comment you build a unique text selection
from before+match+after.
- F2 [doc]: clarified `#<index>` refs (id-less table/cell) are accepted by get_node
but NOT patch_node (id-only).
- F3 [test]: round-trip — each match's nodeId fed to the real getNodeByRef
(attrs.id node + `#<index>` table-cell) to prove the ref format is consumable.
- F4 [test]: before/after edge-pinning (match in first 40 chars of a long
container; index 0 → before==""; container end → after=="").
- New re2 tests: catastrophic patterns complete fast; lookaround/backref → error.
mcp: tsc clean; node --test 472 passed (+5). apps/server: tsc --noEmit clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The listComments Comment[] -> { items, resolvedThreadsHidden } shape change
reached every src/host consumer but not the live-server e2e harness (run via
`node test-e2e.mjs`, not the node --test gate — so the green suite missed it).
The 4 calls now read .items; the post-resolve check passes includeResolved:true
so it still sees the now-resolved root c1 (the default feed hides it).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Editorial roles (Corrector/Factchecker) brute-forced `get_node` block-by-block to
find occurrences (unquoted «ё», straight quotes, «т.е.»), burning tokens. New
`search_in_page(pageId, query, {regex?, caseSensitive?, limit?})` reads the page's
ProseMirror JSON via the existing getPageRaw and searches it IN MEMORY — no server
endpoint, no DB/schema change, no touch to the packages/mcp/src/lib schema mirror.
New pure `searchInDoc(doc, query, opts)` (packages/mcp/src/lib/page-search.ts):
recursive descent to each TEXT CONTAINER (paragraph/heading/table-cell paragraph),
glues its inline text via `blockPlainText` (a match survives inline-mark
boundaries — e.g. «т.е.» split across bold/italic), searches literal (indexOf) or
regex, and returns `{ total, truncated, matches:[{ nodeId, blockIndex, type,
before, match, after }] }`. `nodeId` is the container's attrs.id or the
`#<topLevelIndex>` of the enclosing top-level block — the SAME ref format
get_node/patch_node/comment-anchoring accept (verified identical to getNodeByRef),
so the agent goes straight from a hit to a targeted comment; `before`/`after` are
~40-char windows for a unique selection. `total`/`truncated` always reported (never
silent truncation). Lives in the SHARED_TOOL_SPECS registry → exposed in BOTH
transports (external /mcp + in-app AI-chat), with a SERVER_INSTRUCTIONS line and a
DocmostClientLike signature + contract-test entry. Corrector/Factchecker prompts
get a one-line "use search_in_page first" hint (versions bumped, catalog hash lock
refreshed).
Guards: empty/whitespace query → clear error; invalid regex → clear error (not a
generic 500); zero-length regex matches (`\b`, `a*`) skipped with lastIndex
advanced (no loop/flood); MAX_PATTERN_LENGTH=1000, MAX_CONTAINER_TEXT=100k bound
each exec; limit clamped [1,200] (default 50).
Tests: new page-search.test.mjs (17) — literal+regex, case-sensitivity,
mark-boundary glue, nodeId for paragraph/heading (attrs.id) and table-cell
(#<index> fallback), context bounds, limit/total/truncated + clamp, invalid
regex/empty/over-long errors, zero-length skip, empty-doc null-safety.
mcp: tsc clean; node --test 467 passed (+17). apps/server: tsc --noEmit clean
(DocmostClientLike + wiring). catalog check.mjs OK.
Known limitations (from internal review, non-blocking):
- Residual ReDoS: a crafted catastrophic-backtracking pattern (e.g. `(a+)+$`)
against a large single container can hang the event loop — JS regex is not
interruptible, so the length caps bound the base but not the backtracking.
Realistic exposure is low (containers are small; the pattern is supplied by the
authenticated model). Candidate for a follow-up hardening (safe-regex validation
or a worker+timeout) if it matters.
- Case-insensitive LITERAL search folds via toLowerCase; a char whose lowercase
differs in length (e.g. Turkish İ) BEFORE a match could shift the context
window — negligible for the RU/EN editorial scenario.
- On a `#<index>` table-cell fallback, `type` is the inline container ("paragraph")
while nodeId addresses the top-level block — addressing is correct; the field is
documented as the container's type.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The AI agent (MCP + in-app chat) saw ALL comments incl. resolved via two
channels, cluttering its context and breaking fragment search. Default now:
the agent sees only ACTIVE discussions; resolved is opt-in. Active anchors and
threads are always kept.
Channel 1 — resolved comment anchors on agent reads (converter option):
`convertProseMirrorToMarkdown(content, options?)` gains
`options.dropResolvedCommentAnchors` (default false — zero change for every
existing caller incl. git-sync). Both `case "comment"` emitters (top-level and
the raw-HTML inlineToHtml path) emit BARE text (no `<span data-comment-id>`) when
`resolved && the flag`; active anchors keep their wrapper. mcp `getPage` passes
the flag; `export_page_markdown` does NOT (lossless export must preserve resolved
anchors — that is why it is an opt-in option, not unconditional); `get_page_json`
is untouched (lossless PM JSON). Built on the #293 package converter.
Channel 2 — `list_comments` default active-only: `listComments(pageId,
includeResolved=false)` now returns `{ items, resolvedThreadsHidden }` (was a
bare array). By default a RESOLVED top-level thread is hidden wholesale — the
root AND every reply anchored to it (a thread is gated only by its root's
resolvedAt; a resolved reply under an ACTIVE root stays). `resolvedThreadsHidden`
counts hidden threads so the agent knows to re-query. `includeResolved:true`
returns everything. The `includeResolved` param is added to both tool
registrations (MCP index.ts + in-app ai-chat-tools.service.ts); `DocmostClientLike`
signature updated. Server `findPageComments` is NOT touched — the web UI's tabs
depend on the full feed; filtering is only at the mcp-client level. All internal
call sites (export_page_markdown / checkNewComments / transformPage) updated to
`.items` with `includeResolved:true` to keep their full-feed behavior.
The comment model is assumed FLAT (a reply's parentCommentId points at the
thread root) — documented in the filter; a future reply-of-reply model would
need a root-walk there.
Tests: resolved-comment-anchors.test.ts (6 — anchor dropped with flag / kept
without, for BOTH emitters; active always kept); list-comments-resolved.test.mjs
(4 — resolved thread+reply hidden + counter; includeResolved:true returns all;
an ACTIVE thread with a RESOLVED reply is NOT hidden).
package vitest: 664 passed; tsc clean. mcp: node --test 458 passed; tsc clean.
apps/server + git-sync: tsc clean (converter option default-off).
NOTE: based on feat/293-B (#293/#326 STEP 5) — the converter lives in the
package; this PR is stacked on #333 and its base retargets to develop once #333
merges. mcp/build is gitignored (not committed).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F1 (round 1) wrapped the image alt in escapeLinkText, and that helper also guards
the link-form media captions (attachment/pdf/embed). But its character class
covered only stock CommonMark — NOT the Docmost inline EXTENSIONS this same PR
registers on the marked instance: highlight `==x==` (canon #7), math `$x$`
(canon #6), footnote `^[x]` (canon #2). Their triggers `= $ ^` are not CommonMark
punctuation, so an alt or media filename like `x $A$ y`, `use ==bold==`, `^[fn]`,
or `data $A$.csv` was silently turned into a math/highlight/footnote node on
import — the same class of round-trip data loss F1 closed, reintroduced by this
PR's own canon.
Fix: add `= $ ^` to the escapeLinkText class (`/[\\`*_~[\]<&!()=$^]/g`). `\= \$ \^`
decode back to literals (all ASCII punctuation) AND, being escape tokens, stop
the extension tokenizer from matching — verified lossless byte-stable round-trip.
Updated the helper comment to name the two trigger sets (CommonMark + Docmost
inline extensions). Extended the adversarial round-trip tests: image alt gains
`x $A$ y` / `5$ and 10$` / `use ==bold==` / `^[fn]` / `cost $5 == price`; pdf name
gains `data $A$.csv` / `q3 ==final==.pdf` / `5$ and 10$.pdf` / `note ^[x].pdf` —
all byte-stable with the node intact, so the hole can't reopen.
package vitest: 658 passed; tsc clean. git-sync: 268. mcp: 454.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tail of #244. Three items:
1. Coverage-gate (main). develop had no coverage tooling at all. Added
@vitest/coverage-v8@4.1.6 (pinned to the vitest already in use) to the three
vitest packages — git-sync, editor-ext (which also gains its missing direct
`vitest` devDep), apps/client — and enabled v8 coverage with per-package
thresholds (no root vitest config exists, so per-package is the only
meaningful scope). v8 provider is chosen deliberately: istanbul broke on the
ESM `@docmost/editor-ext` barrel; v8 collects native runtime coverage and
never re-parses ESM. `enabled: true` wires the gate into the plain `test`
script, so `pnpm -r test` (the CI entrypoint) enforces it without a manual
`--coverage`. Thresholds set ~4-5 pts below measured current coverage so the
gate PASSES today and FAILS on regression (verified: forcing lines=95 on
editor-ext exits 1). `all: false` — coverage counts test-touched files;
documented in the configs (with `all: true` the many untested type/barrel
files would sink the % and make the gate meaningless).
Measured→threshold (S/B/F/L): git-sync 91.78/79.16/76.76/92.46 → 88/75/72/88;
editor-ext 58.58/48.1/64.96/58.91 → 54/44/60/54; client 59.93/58/48.47/59.39
→ 55/53/44/55. All exit 0.
2. acceptInvitation atomicity int-spec. New
apps/server/test/integration/workspace-accept-invitation-atomicity.int-spec.ts
(+ createDefaultGroup/createInvitation seeders in test/integration/db.ts per
its convention). Wires the real WorkspaceInvitationService with real
User/Group/GroupUser repos against the test Kysely, stubbing only the
post-commit collaborators. Asserts the invariant protected by
users_email_workspace_id_unique: (a) two CONCURRENT accepts → exactly one
fulfilled, one BadRequestException('Invitation already accepted'), membership
count == 1, invitation consumed; (b) repeated sequential accept → still one
membership; (c) the survivor is in the workspace default group (whole-tx, no
torn state). Ran against real Postgres+Redis: 3/3 pass.
3. turn-end decision unit test. `decideTurnEnd` does not exist as a symbol; the
turn-end logic lives in chat-thread.tsx's onFinish handler. Added a focused
block to the existing chat-thread.test.tsx (matching its hoisted-mock style):
clean finish → flush queued (continue); abort/disconnect/error → queue
preserved (end) with the correct notice; parent notified on every terminal
outcome. 8 passed (3 existing + 5 new).
Verified: git-sync 712, editor-ext 247, client 888 (all with the gate, exit 0);
int-spec 3/3 (real Postgres); tsc --noEmit clean for client + server;
pnpm install --frozen-lockfile consistent (lockfile additive).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F1 [critical, data-loss] — escape the image alt in ``. Canon #4 moved
the top-level image off the lossless <img> form onto markdown ``, but
the alt was inserted raw; the importer re-parses the `![alt]` label as CommonMark
inline, so a markdown-active char in a realistic description ("Figure [1]", "the
*new* logo", "a]b[c") broke the round-trip — the image node vanished or emphasis
collapsed. Now `escapeLinkText(imgAttrs.alt ?? "")`, exactly as the link-form
media (attachment/pdf/embed) already escape their visible text. Regression test
added: six active-punctuation alts round-trip byte-stable with the node intact.
F2 [drift] — re-export `clampCalloutType` / `sanitizeCssColor` from the package
barrel and drop the verbatim copies in the mcp schema shim. The copies had
already drifted (the mcp `clampCalloutType` lost the callout-type alias mapping
the package applies), which is exactly the schema drift #293 exists to kill. The
sanitizers now live only in the package; mcp `schema.test.mjs` exercises the
single alias-aware implementation.
F3 [docs] — AGENTS.md:296 said `packages/mcp/build/` is committed; this branch
gitignored it (git-sync/prosemirror-markdown convention). Updated the line to say
it is gitignored and rebuilt in CI/Docker via `pnpm build`.
F4 [cleanup] — removed the dead `test.typecheck` block from the package
vitest.config.ts and deleted tsconfig.vitest.json. Both were copied verbatim from
git-sync; this package has zero `*.test-d.ts` files, and the ported comments
referenced git-sync-only entities. Kept the `docmost-client` resolve alias
(22 tests use it) and the runtime include/environment.
package vitest: 658 passed (+1 F1 regression); tsc clean. git-sync: 268 passed.
mcp: node --test 454 passed; tsc clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mcp had its OWN drifted copy of the converter (markdown-converter.ts ~900 lines,
docmost-schema.ts ~1270 lines, markdown-document.ts) — older than the shared
package, missing the git-sync fixes AND the #293 canon. This switches mcp's
converter CORE to @docmost/prosemirror-markdown, so mcp jumps straight to the
canonical format and the drift-generating second copy is gone.
- markdown-converter.ts / markdown-document.ts / docmost-schema.ts become thin
re-export shims of the package (convertProseMirrorToMarkdown, the docmost:meta
envelope, docmostExtensions + docmostSchema=getSchema(docmostExtensions)). The
mcp-only helpers clampCalloutType/sanitizeCssColor are preserved verbatim in
the schema shim (the package doesn't expose them via its barrel). ~2170 lines
of the drifted converter/schema bodies deleted.
- collaboration.ts drops its own ~360-line marked pipeline (preprocessCallouts,
bridgeTaskLists, extractFootnotes, the footnoteRef extension) and re-points to
the package's markdownToProseMirror, keeping markdownToProseMirrorCanonical and
all the yjs/collab write glue. footnote-lex/analyze doc comments updated (they
now describe advisory legacy-syntax diagnostics, not an importer).
Schema parity verified: the package schema is a strict SUPERSET of mcp's old
schema — every node and attr mcp declared is present (the package only adds
status/pageEmbed/transclusion*/subpages.recursive/etc.), so nothing is silently
dropped on the switch. The switch actually FIXES two pre-existing mcp data-loss
bugs its own tests documented: htmlEmbed and pageBreak now round-trip (were
dropped by the old mcp converter).
Footnotes: the package assembles inline ^[body] footnotes on import (sequential
fn-N ids, identical bodies merged), so mcp's canonicalizeFootnotes is now an
idempotent no-op after it (verified). Legacy reference footnotes [^id]/[^id]:
are inert literal text (canon #2 no-backward-compat) — lossless, the text
survives verbatim.
Build hygiene: packages/mcp/build/ is now gitignored and untracked, matching the
git-sync/prosemirror-markdown convention (private package, rebuilt in CI/Docker,
so src and prod can never silently diverge). This also removes a dead untracked
build/_vendored_editor_ext/ artifact that a broad `git add` would otherwise
commit.
Dependency: packages/mcp/package.json gains @docmost/prosemirror-markdown
(workspace:*); pnpm-lock.yaml gets the matching link importer (mirrors git-sync).
mcp tests updated deliberately to the canonical forms (highlight ==, math $…$,
image <!--img-->, drawio/media discriminators, subpages/pageBreak
comments, textAlign, inline ^[…] footnotes) with strict assertions; 4 structural
safety-net round-trip tests added.
mcp: node --test 454 passed; tsc clean. package: 657 passed. git-sync: 268 passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The four bugs found during the #293 HTML-emission inventory, fixed in the package:
1. Spoiler mark was silently lost in the raw-HTML path: inlineToHtml (columns /
spanned cells) had no `case "spoiler"`, so spoilered text there dropped the
mark on round-trip. Now emits `<span data-spoiler="true">` — the same form the
top-level serializer uses and exactly what the schema's Spoiler mark parses.
2. Link `title` was dropped in the raw-HTML path: inlineToHtml's link case
emitted `<a href>` without the title. The schema's link mark carries a
`title` global attr (DocmostAttributes), so a titled link inside a column now
round-trips via `<a href … title=…>`.
3. Serializer contract test: emoji/date/toc were flagged as possibly caseless
inline atoms. Verified they exist in NEITHER the package schema NOR
editor-ext, so no node handling is needed today. Added
serializer-contract.test.ts, which derives every node type from the live
schema (getSchema(docmostExtensions)) and asserts each has an explicit
serializer `case` — all 45 current node types are covered and present, and a
future node added without a case will fail this test loudly.
4. codeCombined dead code: `const codeCombined = false` was hardcoded, so every
`codeCombined ? <html> : <markdown>` ternary always took the markdown branch.
Removed the variable and the dead HTML-alternative branches (bold/italic/code/
link/strike). Pure cleanup — output is byte-identical (goldens + full suite
pass unchanged). The `hasCode` early-return (code excludes other marks) stays.
Tests: spoiler-inside-column and link-title-inside-column round-trips, the
serializer contract test + inline-atom non-empty behavioral checks.
package vitest: 657 passed; tsc clean. git-sync: 268 passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_coder
agent_vscode
vvzvlad
agent_coder
claude code agent 227