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>
This commit is contained in:
agent_coder
2026-07-04 21:26:02 +03:00
parent d78b985062
commit 292a0836f4
4 changed files with 214 additions and 237 deletions
@@ -455,59 +455,13 @@ export class AiChatToolsService {
},
}),
// INTENTIONAL per-transport divergence (not shared): the description is
// tuned for the in-app agent (e.g. "retry with a corrected EXACT selection"
// and "Reversible via the comment UI"); the standalone MCP `create_comment`
// keeps its own wording. Kept per-layer.
createComment: tool({
description:
'Add an INLINE comment to a page, or reply to an existing top-level ' +
'comment (one level only — the backend rejects replies to replies). ' +
'The comment is anchored inline to the given exact `selection` text ' +
'(which gets highlighted); page-level comments are NOT supported. A ' +
"new top-level comment REQUIRES a `selection`. Replies inherit the " +
"parent's anchor and take no selection. If the call fails with a " +
'"selection not found" error, retry with a corrected EXACT selection ' +
'copied verbatim from a single paragraph/block. You may also attach a ' +
'`suggestedText` proposing a replacement for the `selection` (a human ' +
'applies it from the UI); when set, the `selection` must occur exactly ' +
'once in the page. Reversible via the comment UI.',
inputSchema: modelFriendlyInput({
pageId: z.string().describe('The id of the page to comment on.'),
content: z.string().describe('The comment body as Markdown.'),
selection: z
.string()
.min(1)
.max(250)
.optional()
.describe(
'EXACT contiguous text from a SINGLE paragraph/block to anchor ' +
'(highlight) the comment on (<=250 chars, avoid spanning across ' +
'formatting boundaries). Required for a new top-level comment; ' +
'omit only when replying via parentCommentId.',
),
parentCommentId: z
.string()
.optional()
.describe(
'Optional id of a TOP-LEVEL comment to reply to (one level ' +
'of replies only).',
),
suggestedText: z
.string()
.min(1)
.max(2000)
.optional()
.describe(
'Optional proposed replacement (PLAIN TEXT) for the `selection`, ' +
'applied by a human via the UI (never auto-applied). REQUIRES a ' +
'`selection`; NOT allowed on a reply. When set, the `selection` ' +
'must be UNIQUE in the page — expand it with surrounding context ' +
'(still <=250 chars) if it occurs more than once, or the call is ' +
'refused.',
),
}),
execute: async ({
// Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
// This layer keeps only its own execute-side guards (require a selection
// for a top-level comment; reject suggestedText on a reply / without a
// selection) — the schema+description are shared.
createComment: sharedTool(
sharedToolSpecs.createComment,
async ({
pageId,
content,
selection,
@@ -548,26 +502,17 @@ export class AiChatToolsService {
const data = (result?.data ?? {}) as { id?: string };
return { commentId: data.id, pageId };
},
}),
),
resolveComment: tool({
description:
'Resolve or reopen a top-level comment thread (reversible — toggle ' +
'the resolved flag). Only top-level comments can be resolved.',
inputSchema: modelFriendlyInput({
commentId: z
.string()
.describe('The id of the top-level comment to resolve/reopen.'),
resolved: z
.boolean()
.describe('true to resolve the thread, false to reopen it.'),
}),
execute: async ({ commentId, resolved }) => {
// Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
resolveComment: sharedTool(
sharedToolSpecs.resolveComment,
async ({ commentId, resolved }) => {
// resolveComment(commentId, resolved) -> { success, commentId, resolved }.
await client.resolveComment(commentId, resolved);
return { commentId, resolved };
},
}),
),
// --- READ tools (added) ---
@@ -673,24 +618,12 @@ export class AiChatToolsService {
await client.getTable(pageId, tableRef),
}),
listComments: tool({
description:
'List comments on a page in one call. By DEFAULT only ACTIVE ' +
'threads are returned; resolved threads (a resolved top-level ' +
'comment and all its replies) are hidden and their count reported ' +
'as `resolvedThreadsHidden` so you can re-query with ' +
'`includeResolved: true` to see everything. Returns ' +
'`{ items, resolvedThreadsHidden }`. Content is returned as Markdown.',
inputSchema: modelFriendlyInput({
pageId: z.string().describe('The id of the page.'),
includeResolved: z
.boolean()
.optional()
.describe('default only active threads; true — include resolved'),
}),
execute: async ({ pageId, includeResolved }) =>
// Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
listComments: sharedTool(
sharedToolSpecs.listComments,
async ({ pageId, includeResolved }) =>
await client.listComments(pageId, includeResolved),
}),
),
getComment: tool({
description: 'Fetch a single comment by id (content as Markdown).',
@@ -700,26 +633,12 @@ export class AiChatToolsService {
execute: async ({ commentId }) => await client.getComment(commentId),
}),
checkNewComments: tool({
description:
'Find new comments across a space (optionally scoped to a subtree) ' +
'created after a given timestamp.',
inputSchema: modelFriendlyInput({
spaceId: z.string().describe('The id of the space to scan.'),
since: z
.string()
.describe('An ISO-8601 timestamp; only comments created after it.'),
parentPageId: z
.string()
.optional()
.describe(
'Optional page id to scope the scan to that page and its ' +
'descendants.',
),
}),
execute: async ({ spaceId, since, parentPageId }) =>
// Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
checkNewComments: sharedTool(
sharedToolSpecs.checkNewComments,
async ({ spaceId, since, parentPageId }) =>
await client.checkNewComments(spaceId, since, parentPageId),
}),
),
listShares: sharedTool(
sharedToolSpecs.listShares,
@@ -108,23 +108,14 @@ export const INLINE_TOOL_TIERS: Record<
tier: 'core',
catalogLine: "listPages — list recent pages, or a space's full page tree.",
},
listComments: {
tier: 'core',
catalogLine: 'listComments — list all comments on a page (including resolved).',
},
// NOTE: createComment, listComments and resolveComment moved to
// @docmost/mcp's SHARED_TOOL_SPECS (#294); they carry their own tier +
// catalogLine there. getComment stays inline (MCP-only shape divergence is
// n/a — it simply has no shared spec).
getComment: {
tier: 'core',
catalogLine: 'getComment — fetch a single comment by id.',
},
createComment: {
tier: 'core',
catalogLine:
'createComment — add an inline comment (optionally with a suggested edit).',
},
resolveComment: {
tier: 'core',
catalogLine: 'resolveComment — resolve or reopen a comment thread.',
},
// --- deferred inline ---
createPage: {
@@ -157,11 +148,8 @@ export const INLINE_TOOL_TIERS: Record<
tier: 'deferred',
catalogLine: 'getTable — read a table as a matrix of cell texts and cell ids.',
},
checkNewComments: {
tier: 'deferred',
catalogLine:
'checkNewComments — find comments in a space created after a timestamp.',
},
// NOTE: checkNewComments moved to @docmost/mcp's SHARED_TOOL_SPECS (#294);
// it carries its own deferred tier + catalogLine there.
getPageHistory: {
tier: 'deferred',
catalogLine:
+14 -115
View File
@@ -721,26 +721,8 @@ server.registerTool(
// --- Comment tools (ported from upstream PR #3 by Max Nikitin) ---
// Tool: list_comments
server.registerTool(
"list_comments",
{
description:
"List comments on a page in one call (pagination is handled " +
"internally). By DEFAULT only ACTIVE threads are returned; resolved " +
"threads (a resolved top-level comment and all its replies) are hidden " +
"and their count reported as `resolvedThreadsHidden` so you can re-query " +
"with `includeResolved: true` to see everything. Returns " +
"`{ items, resolvedThreadsHidden }`. Content is returned as Markdown.",
inputSchema: {
pageId: z.string().describe("ID of the page"),
includeResolved: z
.boolean()
.optional()
.describe(
"default only active threads; true — include resolved",
),
},
},
registerShared(
SHARED_TOOL_SPECS.listComments,
async ({ pageId, includeResolved }) => {
const comments = await docmostClient.listComments(pageId, includeResolved);
return jsonContent(comments);
@@ -748,55 +730,11 @@ server.registerTool(
);
// Tool: create_comment
// INTENTIONAL per-transport divergence (not shared): the in-app copy tunes the
// guidance for the in-app agent (e.g. "retry with a corrected EXACT selection"
// and "Reversible via the comment UI"); this transport keeps its own wording.
server.registerTool(
"create_comment",
{
description:
"Create a new comment on a page. The comment is ALWAYS inline and is " +
"anchored to (highlights) its `selection` text — there are no page-level " +
"comments. Content is provided as Markdown and automatically converted. " +
"A top-level comment REQUIRES an exact `selection`; if the selection " +
"cannot be found in the page the call fails (no orphan comment is left). " +
"Replies (parentCommentId set) inherit the parent's anchor and take no " +
"selection. You may also attach a `suggestedText` proposing a replacement " +
"for the `selection`; a human applies (or rejects) it from the UI. When " +
"`suggestedText` is set the `selection` MUST occur exactly once in the " +
"page — expand it with surrounding context if it is ambiguous.",
inputSchema: {
pageId: z.string().describe("ID of the page to comment on"),
content: z.string().min(1).describe("Comment content in Markdown format"),
selection: z
.string()
.min(1)
// Enforce the documented 250-char cap to match the description above.
.max(250)
.optional()
.describe(
"EXACT contiguous text from a single paragraph/block to anchor the " +
"comment on (<=250 chars). Required for a top-level comment; omit " +
"only when replying via parentCommentId.",
),
parentCommentId: z
.string()
.optional()
.describe("Parent comment ID to create a reply (max 2 nesting levels)"),
suggestedText: z
.string()
.min(1)
.max(2000)
.optional()
.describe(
"Optional proposed replacement (PLAIN TEXT) for the `selection`, " +
"applied by a human via the UI (never auto-applied). REQUIRES a " +
"`selection`; NOT allowed on a reply. When set, the `selection` must " +
"be UNIQUE in the page — expand it with surrounding context (still " +
"<=250 chars) if it occurs more than once, or the call is refused.",
),
},
},
// Schema + description now live in the shared registry (#294). The execute body
// keeps this transport's own guards (require a selection for a top-level
// comment; reject suggestedText on a reply / without a selection).
registerShared(
SHARED_TOOL_SPECS.createComment,
async ({ pageId, content, selection, parentCommentId, suggestedText }) => {
if (!parentCommentId && (!selection || !selection.trim())) {
throw new Error(
@@ -872,28 +810,9 @@ server.registerTool(
);
// Tool: resolve_comment
server.registerTool(
"resolve_comment",
{
description:
"Resolve (close) or reopen a comment thread. Only top-level comments can " +
"be resolved — the server rejects resolving a reply. Reversible: pass " +
"resolved=false to reopen. Resolving keeps the thread and its replies " +
"(unlike delete_comment, which permanently removes them).",
inputSchema: {
commentId: z
.string()
.min(1)
.describe("ID of the top-level comment thread to resolve or reopen"),
resolved: z
.boolean()
.optional()
.default(true)
.describe(
"true (default) marks the thread resolved/closed; false reopens it",
),
},
},
// Schema + description now live in the shared registry (#294).
registerShared(
SHARED_TOOL_SPECS.resolveComment,
async ({ commentId, resolved }) => {
const result = await docmostClient.resolveComment(commentId, resolved);
return jsonContent(result);
@@ -901,30 +820,10 @@ server.registerTool(
);
// Tool: check_new_comments
server.registerTool(
"check_new_comments",
{
description:
"Check for new comments across pages in a space since a given timestamp. " +
"Optionally scope to a page subtree (folder). Returns only comments " +
"created after the specified time.",
inputSchema: {
spaceId: z.string().describe("Space ID to check for new comments"),
since: z
.string()
.min(1)
.describe(
"ISO 8601 timestamp — only return comments created after this time (e.g. '2026-03-10T00:00:00Z')",
),
parentPageId: z
.string()
.optional()
.describe(
"Optional root page ID to scope the check to a subtree (folder). " +
"Only pages under this parent will be checked.",
),
},
},
// Schema + description now live in the shared registry (#294). The execute body
// keeps this transport's own guard rejecting an unparseable `since` timestamp.
registerShared(
SHARED_TOOL_SPECS.checkNewComments,
async ({ spaceId, since, parentPageId }) => {
// Reject an unparseable timestamp up front: otherwise the comparison
// against NaN silently treats every comment as "not new" and the tool
+171
View File
@@ -509,4 +509,175 @@ export const SHARED_TOOL_SPECS = {
pageId: z.string().min(1),
}),
},
// --- comment tools (unified from the per-layer inline definitions, #294) ---
//
// create_comment and resolve_comment previously carried a "per-transport
// divergence" note in BOTH layers; #294 unifies their schema + description
// here. Only the four tools that genuinely exist in BOTH layers live in the
// registry: create/list/resolve comment and check_new_comments.
//
// update_comment and delete_comment are intentionally NOT here: they exist
// ONLY on the standalone MCP server. The in-app agent deliberately exposes no
// hard comment edit/delete tool (comment edits are irreversible / not
// version-tracked; see the guardrail tests in ai-chat-tools.service.spec.ts),
// so there is nothing to unify — they stay inline in index.ts.
createComment: {
mcpName: 'create_comment',
inAppKey: 'createComment',
// CANONICAL: the in-app copy (the more-maintained one). It keeps the same
// rules as the MCP copy — inline-only, top-level requires a `selection`, no
// page-level comments, replies inherit the anchor, suggestedText must be
// unique — and adds the "retry with a corrected EXACT selection" and reply-
// to-reply-rejected guidance the MCP copy lacked. Execute-side validation
// (reject suggestedText on a reply, require a selection) stays per-layer.
description:
'Add an INLINE comment to a page, or reply to an existing top-level ' +
'comment (one level only — the backend rejects replies to replies). ' +
'The comment is anchored inline to the given exact `selection` text ' +
'(which gets highlighted); page-level comments are NOT supported. A ' +
'new top-level comment REQUIRES a `selection`. Replies inherit the ' +
"parent's anchor and take no selection. If the call fails with a " +
'"selection not found" error, retry with a corrected EXACT selection ' +
'copied verbatim from a single paragraph/block. You may also attach a ' +
'`suggestedText` proposing a replacement for the `selection` (a human ' +
'applies it from the UI); when set, the `selection` must occur exactly ' +
'once in the page. Reversible via the comment UI.',
tier: 'core',
catalogLine:
'createComment — add an inline comment (optionally with a suggested edit).',
// Reconciled schema: the field set is identical across both layers; the
// only constraint drift is `content`, which the MCP copy pinned to
// .min(1) while the in-app copy left unbounded — the stricter MCP form is
// kept (an empty comment body is never valid).
buildShape: (z) => ({
pageId: z.string().describe('The id of the page to comment on.'),
content: z.string().min(1).describe('The comment body as Markdown.'),
selection: z
.string()
.min(1)
.max(250)
.optional()
.describe(
'EXACT contiguous text from a SINGLE paragraph/block to anchor ' +
'(highlight) the comment on (<=250 chars, avoid spanning across ' +
'formatting boundaries). Required for a new top-level comment; ' +
'omit only when replying via parentCommentId.',
),
parentCommentId: z
.string()
.optional()
.describe(
'Optional id of a TOP-LEVEL comment to reply to (one level ' +
'of replies only).',
),
suggestedText: z
.string()
.min(1)
.max(2000)
.optional()
.describe(
'Optional proposed replacement (PLAIN TEXT) for the `selection`, ' +
'applied by a human via the UI (never auto-applied). REQUIRES a ' +
'`selection`; NOT allowed on a reply. When set, the `selection` ' +
'must be UNIQUE in the page — expand it with surrounding context ' +
'(still <=250 chars) if it occurs more than once, or the call is ' +
'refused.',
),
}),
},
listComments: {
mcpName: 'list_comments',
inAppKey: 'listComments',
// CANONICAL: the two copies are near-identical; the MCP copy is the
// superset (it keeps the "(pagination is handled internally)" note the
// in-app copy dropped), so it is used verbatim.
description:
'List comments on a page in one call (pagination is handled ' +
'internally). By DEFAULT only ACTIVE threads are returned; resolved ' +
'threads (a resolved top-level comment and all its replies) are hidden ' +
'and their count reported as `resolvedThreadsHidden` so you can re-query ' +
'with `includeResolved: true` to see everything. Returns ' +
'`{ items, resolvedThreadsHidden }`. Content is returned as Markdown.',
tier: 'core',
catalogLine:
'listComments — list all comments on a page (including resolved).',
buildShape: (z) => ({
pageId: z.string().describe('ID of the page'),
includeResolved: z
.boolean()
.optional()
.describe('default only active threads; true — include resolved'),
}),
},
resolveComment: {
mcpName: 'resolve_comment',
inAppKey: 'resolveComment',
// CANONICAL: the MCP copy's richer wording, minus its snake_case reference
// to `delete_comment` (a sibling tool that does NOT exist in the in-app
// layer) — rephrased transport-neutrally per the registry convention.
description:
'Resolve (close) or reopen a top-level comment thread (reversible — ' +
'pass resolved=false to reopen). Only top-level comments can be ' +
'resolved; the server rejects resolving a reply. Resolving keeps the ' +
'thread and its replies intact (it is not a deletion).',
tier: 'core',
catalogLine: 'resolveComment — resolve or reopen a comment thread.',
// Reconciled schema: `resolved` drifted — the MCP copy made it optional
// with .default(true) (resolve is the common case, documented), the in-app
// copy made it required. The MCP form is kept (a strict superset: it never
// rejects a previously-valid input and adds a sensible default), and
// commentId keeps the MCP copy's stricter .min(1).
buildShape: (z) => ({
commentId: z
.string()
.min(1)
.describe('ID of the top-level comment thread to resolve or reopen'),
resolved: z
.boolean()
.optional()
.default(true)
.describe(
'true (default) marks the thread resolved/closed; false reopens it',
),
}),
},
checkNewComments: {
mcpName: 'check_new_comments',
inAppKey: 'checkNewComments',
// CANONICAL: the MCP copy (the more detailed of the two). The MCP layer's
// execute-side guard that rejects an unparseable `since` timestamp stays in
// its execute body (per-layer logic), not in the shared schema.
description:
'Check for new comments across pages in a space since a given ' +
'timestamp. Optionally scope to a page subtree (folder). Returns only ' +
'comments created after the specified time.',
tier: 'deferred',
catalogLine:
'checkNewComments — find comments in a space created after a timestamp.',
// Reconciled schema: `since` keeps the MCP copy's stricter .min(1) (the
// in-app copy left it unbounded); field descriptions use the MCP copy's
// more detailed wording (it carries an example timestamp).
buildShape: (z) => ({
spaceId: z.string().describe('Space ID to check for new comments'),
since: z
.string()
.min(1)
.describe(
"ISO 8601 timestamp — only return comments created after this time " +
"(e.g. '2026-03-10T00:00:00Z')",
),
parentPageId: z
.string()
.optional()
.describe(
'Optional root page ID to scope the check to a subtree (folder). ' +
'Only pages under this parent will be checked.',
),
}),
},
} satisfies Record<string, SharedToolSpec>;