From f4b1d8bb534e7e937d979fa2bf64f4c869b72038 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Thu, 10 Sep 2026 15:06:23 +0200 Subject: [PATCH] fix: clone a remote MCP schema before normalizing it Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JgzuxyafKNF2uaEeL35XQw --- .../copilot/chat/global/mcpTools.test.ts | 20 +++++++++++++++++++ .../copilot/chat/global/mcpTools.ts | 6 +++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts b/frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts index ca4f2a906d..5192dd8d2c 100644 --- a/frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts @@ -260,6 +260,26 @@ describe('loaded remote tools', () => { expect(loadedMcpTools(OWNER)).toEqual([]) }) + // The sanitized schema must not alias the listing cache: the normalize pass rewrites + // nested nodes in place, so a shallow copy would edit the entry `summarizeTool` and + // the failed-call schema echo read from, and that every other chat shares. + it('does not mutate the remote schema it was given', () => { + const remote = { + name: 'aliased', + description: 'x', + inputSchema: { + type: 'object', + properties: { a: { type: 'string', format: '' } }, + required: ['a', 'a'] + } + } as any + + registerMcpTools(OWNER, mcpRegistryGeneration(OWNER), server, [remote]) + + expect(remote.inputSchema.properties.a.format).toBe('') + expect(remote.inputSchema.required).toEqual(['a', 'a']) + }) + it('falls back to an empty object schema when the remote sends no usable one', () => { registerMcpTools(OWNER, mcpRegistryGeneration(OWNER), server, [ { name: 'nada', description: 'x', inputSchema: null } as any diff --git a/frontend/src/lib/components/copilot/chat/global/mcpTools.ts b/frontend/src/lib/components/copilot/chat/global/mcpTools.ts index 1b92fbb13f..844c9017dc 100644 --- a/frontend/src/lib/components/copilot/chat/global/mcpTools.ts +++ b/frontend/src/lib/components/copilot/chat/global/mcpTools.ts @@ -646,7 +646,11 @@ function safeInputSchema(schema: unknown): Record { ) ] : [] - const safe = { ...rest, type: 'object', properties, required } + // Cloned, not spread: the spread would leave `properties` and any `items`/`allOf` + // carried through `rest` pointing at the listing cache's own objects, and the + // normalize pass below rewrites nested nodes in place. A registered tool is + // supposed to be a frozen copy — sharing that structure makes it one in name only. + const safe = structuredClone({ ...rest, type: 'object', properties, required }) // The same pass `createToolDef` runs on every other tool, so a remote schema is not // the one that reaches a provider unnormalized. It recurses, which this does not: // Windmill's own MCP server emits `format: ""` on untyped fields.