mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
test: cover MCP tool argument correctness in the global evals
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
f1bf23c187
commit
be7023500b
@@ -13,6 +13,12 @@ import {
|
||||
getGlobalDraft,
|
||||
listGlobalDrafts,
|
||||
} from "../../../../../frontend/src/lib/components/copilot/chat/global/userDraftAdapter";
|
||||
import {
|
||||
createMcpTools,
|
||||
forgetLoadedMcpTools,
|
||||
loadedMcpTools,
|
||||
loadMcpServers,
|
||||
} from "../../../../../frontend/src/lib/components/copilot/chat/global/mcpTools";
|
||||
import { appendPlanModeInstructions } from "../../../../../frontend/src/lib/components/copilot/chat/planMode";
|
||||
import type { Tool as ProductionTool } from "../../../../../frontend/src/lib/components/copilot/chat/shared";
|
||||
import { createEvalPlanTools } from "./planModeTools";
|
||||
@@ -142,6 +148,10 @@ export async function runGlobalEval(
|
||||
options.workspaceRoot ??
|
||||
(await mkdtemp(join(tmpdir(), "wmill-frontend-global-benchmark-")));
|
||||
|
||||
// The MCP tool registry is keyed by owner so concurrent cases never see each
|
||||
// other's registrations, as concurrent chat sessions don't in production.
|
||||
const mcpOwnerId = `eval:${workspaceRoot}`;
|
||||
|
||||
clearGlobalDrafts(workspaceRoot);
|
||||
registerBenchmarkWorkspaceRunnables(
|
||||
workspaceRoot,
|
||||
@@ -178,11 +188,15 @@ export async function runGlobalEval(
|
||||
chatId: evalArtifacts.helpers.getChatId(),
|
||||
})
|
||||
: undefined;
|
||||
// The `mcp` resources this case seeded; a case that seeds none gets no MCP tools
|
||||
// and no MCP section in the prompt, exactly as a workspace with nothing connected.
|
||||
const mcpServers = await loadMcpServers(workspaceRoot);
|
||||
// Pass the seeded identity straight to the prompt builder rather than mutating
|
||||
// the process-global `userStore`, so concurrent cases never race on it.
|
||||
const baseSystemMessage = prepareGlobalSystemMessage(undefined, {
|
||||
user: options.user,
|
||||
previewTools: options.sessionChat ?? false,
|
||||
mcpServers,
|
||||
});
|
||||
const rawResult = await runEval({
|
||||
userPrompt,
|
||||
@@ -205,7 +219,12 @@ export async function runGlobalEval(
|
||||
tools: [
|
||||
...getGlobalEvalTools(options.sessionChat ?? false),
|
||||
...(planMode?.tools ?? []),
|
||||
...createMcpTools(mcpOwnerId, mcpServers),
|
||||
],
|
||||
// A tool search registers the tools it matched under this owner; they are offered
|
||||
// from the next request on, which is the behaviour the case measures.
|
||||
getDynamicTools: () =>
|
||||
loadedMcpTools(mcpOwnerId) as ProductionTool<unknown>[],
|
||||
helpers: panel
|
||||
? { ...evalArtifacts.helpers, openArtifact: panel.openArtifact }
|
||||
: evalArtifacts.helpers,
|
||||
@@ -243,6 +262,7 @@ export async function runGlobalEval(
|
||||
};
|
||||
} finally {
|
||||
panel?.dispose();
|
||||
forgetLoadedMcpTools(mcpOwnerId);
|
||||
clearGlobalDrafts(workspaceRoot);
|
||||
clearLiveEditorDrafts(workspaceRoot, options.liveEditorDrafts ?? []);
|
||||
unregisterBenchmarkWorkspaceRunnables(workspaceRoot);
|
||||
|
||||
@@ -48,6 +48,10 @@ export interface RunEvalParams<THelpers, TOutput> {
|
||||
isPlanModeActive?: () => boolean;
|
||||
/** Which of `tools` the model is offered on this request. Absent offers all of them. */
|
||||
isToolAvailable?: (name: string) => boolean;
|
||||
/** Tools a previous call registered, re-read before every request as production's
|
||||
* tools getter is: an MCP search registers the tools it matched, and the model has to
|
||||
* be offered them on the very next request or the names it was just handed are dead. */
|
||||
getDynamicTools?: () => ProductionTool<THelpers>[];
|
||||
/** Re-read before every request, as production's systemMessage getter is. Needed when a
|
||||
* tool changes what the prompt should say — plan mode's instructions have to come back
|
||||
* out once the plan is approved. Falls back to the fixed `systemMessage`. */
|
||||
@@ -80,6 +84,7 @@ export async function runEval<THelpers, TOutput>(
|
||||
isPlanModeActive,
|
||||
isToolAvailable,
|
||||
getSystemMessage,
|
||||
getDynamicTools,
|
||||
} = params;
|
||||
let shouldEmitMessageStart = true;
|
||||
|
||||
@@ -98,7 +103,7 @@ export async function runEval<THelpers, TOutput>(
|
||||
// Wrap tools to intercept fn calls for tracking.
|
||||
// Cast to ProductionTool since the eval Tool has a narrower toolCallbacks type
|
||||
// but the actual callbacks passed at runtime will satisfy both interfaces.
|
||||
const wrappedTools = tools.map((tool) => ({
|
||||
const trackCalls = (tool: ProductionTool<THelpers>) => ({
|
||||
...tool,
|
||||
fn: async (p: any) => {
|
||||
toolCallsCount++;
|
||||
@@ -122,7 +127,9 @@ export async function runEval<THelpers, TOutput>(
|
||||
});
|
||||
return tool.fn(p);
|
||||
},
|
||||
}));
|
||||
});
|
||||
|
||||
const wrappedTools = tools.map(trackCalls);
|
||||
|
||||
// No-op callbacks for eval
|
||||
const callbacks: ToolCallbacks & {
|
||||
@@ -160,9 +167,12 @@ export async function runEval<THelpers, TOutput>(
|
||||
// must leave the schema too, or the model keeps being offered a call the run has
|
||||
// moved past — and the token counts a case reports include a tool it cannot use.
|
||||
get tools() {
|
||||
return isToolAvailable
|
||||
? wrappedTools.filter((t) => isToolAvailable(t.def.function.name))
|
||||
const all = getDynamicTools
|
||||
? [...wrappedTools, ...getDynamicTools().map(trackCalls)]
|
||||
: wrappedTools;
|
||||
return isToolAvailable
|
||||
? all.filter((t) => isToolAvailable(t.def.function.name))
|
||||
: all;
|
||||
},
|
||||
helpers,
|
||||
abortController,
|
||||
|
||||
@@ -1318,3 +1318,87 @@ export function handleBenchmarkApiFetch(url: string, init?: RequestInit): Respon
|
||||
}
|
||||
return Response.json({ error: `no benchmark handler for ${path}` }, { status: 404 })
|
||||
}
|
||||
|
||||
// ============= Connected MCP servers (ResourceService.getMcpTools / callMcpTool) =============
|
||||
// The chat reaches a connected MCP server through the backend, which opens a real
|
||||
// connection to a third party — no meaning here. One server is served instead, shaped
|
||||
// like a real listing: `list_issues` carries the constrained `orderBy` the model used to
|
||||
// guess at, which is what the eval measures.
|
||||
|
||||
export const BENCHMARK_MCP_SERVER_PATH = 'f/evals/global/linear_mcp'
|
||||
|
||||
const BENCHMARK_MCP_SERVER_TOOLS = [
|
||||
{
|
||||
name: 'list_issues',
|
||||
description: 'List issues in the user\'s Linear workspace. For my issues, use "me" as the assignee.',
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
assignee: { type: 'string', description: 'User ID, name, email, or "me"' },
|
||||
orderBy: {
|
||||
type: 'string',
|
||||
enum: ['createdAt', 'updatedAt'],
|
||||
default: 'updatedAt',
|
||||
description: 'Sort: createdAt | updatedAt'
|
||||
},
|
||||
limit: { type: 'number', default: 50, maximum: 250, description: 'Max results' }
|
||||
}
|
||||
},
|
||||
annotations: { readOnlyHint: true }
|
||||
},
|
||||
{
|
||||
name: 'create_issue',
|
||||
description: 'Create a new issue in Linear.',
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: { title: { type: 'string' }, team: { type: 'string' } },
|
||||
required: ['title']
|
||||
},
|
||||
annotations: { readOnlyHint: false }
|
||||
}
|
||||
]
|
||||
|
||||
export function listBenchmarkMcpServerTools(path: string) {
|
||||
if (path !== BENCHMARK_MCP_SERVER_PATH) {
|
||||
throw new Error(`No benchmark MCP server at "${path}"`)
|
||||
}
|
||||
return BENCHMARK_MCP_SERVER_TOOLS
|
||||
}
|
||||
|
||||
const BENCHMARK_MCP_ISSUES = [
|
||||
{ identifier: 'ENG-412', title: 'Retry webhook deliveries', updatedAt: '2026-01-14T09:12:00Z' },
|
||||
{ identifier: 'ENG-408', title: 'Flaky worker restart', updatedAt: '2026-01-13T17:40:00Z' },
|
||||
{ identifier: 'ENG-401', title: 'Paginate the runs table', updatedAt: '2026-01-12T08:05:00Z' },
|
||||
{ identifier: 'ENG-399', title: 'Audit log filters', updatedAt: '2026-01-11T15:22:00Z' }
|
||||
]
|
||||
|
||||
/** The result of calling one of the served tools. `list_issues` answers with issues so a
|
||||
* model that asked correctly can report them instead of retrying through the wrapper;
|
||||
* arguments are echoed so a case can assert on what it actually sent. */
|
||||
export function callBenchmarkMcpServerTool(path: string, tool: string, args: unknown) {
|
||||
const known = listBenchmarkMcpServerTools(path).find((t) => t.name === tool)
|
||||
if (!known) {
|
||||
throw new Error(`Unknown tool "${tool}" on ${path}`)
|
||||
}
|
||||
const payload: Record<string, unknown> = { ok: true, tool, arguments: args ?? {} }
|
||||
if (tool === 'list_issues') {
|
||||
// The served schema constrains `orderBy`, and the real server refuses a value
|
||||
// outside it — which is the failure the registered-schema path exists to avoid,
|
||||
// so the fixture has to refuse it too rather than accept anything.
|
||||
const orderBy = (args as { orderBy?: unknown } | undefined)?.orderBy
|
||||
if (orderBy !== undefined && !['createdAt', 'updatedAt'].includes(orderBy as string)) {
|
||||
return {
|
||||
isError: true,
|
||||
content: [
|
||||
{
|
||||
type: 'text',
|
||||
text: `Invalid value "${String(orderBy)}" for orderBy. Expected createdAt or updatedAt.`
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
const limit = (args as { limit?: unknown } | undefined)?.limit
|
||||
payload.issues = BENCHMARK_MCP_ISSUES.slice(0, typeof limit === 'number' ? limit : undefined)
|
||||
}
|
||||
return { content: [{ type: 'text', text: JSON.stringify(payload) }] }
|
||||
}
|
||||
|
||||
@@ -3,7 +3,13 @@ import { expect, it, vi } from 'vitest'
|
||||
import { mkdir, writeFile } from 'fs/promises'
|
||||
// @ts-ignore - Node.js path
|
||||
import { dirname, resolve } from 'path'
|
||||
import { handleBenchmarkApiFetch, hasBenchmarkApiHandler } from './mockBackend'
|
||||
import {
|
||||
BENCHMARK_MCP_SERVER_PATH,
|
||||
callBenchmarkMcpServerTool,
|
||||
handleBenchmarkApiFetch,
|
||||
hasBenchmarkApiHandler,
|
||||
listBenchmarkMcpServerTools
|
||||
} from './mockBackend'
|
||||
|
||||
// The API catalog executor issues relative fetch('/api/...') calls, which have
|
||||
// no meaning in the vitest environment — serve the ones the benchmark handles.
|
||||
@@ -27,6 +33,15 @@ globalThis.fetch = (async (input: unknown, init?: RequestInit) => {
|
||||
return ORIGINAL_FETCH(input as Parameters<typeof fetch>[0], init)
|
||||
}) as typeof fetch
|
||||
|
||||
// Which MCP servers a chat may act through is a per-browser preference, and node has no
|
||||
// localStorage — so nothing is ever enabled and the MCP tools never register. The served
|
||||
// server stands in as the enabled set.
|
||||
vi.mock('$lib/components/mcp/enabledServers', () => ({
|
||||
enabledMcpPaths: () => [BENCHMARK_MCP_SERVER_PATH],
|
||||
isMcpEnabled: (_ws: string, path: string) => path === BENCHMARK_MCP_SERVER_PATH,
|
||||
setMcpEnabled: () => true
|
||||
}))
|
||||
|
||||
vi.mock('monaco-editor', () => ({
|
||||
editor: {},
|
||||
languages: {},
|
||||
@@ -393,7 +408,19 @@ vi.mock('$lib/gen', async () => {
|
||||
return value
|
||||
},
|
||||
queryResourceTypes: async (data: { workspace: string }) =>
|
||||
hasBenchmarkWorkspace(data.workspace) ? [] : actual.ResourceService.queryResourceTypes(data)
|
||||
hasBenchmarkWorkspace(data.workspace) ? [] : actual.ResourceService.queryResourceTypes(data),
|
||||
getMcpTools: async (data: { workspace: string; path: string }) =>
|
||||
hasBenchmarkWorkspace(data.workspace)
|
||||
? listBenchmarkMcpServerTools(data.path)
|
||||
: actual.ResourceService.getMcpTools(data),
|
||||
callMcpTool: async (data: {
|
||||
workspace: string
|
||||
path: string
|
||||
requestBody: { tool: string; arguments?: unknown }
|
||||
}) =>
|
||||
hasBenchmarkWorkspace(data.workspace)
|
||||
? callBenchmarkMcpServerTool(data.path, data.requestBody.tool, data.requestBody.arguments)
|
||||
: actual.ResourceService.callMcpTool(data)
|
||||
}),
|
||||
McpService: wrapService(actual.McpService, {
|
||||
listMcpTools: async (data: { workspace: string }) =>
|
||||
|
||||
@@ -2399,3 +2399,43 @@
|
||||
judgeChecklist:
|
||||
- runs the existing script rather than rewriting it
|
||||
- passes the GitHub resource as the bare string $res:f/evals/global/github_main
|
||||
|
||||
# --- Connected MCP servers (search_mcp_tools + the tools it registers) ---
|
||||
# The harness serves one MCP server itself (mock getMcpTools/callMcpTool in
|
||||
# adapters/frontend), so this does not need a reachable third party.
|
||||
|
||||
- id: global-test41-mcp-tool-argument-from-schema
|
||||
prompt: |-
|
||||
Show me my 3 most recently updated Linear issues.
|
||||
initial: ai_evals/fixtures/frontend/global/initial/linear_mcp_server.json
|
||||
runtime:
|
||||
maxTurns: 8
|
||||
validate:
|
||||
draftCountExactly: 0
|
||||
toolExpect:
|
||||
requiredToolsUsed:
|
||||
- search_mcp_tools
|
||||
- mcp_f_evals_global_linear_mcp__list_issues
|
||||
forbiddenToolsUsed:
|
||||
# The free-form wrapper is the fallback for a tool that was not registered.
|
||||
# Reaching for it here means the model is guessing arguments it could have read.
|
||||
- call_mcp_read_tool
|
||||
- call_mcp_write_tool
|
||||
- write_script
|
||||
- deploy_workspace_item
|
||||
toolCallArgs:
|
||||
# `orderBy` is an enum the server declares and the model cannot infer: the
|
||||
# reported bug was inventing a plausible value ("priority") for it. Universal
|
||||
# over calls, so a wrong first guess fails even if a retry recovers.
|
||||
- tool: mcp_f_evals_global_linear_mcp__list_issues
|
||||
field: orderBy
|
||||
stringEqualsAnyOf:
|
||||
- updatedAt
|
||||
- createdAt
|
||||
# A read-only question produces no draft, and the judge sees only drafts — the
|
||||
# deliverable is the argument shape, checked deterministically above.
|
||||
skipJudge: true
|
||||
judgeChecklist:
|
||||
- finds the Linear issue tool through search_mcp_tools
|
||||
- calls it with arguments the server's schema accepts, first time
|
||||
- reports the issues instead of writing a script to fetch them
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"workspace": {
|
||||
"resources": [
|
||||
{
|
||||
"path": "f/evals/global/linear_mcp",
|
||||
"resource_type": "mcp",
|
||||
"description": "Linear MCP server",
|
||||
"value": {
|
||||
"name": "linear",
|
||||
"url": "https://mcp.linear.app/mcp"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user