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>
This commit is contained in:
agent_coder
@@ -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(
|
||||
'"#<index>" from getOutline, or a block id of any node inside ' +
|
||||
'the table.',
|
||||
'"#<index>" 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('"#<index>" 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('"#<index>" 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('"#<index>" 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,
|
||||
|
||||
@@ -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.',
|
||||
|
||||
+15
-53
@@ -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` = `#<index>` 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` = `#<index>` 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` = `#<index>` 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,
|
||||
|
||||
@@ -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 `#<index>`, 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 `#<index>` ' +
|
||||
'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('"#<index>" 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 `#<index>` ' +
|
||||
'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('"#<index>" 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 `#<index>` 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('"#<index>" 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<string, SharedToolSpec>;
|
||||
|
||||
Reference in New Issue
Block a user