mirror of
https://github.com/twentyhq/twenty
synced 2026-04-21 13:37:22 +00:00
fix(server): scope loadingMessage wrap/strip to AI-chat callers (#19896)
Some checks are pending
CD deploy main / deploy-main (push) Waiting to run
CI Create App E2E minimal / changed-files-check (push) Waiting to run
CI Create App E2E minimal / create-app-e2e-minimal (push) Blocked by required conditions
CI Create App E2E minimal / ci-create-app-e2e-minimal-status-check (push) Blocked by required conditions
CI Create App / changed-files-check (push) Waiting to run
CI Create App / create-app-test (lint) (push) Blocked by required conditions
CI Create App / create-app-test (test) (push) Blocked by required conditions
CI Create App / create-app-test (typecheck) (push) Blocked by required conditions
CI Create App / ci-create-app-status-check (push) Blocked by required conditions
CI Docs / changed-files-check (push) Waiting to run
CI Docs / docs-lint (push) Blocked by required conditions
CI Emails / changed-files-check (push) Waiting to run
CI Emails / emails-test (push) Blocked by required conditions
CI Emails / ci-emails-status-check (push) Blocked by required conditions
CI Example App Hello World / changed-files-check (push) Waiting to run
CI Example App Hello World / example-app-hello-world (push) Blocked by required conditions
CI Example App Hello World / ci-example-app-hello-world-status-check (push) Blocked by required conditions
CI Example App Postcard / example-app-postcard (push) Blocked by required conditions
CI Example App Postcard / changed-files-check (push) Waiting to run
CI Example App Postcard / ci-example-app-postcard-status-check (push) Blocked by required conditions
Push translations to Crowdin / Extract and upload translations (push) Waiting to run
Some checks are pending
CD deploy main / deploy-main (push) Waiting to run
CI Create App E2E minimal / changed-files-check (push) Waiting to run
CI Create App E2E minimal / create-app-e2e-minimal (push) Blocked by required conditions
CI Create App E2E minimal / ci-create-app-e2e-minimal-status-check (push) Blocked by required conditions
CI Create App / changed-files-check (push) Waiting to run
CI Create App / create-app-test (lint) (push) Blocked by required conditions
CI Create App / create-app-test (test) (push) Blocked by required conditions
CI Create App / create-app-test (typecheck) (push) Blocked by required conditions
CI Create App / ci-create-app-status-check (push) Blocked by required conditions
CI Docs / changed-files-check (push) Waiting to run
CI Docs / docs-lint (push) Blocked by required conditions
CI Emails / changed-files-check (push) Waiting to run
CI Emails / emails-test (push) Blocked by required conditions
CI Emails / ci-emails-status-check (push) Blocked by required conditions
CI Example App Hello World / changed-files-check (push) Waiting to run
CI Example App Hello World / example-app-hello-world (push) Blocked by required conditions
CI Example App Hello World / ci-example-app-hello-world-status-check (push) Blocked by required conditions
CI Example App Postcard / example-app-postcard (push) Blocked by required conditions
CI Example App Postcard / changed-files-check (push) Waiting to run
CI Example App Postcard / ci-example-app-postcard-status-check (push) Blocked by required conditions
Push translations to Crowdin / Extract and upload translations (push) Waiting to run
## Summary
MCP tool execution crashed with \`Cannot destructure property
'loadingMessage' of 'parameters' as it is undefined\` whenever
\`execute_tool\` was called without an inner \`arguments\` field. Root
cause: \`loadingMessage\` is an AI-chat UX affordance (lets the LLM
narrate progress so the chat UI can show "Sending email…") but it was
being wrapped into **every** tool schema — including those advertised to
external MCP clients — and \`dispatch\` unconditionally stripped it,
crashing on \`undefined\` args.
The fix scopes the wrap/strip pair to AI-chat callers only:
- Pair wrap and strip inside \`hydrateToolSet\` (they belong together).
- New \`includeLoadingMessage\` option on \`hydrateToolSet\` /
\`getToolsByName\` / \`getToolsByCategories\` (default \`true\` so
AI-chat behavior is unchanged).
- MCP opts out → external clients see clean inputSchemas without a
required \`loadingMessage\` field.
- \`dispatch\` no longer strips; args default to \`{}\` defensively.
- \`execute_tool\` defaults \`arguments\` to \`{}\` at the LLM boundary.
## Test plan
- [x] \`npx nx typecheck twenty-server\` passes
- [x] \`npx oxlint\` clean on changed files
- [x] \`npx jest mcp-protocol mcp-tool-executor\` — 23/23 tests pass
- [ ] Manually: call \`execute_tool\` via MCP with and without inner
\`arguments\` — verify no crash, endpoints execute
- [ ] Manually: inspect MCP \`tools/list\` response — verify
\`search_help_center\` schema no longer contains \`loadingMessage\`
- [ ] Regression: AI chat still streams loading messages as the LLM
calls tools
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
83bc6d1a1b
commit
13afef5d1d
5 changed files with 45 additions and 18 deletions
|
|
@ -118,6 +118,7 @@ export class McpProtocolService {
|
|||
const preloadedTools = await this.toolRegistry.getToolsByName(
|
||||
COMMON_PRELOAD_TOOLS,
|
||||
toolContext,
|
||||
{ includeLoadingMessage: false },
|
||||
);
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -4,4 +4,5 @@ export type ToolRetrievalOptions = {
|
|||
categories?: ToolCategory[];
|
||||
excludeTools?: string[];
|
||||
wrapWithErrorContext?: boolean;
|
||||
includeLoadingMessage?: boolean;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -33,7 +33,6 @@ import { type ToolDescriptor } from 'src/engine/core-modules/tool-provider/types
|
|||
import { type ToolExecutionRef } from 'src/engine/core-modules/tool-provider/types/tool-execution-ref.type';
|
||||
import { type ToolIndexEntry } from 'src/engine/core-modules/tool-provider/types/tool-index-entry.type';
|
||||
import { type ToolOutput } from 'src/engine/core-modules/tool/types/tool-output.type';
|
||||
import { stripLoadingMessage } from 'src/engine/core-modules/tool/utils/wrap-tool-for-execution.util';
|
||||
import { UserEntity } from 'src/engine/core-modules/user/user.entity';
|
||||
import { WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service';
|
||||
|
||||
|
|
@ -77,24 +76,24 @@ export class ToolExecutorService {
|
|||
|
||||
async dispatch(
|
||||
descriptor: ToolIndexEntry | ToolDescriptor,
|
||||
args: Record<string, unknown>,
|
||||
args: Record<string, unknown> | undefined,
|
||||
context: ToolProviderContext,
|
||||
): Promise<ToolOutput> {
|
||||
const cleanArgs = stripLoadingMessage(args);
|
||||
const safeArgs = args ?? {};
|
||||
|
||||
switch (descriptor.executionRef.kind) {
|
||||
case 'database_crud':
|
||||
return this.dispatchDatabaseCrud(
|
||||
descriptor.executionRef,
|
||||
cleanArgs,
|
||||
safeArgs,
|
||||
context,
|
||||
);
|
||||
case 'static':
|
||||
return this.dispatchStaticTool(descriptor, cleanArgs, context);
|
||||
return this.dispatchStaticTool(descriptor, safeArgs, context);
|
||||
case 'logic_function':
|
||||
return this.dispatchLogicFunction(
|
||||
descriptor.executionRef,
|
||||
cleanArgs,
|
||||
safeArgs,
|
||||
context,
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,7 +17,10 @@ import { type ToolDescriptor } from 'src/engine/core-modules/tool-provider/types
|
|||
import { type ToolIndexEntry } from 'src/engine/core-modules/tool-provider/types/tool-index-entry.type';
|
||||
import { wrapWithErrorHandler } from 'src/engine/core-modules/tool-provider/utils/tool-error.util';
|
||||
import { type ToolOutput } from 'src/engine/core-modules/tool/types/tool-output.type';
|
||||
import { wrapJsonSchemaForExecution } from 'src/engine/core-modules/tool/utils/wrap-tool-for-execution.util';
|
||||
import {
|
||||
stripLoadingMessage,
|
||||
wrapJsonSchemaForExecution,
|
||||
} from 'src/engine/core-modules/tool/utils/wrap-tool-for-execution.util';
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
|
||||
@Injectable()
|
||||
|
|
@ -104,23 +107,37 @@ export class ToolRegistryService {
|
|||
hydrateToolSet(
|
||||
descriptors: ToolDescriptor[],
|
||||
context: ToolProviderContext,
|
||||
options?: { wrapWithErrorContext?: boolean },
|
||||
options?: {
|
||||
wrapWithErrorContext?: boolean;
|
||||
includeLoadingMessage?: boolean;
|
||||
},
|
||||
): ToolSet {
|
||||
const toolSet: ToolSet = {};
|
||||
const includeLoadingMessage = options?.includeLoadingMessage ?? true;
|
||||
|
||||
for (const descriptor of descriptors) {
|
||||
const schemaWithLoading = wrapJsonSchemaForExecution(
|
||||
descriptor.inputSchema as Record<string, unknown>,
|
||||
);
|
||||
const baseSchema = descriptor.inputSchema as Record<string, unknown>;
|
||||
const schema = includeLoadingMessage
|
||||
? wrapJsonSchemaForExecution(baseSchema)
|
||||
: baseSchema;
|
||||
|
||||
const executeFn = async (
|
||||
args: Record<string, unknown>,
|
||||
): Promise<ToolOutput> =>
|
||||
this.toolExecutorService.dispatch(descriptor, args, context);
|
||||
): Promise<ToolOutput> => {
|
||||
const cleanArgs = includeLoadingMessage
|
||||
? stripLoadingMessage(args ?? {})
|
||||
: (args ?? {});
|
||||
|
||||
return this.toolExecutorService.dispatch(
|
||||
descriptor,
|
||||
cleanArgs,
|
||||
context,
|
||||
);
|
||||
};
|
||||
|
||||
toolSet[descriptor.name] = {
|
||||
description: descriptor.description,
|
||||
inputSchema: jsonSchema(schemaWithLoading),
|
||||
inputSchema: jsonSchema(schema),
|
||||
execute: options?.wrapWithErrorContext
|
||||
? wrapWithErrorHandler(descriptor.name, executeFn)
|
||||
: executeFn,
|
||||
|
|
@ -148,6 +165,7 @@ export class ToolRegistryService {
|
|||
async getToolsByName(
|
||||
names: string[],
|
||||
context: ToolContext,
|
||||
options?: { includeLoadingMessage?: boolean },
|
||||
): Promise<ToolSet> {
|
||||
const fullContext = this.buildContextFromToolContext(context);
|
||||
|
||||
|
|
@ -164,7 +182,9 @@ export class ToolRegistryService {
|
|||
inputSchema: schemas.get(entry.name)!,
|
||||
}));
|
||||
|
||||
return this.hydrateToolSet(descriptors, fullContext);
|
||||
return this.hydrateToolSet(descriptors, fullContext, {
|
||||
includeLoadingMessage: options?.includeLoadingMessage,
|
||||
});
|
||||
}
|
||||
|
||||
async getToolInfo(
|
||||
|
|
@ -207,7 +227,7 @@ export class ToolRegistryService {
|
|||
|
||||
async resolveAndExecute(
|
||||
toolName: string,
|
||||
args: Record<string, unknown>,
|
||||
args: Record<string, unknown> | undefined,
|
||||
context: ToolContext,
|
||||
_options: ToolExecutionOptions,
|
||||
): Promise<ToolOutput> {
|
||||
|
|
@ -246,7 +266,12 @@ export class ToolRegistryService {
|
|||
context: ToolProviderContext,
|
||||
options: ToolRetrievalOptions = {},
|
||||
): Promise<ToolSet> {
|
||||
const { categories, excludeTools, wrapWithErrorContext } = options;
|
||||
const {
|
||||
categories,
|
||||
excludeTools,
|
||||
wrapWithErrorContext,
|
||||
includeLoadingMessage,
|
||||
} = options;
|
||||
const categorySet = categories ? new Set(categories) : undefined;
|
||||
|
||||
const results = await Promise.all(
|
||||
|
|
@ -279,6 +304,7 @@ export class ToolRegistryService {
|
|||
|
||||
const toolSet = this.hydrateToolSet(filteredDescriptors, context, {
|
||||
wrapWithErrorContext,
|
||||
includeLoadingMessage,
|
||||
});
|
||||
|
||||
if (categories?.includes(ToolCategory.NATIVE_MODEL)) {
|
||||
|
|
|
|||
|
|
@ -54,7 +54,7 @@ export const createExecuteToolTool = (
|
|||
parameters: ExecuteToolInput,
|
||||
options: ToolExecutionOptions,
|
||||
): Promise<ToolOutput> => {
|
||||
const { toolName, arguments: args } = parameters;
|
||||
const { toolName, arguments: args = {} } = parameters;
|
||||
|
||||
if (excludeTools?.has(toolName)) {
|
||||
return {
|
||||
|
|
|
|||
Loading…
Reference in a new issue