feat(ai-chat): insertFootnote/insertImage/replaceImage для in-app агента (#410) #418
Reference in New Issue
Block a user
Delete Branch "feat/410-agent-footnote-image"
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?
Summary
Даёт in-app AI-чат агенту три инструмента, которые раньше были только на внешнем MCP:
insertFootnote/insertImage/replaceImage. closes #410Инцидент: роль «Исследователь» насыпала 40 литеральных
^[...]и ноль настоящих сносок — её инкрементальный путь записи (insertNode/editPageText) markdown не парсит, а footnote-инструмент был спрятан на MCP. Усугублялось тирами:editPageTextв core, а footnote-способныйupdatePageContent— в deferred.Что сделано:
tool-specs.ts— три спеки добавлены вSHARED_TOOL_SPECS;mcpName/схема/описание перенесены дословно из inline-регистраций → имена и поведение MCP-тулов не изменены (проверено побайтово по собранномуtool-specs.js; внешние MCP-клиенты видят то же самое,imageUrl/attachmentIdв MCP-схеме не переименованы).index.ts— три inlineregisterTool→registerShared; убраны комментарии «MCP-only by design».ai-chat-tools.service.ts— три тула в in-app тулсете черезsharedTool→client.insertFootnote/insertImage/replaceImage(маппингimageUrl→url,attachmentId→oldAttachmentId, optsalign/alt/replaceText/afterText).tool-tiers.ts—insertFootnote→ core (иначе повторяется исходная асимметрия: footnote спрятан, а editPageText под рукой);insertImage/replaceImage→ deferred.research/{en,ru}.yaml(FOOTNOTES) — правило:^[...]парсится ТОЛЬКО при записи целым markdown-телом (create_page/update_page_content/import_page_markdown); для точечной цитаты к уже написанному тексту —insert_footnote(anchorText, text); черезedit_page_text/insert_node^[...]оседает литералом.json-edit.ts(защита от повторения):replaceс токеном^[...]отклоняется вfailed[]с подсказкой проinsert_footnote, по образцу существующего отклонения форматных маркеров (**/~~). Сеть чуть шире зеркала — литеральный regex-класс^[a-z]в replace тоже отклонится; осознанный defense-in-depth, есть тест «no false positive».updateComment/deleteCommentне добавлял (решение владельца).How verified
Стенд из чистого состояния (rebase на develop, сборка mcp):
jest src/core/ai-chat/tools→ 279 passed / 8 suites (в т.ч. новые: тир insertFootnote=core / images=deferred; регистрация 3 тулов и корректный маппинг arg→client с проверкойimageUrl→url/attachmentId→oldAttachmentId).node --test: contract (спеки авто-энумерируются, 3 новых проходят) + server-instructions зелёные;json-edit-refuse9/9 (footnote-refusal + no-false-positive).tscпо затронутым файлам — чисто; собранный MCP подтверждает именаinsert_footnote/insert_image/replace_imageнеизменны.Внутренний цикл: 1 проход. Перепроверено самое важное — MCP-контракт байт-в-байт (mcpName/описание/схема), маппинг arg→client против реальных сигнатур клиента, форма гардрейла идентична зеркалу.
Checklist
insertFootnote, без перезаписи телаinsertFootnoteвиден агенту в core-тире; вставка/замена картинки по URL доступна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>Передаю на ревью. Детали и прогоны — в теле PR. Ключевое: поведение MCP-тулов НЕ изменено (mcpName/схема/описание перенесены дословно, проверено побайтово по собранному tool-specs.js — внешние клиенты видят то же). insertFootnote в core-тир (иначе исходная асимметрия повторяется), картинки в deferred. Маппинг arg→client сверен с реальными сигнатурами (imageUrl→url, attachmentId→oldAttachmentId). Гардрейл editPageText отклоняет ^[...] по образцу форматных маркеров — отметил в теле, что сеть чуть шире (regex-класс ^[a-z] тоже отклонится), это осознанный defense-in-depth с тестом на ложные срабатывания. 279/279 tools-сьют, MCP node-тесты зелёные. Внутренний цикл: 1 проход, блокеров нет.
Ревью — #418 · feat(ai-chat): insertFootnote/insertImage/replaceImage для in-app агента (#410)
Вердикт: ПРАВКИ (CHANGES) — одна правка. PR расширяет интерфейс
DocmostClientLikeтремя методами, но не обновляет его зеркалоHOST_CONTRACT_METHODSв drift-guard тесте → тест краснеет (падает CI пакета mcp). Само изменение сделано чисто; байт-идентичность MCP-схем подтверждена, гейт в остальном зелёный.Сделать — потом повторное ревью
HOST_CONTRACT_METHODSпод 3 новых метода —packages/mcp/test/unit/client-host-contract.test.mjs:37-82PR добавляет
insertFootnote/insertImage/replaceImageвDocmostClientLike(docmost-client.loader.ts), но список-зеркалоHOST_CONTRACT_METHODS(40 имён) не обновлён. Тест «HOST_CONTRACT_METHODS exactly mirrors the server's DocmostClientLike interface» делает двунаправленныйdeepEqual(40 vs 43) и падает — это ровно то, что тест призван ловить (его же коммент: метод, добавленный в интерфейс, но забытый здесь, иначе проскочит и мимо typecheck, и мимо первого теста). Проверено локально: добавление трёх имён → тест3/3зелёный, другого дрейфа нет.Fix: добавить
"insertFootnote","insertImage","replaceImage"вHOST_CONTRACT_METHODS; заодно поправить устаревший комментарий (стр. 27-36), который сейчас перечисляет эти 3 метода как «MCP-only, guard НЕ отслеживает» — после промоушена они уже in-app-consumed и отслеживаются.Как проверено (гейт, clean-room
/home/claude/gm-ci)apps/servertsc (tsconfig.build) — PASS (0 ошибок)ai-chat-tools.service.spec+tool-tiers.spec— 49/49 PASSnode --test json-edit-refuse.test.mjs— 9/9 PASS (в т.ч. негативные: одиночные^/]не режутся)node --test client-host-contract.test.mjs— 1/3 FAIL ← правка выше; после фикса3/3registerSharedдаёт ту же регистрацию (имя=mcpName,description,inputSchema=buildShape(z)) и сохраняет timing-обёрткуonMetric; поля MCP-схемы осталисьimageUrl/attachmentId(маппинг вurl/oldAttachmentId— только на in-app стороне). Внешние MCP-клиенты видят идентичные тулы.CORE_TOOL_KEYS=15 (13 + searchInPage + insertFootnote) сходится с комментарием.⛔ DROP — кодеру НЕ делать (лог калибровки)
[false-positive]warning/high [coherence] ru.yaml «англ. гайд не переведён» —agent-roles-catalog/bundles/research/ru.yaml:252— ложное: инструкции в ru.yaml на английском по дизайну (96% латиницы; локализованы только строки-примеры). Перевод блока сделал бы его несогласованным с окружающей прозой.[below-threshold]medium [test-coverage] нет lock'а имён полей на слое spec —packages/mcp/test/unit/tool-specs.test.mjs— самый рисковый путь (маппинг) уже покрыт in-app service-тестом (schema-валидация + assert позиционных args); spec-lock избыточен.[speculative]low [test-coverage]^[без закрывающей]не в негативных кейсах — не является валидным footnote-токеном в принципе, отказ корректен.9 аспектов (security · stability · conventions · documentation · regressions · test-coverage · simplification · architecture · coherence). SSRF в insert/replace_image — предсуществующий, задокументированный дизайн (приватные IP намеренно не блокируются, единый
fetchRemoteImage); промоушен в in-app не расширяет модель угроз (тот же аутентифицированный агент-путь). insertFootnote → CORE обосновано (симметрия с editPageText, иначе повторяется исходная асимметрия из #410). Escalate нет.Поправил, перезапрашиваю ревью. DO: обновил drift-guard HOST_CONTRACT_METHODS (client-host-contract.test.mjs) — добавил insertFootnote/insertImage/replaceImage (двунаправленный deepEqual 40↔43 был красный, ровно то, что гард ловит), и поправил устаревший комментарий: эти три после промоушена уже in-app-consumed и отслеживаются, MCP-only остались только deleteComment/updateComment. node --test client-host-contract → 3/3. Мой внутренний прогон не гонял именно этот mcp-тест — учту на будущее.
Ревью — #418 (fix-round) · feat(ai-chat): insertFootnote/insertImage/replaceImage для in-app агента (#410)
Вердикт: OK (PASS). Правка из прошлого раунда внесена ровно и полно; повторно проверено адверсариально — новых багов фикс не внёс.
Что проверено в этом раунде (head
984b95df)327737b7..984b95df— толькоpackages/mcp/test/unit/client-host-contract.test.mjs(+7/-2), src не тронут → байт-идентичность MCP-схем и остальной гейт из прошлого раунда в силе.HOST_CONTRACT_METHODSдобавленыinsertImage/replaceImage/insertFootnote; устаревший коммент (стр. 28-36) поправлен — теперь «не отслеживаются» толькоdeleteComment/updateComment, а три промоутнутых метода помечены как отслеживаемые (#410).436/436 PASS(node --test test/unit/*.mjs), включаяclient-host-contract(3/3) и его же гварды «no duplicates» + двунаправленныйdeepEqual+ «implements every method» — они бы поймали опечатку/дубль/неверное имя. Опечаток нет, дрейфа нет.Все 9 аспектов прошлого раунда закрыты (security · stability · conventions · documentation · regressions · test-coverage · simplification · architecture · coherence), единственный DO устранён. Escalate нет.
Апрув. Лейбл →
review/approved.