mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: drop registered MCP tools when the conversation rotates
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
5f2b8578b6
commit
1f3251701b
@@ -4170,6 +4170,11 @@ export class AIChatManager {
|
||||
this.#syncMessageFiles()
|
||||
this.syncArtifactsSession()
|
||||
this.planMode.resetRound()
|
||||
// Remote MCP tools are registered by a search in the conversation being left.
|
||||
// Carrying them forward would put their schemas in the next conversation's tool
|
||||
// list — the cost the search indirection exists to avoid — and offer the model a
|
||||
// remote tool nobody there asked for.
|
||||
forgetLoadedMcpTools()
|
||||
this.onChatRotated?.(this.historyManager.getCurrentChatId())
|
||||
}
|
||||
|
||||
@@ -4221,6 +4226,7 @@ export class AIChatManager {
|
||||
this.#automaticScroll = true
|
||||
this.syncArtifactsSession()
|
||||
this.planMode.resetRound()
|
||||
forgetLoadedMcpTools()
|
||||
this.onChatRotated?.(id)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,7 +11,20 @@ vi.mock('../shared', () => ({
|
||||
createToolDef: (_schema: unknown, name: string, description: string) => ({
|
||||
type: 'function',
|
||||
function: { name, description, parameters: {} }
|
||||
})
|
||||
}),
|
||||
// Mirrors the real normalizer closely enough to keep assertions about what
|
||||
// reaches a provider honest: it strips empty/null `format`, recursively.
|
||||
normalizeToolParameterSchema: function strip(schema: any): void {
|
||||
if (!schema || typeof schema !== 'object') return
|
||||
if (schema.format === null || schema.format === '') delete schema.format
|
||||
for (const child of Object.values(schema.properties ?? {})) strip(child)
|
||||
if (Array.isArray(schema.items)) schema.items.forEach(strip)
|
||||
else strip(schema.items)
|
||||
for (const kw of ['allOf', 'anyOf', 'oneOf']) {
|
||||
if (Array.isArray(schema[kw])) schema[kw].forEach(strip)
|
||||
}
|
||||
if (typeof schema.additionalProperties === 'object') strip(schema.additionalProperties)
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('$lib/gen', () => ({
|
||||
@@ -194,7 +207,9 @@ describe('loaded remote tools', () => {
|
||||
inputSchema: {
|
||||
$schema: 'https://json-schema.org/draft/2020-12/schema',
|
||||
type: 'string',
|
||||
properties: { a: { type: 'string' } },
|
||||
// `format: ''` is what Windmill's own MCP server emits for an untyped
|
||||
// field, and only the recursive normalizer removes it.
|
||||
properties: { a: { type: 'string', format: '' } },
|
||||
required: ['a', 'a', 'ghost']
|
||||
}
|
||||
} as any
|
||||
@@ -204,6 +219,7 @@ describe('loaded remote tools', () => {
|
||||
expect(params.type).toBe('object')
|
||||
expect(params.required).toEqual(['a'])
|
||||
expect(params.$schema).toBeUndefined()
|
||||
expect(params.properties.a.format).toBeUndefined()
|
||||
})
|
||||
|
||||
it('falls back to an empty object schema when the remote sends no usable one', () => {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { z } from 'zod'
|
||||
import { ResourceService, type GetMcpToolsResponse } from '$lib/gen'
|
||||
import { createToolDef, type Tool } from '../shared'
|
||||
import { createToolDef, normalizeToolParameterSchema, type Tool } from '../shared'
|
||||
import { enabledMcpPaths } from '$lib/components/mcp/enabledServers'
|
||||
|
||||
/**
|
||||
@@ -78,9 +78,11 @@ export function clearMcpToolsCache() {
|
||||
}
|
||||
|
||||
/**
|
||||
* Remote tools promoted to first-class chat tools for this session, keyed
|
||||
* `${server}::${tool}`. `chatLoop` re-reads its tool list on every iteration, so
|
||||
* one registered while a call is running is callable on the next one.
|
||||
* Remote tools promoted to first-class chat tools for the current conversation,
|
||||
* keyed `${server}::${tool}`. `chatLoop` re-reads its tool list on every iteration,
|
||||
* so one registered while a call is running is callable on the next one, and the
|
||||
* whole set is dropped when the conversation rotates — a tool the model was never
|
||||
* told about in a fresh chat should not be in its list, nor its schema in the bill.
|
||||
*
|
||||
* A registered tool is a frozen copy of an input schema *and* of `readOnlyHint`,
|
||||
* which is the thing `TOOLS_CACHE_TTL_MS` exists to bound — so these are dropped
|
||||
@@ -530,7 +532,12 @@ function safeInputSchema(schema: unknown): Record<string, unknown> {
|
||||
)
|
||||
]
|
||||
: []
|
||||
return { ...rest, type: 'object', properties, required }
|
||||
const safe = { ...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.
|
||||
normalizeToolParameterSchema(safe)
|
||||
return safe
|
||||
}
|
||||
|
||||
function evictLoadedTools() {
|
||||
|
||||
@@ -1398,7 +1398,7 @@ export const createSearchHubScriptsTool = (withContent: boolean = false) => ({
|
||||
/**
|
||||
* Recursively normalizes JSON Schema quirks that specific providers reject.
|
||||
*/
|
||||
function normalizeToolParameterSchema(schema: Record<string, any> | undefined): void {
|
||||
export function normalizeToolParameterSchema(schema: Record<string, any> | undefined): void {
|
||||
if (!schema || typeof schema !== 'object') {
|
||||
return
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user