[Fix] MCP client configuration parsing and tool registration #579
Conversation
- Add fallback to parse MCP servers config when mcpServers key is missing - Fix tool lookup to check both config.tools and service.globalTools - Preserve user-configured tool enabled/selector settings during schema registration - Clean up default config formatting to single line - Return and log actual registered tool count instead of all global tools - Fix tool registration loop to use forked tools copy and count correctly This fixes issues where MCP client configuration wasn't being parsed correctly and ensures tool settings are properly preserved across reloads.
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 improves the MCP client extension's handling of configuration and tool registration. It introduces more resilient parsing for server configurations, ensures user-defined tool settings are respected, and refines the process of tool discovery and reporting. The changes aim to make the client's tool management more reliable, accurate, and easier to maintain by addressing several underlying issues. 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
|
|
Caution Review failedThe pull request is closed. 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本次变更扩展了 MCP 命令的配置解析（支持将原始对象作为 mcpServers）、改为使用公开的 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 用户
participant Ext as 扩展
participant Cmd as 命令
participant Svc as MCP_Service
participant Global as globalTools
User->>Ext: 启动或修改配置
Ext->>Svc: 调用 registerClientTools()
Svc->>Global: 读取全局工具（创建副本）
Global-->>Svc: 返回工具列表副本
Svc->>Svc: 基于 config.tools 过滤/派生 enabled & selector，计数并返回数量
Svc-->>Ext: 返回已注册工具数量 (Promise<number>)
User->>Ext: 请求启用某工具
Ext->>Cmd: 触发 enable 流
Cmd->>Svc: 读取 service.globalTools，优先查找 config.tools[toolName]
Svc-->>Cmd: 返回工具实例/状态
Cmd-->>User: 返回操作结果
Estimated code review effort🎯 3 (中等) | ⏱️ ~25 分钟 Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
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://waps.l3s.uni-hannover.de/live/im_/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 fixes and improvements to the MCP client extension. The changes correctly address issues with configuration parsing, tool lookup, and preserving user settings. The tool registration logic is also refactored for better clarity and accuracy in tool counting. I've identified a potential crash in the configuration parsing logic and a minor typo that should be addressed. Overall, these are solid improvements to the extension's robustness and code quality.
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/extension-mcp/src/command.ts(2 hunks)packages/extension-mcp/src/index.ts(1 hunks)packages/extension-mcp/src/service.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/extension-mcp/src/service.ts (1)
packages/extension-mcp/src/index.ts (1)
logger(7-7)
⏰ 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 (6)
packages/extension-mcp/src/index.ts (1)
23-23: LGTM! 配置格式改进。将默认服务器配置简化为单行 JSON 字符串，提高了可读性和简洁性。
packages/extension-mcp/src/service.ts (3)
57-58: 工具计数逻辑改进。现在正确记录并输出实际注册的工具数量，而不是全局工具的总数。这提高了日志信息的准确性。
157-158: 正确保留用户配置。使用配置中的值作为优先，并提供合理的默认值（enabled 默认为 true，selector 默认为空数组）。这确保了用户自定义的工具设置不会在注册时被重置。
179-235: 工具注册逻辑改进。变更改进了工具注册流程：
- 使用工具副本进行迭代，提高代码清晰度
- 添加计数器追踪实际注册的工具数量
- 使用显式的
enabled === false检查，避免意外跳过工具- 正确返回已注册工具的数量
选择器逻辑正确处理了空选择器（允许所有消息）和特定选择器（按内容匹配）的情况。
packages/extension-mcp/src/command.ts (2)
126-131: 增强配置解析的健壮性。添加了对普通对象格式配置的支持，作为
mcpServers键缺失时的后备处理。这提高了配置解析的容错性。
254-255: 改进工具查找逻辑。变更包含两项改进：
- 使用公共属性
globalTools替代私有属性_globalTools，符合封装最佳实践- 优先使用
config.tools[toolName]中的配置覆盖，允许用户按配置自定义工具状态逻辑正确，确保了配置覆盖和后备查找都能正常工作。
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/extension-mcp/src/service.ts (1)
57-58: 修正变量名拼写错误导致的运行时错误。第 57 行声明了变量
toolLength，但第 58 行使用了toolLegth（拼写错误），这将导致运行时ReferenceError。应用此差异修正错误：
const toolLength = await this.registerClientTools() - logger.info(`MCP client found ${toolLegth} tools`) + logger.info(`MCP client found ${toolLength} tools`)
🧹 Nitpick comments (1)
packages/extension-mcp/src/service.ts (1)
218-220: 建议统一可选链使用方式以提高一致性。第 218 行使用可选链
toolConfig?.selector?.length，而第 192 行使用直接属性访问toolConfig.enabled。由于registerClientToolsToSchema()在第 158 行确保selector属性始终存在（默认为空数组），可选链在此处是不必要的。考虑应用此差异以保持一致性：
- if ((toolConfig?.selector?.length || 0) === 0) { + if (toolConfig.selector.length === 0) { return true } return history.some((message) => toolConfig.selector.some((selector) => getMessageContent(message.content).includes( selector ) ) )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/extension-mcp/src/service.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/extension-mcp/src/service.ts (1)
packages/extension-mcp/src/index.ts (1)
logger(7-7)
🪛 GitHub Check: CodeFactor
packages/extension-mcp/src/service.ts
[warning] 57-57: packages/extension-mcp/src/service.ts#L57
'toolLength' is assigned a value but never used. (@typescript-eslint/no-unused-vars)
🪛 GitHub Check: lint
packages/extension-mcp/src/service.ts
[failure] 57-57:
'toolLength' is assigned a value but never used
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (1)
packages/extension-mcp/src/service.ts (1)
192-195: 无需变更：工具配置在注册时已正确初始化，且未发现toolConfig?.enabled的可选链用法，禁用检查逻辑一致安全。
- Fix typo: toolLegth -> toolLength in logger message - Bump version to 1.3.0-alpha.5
This PR fixes several issues in the MCP client extension related to configuration parsing and tool registration.
Bug Fixes
mcpServerskey is missing from the config objectconfig.toolsandservice.globalTools, ensuring manually configured tools are foundenabledandselectorsettings from user configuration are now properly preserved during schema registration instead of being reset to defaultsOther Changes
globalToolsto use public property instead of private_globalTools