From cbb4a1274b6613dfd0c7dde2a02044f40c14efb8 Mon Sep 17 00:00:00 2001 From: yao <63141491+yaonyan@users.noreply.github.com> Date: Mon, 10 Nov 2025 23:14:35 +0800 Subject: [PATCH] feat: improve agentic mode schema validation - Add validation: when hasDefinitions is empty, definitionsOf must be provided - Add validation: when useTool is specified, hasDefinitions must contain that tool - Prioritize empty hasDefinitions check before other validations - Ensure tools are properly defined before use in agentic mode --- .../src/executors/agentic/agentic-executor.ts | 162 +++++++------- .../core/src/factories/args-def-factory.ts | 96 ++++++--- packages/core/src/prompts/index.ts | 38 ++-- packages/core/src/types.ts | 3 +- .../src/utils/common/tool-tag-processor.ts | 4 +- .../core/tests/unit/agentic_executor_test.ts | 200 ++++++++++++++++++ .../tests/unit/tool_tag_processor_test.ts | 6 +- packages/plugin-code-execution/src/plugin.ts | 2 +- 8 files changed, 379 insertions(+), 132 deletions(-) create mode 100644 packages/core/tests/unit/agentic_executor_test.ts diff --git a/packages/core/src/executors/agentic/agentic-executor.ts b/packages/core/src/executors/agentic/agentic-executor.ts index e740e82..6139abd 100644 --- a/packages/core/src/executors/agentic/agentic-executor.ts +++ b/packages/core/src/executors/agentic/agentic-executor.ts @@ -16,8 +16,7 @@ export class AgenticExecutor { private allToolNames: string[], private toolNameToDetailList: [string, unknown][], private server: ComposableMCPServer, - private ACTION_KEY: string = "action", - private NEXT_ACTION_KEY: string = "nextAction", + private USE_TOOL_KEY: string = "useTool", ) { this.logger = createLogger(`mcpc.agentic.${name}`, server); @@ -48,12 +47,15 @@ export class AgenticExecutor { ): Promise { // Create a span for this execute call const executeSpan: Span | null = this.tracingEnabled - ? startSpan("mcpc.agentic_execute", { - agent: this.name, - action: String(args[this.ACTION_KEY] ?? "unknown"), - nextAction: String(args[this.NEXT_ACTION_KEY] ?? "none"), - args: JSON.stringify(args), - }, parentSpan ?? undefined) + ? startSpan( + "mcpc.agentic_execute", + { + agent: this.name, + selectTool: String(args[this.USE_TOOL_KEY] ?? "unknown"), + args: JSON.stringify(args), + }, + parentSpan ?? undefined, + ) : null; try { @@ -69,7 +71,7 @@ export class AgenticExecutor { this.logger.warning({ message: "Validation failed", - action: args[this.ACTION_KEY], + selectTool: args[this.USE_TOOL_KEY], error: validationResult.error, }); @@ -86,16 +88,16 @@ export class AgenticExecutor { }; } - const actionName = args[this.ACTION_KEY] as string; + const useTool = args[this.USE_TOOL_KEY] as string; + const definitionsOf = (args.definitionsOf as string[]) || []; + const hasDefinitions = (args.hasDefinitions as string[]) || []; - // Update span name to include action - if (executeSpan && actionName) { + // Update span name to include selected tool + if (executeSpan && useTool) { try { - const safeAction = String(actionName).replace(/\s+/g, "_"); + const safeTool = String(useTool).replace(/\s+/g, "_"); if (typeof (executeSpan as any).updateName === "function") { - (executeSpan as any).updateName( - `mcpc.agentic_execute.${safeAction}`, - ); + (executeSpan as any).updateName(`mcpc.agentic_execute.${safeTool}`); } } catch { // Ignore errors while updating span name @@ -104,58 +106,37 @@ export class AgenticExecutor { // First check external tools const currentTool = this.toolNameToDetailList.find( - ([name, _detail]: [string, unknown]) => name === actionName, + ([name, _detail]: [string, unknown]) => name === useTool, )?.[1] as | { execute: (args: unknown) => Promise } | undefined; if (currentTool) { // Execute external tool - const nextAction = args[this.NEXT_ACTION_KEY] as string; - if (executeSpan) { executeSpan.setAttributes({ toolType: "external", - actionName: actionName, - nextAction: nextAction || "none", + selectedTool: useTool, }); } this.logger.debug({ message: "Executing external tool", - action: actionName, - nextAction: nextAction, + selectTool: useTool, }); const currentResult = await currentTool.execute({ - ...(args[actionName] as Record), + ...(args[useTool] as Record), }); - if (args[nextAction]) { - currentResult?.content?.push({ - type: "text", - text: CompiledPrompts.actionSuccess({ - toolName: this.name, - nextAction: nextAction, - currentAction: actionName, - }), - }); - } else { - currentResult?.content?.push({ - type: "text", - text: CompiledPrompts.planningPrompt({ - currentAction: actionName, - toolName: this.name, - }), - }); - } + // Provide tool schemas if requested + this.appendToolSchemas(currentResult, definitionsOf, hasDefinitions); if (executeSpan) { executeSpan.setAttributes({ success: true, isError: !!currentResult.isError, resultContentLength: currentResult.content?.length || 0, - hasNextAction: !!args[nextAction], toolResult: JSON.stringify(currentResult), }); endSpan(executeSpan); @@ -165,54 +146,35 @@ export class AgenticExecutor { } // If not found in external tools, check internal tools - if (this.allToolNames.includes(actionName)) { + if (this.allToolNames.includes(useTool)) { if (executeSpan) { executeSpan.setAttributes({ toolType: "internal", - actionName: actionName, + selectedTool: useTool, }); } this.logger.debug({ message: "Executing internal tool", - action: actionName, + selectTool: useTool, }); try { const result = await this.server.callTool( - actionName, - args[actionName] as Record, + useTool, + args[useTool] as Record, ); - const nextAction = args[this.NEXT_ACTION_KEY] as string; const callToolResult = (result as CallToolResult) ?? { content: [] }; - if (nextAction && this.allToolNames.includes(nextAction)) { - callToolResult.content.push({ - type: "text", - text: CompiledPrompts.actionSuccess({ - toolName: this.name, - nextAction: nextAction, - currentAction: actionName, - }), - }); - } else { - callToolResult.content.push({ - type: "text", - text: CompiledPrompts.planningPrompt({ - currentAction: actionName, - toolName: this.name, - }), - }); - } + // Provide tool schemas if requested + this.appendToolSchemas(callToolResult, definitionsOf, hasDefinitions); if (executeSpan) { executeSpan.setAttributes({ success: true, isError: !!callToolResult.isError, resultContentLength: callToolResult.content?.length || 0, - hasNextAction: - !!(nextAction && this.allToolNames.includes(nextAction)), toolResult: JSON.stringify(callToolResult), }); endSpan(executeSpan); @@ -226,7 +188,7 @@ export class AgenticExecutor { this.logger.error({ message: "Error executing internal tool", - action: actionName, + useTool, error: String(error), }); @@ -234,7 +196,7 @@ export class AgenticExecutor { content: [ { type: "text", - text: `Error executing internal tool ${actionName}: ${ + text: `Error executing internal tool ${useTool}: ${ error instanceof Error ? error.message : String(error) }`, }, @@ -248,7 +210,7 @@ export class AgenticExecutor { if (executeSpan) { executeSpan.setAttributes({ toolType: "not_found", - actionName: actionName || "unknown", + useTool: useTool || "unknown", completion: true, }); endSpan(executeSpan); @@ -256,17 +218,14 @@ export class AgenticExecutor { this.logger.debug({ message: "Tool not found, returning completion message", - action: actionName, + useTool, }); - return { - content: [ - { - type: "text", - text: CompiledPrompts.completionMessage(), - }, - ], + const result: CallToolResult = { + content: [], }; + this.appendToolSchemas(result, definitionsOf, hasDefinitions); + return result; } catch (error) { // Catch any unexpected errors if (executeSpan) { @@ -292,6 +251,45 @@ export class AgenticExecutor { } } + // Append tool schemas to result if requested + private appendToolSchemas( + result: CallToolResult, + definitionsOf: string[], + hasDefinitions: string[], + ): void { + // Filter out tools that are already available + const schemasToProvide = definitionsOf.filter( + (toolName) => !hasDefinitions.includes(toolName), + ); + + if (schemasToProvide.length === 0) { + return; + } + + const definitionTexts: string[] = []; + + for (const toolName of schemasToProvide) { + const toolDetail = this.toolNameToDetailList.find( + ([name]) => name === toolName, + ); + + if (toolDetail) { + const [name, schema] = toolDetail; + const schemaJson = JSON.stringify(schema, null, 2); + definitionTexts.push( + `\n${schemaJson}\n`, + ); + } + } + + if (definitionTexts.length > 0) { + result.content.push({ + type: "text", + text: `${definitionTexts.join("\n\n")}`, + }); + } + } + // Validate arguments using JSON schema validate( args: Record, @@ -300,10 +298,6 @@ export class AgenticExecutor { valid: boolean; error?: string; } { - // Skip validation for complete decision - if (args.decision === "complete") { - return { valid: true }; - } return validateSchema(args, schema); } } diff --git a/packages/core/src/factories/args-def-factory.ts b/packages/core/src/factories/args-def-factory.ts index 837389b..74cd6bb 100644 --- a/packages/core/src/factories/args-def-factory.ts +++ b/packages/core/src/factories/args-def-factory.ts @@ -168,46 +168,92 @@ Workflow step definitions - provide ONLY on initial call. forAgentic: function ( toolNameToDetailList: [string, unknown][], _sampling: boolean = false, - ACTION_KEY: string = "action", - NEXT_ACTION_KEY: string = "nextAction", + USE_TOOL_KEY: string = "useTool", ): JSONSchema { - const allOf = toolNameToDetailList.map( - ([toolName, _toolDetail]: [string, unknown]) => { - return { - if: { - properties: { [ACTION_KEY]: { const: toolName } }, - required: [ACTION_KEY], - }, - then: { - required: [toolName], + const allOf = [ + // When hasDefinitions is empty, definitionsOf must be provided + { + if: { + properties: { + hasDefinitions: { + type: "array", + maxItems: 0, + }, }, - }; + required: ["hasDefinitions"], + }, + then: { + required: ["definitionsOf"], + }, }, - ); + // When useTool is present, hasDefinitions must contain that tool + ...toolNameToDetailList.map( + ([toolName, _toolDetail]: [string, unknown]) => { + return { + if: { + properties: { [USE_TOOL_KEY]: { const: toolName } }, + required: [USE_TOOL_KEY], + }, + then: { + properties: { + hasDefinitions: { + type: "array", + contains: { const: toolName }, + }, + }, + required: ["hasDefinitions"], + }, + }; + }, + ), + // When a specific tool is selected, its parameters must be provided + ...toolNameToDetailList.map( + ([toolName, _toolDetail]: [string, unknown]) => { + return { + if: { + properties: { [USE_TOOL_KEY]: { const: toolName } }, + required: [USE_TOOL_KEY], + }, + then: { + required: [toolName], + }, + }; + }, + ), + ]; - const actionDescription = - `Specifies the action to be performed from the enum. **⚠️ When setting \`action: "example_action"\`, you MUST also provide \`"example_action": { ... }\`**`; + const useToolDescription = + `Specifies which tool to execute from the available options. **When setting \`useTool: "example_tool"\`, you MUST also provide \`"example_tool": { ...parameters }\` with that tool's parameters**`; + + const toolItems = allToolNames.length > 0 + ? { type: "string", enum: allToolNames } + : { type: "string" }; const baseProperties = { - [ACTION_KEY]: { + [USE_TOOL_KEY]: { type: "string", enum: allToolNames, - description: actionDescription, + description: useToolDescription, }, - [NEXT_ACTION_KEY]: { - type: "string", - enum: allToolNames, + hasDefinitions: { + type: "array", + items: toolItems, + description: + "Tool names whose schemas you already have. List all tools you have schemas for to avoid duplicate schema requests and reduce token usage.", + }, + definitionsOf: { + type: "array", + items: toolItems, description: - "Optional: Specify the next action to execute. Only include this when you know additional actions are needed after the current one completes.", + "Tool names whose schemas you need. Request tool schemas before calling them to understand their parameters.", }, - decision: this.decision(), - ...depGroups, + // ...depGroups, }; - const requiredFields = [ACTION_KEY, "decision"]; + const requiredFields: Array = []; const schema: JSONSchema = { - additionalProperties: false, + additionalProperties: true, type: "object", properties: baseProperties, required: requiredFields, diff --git a/packages/core/src/prompts/index.ts b/packages/core/src/prompts/index.ts index 5f3ae9a..faac0fb 100644 --- a/packages/core/src/prompts/index.ts +++ b/packages/core/src/prompts/index.ts @@ -14,7 +14,7 @@ export const SystemPrompts = { * Base system prompt for autonomous MCP execution */ AUTONOMOUS_EXECUTION: - `Agentic tool \`{toolName}\` that executes complex tasks by iteratively calling actions, gathering results, and deciding next steps until completion. Use this tool when the task matches the manual below. + `Agentic tool \`{toolName}\` that executes complex tasks by iteratively selecting and calling tools, gathering results, and continuing until completion. Use this tool when the task matches the manual below. You must follow the , obey the , and use the . @@ -22,31 +22,39 @@ You must follow the , obey the , and use the + +\`useTool\` - Which tool to execute this iteration +\`hasDefinitions\` - Tool names whose schemas you already have +\`definitionsOf\` - Tool names whose schemas you need + + -1. **Execute** one action per call -2. **Collect** feedback from each action result -3. **Decide** next step based on feedback: - - **proceed**: More work needed - - **complete**: Task finished (omit action field) - - **retry**: Current action failed -4. **Provide** parameters matching the action name -5. **Continue** until task is complete -6. Note: You are an agent exposed as an MCP tool - **"action" is an internal parameter, NOT an external MCP tool you can call** +1. **First call**: No tool definitions available—you must request them via \`definitionsOf\` +2. **When executing tools**: Must provide \`hasDefinitions\` with ALL tools you have schemas for (avoid duplicate requests and reduce tokens) +3. **When requesting definitions**: Use \`definitionsOf\` to request tool schemas you need +4. **Both together**: Execute tool AND request new definitions in one call for efficiency +5. **Never request definitions you already have** +6. **Select** one tool to execute per call using \`useTool\` +7. **Provide** parameters matching the selected tool name +8. Note: You are an agent exposed as an MCP tool - **\`useTool\` is an internal parameter for choosing which tool to execute, NOT an external MCP tool you can call** +Initial definition request: \`\`\`json { - "action": "action_name", - "decision": "proceed|retry", - "action_name": { /* action parameters */ } + "hasDefinitions": [], + "definitionsOf": ["tool1", "tool2"] } \`\`\` -When complete: +Execute tool + get new definitions: \`\`\`json { - "decision": "complete" + "useTool": "tool1", + "tool1": { /* parameters */ }, + "hasDefinitions": ["tool1", "tool2"], + "definitionsOf": ["tool3"] } \`\`\` `, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index a1fc16a..3983a18 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -45,8 +45,7 @@ export interface ArgsDefCreator { forAgentic: ( toolNameToDetailList: [string, unknown][], sampling?: boolean, - ACTION_KEY?: string, - NEXT_ACTION_KEY?: string, + USE_TOOL_KEY?: string, ) => JSONSchema; forToolDescription: ( description: string, diff --git a/packages/core/src/utils/common/tool-tag-processor.ts b/packages/core/src/utils/common/tool-tag-processor.ts index 9218f5d..e003616 100644 --- a/packages/core/src/utils/common/tool-tag-processor.ts +++ b/packages/core/src/utils/common/tool-tag-processor.ts @@ -69,8 +69,8 @@ export function processToolTags({ } else { const toolId = findToolId(toolName, tools, toolNameMapping); if (toolId) { - // Replace with in the DOM - $(toolEl).replaceWith(``); + // Replace with in the manual + $(toolEl).replaceWith(``); } else { // Tool not found, remove the tag completely $(toolEl).remove(); diff --git a/packages/core/tests/unit/agentic_executor_test.ts b/packages/core/tests/unit/agentic_executor_test.ts new file mode 100644 index 0000000..29ab406 --- /dev/null +++ b/packages/core/tests/unit/agentic_executor_test.ts @@ -0,0 +1,200 @@ +/** + * Test for Agentic Executor with new useTool parameter + */ + +import { assertEquals, assertExists } from "@std/assert"; +import { mcpc } from "../../mod.ts"; + +Deno.test( + "Agentic mode - uses useTool parameter instead of action", + async () => { + const server = await mcpc( + [ + { name: "test-agentic", version: "1.0.0" }, + { + capabilities: { tools: {} }, + }, + ], + [ + { + name: "test-agent", + description: `Test agent with tools. + +`, + deps: { mcpServers: {} }, + options: { + mode: "agentic", + }, + }, + ], + (server) => { + // Add test tools + server.tool( + "test-tool1", + "Test tool 1", + { type: "object", properties: {} }, + () => ({ + content: [{ type: "text", text: "Tool 1 executed" }], + }), + ); + server.tool( + "test-tool2", + "Test tool 2", + { type: "object", properties: {} }, + () => ({ + content: [{ type: "text", text: "Tool 2 executed" }], + }), + ); + }, + ); + + try { + // Test tool execution with new useTool parameter + const result: any = await server.callTool("test-agent", { + useTool: "test-tool1", + "test-tool1": {}, + hasDefinitions: ["test-tool1"], + }); + + assertEquals(result.isError, undefined); + assertExists(result.content); + assertEquals(result.content.length > 0, true); + + const contentText = result.content.map((c: any) => c.text).join(" "); + assertEquals(contentText.includes("Tool 1 executed"), true); + } finally { + await server.close?.(); + } + }, +); + +Deno.test( + "Agentic mode - provides tool schemas when requested", + async () => { + const server = await mcpc( + [ + { name: "test-schema-provision", version: "1.0.0" }, + { + capabilities: { tools: {} }, + }, + ], + [ + { + name: "schema-agent", + description: `Agent that provides schemas. + +`, + deps: { mcpServers: {} }, + options: { + mode: "agentic", + }, + }, + ], + (server) => { + server.tool( + "tool-alpha", + "Alpha tool", + { + type: "object", + properties: { + param1: { type: "string", description: "First parameter" }, + }, + }, + () => ({ + content: [{ type: "text", text: "Alpha executed" }], + }), + ); + server.tool( + "tool-beta", + "Beta tool", + { + type: "object", + properties: { + param2: { type: "number", description: "Second parameter" }, + }, + }, + () => ({ + content: [{ type: "text", text: "Beta executed" }], + }), + ); + }, + ); + + try { + // Request tool schemas + const result: any = await server.callTool("schema-agent", { + useTool: "tool-alpha", + "tool-alpha": { param1: "test" }, + definitionsOf: ["tool-beta"], + hasDefinitions: ["tool-alpha"], + }); + + assertEquals(result.isError, undefined); + assertExists(result.content); + + const contentText = result.content.map((c: any) => c.text).join("\n"); + + // Should execute the selected tool + assertEquals(contentText.includes("Alpha executed"), true); + + // Should provide schema for requested tool + assertEquals(contentText.includes("tool-beta"), true); + assertEquals(contentText.includes("tool_definition"), true); + assertEquals(contentText.includes("param2"), true); + } finally { + await server.close?.(); + } + }, +); + +Deno.test( + "Agentic mode - does not duplicate schemas for hasDefinitions", + async () => { + const server = await mcpc( + [ + { name: "test-no-duplicate", version: "1.0.0" }, + { + capabilities: { tools: {} }, + }, + ], + [ + { + name: "no-dup-agent", + description: `Agent test. +`, + deps: { mcpServers: {} }, + options: { + mode: "agentic", + }, + }, + ], + (server) => { + server.tool( + "tool-gamma", + "Gamma tool", + { type: "object", properties: {} }, + () => ({ + content: [{ type: "text", text: "Gamma executed" }], + }), + ); + }, + ); + + try { + const result: any = await server.callTool("no-dup-agent", { + useTool: "tool-gamma", + "tool-gamma": {}, + definitionsOf: ["tool-gamma"], + hasDefinitions: ["tool-gamma"], // Already have this schema + }); + + assertEquals(result.isError, undefined); + const contentText = result.content.map((c: any) => c.text).join("\n"); + + // Should NOT include schema section since it's already available + assertEquals(contentText.includes("Tool Schemas"), false); + } finally { + await server.close?.(); + } + }, +); diff --git a/packages/core/tests/unit/tool_tag_processor_test.ts b/packages/core/tests/unit/tool_tag_processor_test.ts index cae2943..855ddcc 100644 --- a/packages/core/tests/unit/tool_tag_processor_test.ts +++ b/packages/core/tests/unit/tool_tag_processor_test.ts @@ -32,11 +32,11 @@ Deno.test("processToolTags replaces existing tools and removes missing ones", () toolNameMapping: new Map([["existing", "existing"]]), }); - // Should replace existing tool + // Should keep the tool tag with normalized name assertEquals( - result.includes('action="existing"'), + result.includes('\n${schemaJson}\n`, + `\n${schemaJson}\n`, ); } }