Feat: Support Credential management and tree argument edit #13301
Conversation
* Support Credential Management and ref credential in mcp server for templates ref * Support edit input schema via argument tree and support object/array argument
|
Thanks for your this PR. 🙏 感谢您提交的PR。 🙏 |
新增凭证管理功能及参数结构树形编辑支持变更文件
时序图sequenceDiagram
participant User as 用户
participant CMPage as CredentialManagement页面
participant API as 后端API
participant DetailPage as 新建凭证页面
User->>CMPage: 访问凭证管理列表
CMPage->>API: 获取凭证列表数据(getConfig)
API-->>CMPage: 返回凭证数据列表
User->>CMPage: 点击新建凭证
CMPage->>DetailPage: 路由跳转到新建页面
DetailPage->>User: 展示JSON编辑器
User->>DetailPage: 编辑凭证内容并提交
DetailPage->>API: 发送创建请求(postConfig)
API-->>DetailPage: 返回操作结果
DetailPage->>User: 显示成功提示并跳转列表
User->>CMPage: 刷新列表查看新凭证
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
There was a problem hiding this comment.
🔍 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 1 | 阻断性问题，需立即修复。例如：系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 6 | 严重问题，高优先级修复。例如：核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 13 | 主要问题，建议修复。例如：非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 10 | 次要问题，酌情优化。例如：代码格式不规范或注释缺失。 |
总计： 30 个问题
⚠️ **需要立即关注的阻断性问题**
console-ui/src/pages/AI/CredentialManagement/CredentialManagement.js
- 删除操作缺少必要权限验证 (L245-L264)
📋 评审意见详情
💡 单文件建议
以下是文件级别的代码建议，聚焦于代码的可读性、可维护性和潜在问题。
📜 console-ui/src/components/NameSpaceList/NameSpaceList.js (4 💬)
- 重复的路由判断逻辑应提取为公共方法 (L146-L148)
- 应使用React Router进行路由判断 (L147-L148)
- 路由判断逻辑应使用精确路径匹配 (L147-L148)
- 条件判断变量命名不明确 (L146)
📜 console-ui/src/locales/en-US.js (2 💬)
- 存在翻译拼写错误 (L72)
- Credential管理功能的英文翻译拼写错误 (L72)
📜 console-ui/src/locales/zh-CN.js (2 💬)
- Credential管理中文翻译不完整 (L70)
- Credential管理中文翻译不专业 (L70)
📜 console-ui/src/pages/AI/CredentialManagement/CredentialManagement.js (10 💬)
- 使用常量替换硬编码的魔法值以提升代码可维护性 (L121-L207)
- 批量删除接口存在注入风险 (L393)
- 状态更新存在组件卸载风险 (L399-L402)
- 删除操作缺少必要权限验证 (L245-L264)
- 分页参数处理存在类型转换风险 (L325)
- URL参数拼接未进行编码导致注入风险 (L126-L128)
- 分页参数传递不完整导致逻辑错误 (L309)
- 组件属性拼写错误影响功能 (L429)
- 直接操作实例变量导致状态不一致 (L159)
- 事件处理缺少参数捕获导致潜在问题 (L289)
📜 console-ui/src/pages/AI/CredentialManagement/index.js (3 💬)
- 组件导出缺少JSDoc注释，影响可维护性 (L17-L19)
- 导入和导出语句可以合并为更简洁的写法 (L17-L19)
- 版权声明的结束年份未更新到当前年份 (L2)
📜 console-ui/src/pages/AI/McpDetail/CreateTools/index.js (7 💬)
- 状态更新逻辑存在直接修改不可变对象的问题 (L67-L75)
- 递归函数可能导致栈溢出风险 (L61-L77)
- 状态更新未使用函数式更新导致数据不一致 (L432-L433)
- 空值处理不完善可能导致运行时错误 (L404-L405)
- 表单值更新未使用受控组件模式 (L600-L605)
- 数组直接修改导致状态更新不可预测 (L109-L110)
- 状态重置类型不匹配风险 (L187)
🚀 跨文件建议
以下是对代码架构和设计的综合分析，聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍 1. 路由常量命名不一致导致系统架构不统一
在src/layouts/menu.js中新增的CredentialManagementRoute常量与原有McpServerManagementRoute命名风格不一致，存在部分单词首字母大写、部分全小写的混合情况。这种命名不一致性会破坏系统架构的规范性，导致后续维护时路由常量难以统一管理和扩展。建议采用统一的帕斯卡命名法（首字母大写）或蛇形命名法，并在全系统中保持一致性。
🔍 2. 参数树结构缺乏统一数据模型
CreateTools组件中定义的参数树节点结构（如args对象的children数组）与CredentialManagement中存储的配置数据结构存在差异。这种数据模型的不一致会导致跨组件数据转换复杂度增加，当需要在McpDetail组件展示参数树时可能引发数据适配问题。建议建立统一的参数树数据规范并提供数据转换工具。
🔍 3. 批量删除接口存在注入风险未全局处理
CredentialManagement组件的批量删除接口在构造URL时直接拼接ID参数（第393行），而类似操作在McpDetail的CreateTools中处理参数树时也存在直接拼接参数的情况。这种硬编码拼接方式存在SQL注入风险，需要在整个系统中统一采用参数化查询或安全的URL编码处理。
🔍 4. 参数树编辑逻辑存在递归深度风险
CreateTools组件的convertPropertiesToTreeData方法采用递归方式处理对象嵌套，当参数树层级超过JavaScript引擎的栈深度限制时（如超过100层）会导致栈溢出。建议改用迭代方式处理嵌套结构，或添加最大层级限制防止无限递归。
🔍 5. Credential配置未实施权限验证
CredentialManagement的删除操作（第245行）和NewCredential的发布接口（第369行）均未实现权限验证逻辑，与系统其他配置管理模块（如McpServerManagement）的权限控制机制不一致。这种安全策略的不统一会破坏系统整体安全性。
🔍 6. 命名空间管理存在架构冲突
Credential配置固定使用'nacos-default-mcp'命名空间（如第121行），而系统其他组件（如McpDetail）支持动态命名空间选择。这种硬编码命名空间的方式与系统多命名空间架构设计存在冲突，限制了功能扩展性。
🔍 7. 参数内容编辑未实施版本控制
NewCredential组件的发布功能（第369行）直接覆盖原有配置，未实现版本历史记录和回滚机制，这与系统其他配置管理模块（如Configuration）的版本控制策略不一致。这种不一致会破坏系统整体的变更管理能力。
🔍 8. 国际化翻译存在术语不一致
en-US.js中的'Credential Managenment'（第72行）存在拼写错误，且中文翻译'Credential管理'（zh-CN第70行）未使用标准术语。这种术语不一致会破坏系统国际化规范，影响用户体验的一致性。
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论，lingma-agents 将自动处理您的请求。例如：
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案，并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如：
-
@Lingma-Agent 分析这个方法的性能瓶颈并提供优化建议。
-
@Lingma-Agent 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如：
-
@Lingma-Agent 请总结上述讨论并提出解决方案。
-
@Lingma-Agent 请根据讨论内容生成优化代码。
| const isAIpage = | ||
| window.location.href.includes(McpServerManagementRoute) || | ||
| window.location.href.includes(CredentialManagementRoute); |
There was a problem hiding this comment.
重复的路由判断逻辑应提取为公共方法
🟡 Major | 🧹 Code Smells
📋 问题详情
在handleNameSpaces和组件渲染逻辑中重复了相同的路由判断逻辑，导致代码冗余且难以维护。若后续路由规则变更需在两处修改，容易引发不一致的bug。
💡 解决方案
将路由判断逻辑提取为组件内部方法：
+ getIsAIPage() {
+ return window.location.href.includes(McpServerManagementRoute) ||
+ window.location.href.includes(CredentialManagementRoute);
+ }
- const isAIpage = ...
+ const isAIpage = this.getIsAIPage();🔧 建议代码
‼️ AI 生成代码 - 请在应用前检查逻辑、规范并测试
| const isAIpage = | |
| window.location.href.includes(McpServerManagementRoute) || | |
| window.location.href.includes(CredentialManagementRoute); | |
| getIsAIPage() { | |
| return window.location.href.includes(McpServerManagementRoute) || | |
| window.location.href.includes(CredentialManagementRoute); | |
| } | |
| // 调用处改为： | |
| const isAIpage = this.getIsAIPage(); | |
| @@ -69,6 +69,7 @@ const I18N_CONF = { | |||
| settingCenter: 'Setting Center', | |||
| AiManagementVirtual: 'AI', | |||
| mcpServerManagement: 'MCP Managenment', | |||
| credentialManagement: 'Credential Managenment', | |||
There was a problem hiding this comment.
存在翻译拼写错误
🟢 Minor | 🐞 Bugs
📋 问题详情
MCP管理功能的英文翻译拼写错误，'Managenment'缺少字母g，应修正为'Management'以确保国际化文本的准确性。
💡 解决方案
修正拼写错误：
- mcpServerManagement: 'MCP Managenment',
+ mcpServerManagement: 'MCP Management',🔧 建议代码
‼️ AI 生成代码 - 请在应用前检查逻辑、规范并测试
| credentialManagement: 'Credential Managenment', | |
| mcpServerManagement: 'MCP Management', |
| window.location.href.includes(McpServerManagementRoute) || | ||
| window.location.href.includes(CredentialManagementRoute); |
There was a problem hiding this comment.
应使用React Router进行路由判断
🟡 Major | 🐞 Bugs
📋 问题详情
直接使用window.location.href.includes进行路由判断不够可靠，可能受URL参数影响。建议使用React Router的useLocation或match进行精确路由匹配。
💡 解决方案
改用React Router的useLocation：
+ import { useLocation } from 'react-router-dom';
...
const location = useLocation();
const isAIpage =
+ location.pathname === McpServerManagementRoute ||
+ location.pathname === CredentialManagementRoute;| @@ -67,6 +67,7 @@ const I18N_CONF = { | |||
| settingCenter: '设置中心', | |||
| AiManagementVirtual: 'AI', | |||
| mcpServerManagement: 'MCP管理', | |||
| credentialManagement: 'Credential管理', | |||
There was a problem hiding this comment.
Credential管理中文翻译不完整
🟢 Minor | 🐞 Bugs
📋 问题详情
CredentialManagement的中文翻译为'Credential管理'，建议补充完整为'凭证管理'或专业术语，确保中文用户的可读性。
💡 解决方案
建议修改为专业术语：
- credentialManagement: 'Credential管理',
+ credentialManagement: '凭证管理',🔧 建议代码
‼️ AI 生成代码 - 请在应用前检查逻辑、规范并测试
| credentialManagement: 'Credential管理', | |
| credentialManagement: '凭证管理', |
| @@ -48,6 +48,7 @@ const configurationMenu = { | |||
| }; | |||
|
|
|||
| export const McpServerManagementRoute = '/mcpServerManagement'; | |||
| export const CredentialManagementRoute = '/credentialManagement'; | |||
There was a problem hiding this comment.
路由常量命名不一致
🟢 Minor | 🧹 Code Smells
📋 问题详情
McpServerManagementRouteName使用全大写命名，而新增的CredentialManagementRoute采用首字母大写，建议统一为全大写命名规范。
💡 解决方案
调整常量命名规范：
- export const CredentialManagementRoute = '/credentialManagement';
+ export const CREDENTIAL_MANAGEMENT_ROUTE = '/credentialManagement';🔧 建议代码
‼️ AI 生成代码 - 请在应用前检查逻辑、规范并测试
| export const CredentialManagementRoute = '/credentialManagement'; | |
| export const CREDENTIAL_MANAGEMENT_ROUTE = '/credentialManagement'; |
| {/* <FormItem label={locale.CredentialRef}> */} | ||
| {/* <Select */} | ||
| {/* mode="multiple" */} | ||
| {/* showSearch */} | ||
| {/* onChange={this.handleCredentialChange} */} | ||
| {/* defaultValue={this.field.getValue('credentials') ? Object.keys(this.field.getValue('credentials')) : []} */} | ||
| {/* dataSource={this.state.credentials} */} | ||
| {/* style={{ width: "100%", marginRight: 8 }} */} | ||
| {/* /> */} |
There was a problem hiding this comment.
注释的表单组件代码未清理，影响代码可维护性
🟢 Minor | 🧹 Code Smells
📋 问题详情
在表单配置区域存在未移除的注释代码块，违反代码规范。残留的注释代码会增加维护成本，可能误导后续开发者认为功能未完整实现。
💡 解决方案
直接删除废弃的注释代码：
- {/* ...注释代码块... */}| ref: key, | ||
| }; | ||
| } | ||
| this.field.setValue('credentials', result); |
There was a problem hiding this comment.
凭证选择组件数据格式不匹配，导致表单提交数据错误
🔴 Blocker | 🐞 Bugs
📋 问题详情
在handleCredentialChange方法中，将Select组件返回的凭证ID数组转换为对象形式，但组件本身使用mode='multiple'模式应直接传递数组。这种转换导致表单字段存储了对象而非预期的数组，将直接导致接口调用时数据格式不符合后端要求。
💡 解决方案
直接传递选中的凭证ID数组而非转换为对象：
- this.field.setValue('credentials', result);
+ this.field.setValue('credentials', value);🔧 建议代码
‼️ AI 生成代码 - 请在应用前检查逻辑、规范并测试
| this.field.setValue('credentials', result); | |
| this.field.setValue('credentials', value); |
| }, | ||
| success(result) { | ||
| if (result.code === 0) { | ||
| self.setState({ |
There was a problem hiding this comment.
组件状态类型不一致，导致潜在运行时错误
🟠 Critical | 🐞 Bugs
📋 问题详情
在getCredentials方法中，credentials状态初始化为对象{}，但接口响应处理时将其设置为数组类型。这种类型不一致会导致后续代码访问credentials时出现Cannot read property ... of undefined等错误。
💡 解决方案
初始化状态时统一为数组类型：
- credentials: {},
+ credentials: [],🔧 建议代码
‼️ AI 生成代码 - 请在应用前检查逻辑、规范并测试
| self.setState({ | |
| credentials: [], |
|
|
||
| getCredentials = () => { | ||
| const self = this; | ||
| const url = `v3/console/cs/config/list?dataId=&groupName=credentials`; |
There was a problem hiding this comment.
凭证接口URL硬编码，违反配置规范
🟡 Major | 🧹 Code Smells
📋 问题详情
API请求路径v3/console/cs/config/list直接硬编码在代码中，未通过常量或配置管理。这种做法违反了代码可维护性规范，当接口地址变更时需要修改多处代码。
💡 解决方案
使用环境变量或常量管理API路径：
- const url = `v3/console/cs/config/list?dataId=&groupName=credentials`;
+ const CREDENTIALS_API = '/v3/console/cs/config/list';
+ const url = `${CREDENTIALS_API}?dataId=&groupName=credentials`;🔧 建议代码
‼️ AI 生成代码 - 请在应用前检查逻辑、规范并测试
| const url = `v3/console/cs/config/list?dataId=&groupName=credentials`; | |
| const CREDENTIALS_API = '/v3/console/cs/config/list'; | |
| const url = `${CREDENTIALS_API}?dataId=&groupName=credentials`; |
| <Row> | ||
| {['stdio', 'mcp-sse', 'mcp-streamble'].map(item => ( | ||
| <Radio key={item} id={item} value={item}> | ||
| {item.charAt(0).toUpperCase() + item.slice(1)} | ||
| </Radio> | ||
| ))} | ||
| </Row> | ||
| <Row> | ||
| {['http', 'dubbo'].map(item => ( | ||
| <Radio key={item} id={item} value={item}> | ||
| {item.charAt(0).toUpperCase() + item.slice(1)} | ||
| </Radio> | ||
| ))} |
There was a problem hiding this comment.
协议选择组件布局逻辑存在潜在业务风险
🟢 Minor | 🧹 Code Smells
📋 问题详情
将协议选项分为两个独立的Row组件，可能导致用户无法同时选择跨组协议（如stdio+http）。若业务逻辑要求协议类型必须属于独立组别，此设计合理；否则可能引发配置冲突。需明确业务需求并确保UI逻辑与之匹配。
💡 解决方案
添加分组说明或业务注释明确选择规则：
+ <Row><Text>基础协议</Text></Row>
<Row>
{['stdio', 'mcp-sse', 'mcp-streamble'].map(...)}
</Row>
+ <Row><Text>网络协议</Text></Row>
<Row>
{['http', 'dubbo'].map(...)}
</Row>…ials-and-args-tree # Conflicts: # console-ui/src/pages/AI/NewMcpServer/NewMcpServer.js
Please do not create a Pull Request without creating an issue first.
What is the purpose of the change
Support Credential management and tree argument edit
Brief changelog
XX
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=trueto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.