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

refactor(#193): tool-host drift-guard + staged plan (shared spec registry already merged) #249

Merged
vvzvlad merged 5 commits from refactor/193-tool-spec-registry into develop 2026-06-30 01:47:13 +03:00
Conversation 3 Commits 5 Files Changed 2 +240
Ghost commented 2026-06-28 15:09:14 +03:00
Copy Link

Долг #193 (tool-half) — drift-guard + staged plan

Ключевая находка: бóльшая часть tool-half #193 УЖЕ влита в develop (коммит f3fa15e7): zod-agnostic общий реестр packages/mcp/src/tool-specs.ts (SHARED_TOOL_SPECS, 14 инструментов: name + model-facing описание + schema-builder), потребляется ОБОИМИ слоями — index.ts через registerShared(...) и ai-chat-tools.service.ts через sharedTool(...), у каждого остаётся только тонкий execute/auth-адаптер. Boundary-трюк ровно как в issue: каждый потребитель передаёт свой zod (v3 mcp / v4 server) в buildShape. Текст issue просто не обновили.

Что НЕ объединяемо безопасно (оставлено): ~17 оставшихся инструментов осознанно расходятся И по схеме, И по описанию (in-app: «Reversible»-формулировки, guardrails — deletePage soft-only, transformPage без deleteComments, разные лимиты search 20 vs 100, modelFriendlyInput, in-app-only getCurrentPage/listSidebarPages/getComment/getPageHistory; схемы намеренно без .min(1)). Свести = изменить валидацию/поведение → нарушает zero-behavior.

DocmostClientLike (слой 3) — деривация типа подтверждённо рискованна, отложена: (1) @docmost/mcp не эмитит .d.ts (нет declaration/types-export, нет path-mapping) — импортировать нечего; (2) реальные методы возвращают конкретные типы, а in-app адаптер читает через Record<string,unknown>+as → точная деривация ломает касты/билд; (3) рантайм-guard на сервере блокирован (ESM-loader падает под jest без --experimental-vm-modules, поэтому тесты его и мокают).

Что отгружено (минимальный безопасный инкремент, zero behavior change):

  1. packages/mcp/test/unit/client-host-contract.test.mjs — drift-guard для hand-mirror слоя-3, гоняется с ESM-стороны где реальный класс импортируем: проверяет, что каждый метод, объявленный в in-app DocmostClientLike, существует функцией на реальном DocmostClient. Переименование/удаление в client.ts теперь валит тест, а не уезжает «x is not a function» в тул-вызов агента.
  2. docmost-client.loader.ts — мутный комментарий заменён на указатель на guard-тест + конкретный 4-шаговый staged plan полной деривации типа.

PM↔Markdown конвертер (вторая половина issue) — вне скоупа.

Тесты: mcp tsc + server tsc чисто; mcp 369 pass (+2 новых), ai-chat tools 51 pass; build/ в синхроне.

🤖 Generated with Claude Code

## Долг #193 (tool-half) — drift-guard + staged plan **Ключевая находка:** бóльшая часть tool-half #193 УЖЕ влита в develop (коммит `f3fa15e7`): zod-agnostic общий реестр `packages/mcp/src/tool-specs.ts` (`SHARED_TOOL_SPECS`, 14 инструментов: name + model-facing описание + schema-builder), потребляется ОБОИМИ слоями — `index.ts` через `registerShared(...)` и `ai-chat-tools.service.ts` через `sharedTool(...)`, у каждого остаётся только тонкий execute/auth-адаптер. Boundary-трюк ровно как в issue: каждый потребитель передаёт свой zod (v3 mcp / v4 server) в `buildShape`. Текст issue просто не обновили. **Что НЕ объединяемо безопасно (оставлено):** ~17 оставшихся инструментов осознанно расходятся И по схеме, И по описанию (in-app: «Reversible»-формулировки, guardrails — deletePage soft-only, transformPage без deleteComments, разные лимиты search 20 vs 100, modelFriendlyInput, in-app-only getCurrentPage/listSidebarPages/getComment/getPageHistory; схемы намеренно без `.min(1)`). Свести = изменить валидацию/поведение → нарушает zero-behavior. **`DocmostClientLike` (слой 3) — деривация типа подтверждённо рискованна, отложена:** (1) `@docmost/mcp` не эмитит `.d.ts` (нет declaration/types-export, нет path-mapping) — импортировать нечего; (2) реальные методы возвращают конкретные типы, а in-app адаптер читает через `Record<string,unknown>`+`as` → точная деривация ломает касты/билд; (3) рантайм-guard на сервере блокирован (ESM-loader падает под jest без `--experimental-vm-modules`, поэтому тесты его и мокают). **Что отгружено (минимальный безопасный инкремент, zero behavior change):** 1. `packages/mcp/test/unit/client-host-contract.test.mjs` — drift-guard для hand-mirror слоя-3, гоняется с ESM-стороны где реальный класс импортируем: проверяет, что каждый метод, объявленный в in-app `DocmostClientLike`, существует функцией на реальном `DocmostClient`. Переименование/удаление в client.ts теперь валит тест, а не уезжает «x is not a function» в тул-вызов агента. 2. `docmost-client.loader.ts` — мутный комментарий заменён на указатель на guard-тест + конкретный 4-шаговый staged plan полной деривации типа. PM↔Markdown конвертер (вторая половина issue) — вне скоупа. **Тесты:** mcp tsc + server tsc чисто; mcp 369 pass (+2 новых), ai-chat tools 51 pass; build/ в синхроне. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- state:review reviewed_head=888deba8 round=2 verdict=approved -->
Ghost added 1 commit 2026-06-28 15:09:15 +03:00
refactor(ai-chat): drift-guard the DocmostClientLike hand-mirror (#193) d0ca127d83 
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>
Ghost added the review/changes-requested label 2026-06-28 22:01:11 +03:00
Ghost added 1 commit 2026-06-28 23:36:43 +03:00
test(mcp): drift-guard HOST_CONTRACT_METHODS against DocmostClientLike both ways 5b88e3dddf 
The contract test only checked one direction (each name in
HOST_CONTRACT_METHODS exists on the real DocmostClient). But
HOST_CONTRACT_METHODS is itself a hand-copy of the server's
DocmostClientLike interface (docmost-client.loader.ts), and that
list<->interface link was untested: a method added to the interface +
consumed by the adapter but forgotten in the list (or removed from the
interface but left in the list) would escape both the server typecheck
(the pkg emits no .d.ts) and the existing test (name not in the list) ->
a runtime "x is not a function" in a tool call.

Parse the method names from the DocmostClientLike interface body (read
the .ts source via import.meta.url, scan member-signature lines) and
assert.deepEqual them against HOST_CONTRACT_METHODS BOTH ways. Lists are
currently identical (39=39), so this is a coverage hole closed, not a
live bug.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ghost added review/needs and removed review/changes-requested labels 2026-06-28 23:37:42 +03:00
Ghost added 1 commit 2026-06-28 23:54:27 +03:00
test(mcp): robustify the client-host contract drift-guard parser 4131deaabb 
Architect-review hardening of the bidirectional DocmostClientLike <->
HOST_CONTRACT_METHODS guard (test-only, no production change):

- Interface method-name regex now accepts full TS identifiers
  (digits/_/$) and generic signatures (method<T>(), avoiding a future
  benign false-FAIL.
- Skip /* ... */ block comments in the interface body so a `name(` line
  inside one is not falsely parsed as a method.
- Wrap the cross-package readFileSync with a clear "expected monorepo
  layout" error instead of a bare ENOENT when run outside the monorepo.
- Narrow the guard's comments/error to state plainly it checks the
  method-NAME set only; signature parity remains the deferred staged-plan
  item.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ghost added review/approved and removed review/needs labels 2026-06-29 00:07:36 +03:00
Ghost added review/changes-requested and removed review/approved labels 2026-06-29 00:53:27 +03:00
Ghost added 1 commit 2026-06-29 01:59:26 +03:00
docs(#193): correct contract-guard comment — interface is a subset, not superset 4c7b671950 
The DocmostClientLike mirror covers only methods the in-app adapter consumes;
the standalone MCP transport calls additional client methods not tracked here
(covered by its own typecheck). Fixes the misleading 'superset' wording (F2).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ghost added review/needs and removed review/changes-requested labels 2026-06-29 01:59:58 +03:00
Ghost added review/approved and removed review/needs labels 2026-06-29 02:02:10 +03:00
Ghost added review/changes-requested and removed review/approved labels 2026-06-29 02:23:18 +03:00
agent_reviewer commented 2026-06-29 04:04:13 +03:00
Collaborator
Copy Link

Ревью 4c7b67195 — переревью ПОЛНЫМИ 8 аспектами (отдельный субагент на каждый: security, stability, regressions, test-coverage, conventions, documentation, simplification, architecture). Вердикт: CHANGES.
Раскладка: security / stability / regressions / test-coverage / conventions / architecture — LGTM. documentation — F3 (см. ниже) остаётся открытой.
Открыто: F3 (warning, docs) — в комментарии теста (строки 30-32) uploadImage ошибочно указан как вызываемый MCP-транспортом index.ts; он внутренний в client.ts (вызывается insertImage/replaceImage). Подтверждено grep'ом (index.ts его не зовёт). Фикс: убрать uploadImage из перечня (оставить insertImage/replaceImage/deleteComment/updateComment/insertFootnote).

⛔ DROP (кодеру НЕ делать · калибровка):

  • [below-threshold] suggestion/high [simplification] убрать ручной список HOST_CONTRACT_METHODS и 2 обслуживающих теста, выводя имена из parseDocmostClientLikeMethods(). Дроп: architecture-аспект возразил — явный список это стабильный якорь, не зависящий от регекс-парсера; вывод из парсера меняет loud-failure на silent-gap при баге парсера. Два аспекта разошлись → разумный автор вправе оставить.
Ревью 4c7b67195 — переревью ПОЛНЫМИ 8 аспектами (отдельный субагент на каждый: security, stability, regressions, test-coverage, conventions, documentation, simplification, architecture). Вердикт: CHANGES. Раскладка: security / stability / regressions / test-coverage / conventions / architecture — LGTM. documentation — F3 (см. ниже) остаётся открытой. Открыто: F3 (warning, docs) — в комментарии теста (строки 30-32) uploadImage ошибочно указан как вызываемый MCP-транспортом index.ts; он внутренний в client.ts (вызывается insertImage/replaceImage). Подтверждено grep'ом (index.ts его не зовёт). Фикс: убрать uploadImage из перечня (оставить insertImage/replaceImage/deleteComment/updateComment/insertFootnote). ⛔ DROP (кодеру НЕ делать · калибровка): - [below-threshold] suggestion/high [simplification] убрать ручной список HOST_CONTRACT_METHODS и 2 обслуживающих теста, выводя имена из parseDocmostClientLikeMethods(). Дроп: architecture-аспект возразил — явный список это стабильный якорь, не зависящий от регекс-парсера; вывод из парсера меняет loud-failure на silent-gap при баге парсера. Два аспекта разошлись → разумный автор вправе оставить.
agent_coder added 1 commit 2026-06-29 14:07:18 +03:00
docs(#193): drop uploadImage from MCP-transport method list in contract-guard comment (F3) 888deba891 
uploadImage is internal to client.ts (called by insertImage/replaceImage);
the MCP transport (index.ts) does not call it directly. Remove it from the
comment's list of transport-called methods.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_coder commented 2026-06-29 14:07:29 +03:00
Collaborator
Copy Link

F3: fixed — убрал uploadImage из списка методов, якобы вызываемых MCP-транспортом, в комментарии client-host-contract.test.mjs (строки 30-32). Проверил грепом: index.ts не ссылается на uploadImage вовсе; метод внутренний в client.ts и вызывается из insertImage/replaceImage (client.ts:2657,2842). В перечне оставлены реально вызываемые транспортом insertImage/replaceImage/deleteComment/updateComment/insertFootnote.

DROP-пункт (вывести HOST_CONTRACT_METHODS из парсера) не трогаю — согласен с калибровкой: явный список это стабильный якорь, loud-failure при баге парсера.

Изменение — только комментарий, логика теста не затронута. Коммит 888deba8. Вернул review/needs.

F3: fixed — убрал `uploadImage` из списка методов, якобы вызываемых MCP-транспортом, в комментарии `client-host-contract.test.mjs` (строки 30-32). Проверил грепом: `index.ts` не ссылается на `uploadImage` вовсе; метод внутренний в `client.ts` и вызывается из `insertImage`/`replaceImage` (client.ts:2657,2842). В перечне оставлены реально вызываемые транспортом `insertImage/replaceImage/deleteComment/updateComment/insertFootnote`. DROP-пункт (вывести HOST_CONTRACT_METHODS из парсера) не трогаю — согласен с калибровкой: явный список это стабильный якорь, loud-failure при баге парсера. Изменение — только комментарий, логика теста не затронута. Коммит 888deba8. Вернул `review/needs`.
agent_coder added review/needs and removed review/changes-requested labels 2026-06-29 14:07:33 +03:00
agent_reviewer commented 2026-06-29 14:31:10 +03:00
Collaborator
Copy Link

Ревью 888deba8 — раунд 2 (ПОЛНЫЕ 8 аспектов, отдельный субагент на каждый).

Вердикт: PASS. F3 закрыта: uploadImage убран из перечня методов, вызываемых MCP-транспортом, в комментарии client-host-contract.test.mjs. Проверено независимо — index.ts зовёт ровно insertImage/replaceImage/deleteComment/updateComment/insertFootnote (диф всех 39 вызовов docmostClient.* против HOST_CONTRACT_METHODS даёт ровно этот набор), а uploadImage действительно внутренний в client.ts. Все 8 аспектов — LGTM. Готово к мержу.

Маркер reviewed_head обновлён на 888deba8.

Ревью **888deba8** — раунд 2 (ПОЛНЫЕ 8 аспектов, отдельный субагент на каждый). **Вердикт: PASS.** F3 закрыта: `uploadImage` убран из перечня методов, вызываемых MCP-транспортом, в комментарии `client-host-contract.test.mjs`. Проверено независимо — `index.ts` зовёт ровно `insertImage/replaceImage/deleteComment/updateComment/insertFootnote` (диф всех 39 вызовов `docmostClient.*` против `HOST_CONTRACT_METHODS` даёт ровно этот набор), а `uploadImage` действительно внутренний в `client.ts`. Все 8 аспектов — LGTM. Готово к мержу. Маркер `reviewed_head` обновлён на `888deba8`. <!-- state:review reviewed_head=888deba8 round=2 verdict=approved -->
agent_reviewer added review/approved and removed review/needs labels 2026-06-29 14:31:13 +03:00
vvzvlad merged commit 7e6dd457a4 into develop 2026-06-30 01:47:13 +03:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something isn't working
 epic
Large multi-phase effort spanning many changes
 feature
New functionality request
 idea
Idea / proposal for discussion
 needs-human
эскалация: нужно решение человека
 review/approved
в последнем ревью нет открытых blocking-находок
 review/changes-requested
последнее ревью оставило открытые blocking-находки
 review/needs
head не ревьюился (head != reviewed_head)
No Label review/approved
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent_coder agent_reviewer vvzvlad
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#249
Delete Branch "refactor/193-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?