Skip to content

mcp: convert tool/prompt schemas eagerly at registration time #1861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ravyg wants to merge 2 commits into
base: main
Choose a base branch
from
+362 −7
Open

mcp: convert tool/prompt schemas eagerly at registration time #1861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2edee91
mcp: convert tool/prompt schemas eagerly at registration time
ravyg Apr 8, 2026
978677f
fix: compute schema conversions before mutating state in update()
ravyg Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 52 additions & 7 deletions packages/server/src/server/mcp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,20 +137,20 @@ export class McpServer {
tools: Object.entries(this._registeredTools)
.filter(([, tool]) => tool.enabled)
.map(([name, tool]): Tool => {
// Use the JSON Schema cached at registration / update
// time instead of re-converting on every request.
const toolDefinition: Tool = {
name,
title: tool.title,
description: tool.description,
inputSchema: tool.inputSchema
? (standardSchemaToJsonSchema(tool.inputSchema, 'input') as Tool['inputSchema'])
: EMPTY_OBJECT_JSON_SCHEMA,
inputSchema: tool.inputJsonSchema ?? EMPTY_OBJECT_JSON_SCHEMA,
annotations: tool.annotations,
execution: tool.execution,
_meta: tool._meta
};

if (tool.outputSchema) {
toolDefinition.outputSchema = standardSchemaToJsonSchema(tool.outputSchema, 'output') as Tool['outputSchema'];
if (tool.outputJsonSchema) {
toolDefinition.outputSchema = tool.outputJsonSchema;
}

return toolDefinition;
Expand Down Expand Up @@ -526,11 +526,13 @@ export class McpServer {
prompts: Object.entries(this._registeredPrompts)
.filter(([, prompt]) => prompt.enabled)
.map(([name, prompt]): Prompt => {
// Use the prompt arguments cached at registration /
// update time instead of recomputing on every request.
return {
name,
title: prompt.title,
description: prompt.description,
arguments: prompt.argsSchema ? promptArgumentsFromStandardSchema(prompt.argsSchema) : undefined,
arguments: prompt.cachedArguments,
_meta: prompt._meta
};
})
Expand Down Expand Up @@ -703,6 +705,11 @@ export class McpServer {
callback: PromptCallback<StandardSchemaWithJSON | undefined>,
_meta: Record<string, unknown> | undefined
): RegisteredPrompt {
// Compute prompt arguments eagerly so any schema-conversion errors
// surface at registration time and we don't recompute on every
// `prompts/list` request.
const cachedArguments = argsSchema ? promptArgumentsFromStandardSchema(argsSchema) : undefined;

// Track current schema and callback for handler regeneration
let currentArgsSchema = argsSchema;
let currentCallback = callback;
Expand All @@ -711,6 +718,7 @@ export class McpServer {
title,
description,
argsSchema,
cachedArguments,
_meta,
handler: createPromptHandler(name, argsSchema, callback),
enabled: true,
Expand All @@ -729,7 +737,10 @@ export class McpServer {
// Track if we need to regenerate the handler
let needsHandlerRegen = false;
if (updates.argsSchema !== undefined) {
// Compute before mutating so state stays consistent if conversion throws.
const newCachedArguments = promptArgumentsFromStandardSchema(updates.argsSchema);
registeredPrompt.argsSchema = updates.argsSchema;
registeredPrompt.cachedArguments = newCachedArguments;
currentArgsSchema = updates.argsSchema;
needsHandlerRegen = true;
}
Expand Down Expand Up @@ -778,6 +789,12 @@ export class McpServer {
// Validate tool name according to SEP specification
validateAndWarnToolName(name);

// Convert schemas to JSON Schema eagerly so any errors (e.g. cycle
// detection) surface at registration time rather than on the first
// `tools/list` request, and so we don't re-convert on every list call.
const inputJsonSchema = inputSchema ? (standardSchemaToJsonSchema(inputSchema, 'input') as Tool['inputSchema']) : undefined;
const outputJsonSchema = outputSchema ? (standardSchemaToJsonSchema(outputSchema, 'output') as Tool['outputSchema']) : undefined;

// Track current handler for executor regeneration
let currentHandler = handler;

Expand All @@ -786,6 +803,8 @@ export class McpServer {
description,
inputSchema,
outputSchema,
inputJsonSchema,
outputJsonSchema,
annotations,
execution,
_meta,
Expand All @@ -809,7 +828,10 @@ export class McpServer {
// Track if we need to regenerate the executor
let needsExecutorRegen = false;
if (updates.paramsSchema !== undefined) {
// Compute before mutating so state stays consistent if conversion throws.
const newInputJsonSchema = standardSchemaToJsonSchema(updates.paramsSchema, 'input') as Tool['inputSchema'];
registeredTool.inputSchema = updates.paramsSchema;
registeredTool.inputJsonSchema = newInputJsonSchema;
needsExecutorRegen = true;
}
if (updates.callback !== undefined) {
Comment on lines 829 to 837
Copy link
Copy Markdown

@claude claude bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Both tool.update() and prompt.update() mutate the registered object's schema field before calling the potentially-throwing conversion function, leaving the object in an inconsistent state if conversion fails. After a failed update, argsSchema/inputSchema holds the new broken schema while cachedArguments/inputJsonSchema remain stale, needsExecutorRegen/needsHandlerRegen are never set (executor/handler not regenerated), and any concurrent callback change is silently dropped; fix by computing the new JSON schema/arguments before mutating state.

Extended reasoning...

What the bug is and how it manifests

In both _createRegisteredTool.update() and _createRegisteredPrompt.update(), this PR introduces a pattern where the registered object's schema field is mutated before the conversion function that can throw is called. If the conversion function throws (e.g., cycle detection from #1563, or any other schema validation error), the object is left in a partially-updated, inconsistent state.

The specific code paths that trigger it

For tools (lines 829–835 in the new code):

if (updates.paramsSchema \!== undefined) {
    registeredTool.inputSchema = updates.paramsSchema;         // (1) mutated
    registeredTool.inputJsonSchema = standardSchemaToJsonSchema(updates.paramsSchema, 'input') // (2) can throw
    needsExecutorRegen = true;                                 // (3) never reached
}
if (updates.callback \!== undefined) { ... }                   // (4) never reached

For prompts (lines 738–744 in the new code):

if (updates.argsSchema \!== undefined) {
    registeredPrompt.argsSchema = updates.argsSchema;         // (1) mutated
    registeredPrompt.cachedArguments = promptArgumentsFromStandardSchema(updates.argsSchema); // (2) can throw
    currentArgsSchema = updates.argsSchema;                   // (3) never reached
    needsHandlerRegen = true;                                 // (4) never reached
}

Why existing code doesn't prevent it

Before this PR, update() never called standardSchemaToJsonSchema or promptArgumentsFromStandardSchema — those conversions happened lazily in the list handlers. This PR moves the conversion into update() (correctly) but forgets to guard the mutation against conversion failure.

Impact

After a failed tool.update({ paramsSchema: cycleSchema, callback: newCb }):

  • inputSchema = new cyclic schema (broken)
  • inputJsonSchema = stale old JSON schema → tools/list serves wrong schema
  • needsExecutorRegen never set → executor NOT regenerated
  • callback update silently dropped (the if (updates.callback)… block is after the throw and never reached)
  • validateToolInput reads tool.inputSchema directly, so subsequent tools/call requests will attempt to validate against the new cyclic/broken schema

After a failed prompt.update({ argsSchema: badSchema }):

  • argsSchema = new broken schema
  • cachedArguments = stale old arguments → prompts/list serves wrong arguments
  • currentArgsSchema (closure variable used for future handler regeneration, line 743) is never updated, so any later update({ callback }) will regenerate the handler with the old schema while registeredPrompt.argsSchema says the new one — a persistent inconsistency that outlives the failing call

The same mutation-before-throw pattern at lines 845–848 for outputSchema leaves outputSchema updated while outputJsonSchema remains stale.

Step-by-step proof

  1. const tool = server.registerTool('foo', { inputSchema: zodGoodSchema }, cb)
    → tool.inputSchema = goodSchema, tool.inputJsonSchema = good JSON, executor uses goodSchema
  2. tool.update({ paramsSchema: zodCycleSchema, callback: newCb })
    → line 830: tool.inputSchema = cycleSchema ✓ (mutated)
    → line 833: standardSchemaToJsonSchema(cycleSchema, 'input') throws (cycle detection)
    → exception propagates; lines 834+ never execute
  3. State after throw: inputSchema = cycleSchema, inputJsonSchema = good JSON (stale), executor still uses goodSchema, newCb never installed
  4. tools/list → returns good JSON schema (stale) — appears fine
  5. tools/call → validateToolInput reads tool.inputSchema (cycleSchema) → crash or unexpected validation behavior

How to fix it

Compute the new value before mutating state:

if (updates.paramsSchema \!== undefined) {
    const newInputJsonSchema = standardSchemaToJsonSchema(updates.paramsSchema, 'input'); // may throw — state untouched
    registeredTool.inputSchema = updates.paramsSchema;
    registeredTool.inputJsonSchema = newInputJsonSchema as Tool['inputSchema'];
    needsExecutorRegen = true;
}

Apply the same pattern for outputSchema and for registeredPrompt.argsSchema / cachedArguments.

Expand All @@ -821,7 +843,12 @@ export class McpServer {
registeredTool.executor = createToolExecutor(registeredTool.inputSchema, currentHandler);
}

if (updates.outputSchema !== undefined) registeredTool.outputSchema = updates.outputSchema;
if (updates.outputSchema !== undefined) {
// Compute before mutating so state stays consistent if conversion throws.
const newOutputJsonSchema = standardSchemaToJsonSchema(updates.outputSchema, 'output') as Tool['outputSchema'];
registeredTool.outputSchema = updates.outputSchema;
registeredTool.outputJsonSchema = newOutputJsonSchema;
}
if (updates.annotations !== undefined) registeredTool.annotations = updates.annotations;
if (updates._meta !== undefined) registeredTool._meta = updates._meta;
if (updates.enabled !== undefined) registeredTool.enabled = updates.enabled;
Expand Down Expand Up @@ -1094,6 +1121,18 @@ export type RegisteredTool = {
description?: string;
inputSchema?: StandardSchemaWithJSON;
outputSchema?: StandardSchemaWithJSON;
/**
* @hidden
* Cached JSON Schema computed from `inputSchema` at registration time.
* Re-computed when `update({ paramsSchema })` is called.
*/
inputJsonSchema?: Tool['inputSchema'];
/**
* @hidden
* Cached JSON Schema computed from `outputSchema` at registration time.
* Re-computed when `update({ outputSchema })` is called.
*/
outputJsonSchema?: Tool['outputSchema'];
annotations?: ToolAnnotations;
execution?: ToolExecution;
_meta?: Record<string, unknown>;
Expand Down Expand Up @@ -1240,6 +1279,12 @@ export type RegisteredPrompt = {
title?: string;
description?: string;
argsSchema?: StandardSchemaWithJSON;
/**
* @hidden
* Cached prompt arguments computed from `argsSchema` at registration time.
* Re-computed when `update({ argsSchema })` is called.
*/
cachedArguments?: Prompt['arguments'];
_meta?: Record<string, unknown>;
/** @hidden */
handler: PromptHandler;
Expand Down
Loading
Loading