diff --git a/apps/server/src/core/ai-chat/tools/parse-node-arg.ts b/apps/server/src/core/ai-chat/tools/parse-node-arg.ts index 6f43625d..e4495c45 100644 --- a/apps/server/src/core/ai-chat/tools/parse-node-arg.ts +++ b/apps/server/src/core/ai-chat/tools/parse-node-arg.ts @@ -3,8 +3,10 @@ // invalid JSON), pass an object through unchanged. Shared by patchNode / // insertNode (and the analogous updatePageJson content parsing). // -// This mirrors `packages/mcp/src/lib/parse-node-arg.ts` byte-for-byte. We -// cannot import that helper here: `@docmost/mcp` is ESM-only and this server +// This is behaviorally identical to `packages/mcp/src/lib/parse-node-arg.ts` +// (the function logic, default/explicit throw messages and branch order match; +// only comments and quote style differ). We cannot import that helper here: +// `@docmost/mcp` is ESM-only and this server // compiles with module:commonjs, so it is loaded at runtime via the // `new Function('import()')` trick (see docmost-client.loader.ts). Sharing // runtime code across that ESM/CJS boundary by a normal import is impossible, diff --git a/docs/backlog/ai-chat-tool-definitions-duplicated.md b/docs/backlog/ai-chat-tool-definitions-duplicated.md new file mode 100644 index 00000000..444628e8 --- /dev/null +++ b/docs/backlog/ai-chat-tool-definitions-duplicated.md @@ -0,0 +1,127 @@ +# Дублирование определений инструментов: in-app агент vs standalone MCP-пакет + +Статус: **частично закрыто.** Квирк «node как объект ИЛИ JSON-строка» вынесен +в общий хелпер `parseNodeArg` (см. «Прогресс» ниже); остальной долг (единый +реестр спеков + унификация конвертера) всё ещё открыт. Это forward-looking +стоимость поддержки, НЕ баг — код корректен сегодня. Держим запись открытой, +чтобы при росте набора инструментов долг не разъезжался молча. + +## Прогресс + +- ✅ **Квирк node-arg вынесен в хелпер** (`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 + инструментов при точных типах. Делать инкрементально. +- ⏳ **Унификация конвертера ProseMirror ↔ Markdown** — открыта (см. раздел + «Расширение …» ниже); на неё опирается план git-синка + (`docs/git-sync-plan.md`). + +## Суть + +Один и тот же набор инструментов поверх одного `DocmostClient` описан +**тремя независимыми рукописными слоями**. Каждое добавление инструмента или +правка его model-facing описания требует синхронной правки в 2–3 местах, а +parity-баги (расхождение копий) приходится чинить/переоткрывать дважды. + +## Где дублируется (три слоя) + +1. **Standalone MCP-сервер** — `packages/mcp/src/index.ts` (~38 `registerTool`). + Для внешних MCP-клиентов (stdio/http). На каждый инструмент: zod-схема + + длинное model-facing описание + тонкий `execute`, вызывающий `DocmostClient`. +2. **Встроенный AI-чат** — `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()`). +3. **Ручная копия сигнатур** — интерфейс `DocmostClientLike` в + `apps/server/src/core/ai-chat/tools/docmost-client.loader.ts:9` (в комментарии + прямо: «Signatures here mirror that file exactly»), скопирован руками из + `packages/mcp/src/client.ts`. + +## Что именно продублировано (с подтверждением по коду) + +- **zod-схема + описание** каждого инструмента — в слоях 1 и 2 целиком. +- ~~**Квирк «node как объект ИЛИ JSON-строка»** реализован дважды (НЕ в общем + клиенте)~~ — **закрыто (PR #114):** вынесен в `parseNodeArg` (по хелперу на + пакет), 6 inline-копий устранены: + - in-app: `patchNode`, `insertNode`, `updatePageJson` → + `apps/server/src/core/ai-chat/tools/parse-node-arg.ts`; + - standalone: `patch_node`, `insert_node`, `update_page_json` → + `packages/mcp/src/lib/parse-node-arg.ts`. +- **Guardrail/семантика `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, а не +только метаданных инструментов. + +- **Копия в gitmost** — `packages/mcp/src/lib/`: `markdown-converter.ts` (~885 + строк), `markdown-document.ts` (~136), `node-ops.ts`, `diff.ts`, + `docmost-schema.ts`. Канонизатора (`canonicalize.ts`) здесь НЕТ. +- **Копия в docmost-sync** — `packages/docmost-client/src/lib/`: тот же набор + + `canonicalize.ts` (~11 КБ, держит идемпотентность round-trip, SPEC §11) + + `markdown-document.ts` с режимом «тело + якоря, без тредов комментов» + (`includeCommentThreads:false`, на ~20 строк больше). +- **Третья копия (планируется)** — план git-синка вендорит чистую часть + конвертера в новый `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`-SDK `tool()` и MCP `registerTool()` имеют разную + форму, поэтому реестр экспортирует транспорт-агностичные `{name, schema, + description}`, а каждая сторона оборачивает их сама. `zod` — общая зависимость + обоих пакетов, типы переносятся.