From 2a5d5b36db4c694b29cb8f6798800a8224024aa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Wed, 22 Apr 2026 12:49:35 +0200 Subject: [PATCH] refactor(tool-provider): kill execute_tool's dual dispatch (#19962) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Stacked on top of #19960.** ## Summary - `execute_tool` used to check `directTools[toolName]` first, falling back to the registry. Same tool name, different wrapping: preloaded went through `wrapToolsWithOutputSerialization`, fallback didn't. Silent divergence — a model calling a CRUD tool via \`learn_tools\`/\`execute_tool\` got raw output, while calling it as a preloaded direct tool got compacted output. - Now: `execute_tool` always routes through `toolRegistry.resolveAndExecute`. One path, no fast-path. - Output serialization (`compactToolOutput`) moves into the registry, gated by a new `serializeOutput` flag on `hydrateToolSet` / `resolveAndExecute` / `getToolsByName` / `getToolsByCategories` / `ToolRetrievalOptions`. Chat passes `true`, MCP and workflow pass `false`. ## Key changes **Registry (`tool-registry.service.ts`)** - `hydrateToolSet` options gain `serializeOutput?: boolean`; when true the execute closure wraps dispatch result with `compactToolOutput`. - `resolveAndExecute` signature: replaces unused \`_options: ToolExecutionOptions\` with `{ serializeOutput?: boolean }`. - `getToolsByName` and `getToolsByCategories` thread `serializeOutput` through to `hydrateToolSet`. **Meta-tool (`execute-tool.tool.ts`)** - API changes from positional `(toolRegistry, context, directTools?, excludeTools?)` to `(toolRegistry, context, options?: { excludeTools?, serializeOutput? })`. - `directTools` fallback removed. All invocations go to the registry. **Chat (`chat-execution.service.ts`)** - Passes `serializeOutput: true` to `getToolsByName` — preloaded tools get compacted output from the hydrator, no external wrap needed. - Drops the external `wrapToolsWithOutputSerialization(preloadedTools)` call. - `createExecuteToolTool` call now passes `{ serializeOutput: true }`. Direct-tool and `execute_tool` paths produce identical output shape. **MCP (`mcp-protocol.service.ts`)** - `createExecuteToolTool` call updated to new options shape with `{ excludeTools: MCP_EXCLUDED_TOOLS }`. No `serializeOutput` flag → raw output as today. **Deletes** - `output-serialization/wrap-tools-with-output-serialization.util.ts` — sole caller removed. ## Behavior changes - **Chat, `execute_tool` fallback path**: now produces compacted output (matches direct path). Net effect: fewer tokens for CRUD results reached via discovery. Intended improvement. - **Chat, `execute_tool({toolName: 'web_search'})` edge**: today silently hits the native tool via `directTools`; now returns \"tool not found, use get_tool_catalog\". Self-correcting, rare — native tools are always directly available to the model. - **MCP**: no change. No `serializeOutput` flag → identical raw output. - **Workflow agent**: no change. Doesn't use `execute_tool`. ## Test plan - [ ] `npx nx typecheck twenty-server` passes (verified: 7 pre-existing unrelated errors, zero new) - [ ] \`npx jest packages/twenty-server/src/engine/api/mcp/services/__tests__/mcp-protocol.service.spec.ts\` passes in CI - [ ] AI chat: call a preloaded tool (e.g. \`search_help_center\`) directly → compacted output - [ ] AI chat: call a non-preloaded CRUD tool via \`learn_tools\`/\`execute_tool\` → compacted output (this is the behavior change) - [ ] AI chat: native \`web_search\` still works when model calls it directly - [ ] MCP: \`tools/call\` on a registry tool → raw output (nulls preserved) - [ ] Workflow AI agent: tool dispatch unchanged Co-authored-by: Claude Opus 4.7 (1M context) --- .../api/mcp/services/mcp-protocol.service.ts | 9 ++--- .../interfaces/tool-retrieval-options.type.ts | 4 +++ ...ap-tools-with-output-serialization.util.ts | 34 ------------------- .../services/tool-registry.service.ts | 31 ++++++++++++++--- .../tool-provider/tools/execute-tool.tool.ts | 28 +++++++-------- .../services/chat-execution.service.ts | 13 +++---- 6 files changed, 53 insertions(+), 66 deletions(-) delete mode 100644 packages/twenty-server/src/engine/core-modules/tool-provider/output-serialization/wrap-tools-with-output-serialization.util.ts diff --git a/packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts b/packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts index af25f2ff0dd..9facdb59141 100644 --- a/packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts +++ b/packages/twenty-server/src/engine/api/mcp/services/mcp-protocol.service.ts @@ -140,12 +140,9 @@ export class McpProtocolService { inputSchema: zodSchema(learnToolsInputSchema), }, [EXECUTE_TOOL_TOOL_NAME]: { - ...createExecuteToolTool( - this.toolRegistry, - toolContext, - preloadedTools, - MCP_EXCLUDED_TOOLS, - ), + ...createExecuteToolTool(this.toolRegistry, toolContext, { + excludeTools: MCP_EXCLUDED_TOOLS, + }), inputSchema: executeToolInputSchema, }, [LOAD_SKILL_TOOL_NAME]: { diff --git a/packages/twenty-server/src/engine/core-modules/tool-provider/interfaces/tool-retrieval-options.type.ts b/packages/twenty-server/src/engine/core-modules/tool-provider/interfaces/tool-retrieval-options.type.ts index 208cbaf9594..de12a0f9d24 100644 --- a/packages/twenty-server/src/engine/core-modules/tool-provider/interfaces/tool-retrieval-options.type.ts +++ b/packages/twenty-server/src/engine/core-modules/tool-provider/interfaces/tool-retrieval-options.type.ts @@ -5,4 +5,8 @@ export type ToolRetrievalOptions = { excludeTools?: string[]; wrapWithErrorContext?: boolean; includeLoadingMessage?: boolean; + // Apply output compaction (strip nulls/empty values) to dispatch results + // before returning. Chat enables this to reduce token usage in the + // conversation context; MCP and workflow agents leave raw output intact. + serializeOutput?: boolean; }; diff --git a/packages/twenty-server/src/engine/core-modules/tool-provider/output-serialization/wrap-tools-with-output-serialization.util.ts b/packages/twenty-server/src/engine/core-modules/tool-provider/output-serialization/wrap-tools-with-output-serialization.util.ts deleted file mode 100644 index 83d4c7a31e3..00000000000 --- a/packages/twenty-server/src/engine/core-modules/tool-provider/output-serialization/wrap-tools-with-output-serialization.util.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { type ToolSet } from 'ai'; - -import { compactToolOutput } from './compact-tool-output.util'; - -// Wraps every tool's execute function with output serialization. -// The wrapper intercepts the raw tool result and applies compaction -// (strip nulls/empty, flatten) before the AI SDK serializes it -// into the conversation context. -// -// This is a composable utility — it can be chained with other -// wrappers like wrapToolsWithErrorContext. -export const wrapToolsWithOutputSerialization = (tools: ToolSet): ToolSet => { - const wrappedTools: ToolSet = {}; - - for (const [toolName, tool] of Object.entries(tools)) { - if (!tool.execute) { - wrappedTools[toolName] = tool; - continue; - } - - const originalExecute = tool.execute; - - wrappedTools[toolName] = { - ...tool, - execute: async (...args: Parameters) => { - const result = await originalExecute(...args); - - return compactToolOutput(result); - }, - }; - } - - return wrappedTools; -}; diff --git a/packages/twenty-server/src/engine/core-modules/tool-provider/services/tool-registry.service.ts b/packages/twenty-server/src/engine/core-modules/tool-provider/services/tool-registry.service.ts index e1b7da4fe2a..dce87b40f1f 100644 --- a/packages/twenty-server/src/engine/core-modules/tool-provider/services/tool-registry.service.ts +++ b/packages/twenty-server/src/engine/core-modules/tool-provider/services/tool-registry.service.ts @@ -1,6 +1,6 @@ import { Inject, Injectable, Logger } from '@nestjs/common'; -import { type ToolExecutionOptions, type ToolSet, jsonSchema } from 'ai'; +import { type ToolSet, jsonSchema } from 'ai'; import { type NativeToolProvider } from 'src/engine/core-modules/tool-provider/interfaces/native-tool-provider.interface'; import { type ToolProvider } from 'src/engine/core-modules/tool-provider/interfaces/tool-provider.interface'; @@ -9,6 +9,7 @@ import { type ToolRetrievalOptions } from 'src/engine/core-modules/tool-provider import { TOOL_PROVIDERS } from 'src/engine/core-modules/tool-provider/constants/tool-providers.token'; import { ToolCategory } from 'twenty-shared/ai'; +import { compactToolOutput } from 'src/engine/core-modules/tool-provider/output-serialization/compact-tool-output.util'; import { NativeModelToolProvider } from 'src/engine/core-modules/tool-provider/providers/native-model-tool.provider'; import { ToolExecutorService } from 'src/engine/core-modules/tool-provider/services/tool-executor.service'; import { type LearnToolsAspect } from 'src/engine/core-modules/tool-provider/tools/learn-tools.tool'; @@ -110,10 +111,12 @@ export class ToolRegistryService { options?: { wrapWithErrorContext?: boolean; includeLoadingMessage?: boolean; + serializeOutput?: boolean; }, ): ToolSet { const toolSet: ToolSet = {}; const includeLoadingMessage = options?.includeLoadingMessage ?? true; + const serializeOutput = options?.serializeOutput ?? false; for (const descriptor of descriptors) { const baseSchema = descriptor.inputSchema as Record; @@ -128,11 +131,15 @@ export class ToolRegistryService { ? stripLoadingMessage(args ?? {}) : (args ?? {}); - return this.toolExecutorService.dispatch( + const result = await this.toolExecutorService.dispatch( descriptor, cleanArgs, context, ); + + return serializeOutput + ? (compactToolOutput(result) as ToolOutput) + : result; }; toolSet[descriptor.name] = { @@ -165,7 +172,10 @@ export class ToolRegistryService { async getToolsByName( names: string[], context: ToolContext, - options?: { includeLoadingMessage?: boolean }, + options?: { + includeLoadingMessage?: boolean; + serializeOutput?: boolean; + }, ): Promise { const fullContext = this.buildContextFromToolContext(context); @@ -184,6 +194,7 @@ export class ToolRegistryService { return this.hydrateToolSet(descriptors, fullContext, { includeLoadingMessage: options?.includeLoadingMessage, + serializeOutput: options?.serializeOutput, }); } @@ -229,7 +240,7 @@ export class ToolRegistryService { toolName: string, args: Record | undefined, context: ToolContext, - _options: ToolExecutionOptions, + options?: { serializeOutput?: boolean }, ): Promise { try { const fullContext = this.buildContextFromToolContext(context); @@ -245,7 +256,15 @@ export class ToolRegistryService { }; } - return await this.toolExecutorService.dispatch(entry, args, fullContext); + const result = await this.toolExecutorService.dispatch( + entry, + args, + fullContext, + ); + + return options?.serializeOutput + ? (compactToolOutput(result) as ToolOutput) + : result; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); @@ -271,6 +290,7 @@ export class ToolRegistryService { excludeTools, wrapWithErrorContext, includeLoadingMessage, + serializeOutput, } = options; const categorySet = categories ? new Set(categories) : undefined; @@ -305,6 +325,7 @@ export class ToolRegistryService { const toolSet = this.hydrateToolSet(filteredDescriptors, context, { wrapWithErrorContext, includeLoadingMessage, + serializeOutput, }); if (categories?.includes(ToolCategory.NATIVE_MODEL)) { diff --git a/packages/twenty-server/src/engine/core-modules/tool-provider/tools/execute-tool.tool.ts b/packages/twenty-server/src/engine/core-modules/tool-provider/tools/execute-tool.tool.ts index f5335c8a6cf..4fe46a61ce4 100644 --- a/packages/twenty-server/src/engine/core-modules/tool-provider/tools/execute-tool.tool.ts +++ b/packages/twenty-server/src/engine/core-modules/tool-provider/tools/execute-tool.tool.ts @@ -1,4 +1,4 @@ -import { jsonSchema, type ToolExecutionOptions, type ToolSet } from 'ai'; +import { jsonSchema } from 'ai'; import { type JSONSchema7 } from 'json-schema'; import { z } from 'zod'; @@ -41,22 +41,24 @@ export const executeToolInputSchema = jsonSchema( }, ); +// All invocations route through the registry — there is no fast path for +// preloaded or native tools. Native tools are exposed to the model directly +// via the top-level ToolSet passed to streamText, not through this meta-tool. export const createExecuteToolTool = ( toolRegistry: ToolRegistryService, context: ToolContext, - directTools?: ToolSet, - excludeTools?: Set, + options?: { + excludeTools?: Set; + serializeOutput?: boolean; + }, ) => ({ description: 'STEP 3: Execute a tool by name with arguments. You MUST call get_tool_catalog (step 1) and learn_tools (step 2) first to discover the tool name and its required input schema.', inputSchema: executeToolInputSchema, - execute: async ( - parameters: ExecuteToolInput, - options: ToolExecutionOptions, - ): Promise => { + execute: async (parameters: ExecuteToolInput): Promise => { const { toolName, arguments: args = {} } = parameters; - if (excludeTools?.has(toolName)) { + if (options?.excludeTools?.has(toolName)) { return { success: false, message: `Tool "${toolName}" is not available`, @@ -64,12 +66,8 @@ export const createExecuteToolTool = ( }; } - const directTool = directTools?.[toolName]; - - if (directTool?.execute) { - return directTool.execute(args, options) as Promise; - } - - return toolRegistry.resolveAndExecute(toolName, args, context, options); + return toolRegistry.resolveAndExecute(toolName, args, context, { + serializeOutput: options?.serializeOutput, + }); }, }); diff --git a/packages/twenty-server/src/engine/metadata-modules/ai/ai-chat/services/chat-execution.service.ts b/packages/twenty-server/src/engine/metadata-modules/ai/ai-chat/services/chat-execution.service.ts index b7af4a47c0c..dd3b7a5ab00 100644 --- a/packages/twenty-server/src/engine/metadata-modules/ai/ai-chat/services/chat-execution.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/ai/ai-chat/services/chat-execution.service.ts @@ -23,7 +23,6 @@ import { CodeInterpreterService } from 'src/engine/core-modules/code-interpreter import { WorkspaceDomainsService } from 'src/engine/core-modules/domain/workspace-domains/services/workspace-domains.service'; import { ExceptionHandlerService } from 'src/engine/core-modules/exception-handler/exception-handler.service'; import { COMMON_PRELOAD_TOOLS } from 'src/engine/core-modules/tool-provider/constants/common-preload-tools.const'; -import { wrapToolsWithOutputSerialization } from 'src/engine/core-modules/tool-provider/output-serialization/wrap-tools-with-output-serialization.util'; import { ToolRegistryService } from 'src/engine/core-modules/tool-provider/services/tool-registry.service'; import { createExecuteToolTool, @@ -152,6 +151,7 @@ export class ChatExecutionService { const preloadedTools = await this.toolRegistry.getToolsByName( toolNamesToPreload, toolContext, + { serializeOutput: true }, ); const resolvedModelId = modelId ?? workspace.smartModel; @@ -175,10 +175,11 @@ export class ChatExecutionService { ? this.getNativeWebSearchTools(registeredModel) : { tools: {}, callableToolNames: [] }; - // Direct tools: native provider tools + preloaded tools. - // These are callable directly AND as fallback through execute_tool. + // Tools the model can call directly: preloaded registry tools (already + // serialized by the hydrator) plus SDK-native tools (opaque, never + // serialized). execute_tool routes discovered tools through the registry. const directTools: ToolSet = { - ...wrapToolsWithOutputSerialization(preloadedTools), + ...preloadedTools, ...nativeSearchTools, }; @@ -188,7 +189,7 @@ export class ChatExecutionService { ]; // ToolSet is constant for the entire conversation — no mutation. - // learn_tools returns schemas as text; execute_tool dispatches to cached tools. + // learn_tools returns schemas as text; execute_tool dispatches via the registry. const activeTools: ToolSet = { ...directTools, [LEARN_TOOLS_TOOL_NAME]: createLearnToolsTool( @@ -198,7 +199,7 @@ export class ChatExecutionService { [EXECUTE_TOOL_TOOL_NAME]: createExecuteToolTool( this.toolRegistry, toolContext, - directTools, + { serializeOutput: true }, ), [LOAD_SKILL_TOOL_NAME]: createLoadSkillTool( (skillNames) =>