-
Notifications
You must be signed in to change notification settings - Fork 403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support MCP server and client #4335
Conversation
|
Walkthrough该 PR 对项目进行了大规模更新,涵盖配置文件、依赖项、前端 UI 组件、MCP 服务器集成、以及后端 AI 模型支持的改进。修改内容包括 Changes
Sequence Diagram(s)MCP 工具调用流程sequenceDiagram
participant U as 用户
participant CV as AIChatView 组件
participant CPS as ChatProxyService
participant MSP as MCPServerProxyService
participant MCR as MCPServerRegistry
participant MH as MCP工具处理器
U->>CV: 发起工具调用请求
CV->>CPS: 调用 handleToolCall 方法
CPS->>MSP: 转发调用请求 ($callMCPTool)
MSP->>MCR: 查询注册的 MCP 工具
MCR->>MH: 调用对应工具处理逻辑
MH-->>MCR: 返回工具执行结果
MCR-->>MSP: 返回工具结果数据
MSP-->>CPS: 返回处理结果
CPS-->>CV: 更新前端视图,显示工具调用结果
AI 模型请求流sequenceDiagram
participant Client as 客户端
participant ABS as AIBackService
participant Model as 模型实例 (Anthropic / DeepSeek / OpenAI)
participant Stream as 响应流
Client->>ABS: 发起请求 (requestStream)
ABS->>Model: 根据配置选择对应模型并调用 request 方法
Model->>Stream: 发送流式响应数据
Stream-->>ABS: 传递响应数据
ABS-->>Client: 返回完整的流数据
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
yarn install v1.22.22 ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🧹 Nitpick comments (40)
packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts (3)
6-6
: 移除无用的注解或导入。
注意到您导入了Injectable
但此处并未实际使用。如果后续不需要使用@Injectable()
装饰器,可以考虑删除它来保持代码整洁。以下是可能的修正示例:
-import { Autowired, Injectable } from '@opensumi/di'; +import { Autowired } from '@opensumi/di';
49-51
: 确认多工作区环境兼容性。
目前仅使用 workspaceRoots[0] 作为项目目录。如果用户打开多个工作区目录,可能需要明确哪一个才是正确的根目录,或者支持更多根目录的处理逻辑。
74-95
: 区分不同错误类型以改善可调试性。
目前所有异常都统一返回 "file not found"。如果是权限问题、读取错误或者文件夹等情况,调试者可能缺少更具体的信息。可考虑在 catch 中捕获更详细的错误类型,并记录或返回更准确的消息,以提升可维护性与排错效率。packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts (1)
39-79
: 在文件内容覆盖前建议考虑多种错误场景处理handler 方法已处理了“无编辑器打开”和“无模型管理对象”的异常情况,不过在某些情况下,可能需要更详细的错误信息或日志内容,例如文件类型不支持、当前编辑器锁定等。若业务场景复杂,建议进一步细化错误返回,以便调用方区分并处理。
packages/startup/entry/sample-modules/ai-native/ai.back.service.ts (2)
52-59
: 模型对象的依赖注入可考虑动态切换目前同时注入三个模型,但在实际调用时只看到 anthropicModel 的使用。如果要支持动态切换模型,可能需要在请求时判断所需模型类型,再调用相应的模型对象,以便更合理地利用依赖注入的优势。
89-90
: 深度模型调用被注释,实际只调用anthropicModel此处注释掉了对 deepseekModel 的请求调用,直接使用 anthropicModel.request。若业务上仅需一种模型,可考虑移除或在情况需要时显式切换,保证代码简洁并减少混淆。
packages/ai-native/src/node/mcp-server.ts (2)
61-63
: onerror 回调仅输出日志,建议考虑重试或自定义错误处理transport.onerror 回调当前仅简单打印错误日志,实际生产环境或关键场景下,可能需要自动重试、报警通知或对异常进行更详细的分类处理,避免关键任务的中断。
82-92
: 调用工具参数解析时增加空参数处理callTool 方法里对 JSON.parse 进行了捕获,但若 arg_string 为空字符串或 null 时,错误提示相同。可在进入 JSON.parse 前判断 arg_string 是否为空,返回相应错误信息,从而减少不必要日志或误解。
packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts (2)
58-65
: 可扩展搜索排除规则
目前仅排除node_modules目录,如需排除其他路径可进一步配置excludePatterns。
78-91
: 错误处理与日志记录建议更加精细
当前仅返回空数组并追加日志,可考虑在抛出异常时包含更多上下文信息,便于排查问题。packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts (3)
42-51
: 无激活编辑器的提示可更友好
目前仅在日志中打印错误,同时返回“no file open”,可视需要提供更丰富的提示。
53-61
: 缺少模型时的错误信息
日志描述较为精简,如能在错误信息中记录当前编辑器或模型上下文可更易排查问题。
83-89
: 异常处理返回“unknown error”
建议在可能的情况下添加具体错误信息,便于后续排障。packages/ai-native/src/node/base-language-model.ts (2)
23-30
: convertToolRequestToAITool 时对参数的处理较简单,可能需要更严谨的异常捕获。当前只是在执行阶段调用 JSON.stringify,若 toolRequest.handler 内部出现错误,可能需要考虑更细致的错误提示或重试机制。
34-117
: handleStreamingRequest 方法体较大,建议适度拆分或提炼逻辑。内部包含多种事件类型处理和异步流操作,若后续需求增加,逻辑的复杂度会进一步提升,建议将对应的事件处理逻辑分拆为更小的函数,以提升可维护性。
packages/ai-native/src/node/mcp-server-manager-impl.ts (2)
16-23
: stopServer 的日志输出可考虑使用更正式的日志方案。目前使用 console.log,对于生产环境建议更换为统一的日志记录工具,以便收集、过滤或分析日志。
116-119
: initBuiltinServer 并未使用异步,但内部调用了 collectTools()。若 future 逻辑中可能包含异步操作,例如网络请求或 IO 操作,可考虑将该方法改为 async 并显式等待,避免后续流程的潜在时序问题。
packages/ai-native/src/node/mcp/sumi-mcp-server.ts (2)
58-61
: initBuiltinMCPServer 没有显式异常处理。若 initBuiltinServer 内部抛出异常,本方法不会捕获,可能导致外部调用难以感知异常。可酌情添加错误捕获或将异常向上抛出。
63-98
: initExposedMCPServer 方法对请求处理器进行注册时结构清晰,但建议考虑可扩展性。若后续需要扩展更多类型的请求或增加权限控制,可将不同处理器拆分到单独的模块或使用更灵活的路由机制。
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileDiagnostics.ts (1)
55-66
: 可考虑进一步强化错误提示信息。
当前在无法获取活动编辑器时,只记录“Error: No active text editor found”,返回空诊断信息。若在调试或测试场景中,需要更明显的提示或可抛出异常,便于进一步排查。packages/ai-native/src/browser/ai-core.contribution.ts (2)
73-77
: 导入存在未使用的依赖?
这里引入了 MCPServerManager 相关符号仅在 221-223 行中注释使用,若后续无计划启用,建议删除无用的导入与注释,保持代码整洁。
221-223
: 注释的 MCPServerManager 代码
若短期内无需使用 MCPServerManager,建议移除相关注释以避免代码冗余。packages/ai-native/src/common/types.ts (1)
22-39
: 新增 IMCPServerProxyService 与 MCPTool 接口
该接口扩展了客户端对 MCP Tool 的调用能力。但 MCPTool 中的 inputSchema 为 any 类型,可能缺少对输入的类型声明,影响类型安全。建议根据实际需求进行更严格的类型定义。packages/ai-native/src/node/deepseek/deepseek-language-model.ts (1)
1-25
: DeepSeekModel 实现建议
当前实现可正常获取 API Key 并抛出英文异常信息。若主要面向中文用户,可考虑使用多语言提示,以便快速定位错误。packages/ai-native/src/node/anthropic/anthropic-language-model.ts (1)
23-23
: 建议将模型标识符设置为可配置项模型标识符 'claude-3-5-sonnet-20241022' 目前是硬编码的。建议将其移至配置文件中,以便于后续更新和维护。
- return provider('claude-3-5-sonnet-20241022'); + const modelId = options.modelId || 'claude-3-5-sonnet-20241022'; + return provider(modelId);packages/ai-native/src/browser/mcp/mcp-server-proxy.service.ts (1)
19-21
: 建议完善参数类型定义
args
参数使用any
类型可能会导致类型安全问题。建议定义具体的接口类型。- $callMCPTool(name: string, args: any) { + interface MCPToolArgs { + [key: string]: unknown; + } + $callMCPTool(name: string, args: MCPToolArgs) {packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts (1)
34-46
: 建议增强错误处理机制
callMCPTool
方法中的错误处理可以更加完善,建议捕获工具执行时可能出现的异常。async callMCPTool( name: string, args: any, ): Promise<{ content: { type: string; text: string }[]; isError?: boolean; }> { const tool = this.tools.find((tool) => tool.name === name); if (!tool) { throw new Error(`MCP tool ${name} not found`); } + try { return tool.handler(args, this.logger); + } catch (error) { + this.logger.appendLine(`Error executing tool ${name}: ${error.message}`); + return { + content: [{ type: 'error', text: error.message }], + isError: true + }; + } }packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts (1)
33-40
: 建议改进错误处理的信息详细度当前的错误信息比较简单,建议提供更具体的原因说明,以帮助用户更好地理解和解决问题。
建议修改如下:
- logger.appendLine('Error: No active text editor found'); + logger.appendLine('Error: 无法获取文件路径 - 未找到活动的文本编辑器或当前文件未保存');packages/ai-native/src/browser/mcp/tools/getOpenEditorFileText.ts (1)
42-48
: 建议添加内容读取日志为了便于调试和监控,建议在成功读取文件内容后添加日志记录。
建议修改如下:
const document = editor.currentDocumentModel; logger.appendLine(`Reading content from: ${document.uri.toString()}`); const content = document.getText(); + logger.appendLine(`Successfully read ${content.length} characters from file`); return { content: [{ type: 'text', text: content }],
packages/ai-native/src/browser/mcp/tools/getSelectedText.ts (2)
33-40
: 建议增强错误处理的可读性建议将错误信息对象化,以便更好地处理和区分不同的错误情况。
- return { - content: [{ type: 'text', text: '' }], - }; + return { + content: [{ type: 'text', text: '' }], + isError: true, + errorType: 'NO_ACTIVE_EDITOR' + };
42-48
: 建议优化空选区的处理逻辑当前实现在没有选区时返回空字符串,建议返回更明确的错误信息。
- return { - content: [{ type: 'text', text: '' }], - }; + return { + content: [{ type: 'text', text: '' }], + isError: true, + errorType: 'NO_SELECTION' + };packages/ai-native/src/browser/components/ChatToolRender.tsx (1)
11-17
: 建议增加类型验证和默认值处理当前的空值检查可以更加严谨,建议使用 TypeScript 的可选链操作符和默认值模式。
- if (!value || !value.function || !value.id) { + if (!value?.function?.name || !value?.id) { return null; }packages/ai-native/__test__/node/mcp-server-manager-impl.test.ts (1)
95-114
: 建议增加边界测试用例
getStartedServers
的测试覆盖了基本场景,建议添加以下测试用例:
- 空服务器列表的情况
- 所有服务器都已启动的情况
- 所有服务器都未启动的情况
packages/ai-native/src/browser/index.ts (1)
91-101
: 建议重构工具注册方式当前 MCP 服务器工具的注册是通过直接列举实现的。建议创建一个工具注册表类来统一管理这些工具,以提高可维护性和可扩展性。
packages/ai-native/src/common/index.ts (1)
124-127
: 建议添加方法文档
ISumiMCPServerBackend
接口的方法缺少详细的文档注释。建议添加以下内容:
- 方法的具体用途
- 参数说明(如果有)
- 返回值说明
- 可能抛出的异常
packages/ai-native/src/browser/chat/chat-model.ts (1)
127-136
: 工具调用状态管理的实现工具调用的状态管理实现得当,通过 ID 匹配来更新现有工具调用或添加新的工具调用。但是有一个小问题需要注意:
- 代码中存在被注释掉的代码
// this.#responseParts[responsePartLength] = find;
- 使用了
@ts-ignore
来忽略类型检查建议进行以下改进:
- 删除注释掉的代码
- 修复类型问题而不是使用
@ts-ignore
- // @ts-ignore - find.content = progress.content; - // this.#responseParts[responsePartLength] = find; + (find as IChatToolContent).content = progress.content;packages/ai-native/src/browser/types.ts (1)
308-319
: 工具定义接口的实现
MCPToolDefinition
接口的设计很好,包含了必要的属性。但是inputSchema
的类型定义可以更具体。建议使用更具体的类型定义:
- inputSchema: any; // JSON Schema + inputSchema: { + type: string; + properties: Record<string, unknown>; + required?: string[]; + };packages/ai-native/src/browser/components/ChatReply.tsx (1)
152-177
: 组件实现良好,建议添加错误处理。组件实现遵循了 React 最佳实践,但建议添加错误边界以处理组件加载失败的情况。
建议添加错误处理:
const ToolCallRender = (props: { toolCall: IChatToolContent['content'] }) => { const { toolCall } = props; const chatAgentViewService = useInjectable<IChatAgentViewService>(ChatAgentViewServiceToken); const [node, setNode] = useState<React.JSX.Element | null>(null); + const [error, setError] = useState<Error | null>(null); useEffect(() => { + try { const config = chatAgentViewService.getChatComponent('toolCall'); if (config) { const { component: Component, initialProps } = config; setNode(<Component {...initialProps} value={toolCall} />); return; } setNode( <div> <Loading /> <span style={{ marginLeft: 4 }}>正在加载组件</span> </div>, ); const deferred = chatAgentViewService.getChatComponentDeferred('toolCall')!; deferred.promise.then(({ component: Component, initialProps }) => { setNode(<Component {...initialProps} value={toolCall} />); + }).catch((err) => { + setError(err); }); + } catch (err) { + setError(err as Error); + } }, [toolCall.state]); + if (error) { + return <div>组件加载失败: {error.message}</div>; + } return node; };packages/ai-native/src/browser/components/ChatToolRender.module.less (2)
3-4
: 建议使用 CSS 变量替代硬编码的颜色值建议将重复使用的颜色值抽取为 CSS 变量,以便于主题切换和统一管理。例如:
:root { + --chat-tool-border-color: #363636; + --chat-tool-header-bg: #2D2D2D; + --chat-tool-header-hover-bg: #363636; + --chat-tool-text-color: #CCCCCC; + --chat-tool-icon-color: #888888; + --chat-tool-content-bg: #1E1E1E; } .chat-tool-render { - border: 1px solid #363636; + border: 1px solid var(--chat-tool-border-color); // ...其他颜色值同样替换 }Also applies to: 12-12, 17-17, 25-25, 32-32, 43-43, 65-65, 84-84
62-69
: 关于内容展开收起的建议当前使用 max-height 实现展开收起可能会在内容很多时出现动画不流畅的问题。建议考虑使用 height: auto 配合 grid 布局或者 JavaScript 计算高度的方案。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (43)
.gitignore
(1 hunks)packages/ai-native/__test__/node/mcp-server-manager-impl.test.ts
(1 hunks)packages/ai-native/package.json
(2 hunks)packages/ai-native/src/browser/ai-core.contribution.ts
(8 hunks)packages/ai-native/src/browser/chat/chat-model.ts
(7 hunks)packages/ai-native/src/browser/chat/chat-proxy.service.ts
(4 hunks)packages/ai-native/src/browser/components/ChatEditor.tsx
(2 hunks)packages/ai-native/src/browser/components/ChatReply.tsx
(5 hunks)packages/ai-native/src/browser/components/ChatToolRender.module.less
(1 hunks)packages/ai-native/src/browser/components/ChatToolRender.tsx
(1 hunks)packages/ai-native/src/browser/components/components.module.less
(1 hunks)packages/ai-native/src/browser/index.ts
(5 hunks)packages/ai-native/src/browser/mcp/mcp-server-proxy.service.ts
(1 hunks)packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/createNewFileWithText.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getDiagnosticsByPath.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getOpenEditorFileDiagnostics.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getOpenEditorFileText.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getSelectedText.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts
(1 hunks)packages/ai-native/src/browser/preferences/schema.ts
(1 hunks)packages/ai-native/src/browser/types.ts
(1 hunks)packages/ai-native/src/common/index.ts
(2 hunks)packages/ai-native/src/common/mcp-server-manager.ts
(1 hunks)packages/ai-native/src/common/tool-invocation-registry.ts
(1 hunks)packages/ai-native/src/common/types.ts
(1 hunks)packages/ai-native/src/node/anthropic/anthropic-language-model.ts
(1 hunks)packages/ai-native/src/node/base-language-model.ts
(1 hunks)packages/ai-native/src/node/deepseek/deepseek-language-model.ts
(1 hunks)packages/ai-native/src/node/index.ts
(1 hunks)packages/ai-native/src/node/mcp-server-manager-impl.ts
(1 hunks)packages/ai-native/src/node/mcp-server.ts
(1 hunks)packages/ai-native/src/node/mcp/sumi-mcp-server.ts
(1 hunks)packages/ai-native/src/node/openai/openai-language-model.ts
(1 hunks)packages/core-common/src/settings/ai-native.ts
(1 hunks)packages/core-common/src/types/ai-native/index.ts
(3 hunks)packages/i18n/src/common/en-US.lang.ts
(1 hunks)packages/i18n/src/common/zh-CN.lang.ts
(1 hunks)packages/startup/entry/sample-modules/ai-native/ai.back.service.ts
(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ai-native/src/browser/components/ChatEditor.tsx
[error] 149-149: The regular expression includes this negated empty character class.
Negated empty character classes match anything.
If you want to match against [, escape it [.
Otherwise, remove the character class or fill it.
(lint/correctness/noEmptyCharacterClassInRegex)
[error] 150-150: The regular expression includes this negated empty character class.
Negated empty character classes match anything.
If you want to match against [, escape it [.
Otherwise, remove the character class or fill it.
(lint/correctness/noEmptyCharacterClassInRegex)
🔇 Additional comments (68)
packages/ai-native/src/browser/components/components.module.less (2)
247-280
: 重构 .monaco_wrapper 类设计
此处将原来与代码块展示相关的样式重构为 .monaco_wrapper 类,命名更加语义化(暗示与 Monaco 编辑器相关),并调整了诸如 min-width (130px)、内边距、滚动条样式等属性。建议确认全局引用该样式的组件或页面已完成相应更新,以避免样式引用混乱。
282-304
: 新增 .code_block 样式类
新增的 .code_block 类专门用于定义代码块的显示样式,包括相对定位、最小宽度以及 margin-top 属性,同时针对代码高亮(.hljs)和代码语言标签 (.code_language) 进行了全局样式设置。这种设计有助于区分不同用途的样式定义,确保在新旧样式之间互不干扰。建议在不同浏览器环境下测试其兼容性,并核实全局样式不会影响页面中其它组件。packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts (1)
63-71
: 🛠️ Refactor suggestion可能存在符号链接带来的安全风险。
当前仅通过 path.relative 判断路径是否越界,若文件为符号链接且指向工作区外部,仍可能绕过该检测。建议进一步校验文件的真实位置或禁用符号链接,以防潜在的安全漏洞。packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts (2)
10-12
: 使用 Zod 描述输入结构的做法很清晰通过 zod 对输入参数进行严格的类型定义和描述,有助于减少调用时参数不正确的问题,也便于自动生成 JSON Schema。此处实现值得肯定。
23-37
: 工具元信息的描述较为详尽在 getToolDefinition 方法中对该工具的名称、描述、输入结构及可能返回的结果都进行了清晰的定义,便于后续在工具列表中展示或被调用时做自动文档化。保持这种详细注释的做法有利于可维护性和可读性。
packages/startup/entry/sample-modules/ai-native/ai.back.service.ts (1)
2-4
: 新增模型依赖导入,使后续实现更灵活导入 AnthropicModel、DeepSeekModel、OpenAIModel 等类,为后面可插拔或可扩展的模型服务打下基础。在多模型并存的场景下,需留意各模型的版本依赖与潜在冲突。
packages/ai-native/src/node/mcp-server.ts (2)
1-13
: IMCPServer 接口定义清晰该接口对 MCP 服务器的核心方法(如 start、callTool、getTools 等)做了完整声明,可与不同实现类配合,保持整体架构灵活。接口方法命名简洁易懂。
38-45
: 启动日志信息对调试十分友好start 方法在启动服务器之前,打印了名称、命令、参数和环境变量等信息,能帮助排查启动参数相关的问题,减少运维和开发阶段的定位成本。
packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts (4)
13-16
: 使用Zod定义输入Schema非常清晰
在需要扩展更多搜索参数时,你可以轻松增加更多字段。
25-44
: 工具定义元数据完整
名称、描述、输入Schema和handler绑定逻辑均较为完善,符合MCP工具的注册流程。
50-56
: 需要确认多工作区场景
当前仅使用workspaceRoots[0],在多工作区环境下可能会忽略其他工作区根目录。
67-76
: 结果映射处理直观
通过相对路径与文件名输出便于调用方解析与显示。packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts (4)
13-15
: 使用Zod定义文本输入非常直观
当有更多自定义字段需求时,可以轻松在Schema中扩展。
26-40
: ToolDefinition信息完备
清晰定义了工具名称、描述、参数和handler,有助于规范化管理。
63-75
: 创建InlineDiff预览逻辑清晰
已根据需求设置disposeWhenEditorClosed等属性,后续可依使用场景灵活调整。
77-82
: 返回“ok”时的确认日志
在操作成功后予以日志提示,方便跟踪操作流程。packages/ai-native/src/common/tool-invocation-registry.ts (5)
5-12
: ToolParameterSchema设计灵活
采用递归结构可满足多层级数据模型需求。
14-29
: ToolParameter与ToolRequest接口定义简洁
对工具参数与请求的结构进行清晰约束,方便后期功能扩展。
31-75
: ToolInvocationRegistry接口条理清晰
涵盖注册、检索、批量检索和注销等常见操作,逻辑完善。
76-81
: ToolProvider接口提供更灵活的注入方式
可根据具体实现返回ToolRequest,实现不同场景的动态工具提供。
82-123
: ToolInvocationRegistryImpl实现细节完备
通过Map存储并提供批量注销、重复注册警告等功能,能满足大部分工具管理需求。packages/ai-native/src/node/base-language-model.ts (3)
1-7
: 导入依赖的方式看起来合理。建议在后续确保第三方库(ai)的版本与项目的整体依赖兼容,以避免潜在冲突或升级难度。
15-16
: 抽象方法的设计有助于模型的可扩展性。抽象方法 initializeProvider(options) 能很好地约束不同模型的初始化流程,当前实现无明显问题。
17-21
: 异步请求方法 request() 逻辑清晰。方法内部能根据配置动态初始化 provider,并调用 handleStreamingRequest,对于后续集成测试需确保所有调用方都正确传入参数。
packages/ai-native/src/node/mcp-server-manager-impl.ts (1)
78-90
: collectTools 方法需要确认如何处理重复注册的工具。将 ToolRequest 注册到 toolInvocationRegistry 可能会在同名工具多次注册时产生冲突,请检查是否有重复命名或在重复注册时覆盖策略是否合理。
packages/ai-native/src/node/mcp/sumi-mcp-server.ts (2)
32-47
: getMCPTools 与 callMCPTool 方法实现简明。二者都在调用前检查 client 是否初始化,符合常见的错误处理模式,且逻辑清晰易理解。
104-166
: BuiltinMCPServer 在 JSON 解析与服务器状态判断上实现得当。已对 JSON.parse 失败进行捕获并输出错误日志,同时对未启动状态进行校验,整体流程较为完善。
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileDiagnostics.ts (2)
19-25
: 注入依赖项设计清晰,类命名合理。
此处将编辑器服务与工作区服务都注入到类中,方便后续在 handler 中访问上下文环境,无明显问题。
107-119
: 严重性映射逻辑健全。
将枚举类型与字符串进行对应,并在不匹配时返回“unknown”,整体实现可读性高,覆盖面完整。packages/ai-native/src/browser/mcp/tools/getDiagnosticsByPath.ts (3)
14-16
: 输入参数定义简洁明了。
使用 Zod 定义必须的 filePathInProject 字段,确保调用方传递所需参数。此做法符合可维护性与可读性。
55-66
: 检查工作区根目录的错误处理合理。
在无法找到工作区时,立即返回空诊断,避免后续操作报错。该模式直观明了。
73-80
: 需验证跨平台的路径比较。
通过相对路径判断文件是否超出项目范围在多数场景下有效,但在 Windows 平台进行大小写比较或分隔符处理时需谨慎。可考虑在后续增加对路径大小写或分隔符的统一化处理。packages/ai-native/src/node/openai/openai-language-model.ts (2)
58-60
: 工具注册与调用设计简洁。
通过 toolInvocationRegistry.getAllFunctions() 与 createTool() 配合,将多工具流程整合到请求中,实现了良好的扩展性。
71-75
: 中断处理可行。
在取消请求时调用 runner.abort(),与 CancellationToken 机制整合,确保流式请求安全退出。该设计能够有效避免无效调用或资源浪费。packages/ai-native/src/browser/ai-core.contribution.ts (6)
101-101
: IMCPServerRegistry 导入正常
此处导入后续在属性注入中有使用场景,暂未发现问题。
106-107
: MCPServerContribution 与 TokenMCPServerRegistry 的导入
在后文注入使用,能够支撑服务端能力扩展,逻辑无冲突。
152-156
: 注入 MCP Server 相关依赖
该注入用于扩展 MCPServerContributions 与 MCPServerRegistry 能力,整体实现合理,可根据业务需求继续完善。
224-226
: 注入 sumiMCPServerBackendProxy
此注入对象主要用于后续 initBuiltinMCPServer 调用,功能对接正常。
311-312
: 延迟调用 initBuiltinMCPServer
在 onDidStart 中延迟执行此初始化操作,请确认其调用时机不会阻塞或者引发依赖问题。
328-331
: 注册 MCP Server Contributions
循环遍历所有服务端贡献点并进行注册,整体实现无明显问题。packages/ai-native/src/node/anthropic/anthropic-language-model.ts (1)
13-20
: 代码实现清晰,错误处理得当初始化提供者的实现简洁明了,错误信息友好且指导性强。
packages/ai-native/src/browser/mcp/mcp-server-proxy.service.ts (1)
24-37
: 建议优化错误处理机制
$getMCPTools
方法中的数组映射操作可能需要添加错误处理机制,以防止在处理工具列表时出现异常。建议添加 try-catch 块来处理潜在的错误:
async $getMCPTools() { + try { const tools = await this.mcpServerRegistry.getMCPTools().map((tool) => ({ name: tool.name, description: tool.description, inputSchema: tool.inputSchema, }), ); this.logger.log('SUMI MCP tools', tools); return tools; + } catch (error) { + this.logger.error('Failed to get MCP tools:', error); + throw error; + } }packages/core-common/src/settings/ai-native.ts (1)
26-30
: 实现清晰,文档完善新增的 API key 设置项定义清晰,注释完整,符合现有代码风格。
packages/ai-native/src/node/index.ts (3)
6-11
: 导入声明结构清晰且合理!新增的MCP服务器相关组件的导入声明组织得当,遵循了良好的代码组织原则。
21-32
: 提供者注册实现完整!新增的MCP服务器管理器、工具调用注册表和服务器代理服务的提供者注册实现完整,符合依赖注入模式。
40-43
: 后端服务注册正确!MCP服务器代理服务的后端服务注册配置正确,确保了服务可以被正确访问。
packages/ai-native/src/common/mcp-server-manager.ts (2)
3-16
: 接口设计全面且合理!
MCPServerManager
接口定义了完整的服务器管理功能集,包括工具调用、服务器生命周期管理和工具收集等关键操作。
22-42
: 服务器描述接口文档完善!
MCPServerDescription
接口的文档注释清晰详细,有助于开发者理解和正确使用该接口。packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts (1)
42-47
: 返回格式规范且统一!返回结果使用统一的内容格式,便于上层应用处理和展示。
packages/ai-native/src/browser/chat/chat-proxy.service.ts (2)
73-77
: 工具调用组件注册实现正确组件注册逻辑清晰,使用了正确的接口和属性。
61-68
: 依赖注入实现完整新增的依赖注入实现完整,包括了 ChatAgentViewService、PreferenceService 和 ApplicationService。
packages/ai-native/__test__/node/mcp-server-manager-impl.test.ts (1)
7-23
: 测试设置结构良好测试用例的设置结构清晰,使用了正确的 Jest 模拟方法,并在每个测试前重置模拟状态。
packages/ai-native/src/browser/index.ts (1)
191-198
: 后端服务配置正确新增的 MCP 服务器管理器和代理服务的配置实现正确,包括了必要的服务路径和令牌。
packages/ai-native/src/common/index.ts (1)
121-129
: 符号和路径定义清晰MCP 服务器相关的符号和路径定义清晰,并且包含了适当的注释说明其用途。
packages/ai-native/src/browser/components/ChatEditor.tsx (2)
133-139
: 代码块组件的改进将
language
属性设为可选是一个很好的改进,这样可以增加组件的灵活性。
152-164
: 避免变量名称冲突通过将变量
language
重命名为_language
,成功避免了与 props 中的language
属性发生命名冲突。这是一个很好的改进。packages/core-common/src/types/ai-native/index.ts (2)
293-306
: 工具调用内容类型的定义新增的
IChatToolContent
接口为工具调用提供了良好的类型定义,包含了必要的属性如 id、type、function 等。
342-342
: 类型系统的扩展将
IChatToolContent
添加到IChatProgress
联合类型中,使得系统能够正确处理工具调用的进度信息。packages/ai-native/src/browser/chat/chat-model.ts (2)
152-154
: 工具调用响应文本的处理工具调用响应文本的处理方式简洁明了,直接返回函数名称。
176-178
: 事件触发逻辑优化移除了
quiet
参数检查,简化了事件触发逻辑。这是一个好的改进,因为它使代码更加直接和可维护。packages/ai-native/src/browser/types.ts (2)
295-302
: MCP服务器贡献点的定义MCP服务器贡献点的定义清晰,使用 Symbol 确保唯一性。接口设计简洁明了。
321-332
: MCP服务器注册表接口的实现
IMCPServerRegistry
接口设计合理,提供了完整的工具注册和调用功能。注释表明未来会支持更多 MCP 功能,这是个好的设计决策。packages/ai-native/src/browser/components/ChatReply.tsx (3)
305-307
: 实现简洁明了。renderToolCall 函数实现清晰,与其他 render 函数保持一致的风格。
319-320
: 工具调用渲染集成良好。新增的工具调用渲染逻辑与现有内容渲染系统集成得当。
250-253
: 验证移除 startTransition 的性能影响。移除 startTransition 可能会影响大型状态更新的性能表现。建议验证此更改在大量消息场景下的性能影响。
packages/i18n/src/common/zh-CN.lang.ts (1)
1298-1303
: 本地化文案表述准确。新增的 AI Native Settings 相关翻译表述清晰,与现有翻译风格保持一致。
packages/i18n/src/common/en-US.lang.ts (1)
1534-1540
: 代码变更看起来不错!新增的本地化条目结构清晰,命名规范统一,且与现有模式保持一致。
packages/ai-native/package.json (1)
22-25
: 建议验证新增依赖的兼容性和安全性新增了多个 AI 相关的 SDK 依赖:
- @ai-sdk/* 系列
- @anthropic-ai/sdk
- @modelcontextprotocol/sdk
- ai
- openai
建议:
- 确认这些版本是否为最新的稳定版本
- 检查是否存在已知的安全漏洞
- 验证这些依赖之间是否存在版本冲突
Also applies to: 45-45, 48-48
✅ Verification successful
🌐 Web query:
1. 搜索 @anthropic-ai/sdk 的最新版本 2. 搜索 openai npm 包的最新版本 3. 检查 @ai-sdk/anthropic 和 @anthropic-ai/sdk 是否有版本兼容性问题
💡 Result:
根据搜索结果,以下是针对您的问题的详细解答: ## 1. @anthropic-ai/sdk 最新版本 根据 npmjs.com 官方页面显示(2025年1月27日更新): - **最新稳定版**:`0.36.3` - **开发动态**:团队正在开发迁移到原生 fetch API 的 alpha 版本[1] - **特性包含**: * Beta 版工具调用支持 * AWS Bedrock API 支持(需单独包) * 详细的错误处理体系(包含7种API错误类型) ## 2. OpenAI npm 包最新版本 根据 GitHub 讨论和官方文档: - **最新主版本**:`4.x`(2024年10月18日发布)[5][8] - **主要变化**: ```bash # v4.x安装命令 npm install openai@^4.0.0 ``` 重要更新包括: - API方法重组(如 createChatCompletion → chat.completions.create) - TypeScript类型重构 - Native fetch替代node-fetch - **遗留版本**:v3.x已停止维护 ## 3. @ai-sdk/anthropic与@anthropic-ai/sdk兼容性分析 | 特性 | @ai-sdk/anthropic (1.1.6) [7] | @anthropic-ai/sdk (0.36.x) [1] | |--------------|-------------------------------|--------------------------------| | SDK类型 | Vercel AI SDK封装 | Anthropic官方原生SDK | | API支持 | Messages API | Messages + Tools API | | Node版本要求 | >=18 | >=16 | | Stream支持 | ✅ | ✅ | | Bedrock支持 | ❌ | ✅ | **兼容性建议**: - **版本差异**:两者采用不同版本体系(1.x vs 0.x),无直接对应关系 - **功能重叠**:@ai-sdk/anthropic是轻量级封装,建议需要完整功能时使用官方SDK - **推荐方案**:若项目已使用Vercel AI生态,可优先选择@ai-sdk;需要底层控制则应直接使用官方SDK 建议开发时参考各自文档: - [Anthropic官方文档](https://docs.anthropic.com)[4] - [Vercel AI SDK文档](https://sdk.vercel.ai/docs)[7] Citations: - 1: https://www.npmjs.com/package/@anthropic-ai/sdk - 2: https://blog.csdn.net/2401_87189860/article/details/143055264 - 3: https://juejin.cn/post/7195462878193778746 - 4: https://docs.anthropic.com/zh-CN/release-notes/api - 5: https://github.com/openai/openai-node/discussions/217 - 6: https://bbs.kanxue.com/thread-278233.htm - 7: https://www.npmjs.com/package/@ai-sdk/anthropic - 8: https://openai.xiniushu.com/docs/libraries - 9: https://www.sihuo.club - 10: https://docs.anthropic.com/zh-TW/release-notes/api
依赖版本及兼容性检查结果:
- 根据最新的信息,@anthropic-ai/sdk 的版本稳定在 0.36.3,与包中使用的版本保持一致,无需更新。
- OpenAI 包目前最新主版本为 4.x,建议确认项目中使用的版本满足所需的 API 接口和功能需求。
- 关于 @ai-sdk/anthropic(1.1.6)与 @anthropic-ai/sdk(0.36.x),二者来自不同的生态体系:
- @ai-sdk/anthropic 是基于 Vercel AI 封装的轻量级包装;
- @anthropic-ai/sdk 为官方原生 SDK,功能更全面(如支持工具调用、底层 API 控制等)。
因此两者没有直接的版本对应关系,选择应依据项目对功能扩展和控制的要求。综合以上,当前依赖的版本均为最新稳定版本,且没有发现明显的版本冲突或已知的安全漏洞。建议在项目整合前后进行适当的集成测试,并密切关注未来版本更新和文档说明,以确保长期兼容性和安全性。
packages/ai-native/src/browser/mcp/tools/createNewFileWithText.ts
Outdated
Show resolved
Hide resolved
packages/ai-native/__test__/node/mcp-server-manager-impl.test.ts
Outdated
Show resolved
Hide resolved
…nsumi/core into feat/mcp-server-client-poc-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/ai-native/src/browser/types.ts (2)
302-304
: 建议添加接口文档为了提高代码可维护性,建议添加 JSDoc 文档说明该接口的用途和参数。
+/** + * MCP 服务器贡献点接口 + */ export interface MCPServerContribution { + /** + * 注册 MCP 服务器 + * @param registry MCP 服务器注册表 + */ registerMCPServer(registry: IMCPServerRegistry): void; }
306-308
: 建议扩展日志接口功能当前日志接口过于简单,建议添加错误级别和格式化功能。
export interface MCPLogger { + /** + * 日志级别 + */ + level: 'info' | 'warn' | 'error'; + + /** + * 添加一行日志 + * @param message 日志消息 + */ appendLine(message: string): void; + + /** + * 添加错误日志 + * @param error 错误对象 + */ + error(error: Error): void; }packages/i18n/src/common/zh-CN.lang.ts (1)
1316-1324
: 建议完善 MCP 服务器设置的描述文案当前描述较为简单,建议添加更详细的说明。
- 'preference.ai.native.mcp.servers.description': '配置 MCP (Model Context Protocol) 服务器', + 'preference.ai.native.mcp.servers.description': '配置 MCP (Model Context Protocol) 服务器,用于扩展 AI 能力。每个服务器需要配置名称、启动命令和环境变量。', - 'preference.ai.native.mcp.servers.command.description': '启动 MCP 服务器的命令', + 'preference.ai.native.mcp.servers.command.description': '启动 MCP 服务器的命令。例如: npx @modelcontextprotocol/server', - 'preference.ai.native.mcp.servers.args.description': 'MCP 服务器的命令行参数', + 'preference.ai.native.mcp.servers.args.description': 'MCP 服务器的命令行参数。例如: ["-y", "--port", "3000"]', - 'preference.ai.native.mcp.servers.env.description': 'MCP 服务器的环境变量', + 'preference.ai.native.mcp.servers.env.description': 'MCP 服务器的环境变量。例如: { "NODE_ENV": "production" }',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (4)
packages/ai-native/src/browser/ai-core.contribution.ts
(8 hunks)packages/ai-native/src/browser/types.ts
(1 hunks)packages/i18n/src/common/en-US.lang.ts
(1 hunks)packages/i18n/src/common/zh-CN.lang.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/i18n/src/common/en-US.lang.ts
🔇 Additional comments (6)
packages/ai-native/src/browser/types.ts (1)
297-300
: 符号声明看起来不错!符号命名遵循标准约定,注释清晰。
packages/ai-native/src/browser/ai-core.contribution.ts (4)
76-77
: 导入和属性声明正确!MCP 服务器相关的导入和属性声明使用了正确的装饰器和类型。
Also applies to: 154-158
314-325
: 建议添加外部 MCP 服务器初始化的错误处理请参考之前的评论,建议为每个外部服务器添加单独的错误处理。
341-344
: 建议添加注册的清理逻辑请参考之前的评论,建议添加工具注册的清理机制。
415-442
: 建议使用安全存储机制保存 API 密钥请参考之前的评论,建议使用系统的安全凭据存储。
packages/i18n/src/common/zh-CN.lang.ts (1)
1305-1315
: API 设置的本地化文案很好!文案清晰准确,描述性强,有助于用户理解各项设置的用途。
* feat: add llmcontext service * feat: integrate rc-collapse for improved chat context UI * feat: implement ChatAgentPromptProvider for enhanced context handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (13)
packages/ai-native/src/browser/chat/chat.view.tsx (2)
500-509
: 优化上下文序列化的性能在
handleAgentReply
中添加了上下文序列化逻辑。对于频繁的消息交互,这可能会影响性能。建议添加缓存机制:
- const context = contextService.serialize(); + const context = this.cachedContext || contextService.serialize(); + this.cachedContext = context;
707-713
: 建议添加 MCP 工具数量为零时的处理当 MCP 工具数量为零时,仍然显示计数可能会困扰用户。
建议添加条件渲染:
{aiNativeConfigService.capabilities.supportsMCP && ( + mcpToolsCount > 0 && ( <div className={styles.tag} onClick={handleShowMCPTools}> {`MCP Tools: ${mcpToolsCount}`} </div> + ) )}packages/ai-native/src/browser/context/llm-context.service.ts (2)
35-42
: 在添加文件到上下文前建议检查重复性或空值当前实现中,使用
this.contextFiles.set(uri.toString(), { ... })
会覆盖同一 URI 的旧信息,且尚未判断uri
是否为空。若用户多次添加相同文件到上下文,可能产生覆盖或误处理。建议在添加文件前先检查this.contextFiles
是否已存在,或确认uri
有效性,以避免数据冲突。
67-116
: 自动收集事件中的 TODO 需要尽快确认或实现代码片段中多处留有 “TODO” 注释,例如自动添加/移除文件到上下文逻辑未实现。若有意在此处完成对文件打开、删除、保存等逻辑的自动收集,请尽早确认需求并完善相关代码,否则可能导致实际使用时无法自动更新上下文。
packages/ai-native/src/browser/context/llm-context.contribution.ts (1)
11-13
: 初始化时应考虑对自动收集流程进行配置或出错处理在
initialize
方法中,直接调用startAutoCollection()
启用自动收集,建议结合实际应用场景提供可配合用户配置或错误捕获的机制。例如,若用户不需要自动收集或相关依赖初始化失败,如何安全地跳过并提示用户?packages/ai-native/src/common/llm-context.ts (2)
11-11
: 添加文件到上下文时可增加返回结果或异常
addFileToContext
接口仅包含参数,若在实现时需要对插入结果或异常信息进行反馈,建议在方法签名中增补返回值或抛出异常,以方便上层逻辑做进一步处理和跟踪。
30-34
: 为选区类型提供更明确的接口或类型定义当前使用
[number, number]
来表示选区范围,若日后扩展到多选区或更复杂的类型,接口可读性和扩展性会受限。建议考虑使用更详细的类型或数据结构,以便更好地表意和维护。packages/ai-native/src/browser/components/ChatContext/index.tsx (2)
29-43
: 建议优化事件监听的性能当前实现中的
useEffect
钩子在每次渲染时都会创建新的 debounce 函数。建议将 debounce 函数移到组件外部或使用useMemo
进行优化。- useEffect(() => { - const disposable = Event.debounce( - contextService.onDidContextFilesChangeEvent, - (_, e) => e!, - 50, - )((files) => { - if (files) { - updateAddedFiles(files); - } - }, contextService); + const debouncedHandler = useMemo( + () => + Event.debounce( + contextService.onDidContextFilesChangeEvent, + (_, e) => e!, + 50, + )((files) => { + if (files) { + updateAddedFiles(files); + } + }, contextService), + [contextService] + ); + + useEffect(() => { + const disposable = debouncedHandler;🧰 Tools
🪛 GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
[failure] 36-36:
Argument of type '{}' is not assignable to parameter of type 'SetStateAction<FileContext[]>'.🪛 GitHub Check: build (ubuntu-latest, 20.x)
[failure] 36-36:
Argument of type '{}' is not assignable to parameter of type 'SetStateAction<FileContext[]>'.
61-71
: 建议添加用户操作反馈在清理文件和移除文件的操作中,建议添加适当的用户反馈机制,比如:
- 操作成功/失败的提示
- 撤销操作的选项
- 操作进行中的加载状态
这样可以提升用户体验,让用户更清楚地了解操作的结果。
packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx (1)
111-146
: 建议增强键盘导航的可访问性当前的键盘导航实现可以通过以下方式增强可访问性:
- 添加 ARIA 属性以支持屏幕阅读器
- 实现 Home/End 键导航到列表首尾
- 添加键盘快捷键提示
return ( - <div className={styles.context_selector} onKeyDown={onDidKeyDown} tabIndex={-1}> + <div + className={styles.context_selector} + onKeyDown={onDidKeyDown} + tabIndex={-1} + role="listbox" + aria-label="文件选择器" + >packages/ai-native/src/browser/components/ChatContext/style.module.less (2)
47-62
: 优化样式声明在
.context_selector
类中存在以下问题:
- 第 53-54 行重复声明了
padding
属性- 阴影变量使用可以简化
- 建议使用 CSS Grid 来优化布局
.context_selector { position: absolute; left: 0px; bottom: 40px; background-color: var(--design-container-background); - padding: 10px; width: 100%; padding: 0px; border-radius: 4px; height: 350px; display: flex; flex-direction: column; user-select: none; - box-shadow: 0px 9px 28px 8px var(--design-boxShadow-primary), - 0px 3px 6px -4px var(--design-boxShadow-secondary), - 0px 6px 16px 0px var(--design-boxShadow-tertiary) !important; + box-shadow: var(--design-elevation-3) !important;
118-168
: 建议增强响应式设计当前的
.file_list
实现使用了固定尺寸,建议:
- 使用相对单位替代固定像素值
- 添加媒体查询以适应不同屏幕尺寸
- 考虑在移动设备上的显示效果
.file_list { margin-top: 5px; - max-height: 300px; + max-height: min(300px, 50vh); overflow-x: hidden; overflow-y: auto; + + @media (max-width: 768px) { + max-height: 70vh; + }packages/ai-native/package.json (1)
49-58
: 添加工具库依赖新增了以下工具库:
- [email protected]:AI 功能支持
- [email protected]:UI 组件
- [email protected] 和 [email protected]:数据验证和模式定义
建议添加这些依赖的具体用途说明到文档中。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (14)
packages/ai-native/package.json
(2 hunks)packages/ai-native/src/browser/chat/chat.view.tsx
(8 hunks)packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx
(1 hunks)packages/ai-native/src/browser/components/ChatContext/index.tsx
(1 hunks)packages/ai-native/src/browser/components/ChatContext/style.module.less
(1 hunks)packages/ai-native/src/browser/components/ChatInput.tsx
(1 hunks)packages/ai-native/src/browser/context/llm-context.contribution.ts
(1 hunks)packages/ai-native/src/browser/context/llm-context.service.ts
(1 hunks)packages/ai-native/src/browser/index.ts
(6 hunks)packages/ai-native/src/browser/types.ts
(3 hunks)packages/ai-native/src/common/llm-context.ts
(1 hunks)packages/ai-native/src/node/openai/openai-language-model.ts
(1 hunks)packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts
(4 hunks)tools/dev-tool/src/webpack.js
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ai-native/src/browser/components/ChatInput.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ai-native/src/node/openai/openai-language-model.ts
- packages/ai-native/src/browser/index.ts
- packages/ai-native/src/browser/types.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts
[error] 501-501: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🪛 GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
packages/ai-native/src/browser/components/ChatContext/index.tsx
[failure] 6-6:
Cannot find module '@opensumi/ide-ai-native/lib/common/llm-context' or its corresponding type declarations.
[failure] 36-36:
Argument of type '{}' is not assignable to parameter of type 'SetStateAction<FileContext[]>'.
[failure] 76-76:
Property 'isActive' does not exist on type '{}'.
packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx
[failure] 5-5:
Cannot find module '@opensumi/ide-ai-native/lib/common/llm-context' or its corresponding type declarations.
🪛 GitHub Check: build (ubuntu-latest, 20.x)
packages/ai-native/src/browser/components/ChatContext/index.tsx
[failure] 6-6:
Cannot find module '@opensumi/ide-ai-native/lib/common/llm-context' or its corresponding type declarations.
[failure] 36-36:
Argument of type '{}' is not assignable to parameter of type 'SetStateAction<FileContext[]>'.
[failure] 76-76:
Property 'isActive' does not exist on type '{}'.
packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx
[failure] 5-5:
Cannot find module '@opensumi/ide-ai-native/lib/common/llm-context' or its corresponding type declarations.
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: build-windows
- GitHub Check: ubuntu-latest, Node.js 20.x
🔇 Additional comments (9)
packages/ai-native/src/browser/chat/chat.view.tsx (2)
656-664
: LGTM! MCP 服务器变更监听实现使用
useEventEffect
监听 MCP 服务器变更并更新工具计数的实现非常清晰。
4-10
:❓ Verification inconclusive
新增的依赖导入需要版本约束
新增了多个关键依赖,包括 MCP 服务相关模块。建议在
package.json
中为这些依赖添加明确的版本约束,以避免潜在的版本不兼容问题。Also applies to: 41-42, 51-52
🏁 Script executed:
#!/bin/bash # 检查 package.json 中是否包含相关依赖的版本约束 cat package.json | jq '.dependencies | keys[] | select(contains("mcp"))'Length of output: 126
请手动确认 MCP 模块依赖的版本约束
目前自动检测脚本执行时出现了错误(jq: error (at <stdin>:152): null (null) has no keys
),因此未能获得关于 package.json 中 MCP 相关依赖版本约束的确切信息。建议您手动检查以下内容:
- 确认 package.json 文件中的
dependencies
是否包含 MCP 模块,并为这些依赖项添加明确的版本约束。- 同步检查文件
packages/ai-native/src/browser/chat/chat.view.tsx
中第 4-10 行,以及其它相关位置(41-42, 51-52 行)新增的 MCP 服务相关代码,确保版本控制的一致性。请在确认之后补充或修正版本约束以防止潜在的版本不兼容问题。
packages/ai-native/src/common/llm-context.ts (1)
38-41
: 确认attachedFiles
的格式与前端或其他调用方一致当序列化后返回
attachedFiles
,其结构包含内容、错误信息、路径、语言等字段。需确保与前端或其他组件调用方的需求相符,以免因字段名称或格式不一致导致后续展示或处理异常。tools/dev-tool/src/webpack.js (2)
214-217
: 优化 EXTENSION_WORKER_HOST 路径构建路径拼接的方式更加清晰和统一,使用
withSlash
确保跨平台兼容性。
225-236
: 改进 ForkTsCheckerWebpackPlugin 配置结构配置结构更加清晰,将
issue
属性移至与typescript
同级,提高了可读性。packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts (2)
1-1
: 添加依赖注入相关的导入和属性通过
@Autowired(INJECTOR_TOKEN)
注入Injector
实例,为动态注册提供支持。Also applies to: 69-70
501-506
: 实现 ChatAgentPromptProvider 注册功能新增的
registerChatAgentPromptProvider
方法允许动态注册聊天代理提示提供者,增强了系统的扩展性。🧰 Tools
🪛 Biome (1.9.4)
[error] 501-501: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.(lint/complexity/noUselessCatch)
packages/ai-native/package.json (2)
27-35
: 添加 OpenSumi 工作区依赖新增了以下工作区依赖:
- ide-addons
- ide-connection
- ide-file-search
这些依赖的添加增强了与 OpenSumi IDE 核心功能的集成。
22-26
:❓ Verification inconclusive
添加 AI 模型相关依赖
新增了多个 AI SDK 依赖,支持与不同 AI 服务的集成:
- Anthropic
- DeepSeek
- OpenAI
- Model Context Protocol (MCP)
建议验证这些依赖的兼容性和安全性。
🌐 Web query:
Are there any known compatibility issues or security vulnerabilities with @ai-sdk/[email protected], @ai-sdk/[email protected], or @ai-sdk/[email protected]?
💡 Result:
Based on analysis of available information up to February 2025:
Compatibility Issues
@ai-sdk/[email protected]
- Known schema validation issues with
o*-mini
models when using optional properties in object schemas ([10][7]).- Error manifests as:
Error [AI_APICallError]: Invalid schema: strict mode: unknown keyword: "optional"
- Workaround: Avoid
optional
keyword in schemas when usingo3-mini
/o1-mini
models ([10][7])@ai-sdk/[email protected]
- No reported compatibility issues found in analyzed sources
- Environment variable handling appears stable when using
DEEPSEEK_API_KEY
([9])@ai-sdk/[email protected]
- No specific compatibility issues reported
- Confirmed working with Claude 3 Sonnet 2024-10-22 v2 model ([5])
Security Vulnerabilities
Critical Considerations
NuGet Ecosystem Risks ([2])
- Malicious packages using homoglyph attacks (e.g., "Gunа" vs "Guna")
- IL weaving techniques modifying binaries post-compilation
General SDK Security Practices
- Validate API base URLs to prevent SSRF (as seen in FastChat vulnerabilities) ([3])
- Sanitize user inputs to avoid XSS (observed in GPT Academic vulnerabilities) ([3])
Recommendations
Package Action ⚠️ OpenAI Avoid optional schema properties with o*-mini models ✅ DeepSeek Monitor API key rotation 🔒 All Verify package signatures against registry hashes No direct security vulnerabilities were found in the specified package versions, but broader ecosystem risks require vigilance against dependency confusion attacks and input validation flaws ([2][3]).
Citations:
- 1: https://www.nuget.org/packages/Anthropic.SDK/
- 2: https://securityboulevard.com/2024/07/malicious-nuget-campaign-uses-homoglyphs-and-il-weaving-to-fool-devs/
- 3: https://protectai.com/threat-research/vulnhuntr-first-0-day-vulnerabilities
- 4: ERROR: pip's dependency resolver does not currently take into account all the packages that are installed apple/turicreate#3383
- 5: https://classic.yarnpkg.com/en/package/@anthropic-ai/bedrock-sdk
- 6: Deepseek support in Eliza eliza-starter repository elizaOS/eliza#2722
- 7: Using openai o*-mini generates an invalid tool's parameters schema. vercel/ai#4662
- 8: https://classic.yarnpkg.com/en/package/@anthropic-ai/sdk
- 9: https://libraries.io/pypi/deepseek-cli
- 10: generateObject throws error with o3-mini when schema has an optional param vercel/ai#4681
关键:验证 AI SDK 依赖的兼容性和安全性
在
packages/ai-native/package.json
中新增了以下 AI SDK 依赖:"@ai-sdk/anthropic": "^1.1.6", "@ai-sdk/deepseek": "^0.1.8", "@ai-sdk/openai": "^1.1.9", "@anthropic-ai/sdk": "^0.36.3", "@modelcontextprotocol/sdk": "^1.3.1",需要注意的是:
- @ai-sdk/[email protected]: 存在在使用 o*-mini 模型时,若 schema 中使用
optional
关键字可能引发校验错误(报错信息如Error [AI_APICallError]: Invalid schema: strict mode: unknown keyword: "optional"
)。建议在定义 schema 时避免使用optional
关键字,或按照文档推荐的方案进行调整。- @ai-sdk/[email protected] 和 @ai-sdk/[email protected]: 到目前为止未发现兼容性问题,但仍建议定期关注更新动态,确保持续的安全性和兼容性。
- 对于 @modelcontextprotocol/sdk 及其他新增依赖,也建议核实包签名和官方更新日志,防范依赖混淆等安全风险。
请在合入前进一步验证这些依赖与现有功能的兼容情况,并适当调整 schema 定义以规避潜在问题。
packages/ai-native/src/browser/components/ChatContext/index.tsx
Outdated
Show resolved
Hide resolved
packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/ai-native/src/browser/types.ts (1)
506-531
: 🛠️ Refactor suggestion建议优化上下文提示模板
当前上下文提示模板存在以下问题:
- 文件内容直接拼接可能导致 XML 注入
- 错误信息直接拼接可能泄露敏感信息
- 缺少错误处理机制
+import { escapeXml } from '@opensumi/ide-core-common/lib/utils/strings'; provideContextPrompt: (context: SerializedContext, userMessage: string) => ` <additional_data> Below are some potentially helpful/relevant pieces of information for figuring out to respond <recently_viewed_files> - ${context.recentlyViewFiles.map((file, idx) => `${idx} + 1: ${file}`)} + ${context.recentlyViewFiles.map((file, idx) => escapeXml(`${idx + 1}: ${file}`))} </recently_viewed_files> <attached_files> ${context.attachedFiles.map( (file) => ` <file_contents> - \`\`\`${file.language} ${file.path} + \`\`\`${escapeXml(file.language)} ${escapeXml(file.path)} - ${file.content} + ${escapeXml(file.content)} \`\`\` </file_contents> <linter_errors> - ${file.lineErrors.join('`n')} + ${file.lineErrors.map(error => escapeXml(error.message)).join('`n')} </linter_errors> `, )} </attached_files> </additional_data> <user_query> - ${userMessage} + ${escapeXml(userMessage)} </user_query>`,
♻️ Duplicate comments (5)
packages/ai-native/src/browser/types.ts (3)
314-325
: 🛠️ Refactor suggestion建议添加外部 MCP 服务器初始化的错误处理
初始化外部 MCP 服务器时缺少错误处理机制,可能导致单个服务器初始化失败影响其他服务器。
if (supportsMCP) { // 初始化内置 MCP Server this.sumiMCPServerBackendProxy.initBuiltinMCPServer(); // 从 preferences 获取并初始化外部 MCP Servers const mcpServers = this.preferenceService.getValid<MCPServerDescription[]>( AINativeSettingSectionsId.MCPServers, ); if (mcpServers && mcpServers.length > 0) { - this.sumiMCPServerBackendProxy.initExternalMCPServers(mcpServers); + mcpServers.forEach(server => { + try { + this.sumiMCPServerBackendProxy.initExternalMCPServer(server); + } catch (error) { + console.error(`初始化外部 MCP 服务器 ${server.name} 失败:`, error); + } + }); } }
330-341
: 🛠️ Refactor suggestion建议完善注册表接口
当前注册表接口缺少以下重要功能:
- 错误处理机制
- 工具注销方法
- 工具存在性检查
export interface IMCPServerRegistry { + /** + * 检查工具是否已注册 + * @param name 工具名称 + */ + hasTool(name: string): boolean; + registerMCPTool(tool: MCPToolDefinition): void; + + /** + * 注销 MCP 工具 + * @param name 工具名称 + */ + unregisterTool(name: string): void; + getMCPTools(): MCPToolDefinition[]; + callMCPTool( name: string, args: any, ): Promise<{ content: { type: string; text: string }[]; isError?: boolean; }>; + + /** + * 错误事件监听 + * @param listener 错误监听器 + */ + onError(listener: (error: Error) => void): IDisposable; }
317-328
: 🛠️ Refactor suggestion建议加强类型定义
当前
MCPToolDefinition
接口存在以下问题:
inputSchema
使用 any 类型过于宽松handler
函数的参数和返回值可以更具体+import { JSONSchema7 } from 'json-schema'; export interface MCPToolDefinition { name: string; description: string; - inputSchema: any; // JSON Schema + inputSchema: JSONSchema7; handler: ( - args: any, + args: Record<string, unknown>, logger: MCPLogger, ) => Promise<{ content: { type: string; text: string }[]; isError?: boolean; + errorMessage?: string; }>; }packages/ai-native/src/browser/ai-core.contribution.ts (2)
314-325
:⚠️ Potential issue建议添加外部 MCP 服务器初始化的错误处理
当前实现缺少错误处理机制,可能导致单个服务器初始化失败影响其他服务器。
请参考之前的评审建议,为每个外部服务器添加独立的错误处理。
416-443
:⚠️ Potential issue建议使用安全存储机制保存 API 密钥
当前实现直接在设置中存储 API 密钥,存在安全风险。
请参考之前的评审建议,使用系统的安全凭据存储机制。
🧹 Nitpick comments (1)
packages/ai-native/src/browser/types.ts (1)
378-384
: 建议优化上下文提示接口
ChatAgentPromptProvider
接口的provideContextPrompt
方法可以增加更多上下文信息。export interface ChatAgentPromptProvider { /** * 提供上下文提示 * @param context 上下文 + * @param userMessage 用户消息 + * @param options 额外选项 */ - provideContextPrompt(context: SerializedContext, userMessage: string): MaybePromise<string>; + provideContextPrompt( + context: SerializedContext, + userMessage: string, + options?: { + maxTokens?: number; + temperature?: number; + systemPrompt?: string; + } + ): MaybePromise<string>; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/ai-native/src/browser/ai-core.contribution.ts
(8 hunks)packages/ai-native/src/browser/types.ts
(3 hunks)packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts
(5 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts
[error] 502-502: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🔇 Additional comments (5)
packages/ai-native/src/browser/types.ts (2)
416-443
: 🛠️ Refactor suggestion建议使用安全存储机制保存 API 密钥
当前 API 密钥直接存储在设置中,建议使用系统的安全凭据存储。
+import { ICredentialsService } from '@opensumi/ide-core-common'; +@Autowired(ICredentialsService) +private readonly credentialsService: ICredentialsService; +private async storeApiKey(key: string, value: string) { + await this.credentialsService.setPassword(`ai-native.${key}`, value); +} +private async getApiKey(key: string): Promise<string> { + return this.credentialsService.getPassword(`ai-native.${key}`); +} if (this.aiNativeConfigService.capabilities.supportsCustomLLMSettings) { registry.registerSettingSection(AI_NATIVE_SETTING_GROUP_ID, { title: localize('preference.ai.native.llm.apiSettings.title'), preferences: [ { id: AINativeSettingSectionsId.LLMModelSelection, localized: 'preference.ai.native.llm.model.selection', }, { id: AINativeSettingSectionsId.DeepseekApiKey, localized: 'preference.ai.native.deepseek.apiKey', + onChange: (value) => this.storeApiKey('deepseek', value), }, // ... 其他 API 密钥设置 ], }); }
342-345
: 🛠️ Refactor suggestion建议添加注册的清理逻辑
MCP 服务器工具的注册缺少清理机制,可能导致内存泄漏。
+private readonly registeredTools = new Set<string>(); // 注册 Opensumi 框架提供的 MCP Server Tools 能力 (此时的 Opensumi 作为 MCP Server) this.mcpServerContributions.getContributions().forEach((contribution) => { + const toolId = contribution.registerMCPServer(this.mcpServerRegistry); + if (toolId) { + this.registeredTools.add(toolId); + } }); +dispose() { + // 清理已注册的工具 + this.registeredTools.forEach(toolId => { + this.mcpServerRegistry.unregisterTool(toolId); + }); + this.registeredTools.clear(); + super.dispose(); +}packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts (1)
14-14
: 代码结构清晰,依赖注入使用恰当!新增的导入和属性定义符合依赖注入的最佳实践。
Also applies to: 28-28, 70-71
packages/ai-native/src/browser/ai-core.contribution.ts (2)
76-77
: 依赖注入和类型定义规范合理!MCP 服务器相关的导入和属性定义遵循了良好的依赖注入实践。
Also applies to: 103-103, 108-109, 154-158
445-456
: MCP 服务器设置注册实现规范!设置注册逻辑结构清晰,正确使用了条件注册和本地化。
packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx (1)
82-109
:⚠️ Potential issue完善搜索功能的错误处理
当前搜索实现仍然缺少错误处理机制。
searchService .find(ev.target.value, { rootUris: [appConfig.workspaceDir], limit: 200, useGitIgnore: true, noIgnoreParent: true, fuzzyMatch: true, }) .then((res) => { const results = res.map((val) => new URI(val)); updateSearchResults(results); setActiveFile(results[0]); }) + .catch((error) => { + console.error('搜索失败:', error); + // 显示错误提示 + updateSearchResults([]); + setActiveFile(null); + }) .finally(() => { toggleSearching(false); });
🧹 Nitpick comments (4)
packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx (4)
28-36
: 建议优化滚动行为实现当前的滚动实现可能会在某些情况下表现不稳定。建议:
- 添加防抖以避免频繁滚动
- 考虑使用 smooth 滚动行为提升用户体验
const scrollBehavior: ScrollIntoViewOptions = { - behavior: 'instant', + behavior: 'smooth', block: 'end', };
38-49
: 建议增强组件的可访问性当前实现缺少必要的可访问性支持。建议:
- 添加适当的 ARIA 属性
- 确保键盘导航支持
- 提供更多的视觉反馈
<div className={cls(styles.candidate_file, active && styles.active)} ref={(ele) => (itemsRef.current = ele)} onClick={() => (selected ? onDidDeselect(uri) : onDidSelect(uri))} + role="option" + aria-selected={selected} + tabIndex={0} + onKeyPress={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + selected ? onDidDeselect(uri) : onDidSelect(uri); + } + }} >
82-83
: 优化 debounce 实现当前的 debounce 实现可能会在组件重渲染时创建新的函数实例。建议:
- 将 debounce 函数移到组件外部
- 使用 useCallback 包装整个函数
- const onDidInput = useCallback( - debounce((ev) => { + const debouncedSearch = debounce((ev) => { // ... search logic - }, 500), - [], - ); + }, 500); + + const onDidInput = useCallback((ev) => { + debouncedSearch(ev); + }, []);
148-172
: 建议添加加载状态和空状态提示当前实现缺少用户反馈。建议:
- 添加搜索加载状态的视觉反馈
- 添加搜索结果为空时的提示信息
- 添加错误状态的提示信息
<div className={styles.context_list}> {searching && <div className={styles.context_search_layer} />} <span className={styles.list_desc}> {searchResults.length > 0 ? 'Search Results' : 'Recent Opened Files'} </span> + {!searching && searchResults.length === 0 && candidateFiles.length === 0 && ( + <div className={styles.empty_state}> + 没有找到相关文件 + </div> + )} {(searchResults.length > 0 ? searchResults : candidateFiles).map((file) => (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx
(1 hunks)packages/ai-native/src/browser/components/ChatContext/index.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ai-native/src/browser/components/ChatContext/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
🔇 Additional comments (1)
packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx (1)
1-57
: 导入和接口定义看起来不错!导入语句组织合理,接口定义清晰完整。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts (1)
502-534
:⚠️ Potential issue需要增加错误处理和修复格式问题
- 需要对
context.recentlyViewFiles
和context.attachedFiles
添加空值检查。- 模板字符串中的索引计算有语法错误。
- 建议优化模板格式以提高可读性。
建议修改为:
registerChatAgentPromptProvider(): void { this.injector.addProviders({ token: ChatAgentPromptProvider, useValue: { provideContextPrompt: (context: SerializedContext, userMessage: string) => ` <additional_data> Below are some potentially helpful/relevant pieces of information for figuring out to respond <recently_viewed_files> - ${context.recentlyViewFiles.map((file, idx) => `${idx} + 1: ${file}`)} + ${(context.recentlyViewFiles || []).map((file, idx) => `${idx + 1}: ${file}`)} </recently_viewed_files> <attached_files> - ${context.attachedFiles.map( + ${(context.attachedFiles || []).map( (file) => ` <file_contents> \`\`\`${file.language} ${file.path} ${file.content} \`\`\` </file_contents> <linter_errors> - ${file.lineErrors.join('`n')} + ${(file.lineErrors || []).join('`n')} </linter_errors> `, )} </attached_files> </additional_data> <user_query> ${userMessage} </user_query>`, }, }); }🧰 Tools
🪛 Biome (1.9.4)
[error] 502-502: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.(lint/complexity/noUselessCatch)
🧹 Nitpick comments (3)
packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx (3)
1-14
: 建议优化导入语句的组织结构建议将导入语句按以下类别分组组织:
- 第三方库导入(React、lodash)
- OpenSumi IDE 相关导入
- 本地模块导入
这样可以提高代码的可读性和可维护性。
import cls from 'classnames'; import { debounce } from 'lodash'; import React, { memo, useCallback, useEffect, useRef, useState } from 'react'; + import { ClickOutside } from '@opensumi/ide-components/lib/click-outside'; import { AppConfig, LabelService } from '@opensumi/ide-core-browser'; import { Icon, Input, Scrollbars } from '@opensumi/ide-core-browser/lib/components'; import { RecentFilesManager } from '@opensumi/ide-core-browser/lib/quick-open/recent-files'; import { useInjectable } from '@opensumi/ide-core-browser/lib/react-hooks/injectable-hooks'; import { FileSearchServicePath, IFileSearchService } from '@opensumi/ide-file-search/lib/common/file-search'; import { URI } from '@opensumi/ide-utils'; + import { FileContext } from '../../../common/llm-context'; + import styles from './style.module.less';
25-52
: 建议增强组件的可访问性当前的文件候选项实现缺少一些重要的可访问性特性:
- 缺少 ARIA 属性
- 键盘交互不完整
- 缺少适当的语义化HTML元素
const CandidateFile = memo(({ uri, active, selected, onDidSelect, onDidDeselect }: CandidateFileProps) => { // ... return ( - <div + <button + type="button" className={cls(styles.candidate_file, active && styles.active)} ref={(ele) => (itemsRef.current = ele)} onClick={() => (selected ? onDidDeselect(uri) : onDidSelect(uri))} + role="option" + aria-selected={selected} + aria-label={`${uri.path.base} in ${URI.file(appConfig.workspaceDir).relative(uri.parent)?.toString()}`} > <Icon iconClass={labelService.getIcon(uri)} /> <span className={styles.basename}>{uri.path.base}</span> <span className={styles.dir}>{URI.file(appConfig.workspaceDir).relative(uri.parent)?.toString()}</span> {selected && <Icon icon='check' style={{ marginLeft: 'auto', color: 'var(--editorGutter-addedBackground)' }} />} - </div> + </button> ); });
113-148
: 建议增强键盘导航体验当前的键盘导航实现可以进一步优化:
- 支持 Home/End 键跳转到列表首尾
- 支持 Page Up/Down 键进行页面级别的跳转
- 添加屏幕阅读器友好的状态提示
const onDidKeyDown = useCallback( (event) => { const { key } = event; - const validKeys = ['ArrowUp', 'ArrowDown']; + const validKeys = ['ArrowUp', 'ArrowDown', 'Home', 'End', 'PageUp', 'PageDown']; // ... existing key handling code ... + + if (key === 'Home') { + setActiveFile(files[0]); + return; + } + + if (key === 'End') { + setActiveFile(files[files.length - 1]); + return; + } + + if (key === 'PageUp' || key === 'PageDown') { + const pageSize = 10; // 或根据实际可视区域计算 + const direction = key === 'PageUp' ? -1 : 1; + const newIndex = Math.max(0, Math.min(files.length - 1, safeIndex + direction * pageSize)); + setActiveFile(files[newIndex]); + return; + } }, [activeFile, searchResults, candidateFiles], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (4)
packages/ai-native/package.json
(2 hunks)packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx
(1 hunks)packages/ai-native/src/browser/types.ts
(3 hunks)packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ai-native/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts
[error] 502-502: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
- GitHub Check: unittest (macos-latest, 18.x, node)
🔇 Additional comments (5)
packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts (1)
1-1
: 依赖注入设置正确!新增的依赖注入相关的导入和属性声明符合项目规范。
Also applies to: 14-14, 70-71
packages/ai-native/src/browser/components/ChatContext/ContextSelector.tsx (1)
84-111
: 建议完善搜索功能的错误处理当前的搜索实现缺少完整的错误处理机制。
packages/ai-native/src/browser/types.ts (3)
350-361
: 建议加强类型定义当前接口存在以下问题:
inputSchema
使用 any 类型过于宽松handler
函数的参数和返回值可以更具体
363-374
: 建议完善注册表接口当前注册表接口缺少以下重要功能:
- 错误处理机制
- 工具注销方法
- 工具存在性检查
409-417
: 接口定义清晰完整ChatAgentPromptProvider 接口设计良好:
- 类型定义准确
- 文档注释清晰
- 参数类型完整
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* chore: update terminal * chore: update exit code * feat: enhance terminal command execution with success message and auto-close feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts (5)
11-14
: 建议将颜色常量与业务逻辑分离。
当前color
对象仅提供简单的控制台样式,若后续需要扩展其他颜色配置或与其他模块复用,可考虑单独抽离到公共工具文件中,增强复用性。
56-63
: Shell 配置简单明了。
目前仅假设了类 Unix 环境,将来若需兼容 Windows 体系,可考虑在此根据平台进行处理。
64-67
: 存在用户审批的 TODO 注释,需进一步实现交互逻辑。
此处仅留下FIXME
,可能导致潜在安全和可用性问题。建议尽快完善用户审批流程或在适当位置补充相应的交互实现。
74-85
: 输出捕获方式合理,Deferred 用法恰当。
在onOutput
中收集日志可以满足大多数场景,但若输出量极大时可能带来内存或性能隐患,可考虑适度做截断或缓冲处理。
86-104
: 自动关闭终端的行为可考虑做成可配置项。
目前固定 3 秒自动关闭,可能在某些场景下过快或过慢,建议提供可自定义的超时时长,或在需要时手动关闭。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/ai-native/src/browser/index.ts
(6 hunks)packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts
- packages/ai-native/src/browser/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: build-windows
- GitHub Check: ubuntu-latest, Node.js 20.x
🔇 Additional comments (7)
packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts (7)
1-3
: 导入 zod 库以进行输入验证很实用,整体实现值得肯定。
这些行只是导入相关依赖,没有发现潜在问题。
29-30
: 使用 @Domain 装饰器便于跨模块注册,思路清晰。
暂无明显问题,整体实现良好。
31-39
: 依赖注入可读性好,方便日后扩展。
使用@Autowired
注解合理,没有明显的并发或生命周期问题。
40-44
: 注册 MCP 工具的方式清晰易懂。
使用registry.registerMCPTool
方法符合 MCP 工具注册模式,没有发现逻辑缺陷。
46-54
: Tool Definition 描述详细,适合后续自动生成文档。
说明信息非常完整,用户在使用时可以准确了解工具功能与规则。
69-73
: 创建 Terminal Widget 逻辑简洁高效。
终端启动参数closeWhenExited: false
有助于查看输出,但请确认是否符合产品需求。
[approve]
105-107
: 异步处理结束后返回 Promise,逻辑清晰并保持良好扩展性。
未发现问题,当前实现可用性高。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (14)
packages/ai-native/__test__/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.test.ts (2)
13-61
: 建议改进模拟对象的类型定义测试设置很完整,但有以下建议:
- 建议为 mockMonacoEditor 和 mockInlineDiffHandler 定义具体的接口类型,而不是使用 any
- 可以使用 TypeScript 的 Partial 类型来更好地定义模拟对象
建议按如下方式改进:
- let mockMonacoEditor: any; + let mockMonacoEditor: Partial<IEditor>; - let mockInlineDiffHandler: any; + let mockInlineDiffHandler: Partial<InlineDiffController>;
62-125
: 建议添加更多边界测试用例当前的测试覆盖了基本场景,建议添加以下测试用例:
- 测试空文本内容的情况
- 测试超大文本内容的处理
- 测试特殊字符(如换行符、制表符等)的处理
- 测试在 diff 预览创建过程中编辑器状态改变的情况
示例测试用例:
it('should handle empty content', async () => { const result = await tool['handler']({ text: '' }, mockLogger); expect(result.content).toEqual([{ type: 'text', text: 'ok' }]); expect(mockDiffPreviewer.setValue).toHaveBeenCalledWith(''); }); it('should handle special characters', async () => { const specialContent = 'line1\nline2\tindented'; const result = await tool['handler']({ text: specialContent }, mockLogger); expect(result.content).toEqual([{ type: 'text', text: 'ok' }]); expect(mockDiffPreviewer.setValue).toHaveBeenCalledWith(specialContent); });packages/ai-native/__test__/browser/mcp/tools/getOpenEditorFileText.test.ts (1)
34-39
: 建议增加更多边缘情况的测试用例当前的测试覆盖了基本场景,但建议添加以下测试用例:
- getText() 方法抛出异常的情况
- 文件内容过大时的处理情况
- 文件内容包含特殊字符的情况
packages/ai-native/__test__/browser/mcp/tools/getSelectedText.test.ts (2)
16-19
: 建议改进 Monaco Editor 的模拟实现当前的模拟实现过于简单,建议:
- 添加更多 Monaco Editor 的核心方法
- 使用
@opensumi/monaco-editor-core
中的类型定义
55-71
: 建议扩展选区测试用例当前仅测试了单行选区,建议添加以下测试场景:
- 多行选区
- 选区跨越文件开始或结束位置
- 空选区(startLineNumber 等于 endLineNumber 且 startColumn 等于 endColumn)
packages/ai-native/__test__/browser/mcp/tools/getOpenEditorFileDiagnostics.test.ts (2)
89-110
: 建议重构测试数据的组织方式建议将模拟的诊断数据抽取到单独的常量或工具函数中,以提高代码的可维护性和重用性:
const createMockDiagnostic = (lineNumber: number, severity: MarkerSeverity, message: string) => ({ startLineNumber: lineNumber, severity, message, }); const mockMarkers = [ createMockDiagnostic(1, MarkerSeverity.Error, 'Error message'), createMockDiagnostic(2, MarkerSeverity.Warning, 'Warning message'), createMockDiagnostic(3, MarkerSeverity.Info, 'Info message'), createMockDiagnostic(4, MarkerSeverity.Hint, 'Hint message'), ];
146-155
: 建议增加更多错误处理测试场景当前仅测试了一般性错误,建议添加以下错误场景的测试:
- 网络超时错误
- 权限错误
- 资源不可用错误
packages/ai-native/__test__/common/mcp-server-manager.test.ts (2)
6-34
: 建议添加类型断言以增强类型安全性建议在 mock 对象创建时使用显式的类型断言,以确保所有必需的方法都被正确实现。
- let mockManager: MCPServerManager; + let mockManager: jest.Mocked<MCPServerManager>; mockManager = { // ... existing mock implementations ... - }; + } as jest.Mocked<MCPServerManager>;
37-65
: 建议添加错误处理测试用例服务器管理测试套件目前只覆盖了成功场景。建议添加以下错误场景的测试:
- 添加重复名称的服务器
- 移除不存在的服务器
- 服务器操作失败时的错误处理
示例测试用例:
it('should handle error when removing non-existent server', async () => { const errorMessage = 'Server not found'; (mockManager.removeServer as jest.Mock).mockRejectedValue(new Error(errorMessage)); await expect(mockManager.removeServer('non-existent')).rejects.toThrow(errorMessage); });packages/ai-native/__test__/browser/mcp/tools/replaceOpenEditorFile.test.ts (5)
12-13
: 建议改进测试设置的类型定义和结构建议对以下几点进行优化:
- 为
mockMonacoEditor
和mockModel
定义具体的接口类型,而不是使用any
- 考虑将 mock 对象的创建抽取到独立的工厂函数中,以提高代码的可维护性
建议修改如下:
- let mockMonacoEditor: any; - let mockModel: any; + let mockMonacoEditor: Pick<IEditor, 'getModel' | 'executeEdits'>; + let mockModel: Pick<IModel, 'getFullModelRange'>; + function createMockModel() { + return { + getFullModelRange: jest.fn(), + }; + } + function createMockEditor(model: ReturnType<typeof createMockModel>) { + return { + getModel: jest.fn().mockReturnValue(model), + executeEdits: jest.fn(), + }; + } beforeEach(() => { const injector = createBrowserInjector([]); - mockModel = { - getFullModelRange: jest.fn(), - }; - mockMonacoEditor = { - getModel: jest.fn().mockReturnValue(mockModel), - executeEdits: jest.fn(), - }; + mockModel = createMockModel(); + mockMonacoEditor = createMockEditor(mockModel);Also applies to: 17-36
39-43
: 建议扩展工具注册测试的覆盖范围当前测试仅验证了工具的名称和描述。建议增加对工具定义其他重要属性的验证,如参数验证等。
建议添加如下测试内容:
it('should register tool with correct name and description', () => { const definition = tool.getToolDefinition(); expect(definition.name).toBe('replace_open_in_editor_file_text'); expect(definition.description).toContain('Replaces the entire content'); + // 验证参数定义 + expect(definition.parameters).toBeDefined(); + expect(definition.parameters.properties.text).toBeDefined(); + expect(definition.parameters.required).toContain('text'); });
45-59
: 建议增加更多错误场景的测试用例当前的错误处理测试覆盖了基本场景,但建议增加更多边界情况的测试。
建议添加以下测试场景:
it('should return error when text parameter is empty', async () => { const result = await tool['handler']({ text: '' }, mockLogger); expect(result.content).toEqual([{ type: 'text', text: 'empty content' }]); expect(result.isError).toBe(true); }); it('should return error when text parameter is too large', async () => { const largeText = 'a'.repeat(1000000); // 假设有文件大小限制 const result = await tool['handler']({ text: largeText }, mockLogger); expect(result.content).toEqual([{ type: 'text', text: 'content too large' }]); expect(result.isError).toBe(true); });
61-82
: 建议完善成功场景的测试用例当前的成功场景测试覆盖了基本功能,但可以增加更多细节验证。
建议添加以下验证:
it('should successfully replace file content', async () => { const mockRange: IRange = { startLineNumber: 1, startColumn: 1, endLineNumber: 10, endColumn: 20, }; const newContent = 'new file content'; mockModel.getFullModelRange.mockReturnValue(mockRange); + const operationOrder: string[] = []; + mockModel.getFullModelRange.mockImplementation(() => { + operationOrder.push('getRange'); + return mockRange; + }); + mockMonacoEditor.executeEdits.mockImplementation(() => { + operationOrder.push('executeEdits'); + return true; + }); const result = await tool['handler']({ text: newContent }, mockLogger); expect(mockModel.getFullModelRange).toHaveBeenCalled(); expect(mockMonacoEditor.executeEdits).toHaveBeenCalledWith('mcp.tool.replace-file', [ { range: mockRange, text: newContent, }, ]); expect(result.content).toEqual([{ type: 'text', text: 'ok' }]); expect(mockLogger.appendLine).toHaveBeenCalledWith('Successfully replaced file content'); + // 验证操作顺序 + expect(operationOrder).toEqual(['getRange', 'executeEdits']); + // 验证只调用了一次 + expect(mockModel.getFullModelRange).toHaveBeenCalledTimes(1); + expect(mockMonacoEditor.executeEdits).toHaveBeenCalledTimes(1); });
84-101
: 建议增强错误处理测试的完整性当前的错误处理测试仅覆盖了基本错误场景,建议增加更多错误类型和阶段的测试。
建议添加以下测试场景:
it('should handle getFullModelRange errors', async () => { mockModel.getFullModelRange.mockImplementation(() => { throw new Error('Range error'); }); const result = await tool['handler']({ text: 'new content' }, mockLogger); expect(result.content).toEqual([{ type: 'text', text: 'unknown error' }]); expect(result.isError).toBe(true); expect(mockLogger.appendLine).toHaveBeenCalledWith('Error getting model range: Error: Range error'); }); it('should handle executeEdits returning false', async () => { mockMonacoEditor.executeEdits.mockReturnValue(false); const mockRange = { startLineNumber: 1, startColumn: 1, endLineNumber: 10, endColumn: 20, }; mockModel.getFullModelRange.mockReturnValue(mockRange); const result = await tool['handler']({ text: 'new content' }, mockLogger); expect(result.content).toEqual([{ type: 'text', text: 'failed to replace content' }]); expect(result.isError).toBe(true); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/ai-native/__test__/browser/mcp/tools/getOpenEditorFileDiagnostics.test.ts
(1 hunks)packages/ai-native/__test__/browser/mcp/tools/getOpenEditorFileText.test.ts
(1 hunks)packages/ai-native/__test__/browser/mcp/tools/getSelectedText.test.ts
(1 hunks)packages/ai-native/__test__/browser/mcp/tools/replaceOpenEditorFile.test.ts
(1 hunks)packages/ai-native/__test__/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.test.ts
(1 hunks)packages/ai-native/__test__/common/mcp-server-manager.test.ts
(1 hunks)packages/ai-native/__test__/node/mcp-server.test.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
🔇 Additional comments (2)
packages/ai-native/__test__/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.test.ts (2)
1-12
: 导入语句组织合理!导入语句包含了所有必要的依赖,并且通过 jest.mock 正确设置了 InlineDiffController 的模拟。
1-125
: 整体测试实现质量良好!测试文件结构清晰,覆盖了主要功能点,包括:
- 工具注册验证
- 错误处理场景
- diff 预览创建
- 选区对象验证
建议在完善类型定义和添加边界测试用例后合并。
Types
Background or solution
Changelog
Summary by CodeRabbit