diff --git a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts index 21933219..81b1f450 100644 --- a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts +++ b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts @@ -601,21 +601,26 @@ export class AiChatToolsService { }), ), + // NOT shared (kept inline): the MCP tool name `table_get` is noun-first + // while this key is `getTable` (verb-first), breaking the + // snake_case(inAppKey) convention the shared registry enforces. Its + // reference parameter is still named `table` (was `tableRef`) so it matches + // the migrated table row/cell tools below. getTable: tool({ description: 'Read a table as a matrix of cell texts (plus a parallel cellIds ' + 'matrix so cells can be addressed for rich edits).', inputSchema: modelFriendlyInput({ pageId: z.string().describe('The id of the page.'), - tableRef: z + table: z .string() .describe( - '"#" from getOutline, or a block id of any node inside ' + - 'the table.', + '"#" from the page outline, or a block id of any node ' + + 'inside the table.', ), }), - execute: async ({ pageId, tableRef }) => - await client.getTable(pageId, tableRef), + execute: async ({ pageId, table }) => + await client.getTable(pageId, table), }), // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294). @@ -766,64 +771,27 @@ export class AiChatToolsService { }, }), - // NOT in the shared registry: this layer names the table argument - // `tableRef`, while the standalone MCP tool names it `table` (index.ts). - // Sharing one buildShape would rename a model-facing parameter on one - // transport, so the table row/cell tools stay per-layer by design. - tableInsertRow: tool({ - description: - 'Insert a row of plain-text cells into a table. Reversible via ' + - 'page history.', - inputSchema: modelFriendlyInput({ - pageId: z.string().describe('The id of the page.'), - tableRef: z - .string() - .describe('"#" from getOutline, or a block id in the table.'), - cells: z.array(z.string()).describe('The cell texts for the row.'), - index: z - .number() - .int() - .optional() - .describe('0-based insert position (omit/out-of-range to append).'), - }), - execute: async ({ pageId, tableRef, cells, index }) => - await client.tableInsertRow(pageId, tableRef, cells, index), - }), + // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294). + // The table reference parameter was unified to `table` (was `tableRef`). + tableInsertRow: sharedTool( + sharedToolSpecs.tableInsertRow, + async ({ pageId, table, cells, index }) => + await client.tableInsertRow(pageId, table, cells, index), + ), - // NOT shared — same `tableRef` (here) vs `table` (MCP) parameter-name - // divergence as tableInsertRow. - tableDeleteRow: tool({ - description: - 'Delete a table row at a 0-based index. Reversible via page history.', - inputSchema: modelFriendlyInput({ - pageId: z.string().describe('The id of the page.'), - tableRef: z - .string() - .describe('"#" from getOutline, or a block id in the table.'), - index: z.number().int().describe('0-based row index to delete.'), - }), - execute: async ({ pageId, tableRef, index }) => - await client.tableDeleteRow(pageId, tableRef, index), - }), + // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294). + tableDeleteRow: sharedTool( + sharedToolSpecs.tableDeleteRow, + async ({ pageId, table, index }) => + await client.tableDeleteRow(pageId, table, index), + ), - // NOT shared — same `tableRef` (here) vs `table` (MCP) parameter-name - // divergence as tableInsertRow. - tableUpdateCell: tool({ - description: - 'Set the plain-text content of a table cell at [row, col] (0-based). ' + - 'Reversible via page history.', - inputSchema: modelFriendlyInput({ - pageId: z.string().describe('The id of the page.'), - tableRef: z - .string() - .describe('"#" from getOutline, or a block id in the table.'), - row: z.number().int().describe('0-based row index.'), - col: z.number().int().describe('0-based column index.'), - text: z.string().describe('The new cell text.'), - }), - execute: async ({ pageId, tableRef, row, col, text }) => - await client.tableUpdateCell(pageId, tableRef, row, col, text), - }), + // Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294). + tableUpdateCell: sharedTool( + sharedToolSpecs.tableUpdateCell, + async ({ pageId, table, row, col, text }) => + await client.tableUpdateCell(pageId, table, row, col, text), + ), copyPageContent: sharedTool( sharedToolSpecs.copyPageContent, diff --git a/apps/server/src/core/ai-chat/tools/tool-tiers.ts b/apps/server/src/core/ai-chat/tools/tool-tiers.ts index 5a5f8cde..e021f1af 100644 --- a/apps/server/src/core/ai-chat/tools/tool-tiers.ts +++ b/apps/server/src/core/ai-chat/tools/tool-tiers.ts @@ -148,6 +148,10 @@ export const INLINE_TOOL_TIERS: Record< tier: 'deferred', catalogLine: 'getTable — read a table as a matrix of cell texts and cell ids.', }, + // NOTE: tableInsertRow, tableDeleteRow and tableUpdateCell moved to + // @docmost/mcp's SHARED_TOOL_SPECS (#294); they carry their own deferred tier + + // catalogLine there. getTable stays inline (its MCP name table_get breaks the + // snake_case(inAppKey) convention, so it has no shared spec). // NOTE: checkNewComments moved to @docmost/mcp's SHARED_TOOL_SPECS (#294); // it carries its own deferred tier + catalogLine there. getPageHistory: { @@ -165,18 +169,6 @@ export const INLINE_TOOL_TIERS: Record< catalogLine: "updatePageJson — overwrite a page's body with a full ProseMirror document.", }, - tableInsertRow: { - tier: 'deferred', - catalogLine: 'tableInsertRow — insert a row of plain-text cells into a table.', - }, - tableDeleteRow: { - tier: 'deferred', - catalogLine: 'tableDeleteRow — delete a table row at a 0-based index.', - }, - tableUpdateCell: { - tier: 'deferred', - catalogLine: 'tableUpdateCell — set the text of a table cell at [row, col].', - }, sharePage: { tier: 'deferred', catalogLine: 'sharePage — make a page publicly accessible and return its URL.', diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index ccb35005..90137182 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -201,6 +201,11 @@ registerShared( ); // Tool: table_get +// Tool: table_get +// NOT in the shared registry: the MCP tool name `table_get` is noun-first while +// the in-app key is `getTable` (verb-first), breaking the snake_case(inAppKey) +// convention the shared registry enforces (shared-tool-specs.contract.spec.ts). +// Renaming the public MCP tool would break external clients, so it stays inline. server.registerTool( "table_get", { @@ -223,25 +228,10 @@ server.registerTool( ); // Tool: table_insert_row -// NOT in the shared registry: this transport names the table argument `table`, -// while the in-app tool names it `tableRef` (ai-chat-tools.service.ts). Sharing -// one buildShape would rename a public MCP parameter, so the table row/cell -// tools stay per-transport by design. -server.registerTool( - "table_insert_row", - { - description: - "Insert a row of plain-text cells into a table. `table` = `#` or " + - "a block id inside it. `cells` = text per column (padded to the table's " + - "column count; error if more cells than columns). `index` = 0-based " + - "insert position (0 inserts before the header); omit to append at the end.", - inputSchema: { - pageId: z.string().min(1), - table: z.string().min(1), - cells: z.array(z.string()), - index: z.number().int().optional(), - }, - }, +// Schema + description now live in the shared registry (#294); the `table` +// parameter name is the canonical one (the in-app layer was unified to it). +registerShared( + SHARED_TOOL_SPECS.tableInsertRow, async ({ pageId, table, cells, index }) => { const result = await docmostClient.tableInsertRow( pageId, @@ -254,22 +244,9 @@ server.registerTool( ); // Tool: table_delete_row -// NOT shared — same `table` (here) vs `tableRef` (in-app) parameter-name -// divergence as table_insert_row. -server.registerTool( - "table_delete_row", - { - description: - "Delete the row at 0-based `index` from a table (`table` = `#` or " + - "a block id inside it). Refuses to delete the table's only row. An " + - "out-of-range `index` throws. Deleting `index` 0 removes the header row, " + - "and the next row becomes the new header.", - inputSchema: { - pageId: z.string().min(1), - table: z.string().min(1), - index: z.number().int(), - }, - }, +// Schema + description now live in the shared registry (#294). +registerShared( + SHARED_TOOL_SPECS.tableDeleteRow, async ({ pageId, table, index }) => { const result = await docmostClient.tableDeleteRow(pageId, table, index); return jsonContent(result); @@ -277,24 +254,9 @@ server.registerTool( ); // Tool: table_update_cell -// NOT shared — same `table` (here) vs `tableRef` (in-app) parameter-name -// divergence as table_insert_row. -server.registerTool( - "table_update_cell", - { - description: - "Set the plain-text content of cell [row,col] (0-based) in a table " + - "(`table` = `#` or a block id inside it). Replaces the cell's " + - "content with a single text paragraph; for rich formatting use patch_node " + - "on the cell's paragraph id from table_get.", - inputSchema: { - pageId: z.string().min(1), - table: z.string().min(1), - row: z.number().int(), - col: z.number().int(), - text: z.string(), - }, - }, +// Schema + description now live in the shared registry (#294). +registerShared( + SHARED_TOOL_SPECS.tableUpdateCell, async ({ pageId, table, row, col, text }) => { const result = await docmostClient.tableUpdateCell( pageId, diff --git a/packages/mcp/src/tool-specs.ts b/packages/mcp/src/tool-specs.ts index a2c80e90..670e6f97 100644 --- a/packages/mcp/src/tool-specs.ts +++ b/packages/mcp/src/tool-specs.ts @@ -680,4 +680,94 @@ export const SHARED_TOOL_SPECS = { ), }), }, + + // --- table tools (unified from the per-layer inline definitions, #294) --- + // + // These tools carried a "NOT shared" note in BOTH layers because of a single + // parameter-NAME drift: the MCP layer named the table reference `table` while + // the in-app layer named it `tableRef`. #294 reconciles that drift by unifying + // on the MCP name `table` — renaming the MCP public parameter would break + // external MCP clients, whereas the in-app parameter is model-facing + // (prompt-only) and safe to rename. The in-app execute bodies now destructure + // `table` instead of `tableRef` (nothing else changes). Descriptions take the + // MCP copy's richer wording (it documented `#`, padding, header-row + // behavior) plus the in-app copy's "Reversible via page history" note; sibling + // tool references are phrased transport-neutrally. + // + // NOT here (kept inline in index.ts): table_get / getTable. Its MCP tool name + // is noun-first (`table_get`) while the in-app key is verb-first (`getTable`), + // so it 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 (its in-app param was still + // aligned to `table` for consistency with the migrated trio below). + + tableInsertRow: { + mcpName: 'table_insert_row', + inAppKey: 'tableInsertRow', + description: + 'Insert a row of plain-text cells into a table. `table` is `#` ' + + 'from the page outline, or a block id inside it. `cells` is the text per ' + + "column (padded to the table's column count; an error if more cells than " + + 'columns). `index` is the 0-based insert position (0 inserts before the ' + + 'header); omit to append at the end. Reversible via page history.', + tier: 'deferred', + catalogLine: 'tableInsertRow — insert a row of plain-text cells into a table.', + buildShape: (z) => ({ + pageId: z.string().min(1).describe('The id of the page.'), + table: z + .string() + .min(1) + .describe('"#" from the page outline, or a block id in the table.'), + cells: z.array(z.string()).describe('The cell texts for the row (one per column).'), + index: z + .number() + .int() + .optional() + .describe('0-based insert position (0 inserts before the header); omit to append.'), + }), + }, + + tableDeleteRow: { + mcpName: 'table_delete_row', + inAppKey: 'tableDeleteRow', + description: + 'Delete the row at 0-based `index` from a table (`table` is `#` ' + + 'from the page outline, or a block id inside it). Refuses to delete the ' + + "table's only row; an out-of-range `index` throws. Deleting `index` 0 " + + 'removes the header row, and the next row becomes the new header. ' + + 'Reversible via page history.', + tier: 'deferred', + catalogLine: 'tableDeleteRow — delete a table row at a 0-based index.', + buildShape: (z) => ({ + pageId: z.string().min(1).describe('The id of the page.'), + table: z + .string() + .min(1) + .describe('"#" from the page outline, or a block id in the table.'), + index: z.number().int().describe('0-based row index to delete.'), + }), + }, + + tableUpdateCell: { + mcpName: 'table_update_cell', + inAppKey: 'tableUpdateCell', + description: + 'Set the plain-text content of cell [row, col] (0-based) in a table ' + + '(`table` is `#` from the page outline, or a block id inside it). ' + + "Replaces the cell's content with a single text paragraph; for rich " + + "formatting, patch the cell's paragraph id (obtained from reading the " + + 'table) instead. Reversible via page history.', + tier: 'deferred', + catalogLine: 'tableUpdateCell — set the text of a table cell at [row, col].', + buildShape: (z) => ({ + pageId: z.string().min(1).describe('The id of the page.'), + table: z + .string() + .min(1) + .describe('"#" from the page outline, or a block id in the table.'), + row: z.number().int().describe('0-based row index.'), + col: z.number().int().describe('0-based column index.'), + text: z.string().describe('The new cell text.'), + }), + }, } satisfies Record;