Skip to content

Commit 978677f

Browse files
committed
fix: compute schema conversions before mutating state in update()
Prevents inconsistent state if standardSchemaToJsonSchema() or promptArgumentsFromStandardSchema() throws during tool/prompt update. Previously the registered object's schema field was mutated before the conversion call, leaving stale cached values on failure.
1 parent 2edee91 commit 978677f

2 files changed

Lines changed: 101 additions & 6 deletions

File tree

packages/server/src/server/mcp.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -737,9 +737,10 @@ export class McpServer {
737737
// Track if we need to regenerate the handler
738738
let needsHandlerRegen = false;
739739
if (updates.argsSchema !== undefined) {
740+
// Compute before mutating so state stays consistent if conversion throws.
741+
const newCachedArguments = promptArgumentsFromStandardSchema(updates.argsSchema);
740742
registeredPrompt.argsSchema = updates.argsSchema;
741-
// Re-cache prompt arguments alongside the schema update.
742-
registeredPrompt.cachedArguments = promptArgumentsFromStandardSchema(updates.argsSchema);
743+
registeredPrompt.cachedArguments = newCachedArguments;
743744
currentArgsSchema = updates.argsSchema;
744745
needsHandlerRegen = true;
745746
}
@@ -827,10 +828,10 @@ export class McpServer {
827828
// Track if we need to regenerate the executor
828829
let needsExecutorRegen = false;
829830
if (updates.paramsSchema !== undefined) {
831+
// Compute before mutating so state stays consistent if conversion throws.
832+
const newInputJsonSchema = standardSchemaToJsonSchema(updates.paramsSchema, 'input') as Tool['inputSchema'];
830833
registeredTool.inputSchema = updates.paramsSchema;
831-
// Re-cache the JSON Schema; surfaces conversion errors
832-
// synchronously like the initial registration does.
833-
registeredTool.inputJsonSchema = standardSchemaToJsonSchema(updates.paramsSchema, 'input') as Tool['inputSchema'];
834+
registeredTool.inputJsonSchema = newInputJsonSchema;
834835
needsExecutorRegen = true;
835836
}
836837
if (updates.callback !== undefined) {
@@ -843,8 +844,10 @@ export class McpServer {
843844
}
844845

845846
if (updates.outputSchema !== undefined) {
847+
// Compute before mutating so state stays consistent if conversion throws.
848+
const newOutputJsonSchema = standardSchemaToJsonSchema(updates.outputSchema, 'output') as Tool['outputSchema'];
846849
registeredTool.outputSchema = updates.outputSchema;
847-
registeredTool.outputJsonSchema = standardSchemaToJsonSchema(updates.outputSchema, 'output') as Tool['outputSchema'];
850+
registeredTool.outputJsonSchema = newOutputJsonSchema;
848851
}
849852
if (updates.annotations !== undefined) registeredTool.annotations = updates.annotations;
850853
if (updates._meta !== undefined) registeredTool._meta = updates._meta;

test/integration/test/server/mcp.test.ts

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2356,6 +2356,64 @@ describe('Zod v4', () => {
23562356
}
23572357
});
23582358
});
2359+
2360+
/***
2361+
* Test: tool.update() leaves state consistent when schema conversion throws (#1847)
2362+
*
2363+
* If the new schema is invalid (e.g. non-object type), the conversion
2364+
* throws. State must remain unchanged — no partial mutation.
2365+
*/
2366+
test('should not mutate tool state when paramsSchema conversion throws', async () => {
2367+
const mcpServer = new McpServer({
2368+
name: 'test server',
2369+
version: '1.0'
2370+
});
2371+
2372+
const originalCb = async () => ({
2373+
content: [{ type: 'text' as const, text: 'original' }]
2374+
});
2375+
2376+
const tool = mcpServer.registerTool('consistent', { inputSchema: z.object({ name: z.string() }) }, originalCb);
2377+
2378+
const originalInputSchema = tool.inputSchema;
2379+
const originalInputJsonSchema = tool.inputJsonSchema;
2380+
2381+
// z.string() is not an object schema, so conversion will throw.
2382+
expect(() => {
2383+
tool.update({
2384+
paramsSchema: z.string() as never,
2385+
callback: async () => ({ content: [{ type: 'text' as const, text: 'new' }] })
2386+
});
2387+
}).toThrow();
2388+
2389+
// State must be unchanged.
2390+
expect(tool.inputSchema).toBe(originalInputSchema);
2391+
expect(tool.inputJsonSchema).toBe(originalInputJsonSchema);
2392+
});
2393+
2394+
test('should not mutate tool state when outputSchema conversion throws', async () => {
2395+
const mcpServer = new McpServer({
2396+
name: 'test server',
2397+
version: '1.0'
2398+
});
2399+
2400+
const tool = mcpServer.registerTool('consistent-output', { outputSchema: z.object({ result: z.number() }) }, async () => ({
2401+
content: [{ type: 'text' as const, text: '' }],
2402+
structuredContent: { result: 1 }
2403+
}));
2404+
2405+
const originalOutputSchema = tool.outputSchema;
2406+
const originalOutputJsonSchema = tool.outputJsonSchema;
2407+
2408+
expect(() => {
2409+
tool.update({
2410+
outputSchema: z.string() as never
2411+
});
2412+
}).toThrow();
2413+
2414+
expect(tool.outputSchema).toBe(originalOutputSchema);
2415+
expect(tool.outputJsonSchema).toBe(originalOutputJsonSchema);
2416+
});
23592417
});
23602418

23612419
describe('resource()', () => {
@@ -4564,6 +4622,40 @@ describe('Zod v4', () => {
45644622
{ name: 'extra', description: undefined, required: false }
45654623
]);
45664624
});
4625+
4626+
/***
4627+
* Test: prompt.update() leaves state consistent when schema conversion throws (#1847)
4628+
*/
4629+
test('should not mutate prompt state when argsSchema conversion throws', async () => {
4630+
const mcpServer = new McpServer({
4631+
name: 'test server',
4632+
version: '1.0'
4633+
});
4634+
4635+
const prompt = mcpServer.registerPrompt(
4636+
'consistent-prompt',
4637+
{
4638+
argsSchema: z.object({ name: z.string() })
4639+
},
4640+
async () => ({
4641+
messages: [{ role: 'user' as const, content: { type: 'text' as const, text: '' } }]
4642+
})
4643+
);
4644+
4645+
const originalArgsSchema = prompt.argsSchema;
4646+
const originalCachedArguments = prompt.cachedArguments;
4647+
4648+
// z.string() is not an object schema, so conversion will throw.
4649+
expect(() => {
4650+
prompt.update({
4651+
argsSchema: z.string() as never
4652+
});
4653+
}).toThrow();
4654+
4655+
// State must be unchanged.
4656+
expect(prompt.argsSchema).toBe(originalArgsSchema);
4657+
expect(prompt.cachedArguments).toBe(originalCachedArguments);
4658+
});
45674659
});
45684660

45694661
describe('Tool title precedence', () => {

0 commit comments

Comments
 (0)