fix: keep a refused MCP schema out of the conversation it broke

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

@@ -145,6 +145,7 @@ import {
 	forgetLoadedMcpTools,
 	invalidateMcpRegistrations,
 	isRequestBodyRejection,
+	withdrawMcpToolsAfterRejection,
 	loadedMcpServers,
 	loadedMcpTools,
 	loadMcpServers,
@@ -1217,6 +1218,8 @@ export class AIChatManager {
 	private mcpServersRefreshId = 0
 	/** The connected set the last refresh settled on, to notice when it changes. */
 	private mcpServersSignature = ''
+	/** The workspace the registered tools were frozen against. */
+	private mcpRegistryWorkspace: string | undefined
 
 	// The GLOBAL prompt's path conventions and folder ACLs, for this chat's operating
 	// workspace (`GlobalPromptIdentity`). Resolved asynchronously alongside skills, never
@@ -2273,6 +2276,14 @@ export class AIChatManager {
 			this.mcpServersSignature = signature
 			invalidateMcpRegistrations(this.mcpOwnerId)
 		}
+		// A registered call runs against the workspace the chat is on when it is made, not
+		// the one it was registered in, and a fork carries the same resource path and
+		// `edited_at` as its parent — so the per-path reconcile below cannot tell those two
+		// servers apart. A workspace change drops the lot instead.
+		if (this.mcpRegistryWorkspace !== undefined && this.mcpRegistryWorkspace !== workspace) {
+			forgetLoadedMcpTools(this.mcpOwnerId)
+		}
+		this.mcpRegistryWorkspace = workspace
 		for (const { path, editedAt } of loadedMcpServers(this.mcpOwnerId)) {
 			if (!live.has(path) || live.get(path) !== editedAt) {
 				forgetLoadedMcpTools(this.mcpOwnerId, path)
@@ -2286,15 +2297,15 @@ 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 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.
+	 * error naming the request, not the tool. So a rejection withdraws those tools: the
+	 * send that follows goes out with the search tool and the wrappers only, and a later
+	 * search leaves them to the wrappers rather than registering the same schema again.
 	 */
 	private dropMcpToolsOnRejectedRequest = (err: unknown) => {
 		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)
+		console.warn('Withdrawing registered MCP tools after a rejected request', err)
+		withdrawMcpToolsAfterRejection(this.mcpOwnerId)
 		if (this.mode === AIMode.GLOBAL) {
 			this.configureGlobalMode()
 		}

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

@@ -37,6 +37,7 @@ import {
 	forgetLoadedMcpTools,
 	invalidateMcpRegistrations,
 	isRequestBodyRejection,
+	withdrawMcpToolsAfterRejection,
 	loadedMcpServers,
 	mcpRegistryGeneration,
 	loadedMcpTools,
@@ -464,6 +465,33 @@ describe('loaded remote tools', () => {
 // 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', () => {
+	// Dropping alone only moves the failure: the model is told to search again, registers
+	// the schema that was just refused, and the next request fails the same way.
+	it('will not register a refused tool again in the same conversation', () => {
+		const server = SERVERS[0]
+		registerMcpTools(OWNER, mcpRegistryGeneration(OWNER), server, [TOOLS[0]])
+
+		withdrawMcpToolsAfterRejection(OWNER)
+
+		expect(loadedMcpTools(OWNER)).toEqual([])
+		expect(registerMcpTools(OWNER, mcpRegistryGeneration(OWNER), server, [TOOLS[0]])).toEqual([
+			undefined
+		])
+		expect(loadedMcpTools(OWNER)).toEqual([])
+	})
+
+	it('registers a refused tool again once its server is reconnected', () => {
+		const server = SERVERS[0]
+		registerMcpTools(OWNER, mcpRegistryGeneration(OWNER), server, [TOOLS[0]])
+		withdrawMcpToolsAfterRejection(OWNER)
+
+		forgetLoadedMcpTools(OWNER, server.path)
+
+		expect(registerMcpTools(OWNER, mcpRegistryGeneration(OWNER), server, [TOOLS[0]])).toEqual([
+			'mcp_u_hugo_github_mcp__get_issue'
+		])
+	})
+
 	it('separates a refused body from a refused account', () => {
 		expect(isRequestBodyRejection(400)).toBe(true)
 		expect(isRequestBodyRejection(422)).toBe(true)

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

@@ -125,6 +125,8 @@ const registries = new Map<string, OwnerRegistry>()
  * otherwise a rotation or disposal during that await is silently undone.
  */
 const generations = new Map<string, number>()
+/** Registry keys `withdrawMcpToolsAfterRejection` will not let an owner register again. */
+const refused = new Map<string, Set<string>>()
 /** Bumped by the all-owner clear. Folded into the token below so it also invalidates an
  * owner that has registered nothing yet, and is therefore in neither map. */
 let allGeneration = 0
@@ -207,12 +209,42 @@ export function mcpServerForToolName(owner: string, toolName: string): string |
 	return undefined
 }
 
+/**
+ * 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 withdrawn 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
+}
+
+/**
+ * Drop an owner's registered tools after the provider refused the request, and refuse
+ * to register those same tools again for the rest of the conversation. Dropping alone
+ * only moves the failure: the model is told to search again, registers the schema that
+ * was just refused, and the next request fails the same way. Which schema was at fault
+ * is not knowable from the error, so every tool that was loaded goes on the list — each
+ * falls back to the free-form wrapper, which is how they were reached before they could
+ * be registered at all. A server turned off, edited, or reconnected clears its entries,
+ * as does a new conversation.
+ */
+export function withdrawMcpToolsAfterRejection(owner: string) {
+	const keys = [...(registries.get(owner)?.tools.keys() ?? [])]
+	forgetLoadedMcpTools(owner)
+	if (keys.length > 0) refused.set(owner, new Set(keys))
+}
+
 /** Drop one owner's loaded tools, or only those belonging to one server. */
 export function forgetLoadedMcpTools(owner: string, serverPath?: string) {
 	// Before the empty-registry check: a conversation can rotate while its first search
 	// is still awaiting a listing, with nothing registered yet, and that search must
 	// still be rejected when it comes back.
 	invalidateOwner(owner)
+	// Whatever drops a tool also lifts its refusal: a new conversation, and a server
+	// turned off or edited, both mean the next listing is worth registering again.
+	clearRefused(owner, serverPath)
 	const registry = registries.get(owner)
 	if (!registry) return
 	if (serverPath === undefined) {
@@ -230,6 +262,20 @@ export function forgetLoadedMcpTools(owner: string, serverPath?: string) {
 	if (registry.tools.size === 0) registries.delete(owner)
 }
 
+function clearRefused(owner: string, serverPath?: string) {
+	if (serverPath === undefined) {
+		refused.delete(owner)
+		return
+	}
+	const keys = refused.get(owner)
+	if (!keys) return
+	const prefix = `${serverPath}::`
+	for (const key of [...keys]) {
+		if (key.startsWith(prefix)) keys.delete(key)
+	}
+	if (keys.size === 0) refused.delete(owner)
+}
+
 /**
  * Drop every owner's loaded tools. Used when a server resource itself changed, which
  * makes the frozen copy every chat holds stale at once — not for one chat rotating.
@@ -237,6 +283,7 @@ export function forgetLoadedMcpTools(owner: string, serverPath?: string) {
 function forgetAllLoadedMcpTools() {
 	allGeneration++
 	registries.clear()
+	refused.clear()
 }
 
 /**
@@ -591,17 +638,6 @@ 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_TOOL_NAME_PREFIX}${sanitizeToolNamePart(serverPath)}__${sanitizeToolNamePart(toolName)}`
 	if (full.length <= MAX_TOOL_NAME_CHARS) return full
@@ -624,12 +660,24 @@ function uniqueRegisteredName(
 	toolName: string
 ): string {
 	const name = registeredToolName(serverPath, toolName)
-	for (const [otherKey, tool] of registry.tools) {
-		if (otherKey !== key && tool.def.function.name === name) {
-			return `${name.slice(0, MAX_TOOL_NAME_CHARS - 8)}_${shortHash(key)}`
+	const taken = (candidate: string) => {
+		for (const [otherKey, tool] of registry.tools) {
+			if (otherKey !== key && tool.def.function.name === candidate) return true
 		}
+		return false
 	}
-	return name
+	if (!taken(name)) return name
+	// A remote names its own tools, so the suffixed candidate can be taken too — by a
+	// tool that sanitizes to exactly it, or by another key whose hash collided. Salt
+	// until it is free; the registry holds MAX_LOADED_TOOLS entries, so this ends.
+	const stem = name.slice(0, MAX_TOOL_NAME_CHARS - 8)
+	for (let salt = 0; salt <= MAX_LOADED_TOOLS; salt++) {
+		const candidate = `${stem}_${shortHash(`${key}#${salt}`)}`
+		if (!taken(candidate)) return candidate
+	}
+	// More collisions than there are entries to collide with: the counter is unique
+	// within the registry by construction.
+	return `${stem}_${registry.counter}`
 }
 
 /**
@@ -721,8 +769,12 @@ export function registerMcpTools(
 	// conversation's tools into the new one, or resurrect a registry nothing can reach.
 	if (generation !== mcpRegistryGeneration(owner)) return tools.map(() => undefined)
 	const registry = registryFor(owner)
+	const refusedKeys = refused.get(owner)
 	const names = tools.map((tool) => {
 		const key = `${server.path}::${tool.name}`
+		// Registering this again is what made the last request fail; the wrapper still
+		// reaches it (see `withdrawMcpToolsAfterRejection`).
+		if (refusedKeys?.has(key)) return undefined
 		const name = uniqueRegisteredName(registry, key, server.path, tool.name)
 		const readOnly = isReadOnly(tool)
 		const parameters = safeInputSchema(tool.inputSchema)

frontend/src/lib/components/copilot/chat/shared.ts

@@ -722,10 +722,12 @@ async function callTool<T>({
 		// 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.
+		// way back, through whichever name that search returns: the tool may come back
+		// registered, or only through the wrappers. 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.`
+				`Unknown tool call: ${functionName}. That MCP tool is no longer loaded — call search_mcp_tools again and use the name it returns.`
 			)
 		}
 		throw new Error(