mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-04-30 04:30:48 +00:00
feat(mcp): add tool validation and invalid tool indicators in MCP management dialog
- Add tool filtering in AnthropicContentGenerator (skip tools without name/description) - Add isValid and invalidReason fields to MCPToolDisplayInfo type - Show invalid tool warnings in ServerList, ServerDetail, ToolList, and ToolDetail steps - Add translations for all 6 languages (en, zh, de, ja, pt, ru) - Add tests for Anthropic converter and MCP utils
This commit is contained in:
LaZzyMan
parent
a608fdd243
commit
fae195eaa8
16 changed files with 399 additions and 9 deletions
|
|
@ -30,6 +30,7 @@ import {
|
|||
createDebugLogger,
|
||||
} from '@qwen-code/qwen-code-core';
|
||||
import { loadSettings, SettingScope } from '../../../config/settings.js';
|
||||
import { isToolValid, getToolInvalidReasons } from './utils.js';
|
||||
|
||||
const debugLogger = createDebugLogger('MCP_DIALOG');
|
||||
|
||||
|
|
@ -105,6 +106,11 @@ export const MCPManagementDialog: React.FC<MCPManagementDialogProps> = ({
|
|||
// Use config.isMcpServerDisabled() to check if server is disabled
|
||||
const isDisabled = config.isMcpServerDisabled(name);
|
||||
|
||||
// Count invalid tools (missing name or description)
|
||||
const invalidToolCount = serverTools.filter(
|
||||
(t) => !t.name || !t.description,
|
||||
).length;
|
||||
|
||||
serverInfos.push({
|
||||
name,
|
||||
status,
|
||||
|
|
@ -112,6 +118,7 @@ export const MCPManagementDialog: React.FC<MCPManagementDialogProps> = ({
|
|||
scope,
|
||||
config: serverConfig,
|
||||
toolCount: serverTools.length,
|
||||
invalidToolCount,
|
||||
promptCount: serverPrompts.length,
|
||||
isDisabled,
|
||||
});
|
||||
|
|
@ -191,13 +198,26 @@ export const MCPManagementDialog: React.FC<MCPManagementDialogProps> = ({
|
|||
mcpTools.push(tool);
|
||||
}
|
||||
}
|
||||
return mcpTools.map((tool) => ({
|
||||
name: tool.name,
|
||||
description: tool.description,
|
||||
serverName: tool.serverName,
|
||||
schema: tool.parameterSchema as object | undefined,
|
||||
annotations: tool.annotations,
|
||||
}));
|
||||
return mcpTools.map((tool) => {
|
||||
// Check if tool is valid (has both name and description required by LLM)
|
||||
const isValid = isToolValid(tool.name, tool.description);
|
||||
|
||||
let invalidReason: string | undefined;
|
||||
if (!isValid) {
|
||||
const reasons = getToolInvalidReasons(tool.name, tool.description);
|
||||
invalidReason = reasons.map((r) => t(r)).join(', ');
|
||||
}
|
||||
|
||||
return {
|
||||
name: tool.name || t('(unnamed)'),
|
||||
description: tool.description,
|
||||
serverName: tool.serverName,
|
||||
schema: tool.parameterSchema as object | undefined,
|
||||
annotations: tool.annotations,
|
||||
isValid,
|
||||
invalidReason,
|
||||
};
|
||||
});
|
||||
}, [config, selectedServer]);
|
||||
|
||||
// View tool list
|
||||
|
|
|
|||
|
|
@ -156,6 +156,13 @@ export const ServerDetailStep: React.FC<ServerDetailStepProps> = ({
|
|||
<Text>
|
||||
{server.toolCount}{' '}
|
||||
{server.toolCount === 1 ? t('tool') : t('tools')}
|
||||
{!!server.invalidToolCount && server.invalidToolCount > 0 && (
|
||||
<Text color={theme.status.warning}>
|
||||
{' '}
|
||||
({server.invalidToolCount}{' '}
|
||||
{server.invalidToolCount === 1 ? t('invalid') : t('invalid')})
|
||||
</Text>
|
||||
)}
|
||||
</Text>
|
||||
</Box>
|
||||
</Box>
|
||||
|
|
|
|||
|
|
@ -154,6 +154,15 @@ export const ServerListStep: React.FC<ServerListStepProps> = ({
|
|||
{server.isDisabled && (
|
||||
<Text color={theme.status.warning}> {t('(disabled)')}</Text>
|
||||
)}
|
||||
{/* 显示无效工具警告 */}
|
||||
{!!server.invalidToolCount && server.invalidToolCount > 0 && (
|
||||
<Text color={theme.status.warning}>
|
||||
{' '}
|
||||
{t('{{count}} invalid tools', {
|
||||
count: String(server.invalidToolCount),
|
||||
})}
|
||||
</Text>
|
||||
)}
|
||||
</Box>
|
||||
);
|
||||
})}
|
||||
|
|
|
|||
|
|
@ -137,6 +137,23 @@ export const ToolDetailStep: React.FC<ToolDetailStepProps> = ({
|
|||
|
||||
return (
|
||||
<Box flexDirection="column" gap={1}>
|
||||
{/* 无效工具警告 */}
|
||||
{!tool.isValid && (
|
||||
<Box flexDirection="column" marginBottom={1}>
|
||||
<Text color={theme.status.error} bold>
|
||||
{t('Warning: This tool cannot be called by the LLM')}
|
||||
</Text>
|
||||
<Text color={theme.status.error}>
|
||||
{t('Reason')}: {tool.invalidReason || t('unknown')}
|
||||
</Text>
|
||||
<Text color={theme.text.secondary}>
|
||||
{t(
|
||||
'Tools must have both name and description to be used by the LLM.',
|
||||
)}
|
||||
</Text>
|
||||
</Box>
|
||||
)}
|
||||
|
||||
{/* 工具描述 */}
|
||||
{tool.description && (
|
||||
<Box>
|
||||
|
|
|
|||
|
|
@ -111,7 +111,18 @@ export const ToolListStep: React.FC<ToolListStepProps> = ({
|
|||
>
|
||||
{tool.name}
|
||||
</Text>
|
||||
{annotations && (
|
||||
{/* 显示无效工具警告 */}
|
||||
{!tool.isValid && (
|
||||
<>
|
||||
<Text color={theme.text.secondary}> </Text>
|
||||
<Text color={theme.status.warning}>
|
||||
{t('invalid: {{reason}}', {
|
||||
reason: tool.invalidReason || t('unknown'),
|
||||
})}
|
||||
</Text>
|
||||
</>
|
||||
)}
|
||||
{annotations && tool.isValid && (
|
||||
<>
|
||||
<Text color={theme.text.secondary}> </Text>
|
||||
<Text color={theme.text.secondary}>{annotations}</Text>
|
||||
|
|
|
|||
|
|
@ -41,6 +41,8 @@ export interface MCPServerDisplayInfo {
|
|||
config: MCPServerConfig;
|
||||
/** 工具数量 */
|
||||
toolCount: number;
|
||||
/** 无效工具数量（缺少name或description） */
|
||||
invalidToolCount?: number;
|
||||
/** Prompt数量 */
|
||||
promptCount: number;
|
||||
/** 错误信息 */
|
||||
|
|
@ -69,6 +71,10 @@ export interface MCPToolDisplayInfo {
|
|||
idempotentHint?: boolean;
|
||||
openWorldHint?: boolean;
|
||||
};
|
||||
/** 工具是否有效（有name和description才能被LLM调用） */
|
||||
isValid: boolean;
|
||||
/** 无效原因（当isValid为false时） */
|
||||
invalidReason?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
159
packages/cli/src/ui/components/mcp/utils.test.ts
Normal file
159
packages/cli/src/ui/components/mcp/utils.test.ts
Normal file
|
|
@ -0,0 +1,159 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Qwen
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import {
|
||||
groupServersBySource,
|
||||
getStatusColor,
|
||||
getStatusIcon,
|
||||
truncateText,
|
||||
formatServerCommand,
|
||||
isToolValid,
|
||||
getToolInvalidReasons,
|
||||
} from './utils.js';
|
||||
import type { MCPServerDisplayInfo } from './types.js';
|
||||
import { MCPServerStatus } from '@qwen-code/qwen-code-core';
|
||||
|
||||
describe('MCP utils', () => {
|
||||
describe('groupServersBySource', () => {
|
||||
it('should group servers by source', () => {
|
||||
const servers: MCPServerDisplayInfo[] = [
|
||||
{
|
||||
name: 'server1',
|
||||
status: MCPServerStatus.CONNECTED,
|
||||
source: 'user',
|
||||
scope: 'user',
|
||||
config: { command: 'cmd1' },
|
||||
toolCount: 1,
|
||||
promptCount: 0,
|
||||
isDisabled: false,
|
||||
},
|
||||
{
|
||||
name: 'server2',
|
||||
status: MCPServerStatus.CONNECTED,
|
||||
source: 'extension',
|
||||
scope: 'extension',
|
||||
config: { command: 'cmd2' },
|
||||
toolCount: 2,
|
||||
promptCount: 0,
|
||||
isDisabled: false,
|
||||
},
|
||||
];
|
||||
|
||||
const result = groupServersBySource(servers);
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0].source).toBe('user');
|
||||
expect(result[0].servers).toHaveLength(1);
|
||||
expect(result[1].source).toBe('extension');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getStatusColor', () => {
|
||||
it('should return correct colors for each status', () => {
|
||||
expect(getStatusColor(MCPServerStatus.CONNECTED)).toBe('green');
|
||||
expect(getStatusColor(MCPServerStatus.CONNECTING)).toBe('yellow');
|
||||
expect(getStatusColor(MCPServerStatus.DISCONNECTED)).toBe('red');
|
||||
expect(getStatusColor('unknown' as MCPServerStatus)).toBe('gray');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getStatusIcon', () => {
|
||||
it('should return correct icons for each status', () => {
|
||||
expect(getStatusIcon(MCPServerStatus.CONNECTED)).toBe('✓');
|
||||
expect(getStatusIcon(MCPServerStatus.CONNECTING)).toBe('…');
|
||||
expect(getStatusIcon(MCPServerStatus.DISCONNECTED)).toBe('✗');
|
||||
expect(getStatusIcon('unknown' as MCPServerStatus)).toBe('?');
|
||||
});
|
||||
});
|
||||
|
||||
describe('truncateText', () => {
|
||||
it('should truncate text longer than maxLength', () => {
|
||||
expect(truncateText('hello world', 8)).toBe('hello...');
|
||||
});
|
||||
|
||||
it('should not truncate text shorter than maxLength', () => {
|
||||
expect(truncateText('hello', 10)).toBe('hello');
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatServerCommand', () => {
|
||||
it('should format http URL', () => {
|
||||
const server = {
|
||||
config: { httpUrl: 'http://localhost:3000' },
|
||||
} as MCPServerDisplayInfo;
|
||||
expect(formatServerCommand(server)).toBe('http://localhost:3000 (http)');
|
||||
});
|
||||
|
||||
it('should format stdio command', () => {
|
||||
const server = {
|
||||
config: { command: 'node', args: ['server.js'] },
|
||||
} as MCPServerDisplayInfo;
|
||||
expect(formatServerCommand(server)).toBe('node server.js (stdio)');
|
||||
});
|
||||
|
||||
it('should return Unknown for empty config', () => {
|
||||
const server = { config: {} } as MCPServerDisplayInfo;
|
||||
expect(formatServerCommand(server)).toBe('Unknown');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isToolValid', () => {
|
||||
it('should return true for valid tool with name and description', () => {
|
||||
expect(isToolValid('toolName', 'A description')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for tool without name', () => {
|
||||
expect(isToolValid(undefined, 'A description')).toBe(false);
|
||||
expect(isToolValid('', 'A description')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for tool without description', () => {
|
||||
expect(isToolValid('toolName', undefined)).toBe(false);
|
||||
expect(isToolValid('toolName', '')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for tool without both name and description', () => {
|
||||
expect(isToolValid(undefined, undefined)).toBe(false);
|
||||
expect(isToolValid('', '')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getToolInvalidReasons', () => {
|
||||
it('should return empty array for valid tool', () => {
|
||||
expect(getToolInvalidReasons('toolName', 'A description')).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return missing name reason', () => {
|
||||
expect(getToolInvalidReasons(undefined, 'A description')).toEqual([
|
||||
'missing name',
|
||||
]);
|
||||
expect(getToolInvalidReasons('', 'A description')).toEqual([
|
||||
'missing name',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should return missing description reason', () => {
|
||||
expect(getToolInvalidReasons('toolName', undefined)).toEqual([
|
||||
'missing description',
|
||||
]);
|
||||
expect(getToolInvalidReasons('toolName', '')).toEqual([
|
||||
'missing description',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should return both reasons when both are missing', () => {
|
||||
expect(getToolInvalidReasons(undefined, undefined)).toEqual([
|
||||
'missing name',
|
||||
'missing description',
|
||||
]);
|
||||
expect(getToolInvalidReasons('', '')).toEqual([
|
||||
'missing name',
|
||||
'missing description',
|
||||
]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -101,3 +101,29 @@ export function formatServerCommand(server: MCPServerDisplayInfo): string {
|
|||
}
|
||||
return 'Unknown';
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a tool is valid (has both name and description required by LLM)
|
||||
* @param name - Tool name
|
||||
* @param description - Tool description
|
||||
* @returns boolean indicating if the tool is valid
|
||||
*/
|
||||
export function isToolValid(name?: string, description?: string): boolean {
|
||||
return !!name && !!description;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the reason why a tool is invalid
|
||||
* @param name - Tool name
|
||||
* @param description - Tool description
|
||||
* @returns Array of missing fields
|
||||
*/
|
||||
export function getToolInvalidReasons(
|
||||
name?: string,
|
||||
description?: string,
|
||||
): string[] {
|
||||
const reasons: string[] = [];
|
||||
if (!name) reasons.push('missing name');
|
||||
if (!description) reasons.push('missing description');
|
||||
return reasons;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue