feat: add terminal command execution tool with user approval #4398
Conversation
|
Warning Rate limit exceeded@Aaaaash has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Walkthrough此次 PR 引入了一个新的 React 组件 Changes
Sequence Diagram(s)sequenceDiagram
participant RT as RunTerminalCommandTool
participant RC as RunCommandHandler
participant TT as TerminalToolComponent
participant U as User
RT->>RC: 调用 handler 执行命令
RC->>TT: 注册并渲染终端组件
TT->>U: 展示命令结果及审批按钮
U->>TT: 点击审批按钮
TT->>RC: 调用 handleApproval 提交审批
RC-->>RT: 返回命令执行结果
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts (1)
29-29: 组件注册：保证在 GUI 中使用新工具组件。
在 MCP 服务器注册新的工具组件，有利于在界面端调度并管理终端交互，对于后续扩展也更直观。packages/ai-native/src/browser/mcp/tools/components/Terminal.tsx (2)
1-9: 模块导入：结构清晰，组件依赖明确。
引入了 React、DI 以及 UI 组件，充分利用了已有的组件和样式处理能力，建议在需要时再补充必要的依赖，避免过度导入。
50-82: UI 渲染与交互：根据 state 切换不同内容。
- 当 state 为 'result' 时展示命令输出，当 state 为 'complete' 时，如果
require_user_approval为真则提供交互按钮。逻辑简洁易懂。- 建议在命令输出位置增加容错提示，如遇到空文本或格式不正确时，给予用户更清晰的信息。
packages/ai-native/src/browser/mcp/tools/handlers/RunCommand.ts (3)
10-13: color 常量：处理终端输出的样式。
此处使用 ANSI 转义码为输出添加斜体和重置风格，可以在将来进行更丰富的配色或日志分类。
43-49: getShellLaunchConfig：自动生成 Shell 配置。
使用工作区目录与自增终端名称，简化了多次调用时的管理，建议在实际生产环境中引入更多可配置项（如 shell 路径、自定义环境变量等）。
51-97: handler 方法：统一执行逻辑并支持用户交互。
- 当
require_user_approval为真时，先通过 deferred 阻塞等待用户输入，对逻辑流程友好且可维护。- 终端输出的收集与退出回调准确，执行完毕后延迟关闭终端，并输出执行成功信息，提高了用户体验。
- 建议增加针对长时间运行或后台执行的处理策略（如超时、手动取消等），以免影响系统资源使用。
packages/ai-native/src/browser/mcp/tools/components/index.module.less (3)
76-86: 评审：头部样式定义
.header样式为组件头部设置了内边距、背景色、边框以及点击指针样式。建议考虑增加键盘聚焦或其它无障碍交互样式，以便在交互性较强的组件中提升可访问性。
100-118: 评审：文件项样式定义
.fileItem样式使用了内边距、字体大小、边距和悬停效果来提升列表项的交互体验。建议可以考虑在悬停效果中增加平滑过渡效果，以提升用户体验。
134-162: 评审：执行命令工具样式定义
.run_cmd_tool内部嵌套的样式结构清晰，.command_title与.command_content的定义符合预期。但注意到后续的.comand_description与.cmmand_footer命名存在不一致或拼写错误问题，建议修改为.command_description和.command_footer，以保持命名的统一和易读性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/ai-native/src/browser/mcp/tools/components/Terminal.tsx(1 hunks)packages/ai-native/src/browser/mcp/tools/components/index.module.less(1 hunks)packages/ai-native/src/browser/mcp/tools/handlers/RunCommand.ts(1 hunks)packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts(3 hunks)
🔇 Additional comments (12)
packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts (3)
10-11: 引入 TerminalToolComponent 与 RunCommandHandler：提升组件与逻辑解耦。
这部分导入有助于将终端 UI 与命令执行逻辑分别封装，提升可维护性与可扩展性。
24-25: 依赖注入 RunCommandHandler：模块化管理命令执行。
通过依赖注入的方式将RunCommandHandler引入，使命令执行逻辑集中在专用类中，实现了更合理的模块化和关注点分离。
42-43: handler 方法：将处理逻辑委托给专用类。
该方法简洁地转调用runCommandHandler.handler，使本类职责明确、体量更小，推荐继续保持此种委托模式以维持易读性。packages/ai-native/src/browser/mcp/tools/components/Terminal.tsx (2)
11-31: getResult 函数：对输出 JSON 的处理较为健壮。
此函数通过捕获 JSON 解析异常来返回空结果，避免了应用崩溃；如果后续需要识别更多字段，宜在此函数中做进一步的校验和处理。
33-49: TerminalToolComponent 基本结构：使用 memo、useCallback 提升性能。
组件中使用memo避免不必要的重渲染，同时使用useCallback来缓存事件处理函数，整体实现比较符合 React 性能优化实践。packages/ai-native/src/browser/mcp/tools/handlers/RunCommand.ts (2)
15-26: inputSchema：使用 Zod 明确命令参数结构。
通过 Zod 验证命令字符串、是否后台执行、用户批准等字段，有助于保障调用者传参的合规性。若有更多高级校验需求或整合业务逻辑，可继续扩展此处。
99-106: handleApproval：集中管理审批结果并解除阻塞。
对于交互场景而言，此函数能及时响应用户意图并唤醒等待流程。可在后续扩展相关失败重试或日志记录功能。packages/ai-native/src/browser/mcp/tools/components/index.module.less (5)
68-74: 评审：容器样式定义
这部分代码添加了.container样式，定义了边框、圆角、外边距以及溢出隐藏处理。整体没有问题，且确保了容器在视觉上的独立性。
88-94: 评审：文件列表样式定义
此处.fileList类通过重置内外边距、列表样式和设置背景色来确保列表项呈现效果统一。样式定义清晰合理。
96-98: 评审：折叠文件列表样式
.fileList.collapsed类通过设置display: none实现了列表的隐藏效果，这一实现简单明了，符合折叠功能需求。
120-124: 评审：文件图标样式定义
.fileIcon类设置了颜色、字体大小和固定宽度，适用于显示文件图标，样式简洁明确。
126-132: 评审：文件路径样式定义
.filePath通过margin-left: auto以及弹性布局属性，成功实现了路径信息的右侧对齐。整体属性设置合理。
Aaaaash
force-pushed
the
feat/add-run-cmd-tool-view
branch
from
9f94f46 to
ada2905
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
packages/ai-native/src/browser/mcp/tools/components/Terminal.tsx (2)
11-31: 建议改进错误处理和类型定义
getResult函数的错误处理和类型定义可以更加严谨：
- 应该对
data.isError和data.content的类型进行明确定义- 在解析失败时，应该返回一个带有错误信息的对象，而不是
null建议按照以下方式重构：
-function getResult(raw: string) { +interface CommandResult { + isError?: boolean; + text?: string; + parseError?: boolean; +} + +function getResult(raw: string): CommandResult { const result: { isError?: boolean; text?: string; } = {}; try { - const data = JSON.parse(raw); + const data: { isError?: boolean; content?: string } = JSON.parse(raw); if (data.isError) { result.isError = data.isError; } if (data.content) { result.text = data.content; } return result; } catch { - return null; + return { parseError: true }; } }
69-69: 修正拼写错误CSS 类名中存在拼写错误：
comand_description应为command_description。-<p className={styles.comand_description}>{args.explanation}</p> +<p className={styles.command_description}>{args.explanation}</p>packages/ai-native/src/browser/mcp/tools/handlers/RunCommand.ts (2)
73-75: 建议添加输出限制终端输出没有大小限制，可能导致内存问题。
建议添加输出大小限制：
+ private readonly MAX_OUTPUT_SIZE = 1024 * 1024; // 1MB + private currentOutputSize = 0; + terminalClient.onOutput((e) => { + const data = e.data.toString(); + if (this.currentOutputSize + data.length > this.MAX_OUTPUT_SIZE) { + result.push('\n... Output truncated due to size limit ...\n'); + return; + } + this.currentOutputSize += data.length; - result.push(e.data.toString()); + result.push(data); });
84-88: 建议改进用户体验终端执行完成消息可以更加友好，并支持国际化。
- terminalClient.term.writeln( - `\n${color.italic}> Command ${args.command} executed successfully. Terminal will close in ${ - 3000 / 1000 - } seconds.${color.reset}\n`, - ); + const message = args.is_background + ? '命令已在后台启动执行' + : `命令执行${e.code === 0 ? '成功' : '失败'}，终端将在 3 秒后关闭`; + terminalClient.term.writeln( + `\n${color.italic}> ${message}${color.reset}\n`, + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/ai-native/src/browser/mcp/tools/components/Terminal.tsx(1 hunks)packages/ai-native/src/browser/mcp/tools/components/index.module.less(1 hunks)packages/ai-native/src/browser/mcp/tools/handlers/RunCommand.ts(1 hunks)packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ai-native/src/browser/mcp/tools/components/index.module.less
🧰 Additional context used
🪛 GitHub Actions: E2E Test
packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts
[error] 38-38: error TS2322: Type '(args: { command: string; explanation: string; is_background: boolean; require_user_approval: boolean; } & { toolCallId: string; }, logger: MCPLogger) => Promise<{ isError?: boolean | undefined; content: string[]; } | { ...; }>' is not assignable to type '(args: any, logger: MCPLogger) => Promise<{ content: { type: string; text: string; }[]; isError?: boolean | undefined; }>'.
🪛 GitHub Actions: CI
packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts
[error] 38-38: error TS2322: Type '(args: { command: string; explanation: string; is_background: boolean; require_user_approval: boolean; } & { toolCallId: string; }, logger: MCPLogger) => Promise<{ isError?: boolean | undefined; content: string[]; } | { ...; }>' is not assignable to type '(args: any, logger: MCPLogger) => Promise<{ content: { type: string; text: string; }[]; isError?: boolean | undefined; }>'.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-windows
packages/ai-native/src/browser/mcp/tools/components/Terminal.tsx
Outdated
Show resolved
Hide resolved
packages/ai-native/src/browser/mcp/tools/handlers/RunCommand.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/ai-native/src/browser/mcp/tools/components/Terminal.tsx (3)
36-38: 建议添加 Props 类型文档为了提高代码可维护性，建议为组件的 props 添加详细的 JSDoc 文档说明。
+/** + * 终端工具组件的属性 + * @param {object} props + * @param {object} props.args - 命令参数 + * @param {string} props.args.command - 要执行的命令 + * @param {boolean} props.args.require_user_approval - 是否需要用户批准 + * @param {string} props.toolCallId - 工具调用ID + * @param {string} props.state - 组件状态 + * @param {string} props.result - 命令执行结果 + */ export const TerminalToolComponent = memo((props: IMCPServerToolComponentProps) => {
11-34: 建议增强类型安全性和错误处理
getResult函数的类型定义和错误处理可以进一步优化。+interface CommandResult { + content: Array<{ + type: string; + text: string; + }>; + isError?: boolean; +} + +interface ProcessedResult { + isError?: boolean; + text?: string; +} + -function getResult(raw: string) { +function getResult(raw: string): ProcessedResult | null { - const result: { - isError?: boolean; - text?: string; - } = {}; + const result: ProcessedResult = {}; try { - const data: { - content: { type: string; text: string }[]; - isError?: boolean; - } = JSON.parse(raw); + const data: CommandResult = JSON.parse(raw); if (data.isError) { result.isError = data.isError; } if (data.content) { result.text = data.content.map((item) => item.text).join('\n'); } return result; - } catch { + } catch (error) { + console.warn('Failed to parse command result:', error); return null; } }
56-86: 建议优化组件的可访问性和错误状态处理组件的可访问性和错误状态展示需要改进。
return ( - <div className={styles.run_cmd_tool}> + <div className={styles.run_cmd_tool} role="region" aria-label="终端命令执行"> {props.state === 'result' && ( <div> - <div className={styles.command_title}> + <div className={styles.command_title} role="heading" aria-level={2}> <Icon icon='terminal' /> <span>输出</span> </div> - {output ? <div className={styles.command_content}>{output.text}</div> : ''} + {output ? ( + <div className={styles.command_content} role="log"> + {output.text} + </div> + ) : ( + <div className={styles.error_state}>无输出内容</div> + )} </div> )} {props.state === 'complete' && args?.require_user_approval && ( <div> - <div className={styles.command_title}> + <div className={styles.command_title} role="heading" aria-level={2}> <Icon icon='terminal' /> <span>是否允许运行命令?</span> </div> - <p className={styles.command_content}>{args.command}</p> - <p className={styles.comand_description}>{args.explanation}</p> + <p className={styles.command_content} role="alert">{args.command}</p> + <p className={styles.command_description} role="alert">{args.explanation}</p> <div className={styles.cmmand_footer}> <Button type='link' size='small' disabled={disabled} + aria-label="允许执行命令" onClick={() => handleClick(true)} > 允许 </Button> <Button type='link' size='small' disabled={disabled} + aria-label="拒绝执行命令" onClick={() => handleClick(false)} > 不允许 </Button> </div> </div> )} </div> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/ai-native/src/browser/mcp/tools/components/Terminal.tsx(1 hunks)packages/ai-native/src/browser/mcp/tools/handlers/RunCommand.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: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
- GitHub Check: unittest (macos-latest, 18.x, node)
🔇 Additional comments (1)
packages/ai-native/src/browser/mcp/tools/handlers/RunCommand.ts (1)
107-114: 建议添加审批状态的内存管理此评论重复了之前的建议，关于在
approvalDeferredMap中添加清理机制以防止内存泄漏。建议在处理完审批后清理相关资源。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4398 +/- ##
==========================================
- Coverage 53.56% 53.55% -0.02%
==========================================
Files 1651 1652 +1
Lines 101866 101891 +25
Branches 22035 22046 +11
==========================================
Hits 54566 54566
- Misses 39351 39373 +22
- Partials 7949 7952 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/ai-native/src/browser/mcp/tools/components/Terminal.tsx(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/ai-native/src/browser/mcp/tools/components/Terminal.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: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: build-windows
- GitHub Check: build (macos-latest, 20.x)
🔇 Additional comments (2)
packages/i18n/src/common/zh-CN.lang.ts (1)
1327-1330: 翻译选择恰当！这些新增的本地化条目翻译简洁明了，符合现有的翻译风格。
packages/i18n/src/common/en-US.lang.ts (1)
1563-1568: 新增的本地化字符串看起来不错！这些字符串清晰简洁，并且与终端命令执行的用户审批功能相匹配。
Types
Background or solution
Kapture.2025-02-21.at.19.52.30.mp4
Changelog
Summary by CodeRabbit
新功能
样式更新
重构