[refactor][ai-chat] Дублирование определений инструментов (in-app агент vs standalone MCP) + дублирование конвертера PM↔Markdown #193
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Дублирование определений инструментов: in-app агент vs standalone MCP-пакет
Статус: частично закрыто. Квирк «node как объект ИЛИ JSON-строка» вынесен
в общий хелпер
parseNodeArg(см. «Прогресс» ниже); остальной долг (единыйреестр спеков + унификация конвертера) всё ещё открыт. Это forward-looking
стоимость поддержки, НЕ баг — код корректен сегодня. Держим запись открытой,
чтобы при росте набора инструментов долг не разъезжался молча.
Прогресс
refactor/ai-chat-tool-spec-registry,PR #114). Шесть рукописных копий нормализации «node как объект ИЛИ
JSON-строка» свёрнуты в
parseNodeArg: по одному источнику на пакет —packages/mcp/src/lib/parse-node-arg.ts(standalone) иapps/server/src/core/ai-chat/tools/parse-node-arg.ts(in-app). Две копиинамеренны (ESM/CJS-граница), поведение тождественно.
DocmostClientLikeиз реального типа — отложены (см. «Фикс»): требуютпересечения ESM/CJS-границы для данных+zod и ломают тест-стабы in-app
инструментов при точных типах. Делать инкрементально.
«Расширение …» ниже); на неё опирается план git-синка
(
docs/git-sync-plan.md).Суть
Один и тот же набор инструментов поверх одного
DocmostClientописантремя независимыми рукописными слоями. Каждое добавление инструмента или
правка его model-facing описания требует синхронной правки в 2–3 местах, а
parity-баги (расхождение копий) приходится чинить/переоткрывать дважды.
Где дублируется (три слоя)
packages/mcp/src/index.ts(~38registerTool).Для внешних MCP-клиентов (stdio/http). На каждый инструмент: zod-схема +
длинное model-facing описание + тонкий
execute, вызывающийDocmostClient.apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts(~39
tool({...})черезai-SDK). Своя zod-схема + своё описание + свойexecuteповерх ТОГО ЖЕ клиента (@docmost/mcpгрузится вtools/docmost-client.loader.ts:188через динамическийimport()).DocmostClientLikeвapps/server/src/core/ai-chat/tools/docmost-client.loader.ts:9(в комментариипрямо: «Signatures here mirror that file exactly»), скопирован руками из
packages/mcp/src/client.ts.Что именно продублировано (с подтверждением по коду)
Квирк «node как объект ИЛИ JSON-строка» реализован дважды (НЕ в общем— закрыто (PR #114): вынесен вклиенте)
parseNodeArg(по хелперу напакет), 6 inline-копий устранены:
patchNode,insertNode,updatePageJson→apps/server/src/core/ai-chat/tools/parse-node-arg.ts;patch_node,insert_node,update_page_json→packages/mcp/src/lib/parse-node-arg.ts.transformPage(dryRun) описана в обоих:ai-chat-tools.service.ts:~935иindex.ts:~1006.Почему разделение слоёв 1 и 2 само по себе оправдано
У путей разный транспорт и auth-контекст, и это правильно держать раздельно:
in-app путь чеканит per-user JWT + provenance collab-токен (подписанная
agent-claim,
docmost-client.loader.ts:159—getCollabToken; см. план §6.5),а standalone обслуживает внешних клиентов по stdio/http. Но это оправдывает
два тонких адаптера (
execute+ auth-обвязка), а НЕ две рукописные копииМЕТАДАННЫХ (схема + описание + квирки). Метаданные можно объявить один раз и
переиспользовать обоими транспортами.
Доказательство стоимости (наблюдалось при фиксе edit_page_text)
При исправлении ложного «успеха»
edit_page_text(refuse форматных правок +verify-отчёт):DocmostClient→ автоматически дошло до обоихагентов ОДНОЙ правкой. Это «хороший» случай — логика в едином источнике.
index.ts(кодером) иотдельно в
ai-chat-tools.service.ts:617, где описание продолжало рекламировать«Markdown wrappers tolerated via strip-and-retry» — ровно ту формулировку, что
ввела исходного агента в заблуждение. Копия молча разъехалась и какое-то время
встроенный агент получал устаревшую подсказку. Это и есть материализованный
parity-баг.
Расширение: дублируется не только описания инструментов — ещё и конвертер (PM ↔ Markdown)
Зафиксировано при планировании встраивания git-синка (
docmost-sync→ gitmost,нативная in-process интеграция). Та же болезнь «несколько рукописных копий одного
кода» теперь касается слоя конвертации ProseMirror ↔ Markdown и его lib, а не
только метаданных инструментов.
packages/mcp/src/lib/:markdown-converter.ts(~885строк),
markdown-document.ts(~136),node-ops.ts,diff.ts,docmost-schema.ts. Канонизатора (canonicalize.ts) здесь НЕТ.packages/docmost-client/src/lib/: тот же набор +canonicalize.ts(~11 КБ, держит идемпотентность round-trip, SPEC §11) +markdown-document.tsс режимом «тело + якоря, без тредов комментов»(
includeCommentThreads:false, на ~20 строк больше).конвертера в новый
packages/git-sync(collab-файл не нужен: запись идётнативно через
openDirectConnection+@docmost/editor-ext).Копии уже молча разъехались (docmost-sync vs
packages/mcp):collaboration.ts~329 изменённых строк,
node-ops.ts~53,markdown-converter.ts~24,markdown-document.ts~20. Отдельно:docmost-schema.tsв lib дублируетреальную схему сервера
@docmost/editor-ext(её использует collab/persistence)— расхождение схем = риск битой конвертации нод.
Вывод: тот же фикс-вектор (единый источник правды), что и для инструментов, стоит
распространить на конвертер — общий пакет конвертации, потребляемый
mcp,git-syncи (в идеале) сервером. До конвергенции git-sync держит вендореннуюкопию валидированного конвертера с гейтом round-trip против схемы
editor-ext(осознанный долг «третья копия сейчас, объединяем позже»).
Фикс
Единый реестр спеков (полное устранение дублирования).** Вынести в
packages/mcpодин источник на инструмент:name+ zod-схема + model-facingописание + общий хелпер нормализации node-строки (для patch/insert/update).
И
index.ts, иai-chat-tools.service.tsимпортируют спеки и добавляют толькосвой
execute/auth.DocmostClientLike— выводить из типа реального клиента(type-only import / генерация), а не копировать руками.
@docmost/mcp— ESM-only, сервер грузит его через трюкnew Function('import(specifier)')(docmost-client.loader.ts:174), потомучто
module:commonjsдаунлевелитimport()вrequire(). Реестр спеков(данные + zod) должен пересекать ту же ESM/CJS-границу — выполнимо тем же
динамическим импортом;
ai-SDKtool()и MCPregisterTool()имеют разнуюформу, поэтому реестр экспортирует транспорт-агностичные
{name, schema, description}, а каждая сторона оборачивает их сама.zod— общая зависимостьобоих пакетов, типы переносятся.
сделай блядь все до конца наконец