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

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

Open
Ghost wants to merge 4 commits from refactor/193-tool-spec-registry into develop
pull from: refactor/193-tool-spec-registry
merge into: vvzvlad:develop
Conversation 1 Commits 4 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: 4c7b67195034a502da7a28bb4bc079a2fadf240a baseline_head: 4c7b67195034a502da7a28bb4bc079a2fadf240a verdict: changes-requested round: 5 max_rounds: 6 open_findings: [F3] reopened: {} -->
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 the status/in-progress label 2026-06-29 00:17:07 +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
vvzvlad removed the status/in-progress label 2026-06-29 03:49:27 +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 при баге парсера. Два аспекта разошлись → разумный автор вправе оставить.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin refactor/193-tool-spec-registry:refactor/193-tool-spec-registry
git checkout refactor/193-tool-spec-registry
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 review/changes-requested
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#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?