feat: add tool execution engine with inline step visualization

[genni613]

User description

Summary

• 新增完整的工具执行引擎框架，包含注册、调度、执行、钩子四层架构
• 新增内联折叠式工具步骤可视化组件，替代原有 ExecutionCard 方案
• 支持 6 个内置工具：bash、read_file、write_file、edit_file、list_files、grep_files
• 对 phase1 已有功能完全无侵入，所有新代码位于独立模块中

Changes

• 调度层：ToolCallDispatcher 中心编排，支持并发批处理和限流
• 注册层：ToolRegistry + ToolSpecs，定义内置工具及其 schema
• 执行层：ToolExecutor 接口及 5 个执行器实现（Bash/Read/Write/Edit/Grep/List）
• 安全层：PermissionMode 三级权限、FileWriteLock 文件写锁、FileChangeReview 用户审批
• 扩展层：ToolHook 前/后置钩子机制 + ToolExecutionLogger 日志钩子
• 前端：ToolStepsBar 容器 + ToolStepRow 步骤行 + FileChangeInline 变更通知
• 设计文档：tool-step-visualization.md & unified-tool-desgin(部分)方案说明

Test plan

• 验证 6 个内置工具的基本功能（读取/写入/编辑/列出/搜索/执行命令）
• 验证权限控制：只读工具不需要审批，写入工具触发用户确认
• 验证并发写入同一文件时的文件锁机制
• 验证前端 ToolStepsBar 展开/折叠行为：运行中自动展开，完成后自动折叠
• 验证 ToolHook 前/后置钩子正常触发
• 验证 phase1 已有功能不受影响，回归测试通过

---

PR Type

Enhancement

---

Description

• Unify 15 bridge callbacks into single onEvent channel for Phase 2 messaging

• Add tool execution engine with 6 built-in tools (bash, read_file, write_file, edit_file, list_files, grep_files)

• Implement 4-layer architecture: ToolRegistry → ToolCallDispatcher → ToolExecutor → ToolHook

• Replace MessageBubble with AssistantGroup + message grouping for proper response ordering

---

Diagram Walkthrough

flowchart LR
  A[ToolRegistry] --> B[ToolCallDispatcher]
  B --> C[ToolExecutor]
  C --> D[ToolHook]
  E[BashExecutor] -.-> C
  F[ReadFileExecutor] -.-> C
  G[WriteFileExecutor] -.-> C
  H[EditFileExecutor] -.-> C
  I[ListFilesExecutor] -.-> C
  J[GrepFilesExecutor] -.-> C
  K[onEvent Channel] --> L[groupReducer]
  L --> M[AssistantGroup]

Loading

File Walkthrough

	Relevant files
Enhancement	22 files ToolCallDispatcher.kt Central tool dispatcher with full pipeline                              +489/-0  ToolSpecs.kt Define 6 built-in tool specifications                                        +226/-0  ToolRegistry.kt Central tool registry with lifecycle                                          +79/-0    ToolExecutor.kt Interface for all tool executors                                                  +12/-0    ToolResult.kt Unified result type for tools                                                        +29/-0    ToolContext.kt Execution context for tools                                                            +13/-0    ToolSpec.kt Tool registration metadata                                                              +21/-0    PermissionMode.kt Three-level permission enum                                                            +17/-0    BashExecutor.kt Bash/powershell command executor                                                  +122/-0  ReadFileExecutor.kt File reader with pagination                                                            +86/-0    WriteFileExecutor.kt Whole-file write with review                                                          +115/-0  EditFileExecutor.kt Precise text replacement executor                                                +156/-0  ListFilesExecutor.kt Directory listing executor                                                              +54/-0    GrepFilesExecutor.kt Pattern search executor                                                                    +92/-0    FileChangeReview.kt File change review dialogs                                                              +127/-0  FileWriteLock.kt Concurrent file write locking                                                        +27/-0    ToolHook.kt Hook interface for tools                                                                  +25/-0    ToolExecutionLogger.kt Logging hook implementation                                                            +25/-0    ChatService.kt Integrate unified tool dispatcher                                                +158/-43 BridgeHandler.kt Unify callbacks into onEvent channel                                          +107/-59 App.tsx Migrate to groupReducer state                                                        +76/-204 eventReducer.ts Unified event handler reducer                                                        +227/-0
Bug fix	1 files ExecutionResult.kt Add toToolResult conversion                                                            +15/-0
Configuration changes	1 files PluginSettings.kt Add unified tools settings                                                              +6/-1
Documentation	1 files bridge.d.ts Update Bridge type for onEvent                                                      +4/-15
Additional files	20 files tool-step-visualization.md +433/-0  unified-tool-design.md +1461/-209 2026-04-19-phase1-unified-event-channel-design.md +391/-0  2026-04-19-phase2-message-grouping-design.md +488/-0  InlineChangeHighlighter.kt +34/-0    PostEditPipeline.kt +35/-0    index.html +222/-70 package.json +1/-1      App.css +166/-0  AssistantGroup.tsx +81/-0    AssistantMarkdown.tsx +83/-0    FileChangeInline.tsx +40/-0    MessageBubble.tsx +6/-7      ToolStepRow.tsx +77/-0    ToolStepsBar.tsx +106/-0  groupReducer.ts +255/-0  useBridge.ts +13/-75  eventReducer.test.mjs +262/-0  groupReducer.test.mjs +436/-0  tsconfig.test.json +4/-2

---

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 29b7e31)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing bridgeNotifiedStart initialization The code uses bridgeNotifiedStart (lines 202, 228, 564, 566, 574, 576) as a mutable set but its declaration is not visible in the diff. If this was removed during refactoring or not properly initialized, it will cause a NullPointerException at runtime when msgId in bridgeNotifiedStart or bridgeNotifiedStart.add(msgId) is called. startStreamingRound(msgId, request, toolsEnabled = commandExecutionEnabled) } fun cancelStream() { val wasStreaming = activeMessageId != null activeStream?.cancel() activeStream = null val msgId = activeMessageId activeMessageId = null if (wasStreaming && msgId != null) { publishStatus() bridgeHandler?.notifyEnd(msgId) } } fun newChat() { activeStream?.cancel() activeStream = null activeMessageId = null truncationHandler.reset() resetToolCallState() session = ChatSession() pendingApprovals.values.forEach { it.complete(false) } pendingApprovals.clear() pendingApprovalCommands.clear() dispatcher.resetSession() sessionStore.clearSession() Unused project resolution In dispatchInternal method (lines 188-192), project is resolved but assigned to null immediately: val project = ...; null // Will be resolved from ToolContext construction in dispatch. This dead code should be removed - the context object is correctly built later in runWithFileLock using resolveProject(). val settings = PluginSettings.getInstance().getState() val project = (registry as? ToolRegistry)?.let { // Get project from registry — we need it from context null // Will be resolved from ToolContext construction in dispatch } Redundant project reference stored in registry At line 189, there's a cast registry as? ToolRegistry which is redundant since registry is already declared as ToolRegistry in the constructor at line 35. This suggests the code was refactored but the check was left behind. val project = (registry as? ToolRegistry)?.let { // Get project from registry — we need it from context null // Will be resolved from ToolContext construction in dispatch } Type error in toToolResult method At line 44-45, inside the buildString block, isNotEmpty() is called on this (the StringBuilder) but it should reference the variable holding accumulated output. This will always return true for an empty StringBuilder and may cause incorrect output formatting (extra newline before stderr). fun toToolResult(): ToolResult = when (this) { is Success -> ToolResult(ok = true, output = buildString { if (stdout.isNotEmpty()) append(stdout) if (stderr.isNotEmpty()) { if (isNotEmpty()) append("\n") append(stderr) } }.ifEmpty { "Command completed with exit code $exitCode" }) is Failed -> ToolResult(ok = false, output = stderr.ifEmpty { "Command failed with exit code $exitCode" }) is Blocked -> ToolResult(ok = false, output = reason) is Denied -> ToolResult(ok = false, output = reason) is TimedOut -> ToolResult(ok = false, output = "Command timed out after ${timeoutSeconds}s") }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix race condition in concurrent batch error tracking The hasBashError flag is accessed and modified inside concurrent async coroutines without proper synchronization, causing a race condition. Use an atomic reference like AtomicBoolean or move the logic to execute sequentially to ensure correct batch failure propagation. src/main/kotlin/com/github/codeplangui/execution/ToolCallDispatcher.kt [136-156] if (batch.isConcurrencySafe && batch.entries.size > 1) { // Concurrent batch - var hasBashError = false + val hasBashError = java.util.concurrent.atomic.AtomicBoolean(false) val batchResults = coroutineScope { batch.entries.map { (index, call) -> async { - if (hasBashError && isBashCommand(call.name)) { + if (hasBashError.get() && isBashCommand(call.name)) { index to (call to ToolResult( ok = false, output = "Skipped: previous bash command in batch failed" )) } else { val result = dispatch(call.name, call.arguments, msgId, bridgeHandler) if (!result.ok && isBashCommand(call.name)) { - hasBashError = true + hasBashError.set(true) } index to (call to result) } } }.awaitAll() } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a real race condition - hasBashError is a regular var that's accessed and modified from multiple concurrent coroutines without synchronization. Using AtomicBoolean is the proper fix for this concurrent access pattern.	High
	Remove redundant JSON.parse on already-parsed diffStats Since useBridge.ts already parses the entire event payload as JSON before passing it to the reducer (line 80: const payload = JSON.parse(payloadJson)), the payload.diffStats will already be a parsed object, not a string. Attempting to parse it again with JSON.parse() would fail or cause unexpected behavior. Remove the unnecessary JSON.parse call. webview/src/eventReducer.ts [216] -diffStats: payload.diffStats ? JSON.parse(payload.diffStats) : undefined, +diffStats: payload.diffStats, Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that useBridge.ts parses the entire event payload as JSON before passing it to the reducer (line 28: const payload = JSON.parse(payloadJson)), so payload.diffStats is already a parsed object. The JSON.parse(payload.diffStats) call is redundant and would cause issues since it's already an object. This is a valid bug fix that improves correctness.	Medium
General	Improve path traversal detection with case-insensitive matching The early path traversal check at line 290 uses command.contains("../") which is case-sensitive, but path traversal can use encoded variants like ..%2F or mixed case. Additionally, the check doesn't align with the later check at line 315 which uses input["path"]?.jsonPrimitive?.contentOrNull. Consider making the check case-insensitive and handling URL-encoded variants. src/main/kotlin/com/github/codeplangui/execution/ToolCallDispatcher.kt [358-362] -// Path traversal - if (command.contains("../") || command.contains("..\\")) return "Path traversal detected" +// Path traversal (case-insensitive and handle URL encoding) + val normalizedCmd = command.lowercase() + if (normalizedCmd.contains("../") || normalizedCmd.contains("..\\") || + normalizedCmd.contains("..%2f") || normalizedCmd.contains("..%2f")) { + return "Path traversal detected" + } Suggestion importance[1-10]: 4 __ Why: The suggestion has a valid point about case sensitivity, but overstates the issue. The existing checkDenyRulesEarly is used as a pre-check, and the later authorization check at line 340 already uses case-sensitive contains. Additionally, URL-encoded path traversal (..%2F) is an uncommon vector in shell commands. This is a minor hardening suggestion rather than a critical fix.	Low

[genni613]

/review

[github-actions]

Persistent review updated to latest commit 29b7e31