Fix: MCP client improvements and core stability enhancements#583
Conversation
- Change blacklist type from boolean to number (0/1) for better configurability - Add warning logs when token limits are exceeded in prompt chain - Add warning logs when message length exceeds model token limits - Update locales to reflect new blacklist configuration (0=disabled, 1=enabled) - Update blacklist middleware to check for value 1 instead of boolean true These changes improve debugging by providing clear warnings when token limits are exceeded and make the blacklist configuration more explicit and safer.
…ients - Fix incorrect import path in model.ts (logger import) - Replace legacy 'chathub' paths with 'chatluna' in temp directory handling - Refactor MCP service to support multiple client connections - Add command string splitting for MCP server configuration - Improve error handling and logging in MCP client initialization This commit addresses path consistency issues and enables the MCP extension to manage multiple client connections simultaneously.
- Add save() call after addDocuments to persist newly added vectors - Add save() call after delete to persist deletion changes This ensures vector store changes are immediately persisted to storage, preventing data loss if the application terminates unexpectedly.
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 delivers a suite of critical fixes and improvements aimed at bolstering the stability and functionality of the Model Context Protocol (MCP) client and core system components. Key enhancements include enabling robust multi-client support for MCP, ensuring data integrity through automatic vector store persistence, standardizing internal file paths, and refining the blacklist configuration. Additionally, improved logging for token limit overages will provide better insights for system optimization. These changes collectively contribute to a more resilient, scalable, and maintainable application. 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将黑名单配置从布尔改为数值（0/1）；在若干 LLM 提示与模型聚合流程加入 token 超限告警并调整计数时机；向量库在增删后立即持久化；临时目录统一为 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Service as MCP Service
participant Cfg as Config Parser
participant C as Client[1..n]
participant Tool as Tool Registry
User->>Service: init()
Service->>Cfg: 读取 MCP 服务器配置
loop 对每个服务器
Service->>C: 创建并连接
alt 连接成功
Service->>Service: _clients.push(client)
else 连接失败
Service->>Service: 跳过该客户端
end
end
Service->>C: 拉取各自工具列表
Service->>Tool: 聚合工具并建立 工具→客户端 映射
User->>Service: 调用工具 X(args)
Service->>Tool: 查找 X 对应客户端
Tool-->>Service: 返回目标客户端
Service->>C: 使用对应客户端执行 X(args)
C-->>Service: 返回结果
Service-->>User: 结果
sequenceDiagram
autonumber
participant Builder as Prompt Builder
participant Model as Model Adapter
participant Logger as Logger
Builder->>Builder: 聚合系统提示与作者备注
Builder->>Builder: 计算 inputTokens 并提前累加 usedTokens
alt usedTokens > sendTokenLimit
Builder->>Logger: warn 超限（系统/输入阶段）
Builder-->>Model: 截断或终止
else
Builder->>Builder: 聚合 messages 占位消息
alt usedTokens + nextMsgTokens > 限制
Builder->>Logger: warn 即将超限（消息阶段）
end
Builder-->>Model: 提交消息
end
Model->>Model: 统计消息 tokens
alt 超过 maxTokenLimit
Model->>Logger: warn 超限（模型层）
Model-->>Builder: 中断
else
Model-->>Builder: 正常继续
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 (26)
📒 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://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, including multi-client support for MCP, enhanced vector store persistence, and better logging for token limits. My review focuses on a few key areas. I've identified a potential bug in the vector store's delete functionality that could lead to data inconsistency. Additionally, I've pointed out a risk of tool name collisions with the new multi-client MCP support, which could cause unexpected behavior. The other changes, such as path corrections and configuration updates, look solid.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
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/platform/model.ts (1)
183-186: 确保记录器未初始化时不会抛错这里直接调用
logger.warn(...)，但logger在插件初始化前可能仍为undefined，会导致TypeError: Cannot read properties of undefined (reading 'warn')。同文件其他位置使用logger?.就是为了规避这一问题。请改成可选调用或提供后备记录器。建议修改如下：- if (usedTokens > this.sendTokenLimit) { - logger.warn( + if (usedTokens > this.sendTokenLimit) { + logger?.warn( `After system prompts, the max tokens exceeded: ${usedTokens} > ${this.sendTokenLimit}. Try increasing the adapter token limit or optimizing the system prompts.` ) }if ( usedTokens + messageTokens > this.sendTokenLimit - (documents.length > 0 ? 480 : 80) ) { - logger.warn( + logger?.warn( `Exceeded token limit (${usedTokens} + ${messageTokens} > ${this.sendTokenLimit}) of the message placeholder` ) break }Also applies to: 393-395
packages/core/src/llm-core/vectorstores/base.ts (1)
93-96: deleteAll 分支遗漏 save 调用此 PR 目标是“每次变更后立即持久化”。不过
deleteAll分支在删除后直接返回，没有调用save()，仍可能造成重启后数据回滚。建议补齐持久化步骤：if (options.deleteAll) { await this._docstore.delete({ deleteAll: true }) + await this.save() return }
🧹 Nitpick comments (1)
packages/core/src/config.ts (1)
26-26: 首选布尔类型表示二元开关文件 packages/core/src/config.ts:26
- 当
blackList仅表示启用/禁用时，应将类型由number改为boolean，符合 TypeScript 及 typescript-eslint 的 strict-boolean-expressions 建议，避免数字在布尔上下文中的隐性转换。- 若确实需要多级状态，使用枚举或字符串联合（如
"off" | "warn" | "error"），并在注释或文档中说明设计意图。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/core/src/locales/en-US.schema.ymlis excluded by!**/*.ymlpackages/core/src/locales/zh-CN.schema.ymlis excluded by!**/*.yml
📒 Files selected for processing (8)
packages/core/src/config.ts(2 hunks)packages/core/src/llm-core/chain/prompt.ts(3 hunks)packages/core/src/llm-core/platform/model.ts(2 hunks)packages/core/src/llm-core/vectorstores/base.ts(2 hunks)packages/core/src/middlewares/auth/black_list.ts(1 hunks)packages/core/src/middlewares/system/wipe.ts(1 hunks)packages/core/src/services/chat.ts(1 hunks)packages/extension-mcp/src/service.ts(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T12:28:21.006Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#543
File: packages/vector-store-service/src/vectorstore/faiss.ts:47-47
Timestamp: 2025-09-15T12:28:21.006Z
Learning: 在 packages/core/src/llm-core/vectorstores/base.ts 中，ChatLunaSaveableVectorStore 基类的 addDocuments 方法已经实现了向量存储和数据库文档存储的同步逻辑：先调用底层存储的 addDocuments，再调用 this._docstore.add() 进行备份，所有继承类（如 FaissVectorStore、MilvusVectorStore 等）都会自动获得这个同步功能。
Applied to files:
packages/core/src/llm-core/vectorstores/base.ts
🧬 Code graph analysis (3)
packages/core/src/llm-core/platform/model.ts (1)
packages/core/src/index.ts (1)
logger(38-38)
packages/core/src/llm-core/chain/prompt.ts (2)
packages/core/src/index.ts (1)
logger(38-38)packages/core/src/utils/string.ts (1)
getMessageContent(150-166)
packages/extension-mcp/src/service.ts (1)
packages/extension-mcp/src/index.ts (4)
logger(8-8)Config(31-45)Config(47-69)name(76-76)
🪛 GitHub Check: CodeFactor
packages/extension-mcp/src/service.ts
[notice] 56-158: packages/extension-mcp/src/service.ts#L56-L158
Complex Method
⏰ 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/core/src/middlewares/auth/black_list.ts (1)
14-14: 统一 blackList 的配置范围与中间件逻辑
config.ts 中 schema 使用 Schema.number().min(0).max(1)，允许 0~1 的任意值，但中间件仅在 resolved === 1 时触发黑名单，会忽略 0.5 等中间情况。
- 将 schema 限制为精确的 0 或 1
- 或将中间件逻辑改为 resolved !== 0
请确认设计意图：是否需要支持中间值（如 0.5 表示部分阻止），或仅需开/关状态（建议改用 boolean）。
- Add step(1) to blackList schema for better UI control - Add optional chaining to logger calls to prevent null reference errors - Fix MCP service destructuring to handle undefined tool mappings safely - Add optional chaining to toolConfig.enabled check for better null safety This commit addresses potential runtime errors and improves the robustness of the configuration system and MCP tool registration process.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/core/src/config.ts (1)
102-104: 仍存在冗余的.default(0)声明
.step(1)的添加有效限制了取值为 0 或 1，解决了之前评审中关于中间值的问题。但第 103 和 104 行仍然存在重复的.default(0)声明，应移除其中一个以保持代码简洁。应用以下修改移除冗余声明：
blackList: Schema.computed( - Schema.number().min(0).max(1).step(1).default(0) -).default(0) + Schema.number().min(0).max(1).step(1).default(0) +)或者保留外层的 default：
blackList: Schema.computed( - Schema.number().min(0).max(1).step(1).default(0) + Schema.number().min(0).max(1).step(1) ).default(0)packages/extension-mcp/src/service.ts (1)
186-189: 处理跨客户端工具名冲突当前循环在遇到同名工具时会直接覆盖已有映射，仍然存在之前指出的多客户端工具冲突隐患。至少应记录告警，必要时跳过或显式保留优先级，以避免无意中调用到错误的客户端工具。
for (const tool of mcpTools.tools) { - toolToClientMap[tool.name] = [client, tool] as const + if (tool.name in toolToClientMap) { + logger.warn( + `跨 MCP 客户端检测到重复的工具名 "${tool.name}"，将沿用最后注册的定义。` + ) + } + toolToClientMap[tool.name] = [client, tool] as const }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core/src/config.ts(2 hunks)packages/core/src/llm-core/chain/prompt.ts(3 hunks)packages/core/src/llm-core/platform/model.ts(2 hunks)packages/extension-mcp/src/service.ts(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/llm-core/platform/model.ts
- packages/core/src/llm-core/chain/prompt.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/extension-mcp/src/service.ts (2)
packages/extension-mcp/src/index.ts (3)
Config(31-45)Config(47-69)name(76-76)packages/extension-mcp/src/utils.ts (1)
callTool(436-486)
🪛 GitHub Check: CodeFactor
packages/extension-mcp/src/service.ts
[notice] 56-158: packages/extension-mcp/src/service.ts#L56-L158
Complex Method
⏰ 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
- Add save() call after deleteAll operation in vector store to persist changes - Add optional chaining to config.tools access to prevent null reference errors This ensures vector store deletions are properly persisted and prevents runtime errors when MCP tools configuration is undefined.
There was a problem hiding this comment.
Actionable comments posted: 0
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/service.ts (1)
115-126: 修复对未设置 type 的 SSE 检测当配置里省略
type（仅提供url或command）时，这里直接调用type.includes('sse')会触发TypeError，导致所有 MCP 客户端初始化失败。请先判空或使用可选链以兼容旧配置。- } else if (url.includes('sse') || type.includes('sse')) { + } else if (url.includes('sse') || type?.includes('sse')) {
♻️ Duplicate comments (2)
packages/extension-mcp/src/service.ts (2)
165-171: 避免在全局工具表中静默覆盖同名工具多个 MCP 客户端返回同名工具时，这里会悄悄覆盖早先的条目，只保留最后一个客户端的定义，造成工具选择不确定。请在写入前检测重复并至少发出告警（或直接跳过后续重复项）。
for (const tool of mcpTools.tools) { + if (schemaValueArray[tool.name]) { + logger.warn( + `检测到多个 MCP 客户端提供同名工具 "${tool.name}"，保留先到的定义并跳过后续重复` + ) + continue + } schemaValueArray[tool.name] = { name: tool.name, enabled: this.config.tools?.[tool.name]?.enabled ?? true, selector: this.config.tools?.[tool.name]?.selector ?? [] }
185-189: 构建 toolToClientMap 时同名工具仍被覆盖与上方逻辑类似，这里会把最后一个同名工具写进映射，导致调用时指向错误的客户端。请先检测是否已存在映射，至少输出警告并跳过后续重复项。
for (const tool of mcpTools.tools) { + if (toolToClientMap[tool.name]) { + logger.warn( + `检测到多个 MCP 客户端提供同名工具 "${tool.name}"，调用时将保留首个匹配` + ) + continue + } toolToClientMap[tool.name] = [client, tool] as const }
🧹 Nitpick comments (1)
packages/core/src/llm-core/vectorstores/base.ts (1)
119-120: 正确添加持久化调用以防止数据丢失。在按 ID 删除文档后调用
save()确保状态被持久化，这与 PR 的目标一致。可选建议： 在高吞吐量场景下，频繁的
save()调用可能影响性能。如果未来遇到性能瓶颈，可以考虑批量操作后再调用save()，或引入防抖机制。注意： 关于第 118 行缺少向量存储删除的问题，之前的审查评论已经标记。根据检索到的学习记录，基类的
delete()方法设计为仅处理文档存储清理，派生类需要重写此方法以实现特定的向量索引删除逻辑。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/llm-core/vectorstores/base.ts(3 hunks)packages/extension-mcp/src/service.ts(7 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-15T12:28:21.006Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#543
File: packages/vector-store-service/src/vectorstore/faiss.ts:47-47
Timestamp: 2025-09-15T12:28:21.006Z
Learning: 在 packages/core/src/llm-core/vectorstores/base.ts 中，ChatLunaSaveableVectorStore 基类的 addDocuments 方法已经实现了向量存储和数据库文档存储的同步逻辑：先调用底层存储的 addDocuments，再调用 this._docstore.add() 进行备份，所有继承类（如 FaissVectorStore、MilvusVectorStore 等）都会自动获得这个同步功能。
Applied to files:
packages/core/src/llm-core/vectorstores/base.ts
📚 Learning: 2025-09-15T12:02:03.617Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#543
File: packages/core/src/llm-core/vectorstores/base.ts:82-113
Timestamp: 2025-09-15T12:02:03.617Z
Learning: 在 packages/core/src/llm-core/vectorstores/base.ts 中，ChatLunaSaveableVectorStore 的 delete() 方法设计为只处理 docstore 清理，具体的向量索引删除由各个向量数据库的具体实现类（如 FaissVectorStore、MilvusVectorStore 等）重写实现，体现了基类处理通用逻辑、派生类处理特定逻辑的设计模式。
Applied to files:
packages/core/src/llm-core/vectorstores/base.ts
🧬 Code graph analysis (1)
packages/extension-mcp/src/service.ts (2)
packages/extension-mcp/src/index.ts (3)
Config(31-45)Config(47-69)name(76-76)packages/extension-mcp/src/utils.ts (1)
callTool(436-486)
🪛 GitHub Check: CodeFactor
packages/extension-mcp/src/service.ts
[notice] 56-158: packages/extension-mcp/src/service.ts#L56-L158
Complex Method
⏰ 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/core/src/llm-core/vectorstores/base.ts (2)
67-68: 正确添加持久化调用以防止数据丢失。在文档添加到向量存储和文档存储后立即调用
save()确保状态被持久化，符合 PR 目标。
95-95: 正确添加持久化调用以确保删除操作的一致性。在批量删除后调用
save()确保状态被正确持久化。
- Bump core version from 1.3.0-alpha.54 to 1.3.0-alpha.55
- Bump MCP extension version from 1.3.0-alpha.6 to 1.3.0-alpha.7
- Update all adapter and extension peer dependencies to match core version
- Add optional chaining to type.includes('sse') check in MCP service
This version bump includes the recent MCP client improvements and core
stability enhancements from the fix/mcps branch.
1 participant
This PR includes several critical fixes and improvements to the MCP (Model Context Protocol) functionality and core system stability.
Bug fixes
Other Changes
Technical Details
Commits included:
ca94f98d: Vector store persistence improvements69a7bf27: MCP client fixes and multi-client support8f61ed32: Blacklist configuration and token limit warningsFiles changed: