vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

refactor(ai-chat): shared parseNodeArg helper (dedupe quirk; full registry deferred) #114

Closed
Ghost wants to merge 0 commits from refactor/ai-chat-tool-spec-registry into develop
pull from: refactor/ai-chat-tool-spec-registry
merge into: vvzvlad:develop
Conversation 0 Commits - Files Changed -
Ghost commented 2026-06-21 06:52:03 +03:00
Copy Link

Рефакторинг: устранение дублирования определений инструментов (безопасный шаг)

Отчёт по скиллу архитектора.

1. Задача

Из docs/backlog/ai-chat-tool-definitions-duplicated.md: один и тот же набор инструментов описан рукописно в нескольких местах (standalone MCP packages/mcp/src/index.ts, in-app агент ai-chat-tools.service.ts, плюс ручная копия сигнатур DocmostClientLike). Это forward-looking долг сопровождения (НЕ баг — код корректен сегодня): каждое добавление/правка инструмента требует синхронной правки в 2–3 местах, parity-баги (разъезд копий) приходится чинить дважды.

2. Анализ — что реально достижимо безопасно

Полное устранение (единый transport-agnostic реестр спеков) архитектурно заблокировано, и это подтверждено фактами:

  • @docmost/mcp — ESM-only и НЕ отдаёт .d.ts (билд только JS), а сервер на module:commonjs физически не может его import-ить (грузит в рантайме трюком Function('import()')). Значит рантайм-код нельзя разделить через границу обычным импортом, а type-only вывод требует эмита деклараций + правки tsconfig-paths.
  • Два транспорта намеренно расходятся: имена инструментов (snake_case ×38 vs camelCase ×41), состав (in-app добавляет getCurrentPage/listSidebarPages, не отдаёт delete_comment/image-инструменты) и model-facing описания. Единый реестр поменял бы поведение И агента, И внешних MCP-клиентов — это отдельная фича со своей верификацией.

Поэтому в этом PR — безопасный, поведенчески-нейтральный шаг, который убирает конкретный «материализованный parity-баг», на который указывает доку.

3. Решение

Квирк «node как объект ИЛИ JSON-строка» был рукописно скопирован в 6 точках (3 в standalone, 3 in-app). Вынес его в один хелпер parseNodeArg() на пакет:

  • packages/mcp/src/lib/parse-node-arg.ts (standalone: patch_node, insert_node, update_page_json);
  • apps/server/src/core/ai-chat/tools/parse-node-arg.ts (in-app: patchNode, insertNode, updatePageJson).

Две копии — намеренно (ESM/CJS-граница, объяснено в комментарии серверной копии): теперь каждая — единый источник внутри своего пакета (6 inline-копий → 2 хелпера). Поведение байт-в-байт: сообщения throw сохранены точно (node was a string but not valid JSON / content was a string but not valid JSON); ветка null/object у update_page_json не тронута. Имена/описания/zod-схемы инструментов не менялись. packages/mcp/build пересобран в синк.

Отложено (зафиксировано здесь, т.к. доку удаляется этим коммитом): полный реестр спеков + вывод DocmostClientLike из реального типа — по причинам из §2 (Part 2 пробовался и откатан: реальные точные типы клиента ломают тест-стабы in-app инструментов, tsc краснеет). Делать инкрементально.

4. Найденные баги

Part 2 (DocmostClientLike из реального типа) — при попытке tsc дал 3 ошибки в ai-chat-tools.service.spec.ts (стабы возвращают упрощённые { ok: true }, несовместимые с точными типами). Откатил целиком, чтобы tsc остался зелёным — это и есть ожидаемая regression-поверхность, описанная в §2.
Ревью-субагент: проблем не найдено — APPROVE, поведение тождественно, билд эмпирически байт-в-байт совпадает с исходником.

5. Тестирование — статистика

Юнит-тесты: server Jest parse-node-arg.spec.ts 5/5; mcp node:test — 254 pass; server tsc --noEmit exit 0.
Циклы ревью: 1 (APPROVE; 1 посторонний build-дрейф docmost-schema.js исключён из PR через git restore).
Живая браузерная проверка: 1 цикл, автономный субагент, реальный стенд+LLM, 0 багов. Агент по инструкции вставил абзац INSERTED-BY-AGENT-F4 → подтверждено в персистентном ProseMirror-документе через POST /api/pages/info (новая нода с собственным attrs.id), не только в клиенте. Значит путь insertNode/patchNode → parseNodeArg работает вживую без регресса. Скрины /tmp/f4-*.png.

🤖 Generated with Claude Code

## Рефакторинг: устранение дублирования определений инструментов (безопасный шаг) Отчёт по скиллу архитектора. ### 1. Задача Из `docs/backlog/ai-chat-tool-definitions-duplicated.md`: один и тот же набор инструментов описан рукописно в нескольких местах (standalone MCP `packages/mcp/src/index.ts`, in-app агент `ai-chat-tools.service.ts`, плюс ручная копия сигнатур `DocmostClientLike`). Это forward-looking долг сопровождения (НЕ баг — код корректен сегодня): каждое добавление/правка инструмента требует синхронной правки в 2–3 местах, parity-баги (разъезд копий) приходится чинить дважды. ### 2. Анализ — что реально достижимо безопасно Полное устранение (единый transport-agnostic реестр спеков) **архитектурно заблокировано**, и это подтверждено фактами: - `@docmost/mcp` — **ESM-only и НЕ отдаёт .d.ts** (билд только JS), а сервер на `module:commonjs` физически не может его `import`-ить (грузит в рантайме трюком `Function('import()')`). Значит рантайм-код нельзя разделить через границу обычным импортом, а type-only вывод требует эмита деклараций + правки tsconfig-paths. - Два транспорта **намеренно расходятся**: имена инструментов (snake_case ×38 vs camelCase ×41), состав (in-app добавляет `getCurrentPage`/`listSidebarPages`, не отдаёт `delete_comment`/image-инструменты) и model-facing описания. Единый реестр поменял бы поведение И агента, И внешних MCP-клиентов — это отдельная фича со своей верификацией. Поэтому в этом PR — безопасный, поведенчески-нейтральный шаг, который убирает конкретный «материализованный parity-баг», на который указывает доку. ### 3. Решение Квирк «node как объект ИЛИ JSON-строка» был рукописно скопирован в **6 точках** (3 в standalone, 3 in-app). Вынес его в один хелпер `parseNodeArg()` на пакет: - `packages/mcp/src/lib/parse-node-arg.ts` (standalone: patch_node, insert_node, update_page_json); - `apps/server/src/core/ai-chat/tools/parse-node-arg.ts` (in-app: patchNode, insertNode, updatePageJson). Две копии — **намеренно** (ESM/CJS-граница, объяснено в комментарии серверной копии): теперь каждая — единый источник внутри своего пакета (6 inline-копий → 2 хелпера). Поведение байт-в-байт: сообщения throw сохранены точно (`node was a string but not valid JSON` / `content was a string but not valid JSON`); ветка null/object у update_page_json не тронута. Имена/описания/zod-схемы инструментов не менялись. `packages/mcp/build` пересобран в синк. **Отложено (зафиксировано здесь, т.к. доку удаляется этим коммитом):** полный реестр спеков + вывод `DocmostClientLike` из реального типа — по причинам из §2 (Part 2 пробовался и откатан: реальные точные типы клиента ломают тест-стабы in-app инструментов, tsc краснеет). Делать инкрементально. ### 4. Найденные баги **Part 2** (DocmostClientLike из реального типа) — при попытке tsc дал 3 ошибки в `ai-chat-tools.service.spec.ts` (стабы возвращают упрощённые `{ ok: true }`, несовместимые с точными типами). Откатил целиком, чтобы tsc остался зелёным — это и есть ожидаемая regression-поверхность, описанная в §2. **Ревью-субагент:** проблем не найдено — **APPROVE**, поведение тождественно, билд эмпирически байт-в-байт совпадает с исходником. ### 5. Тестирование — статистика **Юнит-тесты:** server Jest `parse-node-arg.spec.ts` 5/5; mcp `node:test` — **254 pass**; server `tsc --noEmit` exit 0. **Циклы ревью:** 1 (APPROVE; 1 посторонний build-дрейф `docmost-schema.js` исключён из PR через `git restore`). **Живая браузерная проверка:** 1 цикл, автономный субагент, реальный стенд+LLM, 0 багов. Агент по инструкции вставил абзац `INSERTED-BY-AGENT-F4` → подтверждено в персистентном ProseMirror-документе через `POST /api/pages/info` (новая нода с собственным attrs.id), не только в клиенте. Значит путь insertNode/patchNode → `parseNodeArg` работает вживую без регресса. Скрины `/tmp/f4-*.png`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Ghost added 1 commit 2026-06-21 06:52:04 +03:00
refactor(ai-chat): dedupe node-arg JSON normalization into a shared helper f9757fda12 
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>
vvzvlad added 1 commit 2026-06-21 14:30:48 +03:00
fix(ai-chat): keep tool-duplication backlog doc; fix parseNodeArg comment c7f0b51389 
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>
Ghost closed this pull request 2026-06-21 14:48:39 +03:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 epic
Large multi-phase effort spanning many changes
 feature
New functionality request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 idea
Idea / proposal for discussion
 invalid
This doesn't seem right
 needs-human
эскалация: нужно решение человека
 question
Further information is requested
 refactor
Code cleanup / refactoring
 review/approved
в последнем ревью нет открытых blocking-находок
 review/changes-requested
последнее ревью оставило открытые blocking-находки
 review/needs
head не ревьюился (head != reviewed_head)
 security
Security / hardening issue
 status/blocked
ждёт зависимость blocked_by
 status/done
закрыто и проверено
 status/in-progress
в активной работе (мягкая заявка)
 status/ready
специфицировано, не заблокировано, ждёт исполнителя
 test
Test coverage / test infrastructure
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent_coder agent_reviewer vvzvlad
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#114
Delete Branch "refactor/ai-chat-tool-spec-registry"

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?