mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 08:02:38 +00:00
fix: point the model back to a search when an MCP tool is withdrawn
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JgzuxyafKNF2uaEeL35XQw
This commit is contained in:
co-authored by
Claude Opus 5
parent
be7023500b
commit
28e793db89
@@ -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)
|
||||
|
||||
@@ -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', {
|
||||
|
||||
@@ -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)}`
|
||||
}
|
||||
|
||||
@@ -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<T>({
|
||||
}): Promise<string> {
|
||||
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.`
|
||||
)
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
|
||||
Reference in New Issue
Block a user