[Fix] Improve MCP tool handling and timeout support#613
Conversation
- Add timeout configuration support for MCP tool calls - Improve tool schema generation and display formatting - Enhance error handling in agent executor
Summary of ChangesHello @dingyi222666, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Model Context Protocol (MCP) extension by introducing configurable timeouts for tool executions, improving the reliability of file handling through accurate MIME type-based extension generation, and making the agent executor more resilient to unexpected tool observation formats. These changes aim to provide a more stable and configurable environment for MCP tool interactions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (28)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough此 PR：将工具非字符串观察值从致命错误改为记录并 stringify；为 MCP 配置与工具项添加可选/默认超时并把超时传入调用包装；将 MCP 客户端存储从数组改为 Map 并相应调整迭代与注册逻辑；修复 MIME 类型到文件后缀的映射与拼写错误，并新增 ToolException 导出。 Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent Executor
participant Service as MCP Client Service
participant MCP as MCP / Tool
participant Schema as Schema/Config
participant Result as Result Handler
rect rgba(120,200,255,0.08)
Note over Service,Schema: 启动时注册工具并计算超时（tool/server/default）
Service->>Schema: 读取 server/tool timeout
Schema-->>Service: 返回 timeout
end
Agent->>Service: 构建工具调用（含 config.timeout）
Service->>MCP: 调用工具 (含 timeout)
alt 工具返回字符串
MCP-->>Service: 字符串观察值
Service-->>Agent: 字符串观察值
Agent->>Result: 继续处理
else 工具返回非字符串
MCP-->>Service: 非字符串观察值
Service->>Service: 记录警告并 stringify
Service-->>Agent: 字符串观察值
Agent->>Result: 继续处理
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several valuable improvements to the MCP extension. The support for configurable timeouts at both the server and tool level is a great addition for managing tool execution. The refactoring of client storage to a Map is a good architectural change, and the fix for file extension generation using mime-types makes file handling more robust. The change to gracefully handle non-string tool observations also improves the agent's resilience.
I've found one issue in the timeout calculation logic where there's a unit mismatch and incorrect handling of a 0 timeout value. My review includes a specific suggestion to fix this. Overall, this is a solid set of changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/llm-core/agent/executor.ts (1)
587-591: 存在不一致的错误处理逻辑。Lines 728-733 已将非字符串观察值的处理从抛出错误改为记录警告并转换为 JSON 字符串，但此处（lines 587-591）仍然抛出错误。这导致两个相似的代码路径对相同情况的处理不一致，可能造成不可预测的行为。
建议应用以下修复以保持一致性：
observation = tool ? await tool.invoke( action.toolInput, patchConfig(config, { callbacks: runManager?.getChild() }) ) : `${action.tool} is not a valid tool, try another one.` if (typeof observation !== 'string') { - throw new Error( - 'Received unsupported non-string response from tool call.' - ) + logger.warn( + `Tool ${action.tool} returned non-string observation`, + observation + ) + observation = JSON.stringify(observation) }packages/extension-mcp/src/utils.ts (1)
492-492: 修复参数名称中的拼写错误。函数参数名称仍为
mineType（拼写错误），但在函数内部使用时却是正确的mimeType（line 499, 502）。这与 PR 目标（修复 issue #611 中的mineType拼写错误）不一致。应用以下修复：
async function putResourceToStorage( ctx: Context, blob: string | Buffer, - mineType: string + mimeType: string ) { if (!ctx.chatluna_storage) { return } const buffer = typeof blob === 'string' ? Buffer.from(blob, 'base64') : blob - const extension = mimeTypes.extension(mineType) + const extension = mimeTypes.extension(mimeType) if (!extension) { - throw new Error(`Unsupported mime type: ${mineType}`) + throw new Error(`Unsupported mime type: ${mimeType}`) } const fileName = `file.${extension}` return await ctx.chatluna_storage.createTempFile(buffer, fileName) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/extension-mcp/package.jsonis excluded by!**/*.json
📒 Files selected for processing (4)
packages/core/src/llm-core/agent/executor.ts(1 hunks)packages/extension-mcp/src/index.ts(3 hunks)packages/extension-mcp/src/service.ts(6 hunks)packages/extension-mcp/src/utils.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/extension-mcp/src/service.ts (1)
packages/extension-mcp/src/index.ts (2)
Config(26-41)Config(43-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (9)
packages/core/src/llm-core/agent/executor.ts (1)
728-733: 改进非字符串观察值的处理逻辑。将抛出错误改为记录警告并转换为 JSON 字符串是一个很好的防御性修复，使得工具调用更加健壮。不过请确保同时修复 lines 587-591 处的类似逻辑以保持一致性。
packages/extension-mcp/src/index.ts (2)
32-32: 超时配置已正确添加。为工具配置添加了超时字段，默认值为 60 秒。此配置将在 service.ts 中被转换为毫秒（乘以 1000），逻辑合理。
53-53: 服务器和工具级别的超时配置已添加。在配置接口中正确添加了可选的 timeout 字段，支持在服务器级别和工具级别设置超时。这满足了 issue #612 中要求的自定义超时支持。
Also applies to: 62-62
packages/extension-mcp/src/utils.ts (2)
499-506: MIME 类型到文件扩展名的转换逻辑正确。使用
mime-types库正确地将 MIME 类型转换为文件扩展名，并在不支持的 MIME 类型时抛出错误。这修复了 issue #611 中提到的问题，不再使用硬编码的 MIME 类型作为文件后缀（如xxx.image/png），而是使用正确的扩展名（如file.png）。
163-163: Line 163 的拼写修复正确。此处已将
content.mineType正确修复为content.mimeType，符合 PR 目标。packages/extension-mcp/src/service.ts (4)
15-15: 客户端存储重构为 Map 结构。将
_clients从数组重构为Map<Config['server'][0], Client>是一个很好的改进，能够更好地追踪服务器配置与客户端的对应关系，便于后续根据服务器配置查找和管理客户端。
226-228: 超时配置已正确传递给工具调用。将超时配置通过
config对象传递给callTool函数，确保工具调用时能够使用配置的超时值。这满足了 issue #612 的需求，支持为 MCP 工具设置自定义超时。
146-146: Map 操作已正确更新。客户端的添加和存在性检查已正确更新为使用 Map 的
set和size方法，与新的数据结构保持一致。Also applies to: 158-158
266-269: 停止方法已正确更新。
stop()方法正确地遍历 Map 的所有客户端值，关闭每个客户端连接，并在最后清空 Map。这确保了资源的正确释放。
Fix timeout calculation to properly handle default values and convert to milliseconds correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/extension-mcp/src/utils.ts (1)
489-507: 修复参数名称中的拼写错误。函数参数名称在第 492 行仍然是
mineType（拼写错误），应该是mimeType。这与 PR 目标不一致，PR 的目标是修复所有mimeType拼写错误的出现位置。此外，使用
mime-types库来正确映射 MIME 类型到文件扩展名是一个很好的改进，解决了 issue #611 中提到的文件扩展名问题。应用此差异修复参数名称：
async function putResourceToStorage( ctx: Context, blob: string | Buffer, - mineType: string + mimeType: string ) { if (!ctx.chatluna_storage) { return } const buffer = typeof blob === 'string' ? Buffer.from(blob, 'base64') : blob - const extension = mimeTypes.extension(mineType) + const extension = mimeTypes.extension(mimeType) if (!extension) { - throw new Error(`Unsupported mime type: ${mineType}`) + throw new Error(`Unsupported mime type: ${mimeType}`) } const fileName = `file.${extension}` return await ctx.chatluna_storage.createTempFile(buffer, fileName) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/extension-mcp/src/service.ts(6 hunks)packages/extension-mcp/src/utils.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension-mcp/src/service.ts
🧰 Additional context used
🪛 ESLint
packages/extension-mcp/src/utils.ts
[error] 20-20: 'Logger' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Check: CodeFactor
packages/extension-mcp/src/utils.ts
[warning] 20-20: packages/extension-mcp/src/utils.ts#L20
'Logger' is defined but never used. (@typescript-eslint/no-unused-vars)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (2)
packages/extension-mcp/src/utils.ts (2)
28-42: 很好的错误处理改进！新增的
ToolException类和isToolException类型守卫为工具异常提供了结构化的错误处理。这使得错误识别更加明确，并支持更好的错误处理逻辑。
163-163: 正确修复了拼写错误。已将
content.mineType正确更正为content.mimeType，解决了 PR 目标中提到的拼写错误问题。
- Bump core package version to 1.3.0-alpha.74 - Bump extension-mcp version to 1.3.0-alpha.13 - Update peer dependencies across all adapter and extension packages - Add timeout locale entries for MCP tool configuration
1 participant
This PR improves the MCP extension with better tool handling, timeout configuration support, and bug fixes.
Bug Fixes
Other Changes
mime-typesdependency for proper MIME type to file extension conversionmineType→mimeType