diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index 13378da13b..32227d7f0c 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -144,6 +144,7 @@ import { createMcpTools, forgetLoadedMcpTools, invalidateMcpRegistrations, + isRequestBodyRejection, loadedMcpServers, loadedMcpTools, loadMcpServers, @@ -2286,14 +2287,11 @@ export class AIChatManager { * A registered MCP tool carries a schema a third party wrote, and a provider that * refuses it refuses every later request in the conversation the same way — with an * error naming the request, not the tool. So a rejection drops the registered tools: - * the next send goes out with the search tool and the wrappers only, and the model - * can register again. A false positive costs one re-search. Statuses about the - * account rather than the body (auth, quota) are left alone. + * the send that follows goes out with the search tool and the wrappers only, and the + * model can register again. A false positive costs one re-search. */ private dropMcpToolsOnRejectedRequest = (err: unknown) => { - const status = getErrorStatus(err) - if (status === undefined || status < 400 || status >= 500) return - if (status === 401 || status === 403 || status === 429) return + if (!isRequestBodyRejection(getErrorStatus(err))) return if (loadedMcpTools(this.mcpOwnerId).length === 0) return console.warn('Dropping registered MCP tools after a rejected request', err) forgetLoadedMcpTools(this.mcpOwnerId) 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 fc9816d407..e4feaee099 100644 --- a/frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/mcpTools.test.ts @@ -36,6 +36,7 @@ import { createMcpTools, forgetLoadedMcpTools, invalidateMcpRegistrations, + isRequestBodyRejection, loadedMcpServers, mcpRegistryGeneration, loadedMcpTools, @@ -459,6 +460,22 @@ describe('loaded remote tools', () => { }) }) +// Dropping the registered tools is how a conversation recovers from a schema the +// provider refuses, and it must not fire on an account problem: a quota error would +// then cost the user every tool they had searched for. +describe('request rejections that withdraw registered tools', () => { + it('separates a refused body from a refused account', () => { + expect(isRequestBodyRejection(400)).toBe(true) + expect(isRequestBodyRejection(422)).toBe(true) + expect(isRequestBodyRejection(401)).toBe(false) + expect(isRequestBodyRejection(403)).toBe(false) + expect(isRequestBodyRejection(429)).toBe(false) + expect(isRequestBodyRejection(500)).toBe(false) + // An abort carries no status. + expect(isRequestBodyRejection(undefined)).toBe(false) + }) +}) + describe('read/write split', () => { it('refuses a mutating tool on the read path', async () => { const result = await run('call_mcp_read_tool', { diff --git a/frontend/src/lib/components/copilot/chat/global/mcpTools.ts b/frontend/src/lib/components/copilot/chat/global/mcpTools.ts index 2f866ef431..585b0dd70f 100644 --- a/frontend/src/lib/components/copilot/chat/global/mcpTools.ts +++ b/frontend/src/lib/components/copilot/chat/global/mcpTools.ts @@ -1,7 +1,7 @@ import { z } from 'zod' import { ResourceService, type GetMcpToolsResponse } from '$lib/gen' import { createToolDef, type Tool } from '../shared' -import { normalizeToolParameterSchema } from '../toolSchema' +import { MCP_TOOL_NAME_PREFIX, normalizeToolParameterSchema } from '../toolSchema' import { enabledMcpPaths } from '$lib/components/mcp/enabledServers' /** @@ -591,8 +591,19 @@ function shortHash(text: string): string { * `list_issues` stay apart, and nothing parses it back: the registry keys on the * pair, so truncation only has to stay unique. */ +/** + * Whether a failed chat request was the provider refusing the body rather than the + * account. A remote schema this provider will not accept refuses every later request + * in the conversation the same way, so the registered tools are dropped on the first + * of these; auth and quota statuses say nothing about the schemas. + */ +export function isRequestBodyRejection(status: number | undefined): boolean { + if (status === undefined || status < 400 || status >= 500) return false + return status !== 401 && status !== 403 && status !== 429 +} + function registeredToolName(serverPath: string, toolName: string): string { - const full = `mcp_${sanitizeToolNamePart(serverPath)}__${sanitizeToolNamePart(toolName)}` + const full = `${MCP_TOOL_NAME_PREFIX}${sanitizeToolNamePart(serverPath)}__${sanitizeToolNamePart(toolName)}` if (full.length <= MAX_TOOL_NAME_CHARS) return full return `${full.slice(0, MAX_TOOL_NAME_CHARS - 8)}_${shortHash(full)}` } diff --git a/frontend/src/lib/components/copilot/chat/shared.ts b/frontend/src/lib/components/copilot/chat/shared.ts index d3625f6707..e542968373 100644 --- a/frontend/src/lib/components/copilot/chat/shared.ts +++ b/frontend/src/lib/components/copilot/chat/shared.ts @@ -7,7 +7,7 @@ import type { UserDraftItemKind } from '$lib/gen' // The gate's two refusals, from a module that holds prose and one size limit: under the // shallow-import rule below, the rest of plan mode is not reachable from here. import { PLAN_MODE_MESSAGES } from './planModeMessages' -import { normalizeToolParameterSchema } from './toolSchema' +import { MCP_TOOL_NAME_PREFIX, normalizeToolParameterSchema } from './toolSchema' // Import-free leaf, so it satisfies the shallow-import rule below. import { openItemPreviewAction, @@ -719,6 +719,15 @@ async function callTool({ }): Promise { const tool = tools.find((t) => t.def.function.name === functionName) if (!tool) { + // A registered MCP tool is withdrawn whenever its registration is dropped — a + // server turned off, an eviction, a schema the provider refused — while the search + // result that advertised its name stays in the transcript. Searching again is the + // way back; the mode advice below would send the model after a tool it has not got. + if (functionName.startsWith(MCP_TOOL_NAME_PREFIX)) { + throw new Error( + `Unknown tool call: ${functionName}. That MCP tool is no longer loaded — call search_mcp_tools again to load it.` + ) + } throw new Error( `Unknown tool call: ${functionName}. Probably not in the correct mode, use the change_mode tool to switch to the correct mode.` ) diff --git a/frontend/src/lib/components/copilot/chat/toolSchema.ts b/frontend/src/lib/components/copilot/chat/toolSchema.ts index d6a41945f3..e919a5f896 100644 --- a/frontend/src/lib/components/copilot/chat/toolSchema.ts +++ b/frontend/src/lib/components/copilot/chat/toolSchema.ts @@ -1,3 +1,10 @@ +/** + * Marks a chat tool name as a remote MCP tool registered for the conversation; built by + * `registeredToolName` in `global/mcpTools`. Here rather than there so `shared` can read + * it without an import cycle. + */ +export const MCP_TOOL_NAME_PREFIX = 'mcp_' + /** * Recursively normalizes JSON Schema quirks that specific providers reject. *