applyElkLayout крутит elkjs СИНХРОННО в процессе на mxGraph-XML от LLM
(layout:'elk' в drawio_create/update) без лимита размера и без таймаута —
большой граф (тысячи узлов, ~1МБ XML проходит stage-1 cap 16МБ) блокирует
event-loop MCP-сервера на секунды-минуты. try/catch ловил только брошенную
ошибку, но не зависание.
- кап ДО построения графа: >500 узлов или >1000 рёбер → вернуть исходный XML
(best-effort, как существующий catch); синхронный elkjs → кап и есть
реальная защита;
- Promise.race с 5s-таймаутом (defense-in-depth на случай async-elkjs); таймер
гасится в finally → нет утечки хендла и unhandled-rejection (проигравший
timeout остаётся pending с погашенным таймером);
- тест: 600-узловой граф возвращается без изменений и быстро (<2s) — кап-путь.
79/79 drawio-тестов зелёные.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Стадия-1 (#434) уже была довяжена in-app в develop (f46d89ea, agent_vscode)
для CRUD-тулов; два новых чистых read-only хелпера стадии-2 остались
незаброшенными → contract-parity спека падала 6 ассертами (по 3 на
drawio_shapes/drawio_guide). В отличие от CRUD-тулов это ЧИСТЫЕ функции без
сетевого вызова, поэтому НЕ client-методы:
- реэкспорт searchShapes / getGuideSection (+ тип SearchShapesOptions) из
entry пакета @docmost/mcp; loadDocmostMcp() пробрасывает их так же, как
sharedToolSpecs (типы SearchShapesFn/GetGuideSectionFn);
- две записи sharedTool(...) в forUser() после drawioUpdate: drawioShapes
повторяет серверный вызов searchShapes(query,{category,limit}) и форму
{ query, count, results }; drawioGuide — getGuideSection(section)
(omit section -> index); голый объект без jsonContent-envelope, как у
соседних in-app хендлеров;
- DocmostClientLike и HOST_CONTRACT_METHODS-вайтлист НЕ тронуты (это не
методы клиента);
- три тест-мока (contract/service/tool-tiers) получили type-only no-op
заглушки под расширенный тип loadDocmostMcp() — тела инструментов в этих
тестах не исполняются, contract-спека реально гоняет настоящий
SHARED_TOOL_SPECS.
Внутреннее ревью обвязки: APPROVE, 0 находок. shared-tool-specs.contract:
211/211 (было 6 падений); client-host-contract drift-guard 3/0; tsc EXIT 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ядро Фазы 1б. Реестр (#294) шарил только метаданные (имя/схема/описание/tier),
но НЕ execute-логику — у каждого shared-тула было ДВА рукописных execute-тела
с копией маппинга аргументов (MCP registerShared в index.ts; in-app sharedTool
в ai-chat-tools.service, зеркалящий MCP-транспорт вручную). Корень
повторяющихся parity-багов (f46d89ea drawio, f8d26420 stashPage, fc9088b7
node-args): добавление одного тула = 7-9 согласованных ручных правок в двух
пакетах.
- SharedToolSpec расширен: канонический execute(client, args) (чистый JS —
свободно пересекает zod-мажорную границу v3/v4) + оверрайды
mcpExecute/inAppExecute/mcpOnly/inAppOnly для ОСОЗНАННЫХ per-layer различий.
client: DocmostClientLike (Pick из #446). Канон возвращает СЫРЬЁ, каждый хост
накладывает свой конверт (MCP jsonContent, in-app как есть); override владеет
результатом хоста целиком.
- Оба хоста → циклы по Object.values(SHARED_TOOL_SPECS): index.ts registerShared
39→0 (цикл), ai-chat-tools.service sharedTool ~40→1 (цикл). Добавление спека
автоматически регистрирует тул в ОБОИХ хостах — сценарий PR #434 невозможен
по построению.
- Осознанные различия через overrides (ни одно не сплющено к одному хосту):
оба mcpExecute+inAppExecute — createPage/movePage/deletePage/
exportPageMarkdown/createComment (guardrails, конверты, проекции, тексты
ошибок); execute+inAppExecute — getPage/renamePage/resolveComment;
execute+mcpExecute — stashPage (resource_link+structuredContent),
checkNewComments (since-guard только на MCP).
- Оставлены inline (по делу): update_comment/delete_comment (MCP-only, in-app
не даёт хард-правку/удаление комментов), search/transformPage (per-transport
дивергенция — hybrid RRF / без deleteComments), table_get (noun-vs-verb
naming clash — уедет после camelCase #412), getCurrentPage/updatePageContent/
listSidebarPages/getComment/getPageHistory (in-app-only, per-request state).
- Guard-тесты (contract-parity, phantom-catalog) сохранены — теперь инварианты,
не «последняя линия».
Внутреннее ревью: APPROVE, построчная BEFORE/AFTER-сверка по каждому shared-тулу
на обоих хостах — ни одного изменённого per-host поведения (порядок/дефолты
аргументов, guard'ы, конверты, проекции сохранены), множества тулов побайтово
совпадают (48 in-app, 45 MCP), кросс-zod-граница чистая (нет z. в execute),
611 mcp + 260 server тестов зелёные. Ядро Фазы 1б, стоит на #446 — мержить после.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Восстановленный отложенный долг #294: @docmost/mcp не отдавал .d.ts, поэтому в
сервере жили ТРИ дрейфующие ручные копии одних и тех же имён/сигнатур
(DocmostClientLike ~230 строк, копия SharedToolSpec, name-only HOST_CONTRACT_
METHODS-тест). In-app execute-тела зовут клиент ПОЗИЦИОННО, так что перестановка
параметра в client.ts доезжала до прода рантайм-ошибкой без сигнала на компиляции.
- declaration:true (+declarationMap) в packages/mcp/tsconfig.json; types-экспорт
в package.json (exports → conditional {types, default} для . и ./http;
require.resolve/dynamic-import резолвят default → build/index.js, рантайм не
тронут). build/index.d.ts эмитится, реэкспортит DocmostClient + SharedToolSpec.
Правок исходников пакета для эмита НЕ потребовалось.
- DocmostClientLike → Pick<DocmostClient, 48 методов> из type-only import
(стёрт на компиляции, ESM/CJS-границу не задевает); ручное зеркало удалено.
- SharedToolSpec → type-only реэкспорт из пакета; ручная копия удалена.
- client-host-contract.test.mjs удалён целиком — имена И сигнатуры теперь
проверяет tsc.
- Позиционная безопасность: never-called __assertClientCallContract(client:
DocmostClientLike) воспроизводит каждый позиционный вызов с типизированными
плейсхолдерами (AI-SDK стирает вход execute-замыканий в any, иначе позиционные
вызовы не проверялись). Перестановка параметров client.ts → ошибка компиляции
сервера ровно тут. Loose as-касты в ai-chat-tools.service не потребовали
правок; as any не добавлялся.
Внутреннее ревью: APPROVE. Runtime resolution через conditional exports не сломан
(разобрано для прод-инсталляции, не только symlink); покрытие
__assertClientCallContract полное (48 call-sites == union == assert, сверено
программно); Pick полон; демонстрация reorder → TS2345 в assert. Единственная
находка (Promise<any> в части возвратов) предсуществующая в client.ts, вне
цели PR. Стоит на #447 (закрытие skew build/vs/src) — мержить после него.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Два структурных слепых пятна: (1) сервер грузит СКОМПИЛИРОВАННЫЙ
packages/mcp/build/index.js, а серверные guard-тесты читают src/tool-specs.ts —
правка src без пересборки оставляет тесты зелёными, но рантайм расходится со
спеками; (2) спеки добавляют в packages/mcp, а parity-тесты живут в jest-сьюте
apps/server — PR, трогающий только пакет, проходит зелёным, сломанная in-app-
проводка всплывает уже на develop (кейс f46d89ea).
- REGISTRY_STAMP: codegen (scripts/gen-registry-stamp.mjs) считает sha256 от
нормализованного (CRLF→LF, один хвостовой \n снят) сырого текста
src/tool-specs.ts, пишет src/registry-stamp.generated.ts (gitignored),
index.ts реэкспортит → попадает в build/. Вшит в build/pretest/watch ДО tsc.
- Loader (dev/test): computeSrcRegistryStamp пересчитывает стамп из src рядом с
build/index.js (dev-vs-prod по existsSync, любая ошибка → null), сверяет с
build-стампом → при рассинхроне бросает «build is stale — rebuild». В prod
(src нет) и на pre-#447 билдах (нет REGISTRY_STAMP) — чистый no-op.
- CI: job mcp-server-parity собирает shared-deps+mcp (регенерит стамп) и гоняет
ОБА сьюта вместе (mcp node:test + server guard-спеки) — именованный гейт, его
нельзя случайно расщепить.
- AGENTS.md: правка спеков требует ребилда @docmost/mcp.
Тесты (20): mcp-сайд (детерминизм, нормализация, desync-гард стамп-vs-билд) +
server-сайд (null при отсутствии src = prod no-op; mismatch → throw точного
сообщения; pre-#447 no-op). Кросс-импл equality-гард: один фиксированный вход →
один хэш на ОБЕИХ сторонах, ловит рассинхрон двух нормализаций. Внутреннее
ревью: APPROVE WITH SUGGESTIONS (обе — покрытие guard'а — закрыты этим тестом).
Мутационно: любой из двух normalize-имплов расходится → equality-тест краснеет.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Третий класс петли (инцидент 2026-07-10): ран упёрся в 20-шаговый кап, спалив
все шаги на чтение; на 20-м шаге final-step lockdown отнял инструменты
(toolChoice:'none') посреди незаконченной работы → модель выродилась в
текст-повтор («loadTools.» ×20416, 255КБ). Пакет защит по дизайну владельца:
- MAX_AGENT_STEPS 20→50; спеки выводятся из константы (нет захардкоженных 19/20).
- Final-step lockdown под env-тогглом AI_CHAT_FINAL_STEP_LOCKDOWN (дефолт OFF,
по образцу AI_CHAT_DEFERRED_TOOLS): OFF — инструменты доступны на всех шагах +
мягкий финальный нудж; ON — легаси toolChoice:'none'+FINAL_STEP_INSTRUCTION.
Спеки параметризованы по тогглу. .env.example задокументирован.
- Пустой ход (все шаги без текста, шаги исчерпаны) получает синтетический
маркер-текст — виден в UI и реплее.
- Детектор токен-деградации в onChunk (единственная защита от болтовни, БЕЗ
maxOutputTokens — tool-аргументы это выходные токены): чистые правила
(≥25 одинаковых строк ИЛИ периодический хвост), при срабатывании abort через
внутренний AbortController ∪ effectiveSignal (AbortSignal.any), финализация в
onAbort: усечение хвоста, ai_chat_runs.error=Output degeneration detected,
лизы MCP/снапшоты освобождаются (существующий lifecycle).
- Предупреждение о бюджете шагов на MAX-6…MAX-2 с убывающим N.
- loadTools-описание и преамбула каталога явно говорят, что CORE-тулы всегда
активны (список из CORE_TOOL_KEYS динамически).
Внутренний цикл: 2 прохода. Ревью нашло data-loss-риск: правило периодичности
детектора ложно срабатывало на markdown-разделителях/setext-подчёркиваниях/
хвостовых пробелах (монохар-хвост p-периодичен при ЛЮБОМ p → ложный abort с
пометкой error и усечением). Починка: отдельная монохар-проверка (порог 60,
выше любого реального разделителя) + требование ≥2 различных символов в
периодическом блоке при p≥2. Реальный loadTools-цикл (период ~10) ловится.
Мутационно: 59 одинаковых — не флаг, 60 — флаг; loadTools×20416 — флаг.
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>
Аудит при подготовке #413 нашёл дрейфующие дубли между packages/mcp и
packages/prosemirror-markdown. Четыре дедупа (поведение тулов не меняется):
1. node-ops: форк ~960 строк сведён в ОДНУ копию в prosemirror-markdown (живая
mcp-версия — строгое надмножество замороженного #293-seed'а пакета; сверено по
git-истории, новая пакет-копия байт-в-байт == прежней mcp-копии). Barrel-экспорт
полной поверхности; mcp/client.ts/page-search.ts/transforms.ts/collaboration.ts
импортируют из пакета; тесты переехали. node-ops тянет stripInlineMarkdown ->
пакет-локальная text-normalize.ts несёт только этот примитив (mcp-версия —
домен #408; заголовок документирует дубликацию + источник истины).
2. footnote-lex/footnote-analyze (vestigial legacy [^id]: диагностика): сведены к
одному fence-aware предупреждению 'reference-style footnotes -> use ^[...]'
(полезно для класса #410); footnote-lex удалён.
3. footnote-authoring -> примитивы (footnoteContentKey/makeFootnoteDefinition/
generateFootnoteId) перенесены в пакетный footnote.ts, одна реализация конвенции.
4. parse-node-arg -> перенесён в prosemirror-markdown (не mcp: сервер CommonJS не
импортирует ESM-only @docmost/mcp, но нативно импортирует пакет), обе копии
удалены, консьюмеры перенаправлены.
canonicalizeFootnotes/ENFORCEMENT RULE #228 и comment-anchor/json-edit/text-normalize
(mcp) не тронуты. API-поверхность node-ops оставлена чистой для #409/#413.
closes#414
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
wrapToolsWithCommentSignal всегда ставил свой toModelOutput, молча выбрасывая
собственный toModelOutput инструмента (латентная ловушка — будущий тул со своим
toModelOutput тихо сломался бы). Теперь база = origToModelOutput(info) при наличии,
иначе воспроизведённый дефолт SDK; no-signal путь возвращает базу дословно, signal-
путь = части базы (modelOutputToParts: text/json/content) + элемент сигнала последним.
execute по-прежнему возвращает СЫРОЙ результат -> part.output/цитаты байт-идентичны.
Дефолтный путь (единственный исполняемый сегодня) байт-идентичен и SDK-дефолту, и
до-фиксовому signal-пути (проверено повторным ревью). json-ветку загардил ?? null
для симметрии с fallback. +2 теста: тул со своим text/content toModelOutput —
база честно сохраняется и в no-signal, и в signal (сигнал добавлен последним).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Комментарии человека находят агента сами: короткая эфемерная строка
'new comments: N on page … — call listComments(pageId)' в результате ЛЮБОГО
tool-вызова, mid-turn. Общий хелпер packages/mcp/src/comment-signal.ts
(createCommentSignalTracker: watermark + per-page debounce + working-set;
buildCommentSignalLine; defangCommentSignalTitle).
- Standalone MCP (index.ts): второй wrapper в choke point registerTool (паттерн
метрик #402) — отдельный {type:'text'} content-элемент, форма результата не
меняется. Источник: rate-limited listComments по working-set, title через
getPageRaw только на hit. State per-session.
- In-app (ai-chat-tools.service.ts): execute ВСЕГДА возвращает сырой результат
(part.output/цитаты не трогаются), сигнал доставляется модели через отдельный
toModelOutput ({type:'content', value:[raw, signal]}) — зеркало MCP; no-signal
ветка точно воспроизводит дефолт SDK. Источник: REST-probe (осознанный форк от
DB-count из ТЗ — чтобы не менять конструктор сервиса и не ломать спеки).
- Инъекционная защита: в сигнал идут только count+pageId+defanged-title, НИКОГДА
текст комментария (untrusted). Per-page watermark (не глобальный) — комментарии
на второй странице не теряются.
closes#417
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>
Snapshot the editor selection at send time (same live-ref pattern as
openPageRef in prepareSendMessagesRequest), carry it nested inside
openPage so it dies with the page on a fail-closed resolve, and surface
it to the model only through the existing core getCurrentPage tool.
The selection TEXT is returned exclusively in the tool result (untrusted
collaborative-page content, treated as data by SAFETY_FRAMEWORK); the
system prompt gets only a fixed one-line flag, never the text/before/
after. sanitizeSelection caps text (4000), before/after (200), blockIds
(<=64 chars, <=20). Selection is a hint, not ground truth — the tool
description tells the agent to localize the fragment before editing.
closes#388
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>
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>
- F1: document AI_CHAT_DEFERRED_TOOLS in .env.example (AI_* section) — default
ON = deferred loading (compact catalog + loadTools), =false restores the old
"all tools always active" behavior.
- F2: integration test of the ON path in ai-chat-stream.int-spec.ts — a deferred
tool activated via loadTools is active on the SAME turn's next step but a fresh
turn starts cold (CORE + loadTools only), proving the per-turn activatedTools
Set does not leak across turns/chats. Drives the real streamText loop with a
MockLanguageModelV3 and inspects recorded per-step activeTools-filtered tools.
- F3: replace the magic toHaveLength(28) in tool-tiers.spec.ts with a two-way
partition against the LIVE in-app toolset (AiChatToolsService.forUser keys):
every non-core tool must appear in buildInAppDeferredCatalog and every catalog
entry must map to a real non-core tool — so a future tool forgotten in
INLINE_TOOL_TIERS fails the suite instead of silently vanishing from the agent.
No production logic change (mechanism was already reviewed correct).
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>
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>
Audit of all 41 tool descriptions against the actual implementation found
factually wrong or misleading texts:
- list_comments claimed '(paginated)' — it takes only pageId and returns ALL
comments in one call (internal pagination); now also states that RESOLVED
threads are included and how to filter them. In-app twin synced.
- search claimed the limit default is 'applied by the client' — the client
deliberately omits it so the SERVER applies its default.
- create_page's '(automatically moves it to the correct hierarchy)' said
nothing useful — now documents parentPageId nesting semantics; move_page
drops the stale 'essential for organizing pages created via create_page'.
- share_page now warns the page becomes accessible to ANYONE with the URL.
- get_page (both transports) now explains inline <span data-comment-id> tags
are comment anchors (incl. resolved) — markup, not page text.
- patch_node/delete_node/insert_node pointed only at the expensive page-JSON
view for block ids — now route through the cheap page outline first.
- docmost_transform marks 'Примечания переводчика' as the DEFAULT
notesHeading, overridable for non-Russian pages.
Checks: @docmost/mcp tests 450/450 (incl. the server-instructions guard);
server ai-chat-tools spec 20/20; mcp build/ artifacts rebuilt.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Agents can attach a suggested replacement when creating an inline comment, via
both the MCP create_comment tool and the AI-chat createComment tool.
Because applying a suggestion edits the EXACT anchored text, an ambiguous anchor
would let Apply corrupt the wrong occurrence. So when suggestedText is set the
selection must occur EXACTLY ONCE:
- new countAnchorMatches(doc, selection) counts occurrences across all blocks
(same normalization/traversal as canAnchorInDoc), counting occurrences (2 in
one block => 2) — stricter than block-count, never under-counting distinct
occurrences (false-unique is the dangerous direction).
- client.createComment gains suggestedText: a pre-check (getPageJson +
countAnchorMatches: 0 => not-found, >=2 => ambiguity error) before create, and
an AUTHORITATIVE live check inside the anchoring mutation that recomputes on the
live doc and, if != 1, aborts and rolls back the just-created comment (reusing
the existing safeDeleteComment "anchor not found" path). Ordinary comments keep
first-occurrence behavior unchanged.
- suggestedText is rejected on a reply or without selection in all three layers
(MCP handler, MCP client, AI-chat tool), mirroring the server DTO/service.
- filterComment surfaces suggestedText/suggestionAppliedAt/suggestionAppliedById.
- DocmostClientLike.createComment signature updated. MCP build/ rebuilt.
Tests: countAnchorMatches (0/1/N, within/across/nested block, span nodes,
quote normalization); createComment (ambiguous refused pre-create, reply and
no-selection rejected, unique succeeds and forwards suggestedText, filterComment
surfaces it); ai-chat schema accepts suggestedText. MCP 443 pass; ai-chat 601 pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The same tool metadata (zod schema + model-facing description) was hand-duplicated
between the standalone MCP server and the in-app AI-chat agent, so every tweak had to
land in two places and copies drifted (a materialized parity bug). The shared
transport-agnostic registry (packages/mcp/src/tool-specs.ts) already de-duplicates 14
tools; this migrates two more genuinely-identical ones — patch_node/patchNode and
insert_node/insertNode. The canonical description is a strict SUPERSET of both originals
(keeps MCP's "without resending the whole document" + table-structure/anchor guidance
AND the in-app "reversible via page history" / "exactly one of anchorNodeId or
anchorText" framing — no model-facing guidance dropped); the schema is identical (the
in-app side just gains MCP's .min(1) on ids, a safe tightening). Each transport keeps its
own execute/auth wrapper, and the in-app parseNodeArg node-arg normalization is unchanged.
The three table tools are intentionally NOT merged (a real param-name divergence:
table vs tableRef) — documented on both sides. Other per-transport divergences
(search/share/create_comment/transform/list_pages) are left separate with a short comment
explaining why (the issue asked to flag these as intentional). DocmostClientLike stays a
hand-mirror (the ESM/CJS boundary blocks a compile-time type import; a runtime drift-guard
already pins it). Also fixes a latent contract-spec bug: derive `required` from
`instanceof z.ZodOptional` (matches the emitted JSON schema) instead of `isOptional()`,
which wrongly reported z.any() fields as optional.
Partially addresses #294.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The agent rebuilds context from DB each turn and didn't know the user manually
edited the open page since its last response, so it could overwrite those edits.
Add a per-turn ephemeral <page_changed> note in the system prompt (twin of
INTERRUPT_NOTE, self-clearing) carrying a unified Markdown diff of what changed
since the END of the agent's previous turn.
- New ai_chat_page_snapshots table (migration + hand-declared db.d.ts/entity
types) storing the page Markdown per (chat,page) at each turn's end.
- Pure computePageChange util (whitespace-normalized unified diff via the
existing jsdiff dep, 6KB cap + getPage hint).
- Turn start: if the open page's updatedAt moved past the snapshot, diff current
vs snapshot; non-empty -> PAGE_CHANGED_NOTE in the safety sandwich.
- Turn end: upsert the snapshot on EVERY terminal path (onFinish/onError/onAbort,
once) so the agent's own edits are excluded by construction even on aborted
turns.
All best-effort (never breaks/latency-regresses a turn); fast path when updatedAt
is unchanged. Server-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
guardedFetch blocks loopback/private/link-local/metadata IPs and never calls
fetch; decryptHeaders fails open (returns undefined, warns once, no blob leak).
yjs.util setYjsMark/removeYjsMarkByAttribute/updateYjsMarkAttribute on real
Y.Docs. SHARED_TOOL_SPECS<->in-app parity (name/desc/input-schema; a dropped or
renamed wiring fails). Replace the tautological storage.service spec with
driver-delegation checks across every public method.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The blob-sandbox feature (#243/#250) made AiChatToolsService.forUser()
eagerly call this.sandboxStore.asSink() while wiring the stash tool, but
the spec still passed an empty {} as the sandboxStore constructor arg.
That object has no asSink method, so all 19 tests in the suite failed in
CI with 'TypeError: this.sandboxStore.asSink is not a function'.
Replace the stale {} mock at all 4 constructor sites with a no-op sink
exposing asSink() -> { put, has, evict } (jest.fn()). These tests never
execute the stash tool, so a no-op sink is sufficient for forUser() to
wire successfully. Test-only change; production code is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Security (must-fix):
- sandbox.controller: the anonymous GET /api/sb/:id response now sets
X-Content-Type-Options: nosniff, a restrictive CSP, and Content-Disposition=
attachment for any mime outside a raster-image allowlist (png/jpeg/gif/webp/
avif). entry.mime is attacker-controlled, so an evil.svg/evil.html could
otherwise execute script inline on the Docmost origin (stored XSS). Mirrors
the public attachment route's hardening.
Stability:
- client.stashPage: reconcile mirrors AFTER the final document put, not only
before it. The doc blob is the newest entry and FIFO eviction drops the
oldest = this stash's own images, so the stored doc could reference an
evicted blob (consumer 404) and over-report images.mirrored. A bounded loop
now reverts doc-put-evicted mirrors, drops the stale doc blob, and re-puts
until stable. Regenerated packages/mcp/build/.
- sandbox.controller: emit Cache-Control on the 304 branch too (ttlSeconds is
computed before the conditional check).
Docs:
- Bump the MCP tool count 39 -> 40 across all READMEs and AGENTS.md (the
registry now exposes exactly 40 tools).
Refactor:
- SandboxStore.asSink() centralizes the {put,has,evict} sink + uri<->id
mapping; the embedded-MCP and in-app agent-tools wiring sites share it.
Tests:
- security headers (inline vs attachment, nosniff, CSP), 304 Cache-Control,
putAndLink URL form, has()/remove(), asSink() round-trip, getSandboxPublicUrl
(trailing-slash trim + APP_URL fallback), and a stash test where the doc put
itself evicts a mirrored image.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Security:
- stash_page: reject path-traversal / percent-encoded srcs before the authed
loopback fetch (resolveInternalFilePath), closing an SSRF/exfiltration hole
where a crafted node.attrs.src could read an arbitrary internal GET endpoint
into the anonymous sandbox.
Stability:
- stash_page: revert + recount mirrors FIFO-evicted by a later put in the same
stash (no dangling sandbox refs, honest images.mirrored/failed); free image
blobs if the final document put throws.
- Reject/clamp non-positive SANDBOX_TTL_MS to the 1h default (warn once).
- Log mirror failures unconditionally (console.warn, no blob bodies).
Cleanup / architecture:
- Remove dead expiresAt from SandboxPutResult.
- Centralize the /api/sb route in SANDBOX_ROUTE_SEGMENT/SANDBOX_API_PATH and
move URL composition into SandboxStore.putAndLink; drop the duplicated sink
closures and the now-unused EnvironmentService injection from McpService and
AiChatToolsService.
- Un-export isInternalFileUrl; document the process-local (instance-bound)
sandbox limitation in the tool description and .env.example.
Docs/tests:
- README/README.ru: 38 -> 39 tools + stash_page entry.
- Add traversal/normalize/recursion unit tests, stash self-eviction +
doc-put-throw + empty/octet-stream mock tests, controller If-None-Match
(wildcard/weak/list) + Cache-Control tests, and SANDBOX_TTL_MS validation
tests. Regenerate packages/mcp/build.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an ephemeral, process-local blob store so the in-app agent (and the
embedded MCP) can hand a large page document and its images to an external
consumer WITHOUT routing the bytes through the model context or Docmost auth.
- SandboxStore (@Injectable singleton): Map<uuid,{buf,mime,sha256,expiresAt}>
in RAM only. put() picks a per-blob cap by mime (image vs doc), enforces a
total-bytes RAM guard with oldest-first eviction, and stamps a TTL; get()
lazily expires. sha256 computed at put() doubles as the strong ETag. An
unref'd sweep interval clears expired entries and is cleared on destroy.
- GET /api/sb/:uuid anonymous controller: serves raw bytes with Content-Type,
Content-Length and ETag=sha256; 404 on missing/expired/non-UUID (anti-
traversal), 304 on a matching If-None-Match. No tokens, no 401 — the
capability is the unguessable UUID + short TTL + TLS. Auth-exempt the same
way as /api/files/public (no JwtAuthGuard) plus an /api/sb entry in main.ts's
workspace-resolution preHandler so a remote consumer with no workspace host
is not rejected.
- stash_page tool in both layers (MCP resource_link + in-app {uri,size,sha256,
images}). client.stashPage serializes the get_page_json shape, mirrors every
INTERNAL file/image src (type-agnostic, covers drawio/excalidraw/video/file)
into the sandbox under Docmost auth and rewrites src to the sandbox URL;
external http(s) srcs are left untouched; dedup by src; a failed image fetch
is counted, never aborts the doc.
- SANDBOX_PUBLIC_URL / SANDBOX_TTL_MS / SANDBOX_MAX_BYTES /
SANDBOX_MAX_IMAGE_BYTES / SANDBOX_MAX_TOTAL_BYTES wired through the
environment service + validation + .env.example.
- SandboxModule (@Global) provides the shared store to the controller,
McpService and AiChatToolsService (same instance for put and get).
Tests: SandboxStore (round-trip, sha256, TTL lazy + sweep, caps, eviction),
SandboxController (200+ETag+CT+CL, 404 missing/expired/non-UUID, 304), and a
mock-HTTP stashPage test (mirror+rewrite internal, keep external, dedup, failed
image counted, returns only a link). Interoperates with the vvzvlad/habr-mcp
consumer's anonymous-GET + sha256-ETag + resource_link contract.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Issue #193's tool-half has two open items. The shared, zod-agnostic tool-spec
registry (SHARED_TOOL_SPECS) for the identical tools is already merged
(f3fa15e7) and consumed by both layers, so that subset is done. The remaining
items are: (a) deriving the layer-3 hand-mirror `DocmostClientLike` from the
real client type, and (b) folding more tools into the registry. Both were
deferred as risky, and that deferral still holds (verified, see below) — so
this change ships the safest concrete increment instead of forcing the risk.
What this adds (behaviour-neutral, test-only + a doc comment):
- packages/mcp/test/unit/client-host-contract.test.mjs: pins the layer-3
contract from the ESM side, where the real DocmostClient is importable. It
asserts every method the in-app `DocmostClientLike` mirror declares exists as
a function on a real DocmostClient instance (constructor is side-effect-free).
A rename/removal in client.ts now fails this test instead of silently shipping
a runtime "x is not a function" into an agent tool call. Negative-case
verified (a bogus method name is detected).
- docmost-client.loader.ts: replaces the vague mirror comment with a pointer to
the guard test and a concrete, empirically-grounded staged plan for the full
type-derivation. Verified blockers kept it deferred: @docmost/mcp emits no
.d.ts (no `declaration`, no `types` export) and the server has no path mapping
for it, so there is no type to import today; and the real methods' inferred
CONCRETE return types conflict with the in-app adapter's loose
Record<string,unknown> + `as`-cast result handling (deriving the exact type
breaks the build / forces pervasive double-casts and full-surface test stubs).
Out of scope (noted in the issue): the PM<->Markdown converter unification.
Verified: server tsc clean; mcp tsc clean; mcp tests 369 pass (367 + 2 new);
ai-chat tools specs 51 pass. No behaviour change; committed mcp build untouched
(no mcp src changed).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In-app AI-chat tools used bare zod schemas, so when the model dropped a
required arg (typically pageId) in a parallel/batch tool call, the AI SDK
forwarded zod's raw "expected string, received undefined" text to the model
— not actionable. Add a centralized modelFriendlyInput(shape) wrapper that
keeps the exact JSON Schema contract (required/description/constraints via
z.toJSONSchema draft-7) but replaces the raw zod text with a message naming
each missing/invalid parameter and reminding the model not to drop ids like
pageId in parallel batches. No value is guessed/backfilled (cf. #159).
Applied to every in-app tool: the sharedTool() builder and all inline
inputSchema in ai-chat-tools.service.ts, plus public-share-chat-tools.service.ts.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The in-app AI chat hardcoded type='page' and the shared createComment
swallowed anchoring failures silently, so agent comments never got a
text anchor/highlight.
- Forbid page-type comments for the agent: top-level comments are always
inline and require an exact `selection`; replies inherit the parent
anchor (stored as the historical `page` type).
- Throw and roll back the just-created comment when the selection cannot
be anchored, instead of leaving an orphan unanchored comment.
- Add comment-anchor module: text normalization (smart quotes, dashes,
nbsp, collapsed whitespace) and matching across adjacent text nodes
within a block, so selections crossing inline-code/bold/link anchor.
- Update create_comment (MCP) and createComment (ai-chat) tool schemas
and descriptions; add unit + mock-HTTP orchestration tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Batches 6-9: behaviour-preserving extractions of testable pure cores plus the
tests they unblock, and a fix for the broken client test environment.
Full suites green: server 113 suites / 1117 + 1 todo, client 30 files / 338.
client (R0 infra):
- vitest.setup.ts: in-memory localStorage/sessionStorage Storage stub wired via
setupFiles. Unblocks menu-items.gating.test.ts (was 9 failing) -> client suite
fully green. + menu-items.suggestions.test.ts (getSuggestionItems filter/sort).
share:
- extract buildShareMetaHtml (share-seo.util.ts) from the SEO controller; tests
for reflected-XSS escaping in <title>/og/twitter meta, noindex, truncation;
extractPageSlugId; updateAttachmentAttr; prepareContentForShare comment-strip
(anonymous-viewer metadata-leak guard).
ai-chat (security extractions):
- selectAccessibleHits: CASL post-filter for semantic search (restricted page in
an accessible space must NOT leak to the agent).
- validateResolvedAddresses: SSRF connect-time guard (block if ANY resolved
address is private).
- resolveAudioFormat: mime whitelist (dead `?? 'webm'` fallback dropped, set
unchanged). + mcp-servers toView header-leak guard, MCP tool namespacing.
collaboration (data-loss area):
- extract computeHistoryJob (pins the "agent delay MUST stay 0" invariant) and
resolveSource. Integration: onAuthenticate read-only matrix (collab auth
bypass), HistoryProcessor (contributor restore on save failure), onStoreDocument
Approach-A boundary snapshot (human revision pinned before agent overwrite).
Reviewed (APPROVE WITH SUGGESTIONS): extractions behaviour-preserving, security
tests mutation-resistant.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements two architecture follow-ups from the multi-aspect review.
1. Shared, zod-agnostic tool-spec registry (packages/mcp/src/tool-specs.ts)
for the 14 AI tools whose name + schema + model-facing description are
genuinely identical across the standalone MCP server and the in-app
AI-SDK chat. Both layers consume it (registerShared in index.ts;
sharedTool in ai-chat-tools.service.ts) and keep their own execute/auth.
- Zod-agnostic builders (z) => ZodRawShape bridge the zod v3 (mcp) vs
zod v4 (server) split; the registry imports no zod.
- Folds in the documented edit_page_text drift-bug fix: the stale
"strip-and-retry tolerated" claim is gone; canonical wording states a
formatting-only change is refused into failed[].
- Sibling-tool references in shared descriptions are transport-neutral so
one description is correct for both snake_case (MCP) and camelCase
(in-app) tool names.
- Loader fail-fast guard for a stale @docmost/mcp build.
- The ~17 intentionally-divergent tools (security guardrails, tuned UX)
stay per-layer, untouched.
- Rebuilt committed mcp artifacts (also regenerates a previously stale
build/lib/docmost-schema.js to match its already-committed source).
2. Formalize apps/server/test/integration/db.ts as the canonical
integration-test seed factory (module doc + a shortId helper); the
hand-written minimal seeders are kept on purpose, decoupled from the
app service-layer side effects.
Verified: server tsc + lint clean, mcp build clean; mcp unit tests 261 pass,
ai-chat-tools.service 16 pass, public-share-chat-tools 8 pass, ai-chat suite
224 pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pre-merge review follow-up for the parseNodeArg dedupe (PR #114):
- Restore docs/backlog/ai-chat-tool-definitions-duplicated.md instead of
deleting it: it still tracks open debt (unified spec registry + ProseMirror
<-> Markdown converter unification) that this branch defers, and
docs/git-sync-plan.md links to its converter section. Mark the node-arg
quirk as done and add a Progress section.
- Reword the in-app helper header from "byte-for-byte" to "behaviorally
identical": the two copies differ in comments/quote style; only the logic,
throw messages and branch order match.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First, safe step of docs/backlog/ai-chat-tool-definitions-duplicated.md: the
"node may be a JSON object OR a JSON string" quirk was hand-copied at 6 tool
sites. Extract it into a single parseNodeArg() helper per package and call it at
every site. Behavior-preserving — each site's throw message is byte-identical
(patch/insert: 'node was a string but not valid JSON'; update_page_json: 'content
was a string but not valid JSON'); no tool name/description/schema changed.
Two helper copies (packages/mcp/src/lib/parse-node-arg.ts and
apps/server/src/core/ai-chat/tools/parse-node-arg.ts) are intentional: the
ESM-only @docmost/mcp cannot be imported by the CommonJS server (it is loaded at
runtime via the Function('import()') trick), so runtime code cannot cross that
boundary by a normal import. Each copy is now the single source within its
package (6 inline copies -> 2 helpers). packages/mcp/build rebuilt in sync.
Tests: parse-node-arg.spec.ts (server, Jest) + parse-node-arg.test.mjs (mcp,
node:test) — object passthrough, valid-string parse, invalid-string throw with
the right message. Server tsc clean; mcp suite 254 pass; agent structural-edit
path verified live in-browser (agent inserted a node, persisted to the doc).
Deferred (documented for the record, since the backlog doc is removed with this
commit): the FULL transport-agnostic tool-spec registry (one name+schema+
description per tool shared by both transports) and deriving DocmostClientLike
from the real client type. Both are blocked by the current architecture, not by
effort: (1) @docmost/mcp ships no type declarations and is ESM-only, so a
type-only derivation needs declaration emission + tsconfig path wiring, and the
real client's precise return types break the in-app tool test stubs (attempted,
reverted to keep tsc green); (2) the two transports intentionally DIVERGE in tool
NAMES (snake_case x38 vs camelCase x41), membership (in-app adds getCurrentPage/
listSidebarPages, omits delete_comment/image tools) and model-facing
DESCRIPTIONS, so a unified registry would change behavior on BOTH the agent and
external MCP clients and needs its own verification pass. This is forward-looking
debt (the code is correct today), to be done incrementally.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 'current page' feature (client useMatch openPage + server getCurrentPage
tool + system-prompt injection) was already implemented & merged; this backfills
its missing test coverage and removes the completed backlog doc.
- extract pure resolveCurrentPageResult(openedPage) into current-page.util.ts
(byte-identical to the prior inline getCurrentPage tool body) so it is
unit-testable without the dynamically-imported ESM Docmost client; the tool
now delegates to it.
- current-page.util.spec.ts: 7 cases (null/undefined/no-id/empty-id/full/no-title).
- ai-chat.prompt.spec.ts: +8 cases for the openedPage context line (title+pageId
present, Untitled fallback for blank/whitespace title, no line when absent/blank
id, and sandwich ordering before the trailing safety block).
Verified live in-browser: client sends openPage{id,title} on a page and null
off-page; the agent invokes getCurrentPage and answers with the real title+id.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- ai-chat: drop the unused pagePermissionRepo injection from
PublicShareChatToolsService (its only use moved into
ShareService.resolveReadableSharePage); update all 5 positional
test construction sites to match the 3-arg constructor.
- env: correct the anonymous share-AI per-workspace cap comment —
the limiter FAILS CLOSED on Redis failure (#62), not open.
- docs: sync README.ru.md with README.md — move "Page templates"
from Planned to Done and drop the dead plan-doc link.
Remaining test-coverage gaps tracked as #102, #103, #104, #105, #106.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The '(shareId,pageId) -> usable non-restricted page in THIS share' boundary was
written as 3 must-be-identical async sequences. They weren't: the chat funnel
omitted an explicit page.deletedAt check (latently safe via getShareForPage's
CTE) and layered isSharingAllowed separately. Add ShareService.resolveReadable-
SharePage(shareId,pageId,workspaceId) running the single canonical sequence
(getShareForPage -> id match (skipped when null) -> findById -> !deletedAt ->
!hasRestrictedAncestor) returning {share,page}|null; getSharedPage, the funnel,
and the getSharePage tool all use it. hasRestrictedAncestor now lives in the one
method no caller can skip; the funnel still returns uniform 404s and keeps
isSharingAllowed. Adds a direct security-invariant test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The anonymous share page-fetch tool's positive branch (sanitize via
updatePublicAttachments then jsonToMarkdown before returning to the model) was
untested, so a dropped/reordered sanitizer would ship a comment-mark/raw-
attachment leak with green tests. Add a positive-branch test pinning the
sanitizer call + that markdown derives from sanitized content, and a soft-deleted
test asserting a generic error with no content fetch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add ~330 tests across server (Jest), client (Vitest), editor-ext (Vitest)
and packages/mcp (node:test) for the gitmost features added since
053a9c0d: AI chat, AI agent roles, public-share assistant, MCP per-user
auth, HTML embed, page templates/embed, realtime tree, tree
expand/collapse, and the AI-settings UI.
Test-tooling fixes (prerequisite, were silently hiding coverage):
- Repair 3 page-template specs broken by the 11-arg TransclusionService
constructor; they never compiled, so template access-control / content
-leak / unsync-strip coverage was fictitious.
- Build @docmost/editor-ext before server tests via a `pretest` hook;
the stale dist omitted the new HtmlEmbed/PageEmbed exports (TS2305).
- Let jest resolve the .tsx email templates: add `tsx` to
moduleFileExtensions and widen the ts-jest transform to (t|j)sx?.
Behaviour-preserving "extract pure core" refactors that the tests drive:
- server: resolveShareAssistantRequest + uiMessageTextLength
(public-share controller), decideBasicGate + mapAuthResultToResponse
(mcp), buildErrorAssistantRecord (ai-chat), jsonbObject export (roles).
- client: render-raw-html + shouldExecute/canEdit, decide-embed-state,
page-embed picker utils, tree-socket reducers, open/close branch maps,
isEndpointConfigured/resolveKeyField; buildTreeWithChildren now treats
a permission-trimmed orphan as a root instead of crashing.
Deferred (need a test DB or HTTP harness, documented in the specs):
repo-level Postgres integration tests and the public-share XFF E2E.
Pre-existing DI/lib0-ESM suite failures are untouched and out of scope.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
agent_coder
vvzvlad
agent_vscode
agent_coder
claude code agent 227
agent_coder