Skip to content

Commit 8a04896

Browse files
bjohascaio-pizzolclaude
authored
fix(mcp): rename wire fields to contract names before dispatch (#3215)
* fix(mcp): rename wire fields to contract names before dispatch The MCP wire schema (generated from apps/cli operation params) exposes PARAM_FLAG_OVERRIDES renames such as `commentId` → `id` and `parentCommentId` → `parentId`. Without an inverse translation at dispatch time, the contract validator rejects the wire field names because it expects the canonical contract names — so `superdoc_comment` actions `delete`/`get`/`update` fail with `comments.<action> commentId must be a non-empty string` even when the caller follows the documented schema. The CLI applies the inverse rename in extractInvokeInput() (apps/cli/src/lib/invoke-input.ts PARAM_RENAMES). This commit adds the matching layer for the MCP path: a small `applyParamRenames` helper in a standalone module, applied in `executeOperation` immediately before `api.invoke`. Affected operations: comments.create (parentId → parentCommentId), comments.patch / comments.delete / comments.get (id → commentId), getNodeById (id → nodeId). Includes unit tests for the rename helper. * review: fix comment count/style and document output-rename asymmetry - param-renames.ts: fix "Three" → "Five" (5 operations, not 3) and replace em-dash per style rule (caio-pizzol review suggestion). - intent.ts: add AIDEV-NOTE explaining why input renames are not inverted on output — canonical names in responses are usable directly, so inverting adds complexity for no practical benefit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * review: clarify output-rename asymmetry is incidental, not deliberate Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent cb1330a commit 8a04896

3 files changed

Lines changed: 86 additions & 1 deletion

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { describe, it, expect } from 'bun:test';
2+
import { applyParamRenames } from '../tools/param-renames.js';
3+
4+
describe('applyParamRenames', () => {
5+
it('renames id → commentId for comments.delete', () => {
6+
expect(applyParamRenames('comments.delete', { id: 'c1' })).toEqual({ commentId: 'c1' });
7+
});
8+
9+
it('renames id → commentId for comments.get', () => {
10+
expect(applyParamRenames('comments.get', { id: 'c1' })).toEqual({ commentId: 'c1' });
11+
});
12+
13+
it('renames id → commentId for comments.patch and preserves other fields', () => {
14+
expect(applyParamRenames('comments.patch', { id: 'c1', text: 'updated', isInternal: true })).toEqual({
15+
commentId: 'c1',
16+
text: 'updated',
17+
isInternal: true,
18+
});
19+
});
20+
21+
it('renames parentId → parentCommentId for comments.create', () => {
22+
expect(applyParamRenames('comments.create', { text: 'reply', parentId: 'c1' })).toEqual({
23+
text: 'reply',
24+
parentCommentId: 'c1',
25+
});
26+
});
27+
28+
it('renames id → nodeId for getNodeById', () => {
29+
expect(applyParamRenames('getNodeById', { id: 'n1' })).toEqual({ nodeId: 'n1' });
30+
});
31+
32+
it('passes through unchanged for operations with no renames', () => {
33+
expect(applyParamRenames('comments.list', { limit: 10 })).toEqual({ limit: 10 });
34+
expect(applyParamRenames('getText', {})).toEqual({});
35+
});
36+
37+
it('does not strip non-renamed keys when a rename map exists', () => {
38+
expect(applyParamRenames('comments.create', { text: 'top-level' })).toEqual({ text: 'top-level' });
39+
});
40+
});

apps/mcp/src/tools/intent.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type { SessionManager } from '../session-manager.js';
1212
import type { DocumentApi, DynamicInvokeRequest } from '@superdoc/document-api';
1313
import { MCP_TOOL_CATALOG } from '../generated/catalog.js';
1414
import { dispatchIntentTool } from '../generated/intent-dispatch.generated.js';
15+
import { applyParamRenames } from './param-renames.js';
1516

1617
// ---------------------------------------------------------------------------
1718
// Types for the generated catalog
@@ -121,7 +122,14 @@ function buildZodSchema(tool: CatalogTool): Record<string, z.ZodTypeAny> {
121122
function executeOperation(api: DocumentApi, operationId: string, input: Record<string, unknown>): unknown {
122123
// Generated dispatch uses 'doc.' prefix (e.g. 'doc.query.match'); strip it for DocumentApi.invoke()
123124
const opId = operationId.startsWith('doc.') ? operationId.slice(4) : operationId;
124-
return api.invoke({ operationId: opId, input } as DynamicInvokeRequest);
125+
// AIDEV-NOTE: renames are applied on input only — output is not inverted.
126+
// This is incidental rather than deliberate: the fix only needed to address
127+
// the input path. The asymmetry is harmless: the contract response uses
128+
// canonical names (e.g. commentId), and agents can use either name on
129+
// subsequent calls (canonical names pass through PARAM_RENAMES unchanged;
130+
// short names are renamed here). A follow-up could invert on output for
131+
// symmetry, but it is not load-bearing.
132+
return api.invoke({ operationId: opId, input: applyParamRenames(opId, input) } as DynamicInvokeRequest);
125133
}
126134

127135
// ---------------------------------------------------------------------------
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
* Wire → contract field-name renames for intent tool dispatch.
3+
*
4+
* The MCP wire schema (apps/mcp/src/generated/catalog.ts) is generated from
5+
* apps/cli/src/cli/operation-params.ts, where PARAM_FLAG_OVERRIDES rewrites
6+
* a handful of contract field names to shorter CLI flag names (e.g.
7+
* `commentId` → `id`, `parentCommentId` → `parentId`). The same renamed
8+
* names end up in the MCP wire schema. Without an inverse translation at
9+
* dispatch time, the contract validator rejects the input because it expects
10+
* the canonical field names.
11+
*
12+
* The CLI applies the inverse rename in extractInvokeInput()
13+
* (apps/cli/src/lib/invoke-input.ts PARAM_RENAMES). This module is the MCP
14+
* mirror, kept in lockstep by hand. Five operations are affected today
15+
* (comments.delete, comments.get, comments.patch, comments.create's
16+
* parentId, and getNodeById's id), so duplication is small.
17+
*
18+
* Keys are bare operation IDs (no `doc.` prefix) to match the form
19+
* executeOperation passes to api.invoke().
20+
*/
21+
const PARAM_RENAMES: Record<string, Record<string, string>> = {
22+
getNodeById: { id: 'nodeId' },
23+
'comments.create': { parentId: 'parentCommentId' },
24+
'comments.patch': { id: 'commentId' },
25+
'comments.delete': { id: 'commentId' },
26+
'comments.get': { id: 'commentId' },
27+
};
28+
29+
export function applyParamRenames(opId: string, input: Record<string, unknown>): Record<string, unknown> {
30+
const renames = PARAM_RENAMES[opId];
31+
if (!renames) return input;
32+
const renamed: Record<string, unknown> = {};
33+
for (const [key, value] of Object.entries(input)) {
34+
renamed[renames[key] ?? key] = value;
35+
}
36+
return renamed;
37+
}

0 commit comments

Comments
 (0)