fix: fix const McpStreamableProtocol spell mistake #2405
Conversation
Erica177
requested review from
2456868764,
NameHaibinZhang and
johnlanni
as code owners
修复McpStreamableProtocol常量拼写错误及关联配置更新变更概述问题修复
变更统计
变更文件
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2405 +/- ##
===========================================
+ Coverage 35.91% 46.03% +10.12%
===========================================
Files 69 81 +12
Lines 11576 13018 +1442
===========================================
+ Hits 4157 5993 +1836
+ Misses 7104 6679 -425
- Partials 315 346 +31 🚀 New features to boost your workflow:
|
![lingma-agents[bot]](https://avatars.githubusercontent.com/in/1198481?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题，需立即修复。例如：系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 3 | 严重问题，高优先级修复。例如：核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 0 | 主要问题，建议修复。例如：非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 0 | 次要问题，酬情优化。例如：代码格式不规范或注释缺失。 |
总计: 3 个问题
📋 评审意见详情
💡 代码实现建议
以下是文件级别的代码建议，聚焦于代码的可读性、可维护性和潜在问题。
🔹 registry/nacos/mcpserver/watcher.go (3 💬)
- 修正映射配置中的常量名称拼写错误，确保协议支持逻辑正确。 (L59-L62)
- 修正上游类型映射中的常量名称拼写错误，确保协议类型正确关联。 (L65-L68)
- 修正路由重写协议配置中的常量名称拼写错误，确保路由规则生效。 (L71-L72)
🚀 架构设计建议
以下是对代码架构和设计的综合分析，聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍1. 存在未修正的旧常量引用风险
在修正McpStreambleProtocol拼写错误后，需确认其他文件是否仍存在旧名称引用。当前PR仅修复了watcher.go中的相关引用，但未检查其他可能使用该常量的模块。若其他文件（如配置解析、协议处理模块）仍保留旧拼写，可能导致协议类型不匹配或路由规则失效等系统性问题。建议全局搜索并修正所有旧名称引用。
📌 关键代码
McpSSEProtocol = "mcp-sse"
McpStreamableProtocol = "mcp-streamable"provider.McpStreamableProtocol: true
provider.McpStreamableProtocol: mcpserver.UpstreamTypeStreamable
provider.McpStreamableProtocol: true若其他模块仍使用旧名称，可能导致协议类型判断错误、路由规则失效或配置加载失败。
🔍2. 协议常量分散管理存在维护风险
当前协议常量（如HttpProtocol、McpStreamableProtocol）虽集中定义在mcp_model.go，但未通过枚举或接口约束强制引用，可能导致开发人员在新模块中直接硬编码字符串。建议将协议常量定义为枚举类型或提供只读访问方法，减少拼写错误和维护成本。
📌 关键代码
StdioProtocol = "stdio"
HttpProtocol = "http"
...
McpStreamableProtocol = "mcp-streamable"新增协议类型时可能因重复定义导致不一致，或因拼写错误引发隐蔽性故障。
🔍3. 路由重写协议的映射逻辑需全局验证
修正McpStreamableProtocol后，需验证所有涉及协议类型映射的模块（如protocolUpstreamTypeMapping、routeRewriteProtocols）是否覆盖了新名称。当前PR仅修正了watcher.go中的映射，但未检查如server.go或config_parser.go等可能依赖旧名称的文件，可能导致部分场景下协议类型未被正确识别。
📌 关键代码
provider.McpStreamableProtocol: true路由规则或上游类型映射失效，导致服务发现异常或通信协议不兼容。
审查详情
📒 文件清单 （2 个文件）
📝 变更: 2 个文件
📝 变更文件:
registry/mcp_model.goregistry/nacos/mcpserver/watcher.go
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论，lingma-agents 将自动处理您的请求。例如：
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案，并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如：
-
@lingma-agents 分析这个方法的性能瓶颈并提供优化建议。
-
@lingma-agents 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如：
-
@lingma-agents 请总结上述讨论并提出解决方案。
-
@lingma-agents 请根据讨论内容生成优化代码。
| provider.HttpProtocol: true, | ||
| provider.HttpsProtocol: true, | ||
| provider.McpSSEProtocol: true, | ||
| provider.McpStreamableProtocol: true, |
There was a problem hiding this comment.
修正映射配置中的常量名称拼写错误，确保协议支持逻辑正确。
🟠 Critical | 🐞 Bugs
📋 问题详情
在supportedProtocols、protocolUpstreamTypeMapping和routeRewriteProtocols中，McpStreambleProtocol拼写错误可能导致协议不被识别或映射失败，需修正为McpStreamableProtocol。
💡 解决方案
更新所有协议映射的常量引用：
- provider.McpStreambleProtocol: true,
+ provider.McpStreamableProtocol: true,您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
| provider.HttpProtocol: mcpserver.UpstreamTypeRest, | ||
| provider.HttpsProtocol: mcpserver.UpstreamTypeRest, | ||
| provider.McpSSEProtocol: mcpserver.UpstreamTypeSSE, | ||
| provider.McpStreamableProtocol: mcpserver.UpstreamTypeStreamable, |
There was a problem hiding this comment.
修正上游类型映射中的常量名称拼写错误，确保协议类型正确关联。
🟠 Critical | 🐞 Bugs
📋 问题详情
在protocolUpstreamTypeMapping中，McpStreambleProtocol拼写错误可能导致协议类型无法正确映射到UpstreamTypeStreamable，需修正为McpStreamableProtocol。
💡 解决方案
更新上游类型映射的常量引用：
- provider.McpStreambleProtocol: mcpserver.UpstreamTypeStreamable,
+ provider.McpStreamableProtocol: mcpserver.UpstreamTypeStreamable,您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
| provider.McpSSEProtocol: true, | ||
| provider.McpStreamableProtocol: true, |
There was a problem hiding this comment.
修正路由重写协议配置中的常量名称拼写错误，确保路由规则生效。
🟠 Critical | 🐞 Bugs
📋 问题详情
在routeRewriteProtocols中，McpStreambleProtocol拼写错误可能导致路由重写规则失效，需修正为McpStreamableProtocol。
💡 解决方案
更新路由重写协议的常量引用：
- provider.McpStreambleProtocol: true,
+ provider.McpStreamableProtocol: true,🔧 建议代码
[!NOTE]
AI 生成代码 - 请在应用前检查逻辑、规范并测试
| provider.McpSSEProtocol: true, | |
| provider.McpStreamableProtocol: true, | |
| provider.McpSSEProtocol: true, | |
| provider.McpStreamableProtocol: true, | |
您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
4 participants
Ⅰ. Describe what this PR did
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews