refactor(ai-chat): единый реестр спеков инструментов — унификация tables/pages/misc/comments (#294) #367

Merged
vvzvlad merged 6 commits from refactor/294-spec-registry-cont into develop 2026-07-05 20:39:42 +03:00

6 Commits

Author SHA1 Message Date
agent_coder 36b940fdb8 fix(#294 review F1-F2): test the changed execute wirings + transport-neutral descriptions
- F1: added in-app execute tests for the two wirings that ACTUALLY changed in the
  migration (the contract-parity test only checks advertised schema keys, not
  execute bodies): movePage forwards the newly-added optional `position` to
  client.movePage (and passes undefined position + null parent when omitted); the
  table trio (insert/delete/updateCell) forwards the unified `table` param
  positionally. A field destructured under the wrong name would have silently
  passed undefined to the client (execute is any-cast, tsc won't catch it).
- F2: rewrote the three migrated descriptions that hardcoded snake_case sibling
  tool names (which the in-app camelCase layer exposes under different ids,
  violating the registry's own transport-neutral-prose convention) into neutral
  prose: getPage "use get_page_json" -> "use the lossless page-JSON read tool";
  updatePageJson "get_page_json -> ... -> update_page_json" -> "read the page-JSON
  view -> modify -> write it back", "prefer rename_page" -> "prefer the rename-page
  tool"; exportPageMarkdown "import_page_markdown round-trip" -> "page-Markdown
  import round-trip" (the last was a direct regress — the in-app base said the
  camelCase importPageMarkdown). (stashPage's pre-existing get_page_json mention is
  out of scope, per the reviewer.)

Gate: mcp build 0; ai-chat-tools.service + tool-tiers (catalog-partition) pass,
incl. the 5 new execute-wiring tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 03:10:42 +03:00
agent_coder ce70fab1df refactor(ai-chat): unify share_page into SHARED_TOOL_SPECS (#294, misc family)
Migrates share_page / sharePage into the transport-agnostic spec registry
(schema + description declared once; each transport keeps only its execute/auth):
- sharePage (deferred) -> SHARED_TOOL_SPECS; index.ts uses registerShared(),
  ai-chat uses sharedTool(); removed from INLINE_TOOL_TIERS.

Drift reconciled (documented inline): both inline copies already carried the
"only share when the user explicitly asked" security framing, so the old
"per-transport divergence" note in BOTH layers was STALE — there was no real
behavioral divergence, only wording drift. The canonical description merges the
MCP copy's URL-format + idempotency detail with the in-app copy's reversibility
note and keeps the shared security framing. pageId keeps the MCP copy's stricter
.min(1). The MCP execute keeps its own `searchIndexing ?? true` default
(per-layer, not part of the shared schema).

Intentionally NOT migrated (kept inline — genuinely divergent, as their existing
notes state):
- search / searchPages: the in-app tool is a semantic+keyword hybrid (RRF) with
  in-process access control and a tuned schema (limit 1-20); the MCP `search` is
  a plain REST full-text search (limit up to 100). Different behavior AND schema.
- docmost_transform / transformPage: the in-app tool deliberately omits the
  `deleteComments` schema field (a comment-deletion guardrail) and carries a
  shorter description. Different schema.

Gate: mcp build 0 + node --test 458/458 (page-search excluded — hangs only under
the local re2->RegExp type-shim, its source untouched), server jest 775 incl.
tool-tiers catalog-partition + shared-spec contract parity, server tsc 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 02:27:50 +03:00
agent_coder b51dae16a6 docs(mcp): mark media tools MCP-only in index.ts (#294, media family)
The media tools — insert_image, replace_image, insert_footnote — are MCP-only
by design: the in-app AI-chat agent exposes no image or footnote tools, so there
is no second layer to unify into SHARED_TOOL_SPECS. A registry spec's
tier/catalogLine are in-app metadata and the catalog-partition test forbids a
spec without a live in-app tool, so forcing them into the registry would break
the invariant. They stay per-transport (inline in index.ts).

No behavior change — documentation only (adds the rationale above each tool so a
future migrator does not re-investigate why these are not shared).

Gate: mcp tsc 0 (comment-only change).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 02:14:39 +03:00
agent_coder 39735afd73 refactor(ai-chat): unify page tools into SHARED_TOOL_SPECS (#294, pages family)
Migrates the three-layer page tools into the transport-agnostic spec registry
(schema + description declared once; each transport keeps only its execute/auth):
- getPage, listPages (core), createPage, movePage, renamePage, deletePage,
  updatePageJson, exportPageMarkdown (deferred) -> SHARED_TOOL_SPECS; index.ts
  uses registerShared(), ai-chat uses sharedTool(); removed from
  INLINE_TOOL_TIERS. Tiers preserved from CORE_TOOL_KEYS (getPage/listPages =
  core, the rest deferred).

delete_page is genuinely three-layer (in-app deletePage exists), so it IS
migrated — not MCP-only. Its H4 guardrail is preserved: the shared schema
exposes ONLY pageId, so no permanentlyDelete/forceDelete flag can reach the
client (still asserted by ai-chat-tools.service.spec.ts).

Descriptions merged (documented inline): each canonical text takes the MCP
copy's richer structural notes plus the in-app copy's reversibility framing.

Schema DRIFT reconciled (documented inline):
- createPage.content: MCP pinned .min(1) but the in-app copy left it unbounded
  and DOCUMENTS an empty body as valid ("may be empty" — creating an empty page
  to fill later is a real use). Kept the looser no-min form: create_page now also
  accepts an empty body (harmless) and no previously-valid in-app input is
  rejected. title/spaceId keep the MCP .min(1) (empty is never valid).
- movePage: MCP exposed an optional `position` (fractional-index) field the
  in-app copy lacked. Unified by KEEPING position — the in-app client already
  accepts an optional position arg, so the in-app execute now forwards it;
  optional, so no previously-valid call is rejected. `parentPageId` is nullable
  on both (real JSON null -> root); the MCP execute keeps its 'null'/'' string
  coercion as a per-layer robustness fallback.
- getPage/renamePage/updatePageJson/exportPageMarkdown/listPages: kept the MCP
  copy's stricter .min(1) on ids where the in-app copy was unbounded.

Per-transport execute logic preserved: getPage's {title,markdown} projection,
updatePageJson's JSON-string normalization, list_pages' default limit/tree, and
move_page's cycle guard + positive-confirmation check all stay in their execute
bodies.

Intentionally NOT touched: updatePageContent (Markdown-based body update; no MCP
equivalent) and getTable (name-convention divergence, see tables family) stay
inline.

Gate: mcp build 0 + node --test 458/458 (page-search excluded — hangs only under
the local re2->RegExp type-shim, its source untouched), server jest 770 incl.
tool-tiers catalog-partition + shared-spec contract parity + deletePage H4
guardrail, server tsc 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 02:13:41 +03:00
agent_coder eebbe6717c refactor(ai-chat): unify table row/cell tools into SHARED_TOOL_SPECS (#294, tables family)
Migrates the three-layer table WRITE tools into the transport-agnostic spec
registry (schema + description declared once; each transport keeps only its
execute/auth):
- tableInsertRow, tableDeleteRow, tableUpdateCell -> SHARED_TOOL_SPECS;
  index.ts uses registerShared(), ai-chat uses sharedTool(); removed from
  INLINE_TOOL_TIERS (all three are deferred; not in CORE_TOOL_KEYS).

Drift reconciled (documented inline): the four table tools previously carried a
"NOT shared" note in both layers over a single parameter-NAME drift — the MCP
layer named the table reference `table`, the in-app layer `tableRef`. Unified on
the MCP name `table` (renaming the public MCP parameter would break external MCP
clients; the in-app parameter is model-facing/prompt-only and safe to rename).
The in-app execute bodies now destructure `table`. Descriptions took the MCP
copy's richer wording (documents `#<index>`, padding, header-row behavior) plus
the in-app copy's "Reversible via page history" note; both fields keep the MCP
copy's stricter .min(1) (in-app left them unbounded); sibling tool references
phrased transport-neutrally.

Intentionally NOT migrated (kept inline): table_get / getTable. Its MCP tool
name is noun-first (`table_get`) while the in-app key is verb-first (`getTable`),
which breaks the snake_case(inAppKey) naming convention the registry enforces
(shared-tool-specs.contract.spec.ts). Renaming the public MCP tool would break
external clients, so it stays per-transport — but its in-app reference param was
still aligned to `table` (was `tableRef`) for consistency with the migrated trio.

Gate: mcp tsc 0 + node --test 458/458 (page-search excluded — hangs only under
the local re2->RegExp type-shim, its source is untouched), server jest 730 incl.
tool-tiers catalog-partition + shared-spec contract parity, server tsc 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 02:06:52 +03:00
agent_coder e348433a39 refactor(ai-chat): unify comment tools into SHARED_TOOL_SPECS (#294, comments family)
Migrates the three-layer comment tools into the single transport-agnostic spec
registry (schema + model-facing description declared once; each transport keeps
only its execute/auth):
- createComment, listComments, resolveComment, checkNewComments — moved to
  SHARED_TOOL_SPECS; index.ts uses registerShared(), ai-chat uses sharedTool();
  removed from INLINE_TOOL_TIERS (tier/catalogLine now on the spec). Tiers
  preserved from CORE_TOOL_KEYS (create/list/resolve = core, check = deferred).

Intentionally NOT migrated (kept MCP-inline): update_comment / delete_comment —
they are MCP-only by design; the in-app AI-chat layer deliberately has no
updateComment/deleteComment (comment edits are irreversible / not
version-tracked), asserted by ai-chat-tools.service.spec.ts. A registry spec's
tier/catalogLine are in-app metadata and the catalog-partition test forbids a
deferred spec without a live in-app tool, so these stay per-transport.

Drift reconciled (documented inline): createComment/listComments/checkNewComments
took the more-maintained/superset description + stricter .min(1) guards.
resolveComment: `resolved` drifted (MCP optional+default(true) vs in-app
required) — kept the MCP superset, so in-app resolveComment now accepts an
omitted `resolved` (defaults to resolve) — a deliberate, backward-compatible
unification (never rejects a previously-valid input).

Gate: mcp build 0 + node --test 480/480, ai-chat 654, tool-tiers (incl. F3
catalog-partition) 16/16, server tsc 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 01:45:43 +03:00