fix: let the MCP listing decide what a registered call runs

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JgzuxyafKNF2uaEeL35XQw

frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts

@@ -2253,7 +2253,16 @@ export class AIChatManager {
 	// a different server, and one the user has not opted into.
 	refreshMcpServers = async (workspace = this.operatingWorkspace ?? '') => {
 		const refreshId = ++this.mcpServersRefreshId
-		const servers = await loadMcpServers(workspace)
+		let servers: McpServer[]
+		try {
+			servers = await loadMcpServers(workspace)
+		} catch (e) {
+			// A listing that failed is not a workspace with nothing connected: reconciling
+			// against it would drop every registered tool and lift every refusal because a
+			// request happened to fail. Keep what the last good refresh settled on.
+			console.error('Failed to load MCP servers', e)
+			return
+		}
 		if (refreshId !== this.mcpServersRefreshId) {
 			return
 		}

frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts

@@ -549,6 +549,65 @@ describe('request rejections that withdraw registered tools', () => {
 	})
 })
 
+// A registration freezes what the remote said, but the remote can change a tool without
+// its Windmill resource being touched — the listing TTL is what bounds that everywhere else.
+describe('registrations against a changed remote', () => {
+	const server = SERVERS[0]
+
+	function callArgs() {
+		return {
+			args: { owner: 'windmill-labs', repo: 'windmill' },
+			workspace: 'test-ws',
+			helpers: {},
+			toolCallbacks: createToolCallbacks(),
+			toolId: 'tool-1'
+		} as any
+	}
+
+	it('refuses to send arguments built from a schema the remote has changed', async () => {
+		registerMcpTools(OWNER, mcpRegistryGeneration(OWNER), server, [TOOLS[0]])
+		const registered = loadedMcpTools(OWNER)[0]
+		getMcpToolsMock.mockResolvedValue([
+			{ ...TOOLS[0], inputSchema: { type: 'object', properties: { issue_id: { type: 'string' } } } }
+		])
+
+		const result = JSON.parse(await registered.fn(callArgs()))
+
+		expect(callMcpToolMock).not.toHaveBeenCalled()
+		expect(result.success).toBe(false)
+		expect(result.schema).toEqual({ type: 'object', properties: { issue_id: { type: 'string' } } })
+		// Dropped, so the next request stops advertising it and the model searches again.
+		expect(loadedMcpTools(OWNER)).toEqual([])
+	})
+
+	it('reports a tool the remote no longer exposes', async () => {
+		registerMcpTools(OWNER, mcpRegistryGeneration(OWNER), server, [TOOLS[0]])
+		const registered = loadedMcpTools(OWNER)[0]
+		getMcpToolsMock.mockResolvedValue([TOOLS[1]])
+
+		const result = JSON.parse(await registered.fn(callArgs()))
+
+		expect(callMcpToolMock).not.toHaveBeenCalled()
+		expect(result.success).toBe(false)
+		expect(result.error).toContain('no longer exposed')
+		expect(loadedMcpTools(OWNER)).toEqual([])
+	})
+
+	// The listing is a live call to a third party, and the backend re-asserts read_only
+	// on every call — so a blip must not take down a call the frozen copy can still make.
+	it('still calls on the frozen copy when the listing cannot be reached', async () => {
+		registerMcpTools(OWNER, mcpRegistryGeneration(OWNER), server, [TOOLS[0]])
+		const registered = loadedMcpTools(OWNER)[0]
+		getMcpToolsMock.mockRejectedValue(new Error('connection refused'))
+		callMcpToolMock.mockResolvedValue({ content: [{ type: 'text', text: 'ok' }] })
+
+		const result = JSON.parse(await registered.fn(callArgs()))
+
+		expect(result.success).toBe(true)
+		expect(callMcpToolMock).toHaveBeenCalled()
+	})
+})
+
 describe('read/write split', () => {
 	it('refuses a mutating tool on the read path', async () => {
 		const result = await run('call_mcp_read_tool', {
@@ -887,6 +946,15 @@ describe('loadMcpServers', () => {
 		expect(listResourceMock).not.toHaveBeenCalled()
 	})
 
+	// The caller reconciles registered tools and refusals against this list, so a failed
+	// request must not read as "nothing is connected".
+	it('fails rather than reporting an empty list when the resources cannot be read', async () => {
+		setMcpEnabled('test-ws', 'u/hugo/github_mcp', true)
+		listResourceMock.mockRejectedValue(new Error('503'))
+
+		await expect(loadMcpServers('test-ws')).rejects.toThrow()
+	})
+
 	it('advertises only the enabled server', async () => {
 		setMcpEnabled('test-ws', 'u/hugo/github_mcp', true)
 		expect(await loadMcpServers('test-ws')).toEqual([{ path: 'u/hugo/github_mcp' }])

frontend/src/lib/components/copilot/chat/global/mcpTools.ts

@@ -185,8 +185,8 @@ export function loadedMcpTools(owner: string): Tool<{}>[] {
 }
 
 /**
- * The servers holding a registered tool, with the revision each was frozen at, for the
- * reconcile in `refreshMcpServers`. The revision matters as much as the path: a
+ * The servers holding a registered tool, with the revision each was frozen at, for
+ * `reconcileMcpRegistry`. The revision matters as much as the path: a
  * registered call bypasses the listing cache, so a connection edited elsewhere would
  * keep running against the schema it had before the edit.
  */
@@ -299,6 +299,15 @@ export function forgetLoadedMcpTools(owner: string, serverPath?: string) {
 	if (registry.tools.size === 0) registries.delete(owner)
 }
 
+function forgetRegisteredTool(owner: string, key: string) {
+	const registry = registries.get(owner)
+	if (!registry) return
+	registry.tools.delete(key)
+	registry.lastUsed.delete(key)
+	registry.editedAt.delete(key)
+	if (registry.tools.size === 0) registries.delete(owner)
+}
+
 function clearRefused(owner: string, serverPath?: string) {
 	if (serverPath === undefined) {
 		refused.delete(owner)
@@ -349,19 +358,17 @@ export async function loadMcpServers(workspace: string): Promise<McpServer[]> {
 	// without a request — this runs before every send.
 	const enabled = enabledMcpPaths(workspace)
 	if (enabled.length === 0) return []
-	try {
-		const resources = await ResourceService.listResource({
-			workspace,
-			resourceType: 'mcp',
-			perPage: 100
-		})
-		return resources
-			.filter((r) => enabled.includes(r.path))
-			.map((r) => ({ path: r.path, editedAt: r.edited_at }))
-	} catch (e) {
-		console.error('Failed to load MCP servers', e)
-		return []
-	}
+	// Throws rather than answering `[]`: the caller reconciles registered tools and
+	// refusals against this list, and a failed request read as "nothing connected"
+	// would drop both on a blip.
+	const resources = await ResourceService.listResource({
+		workspace,
+		resourceType: 'mcp',
+		perPage: 100
+	})
+	return resources
+		.filter((r) => enabled.includes(r.path))
+		.map((r) => ({ path: r.path, editedAt: r.edited_at }))
 }
 
 function tokenize(text: string): string[] {
@@ -446,6 +453,50 @@ async function resolveTool(
 	return { server, tool }
 }
 
+/**
+ * The current definition of a registered tool, or the reason not to run the call.
+ *
+ * A registration freezes a schema and a `readOnlyHint` for the conversation, but a remote
+ * can change or withdraw a tool without its Windmill resource being touched — which is
+ * what `TOOLS_CACHE_TTL_MS` bounds for every other path. So the listing decides what runs
+ * here too, as it does for the wrapper. Arguments the model built from the old schema are
+ * not sent to a new one: the registration is dropped and the model searches again.
+ */
+async function revalidateRegistration(
+	owner: string,
+	workspace: string,
+	server: McpServer,
+	key: string,
+	frozen: McpToolDef
+): Promise<{ tool: McpToolDef } | { error: string; schema?: unknown }> {
+	let tools: McpToolDef[]
+	try {
+		tools = await loadServerTools(workspace, server.path, server.editedAt)
+	} catch {
+		// Listing is a live call to a third party. One failure must not take down a call
+		// the frozen copy can still make — the backend re-asserts `read_only` regardless.
+		return { tool: frozen }
+	}
+	const current = tools.find((t) => t.name === frozen.name)
+	if (!current) {
+		forgetRegisteredTool(owner, key)
+		return {
+			error: `${frozen.name} is no longer exposed by ${server.path}. Use search_mcp_tools to find what is.`
+		}
+	}
+	if (
+		JSON.stringify(current.inputSchema) !== JSON.stringify(frozen.inputSchema) ||
+		isReadOnly(current) !== isReadOnly(frozen)
+	) {
+		forgetRegisteredTool(owner, key)
+		return {
+			error: `${frozen.name} changed on ${server.path} since it was loaded, so these arguments were not sent. Call search_mcp_tools again to load it as it is now.`,
+			schema: current.inputSchema
+		}
+	}
+	return { tool: current }
+}
+
 /** Flatten the MCP content blocks into the text the model can act on. */
 function extractResultData(result: unknown): unknown {
 	const content = (result as { content?: unknown })?.content
@@ -846,7 +897,11 @@ export function registerMcpTools(
 					content: `Calling ${tool.name}...`,
 					mcpServer: server.path
 				})
-				const result = await executeTool(owner, workspace, server, tool, args ?? {}, readOnly)
+				const current = await revalidateRegistration(owner, workspace, server, key, tool)
+				const result =
+					'error' in current
+						? bounded({ success: false, ...current })
+						: await executeTool(owner, workspace, server, current.tool, args ?? {}, readOnly)
 				const ok = JSON.parse(result).success === true
 				toolCallbacks.setToolStatus(toolId, {
 					content: ok ? `Called ${tool.name}` : `Call to ${tool.name} failed`,