Skip to content

feat(tools): phase2 tool system skeleton (M1)#28

Merged
genni613 merged 1 commit into
phase2from
feat/tools-m1-skeleton
Apr 21, 2026
Merged

feat(tools): phase2 tool system skeleton (M1)#28
genni613 merged 1 commit into
phase2from
feat/tools-m1-skeleton

Conversation

@TuYv

@TuYv TuYv commented Apr 21, 2026

Copy link
Copy Markdown
Owner

User description

Summary

Phase2 M1 — the skeleton of the new tool abstraction layer, running in parallel to existing command-mode. No UI wiring, command-mode completely untouched. Cutover is planned for M5.

What's new (under com.github.codeplangui.tools):

  • Tool.kt — generic Tool<Input, Output> interface (4 required methods + defaults for ~10 more)
  • ToolTypes.ktValidationResult / PermissionResult (Allow/Ask/Deny) / ToolResult / Progress / ToolUpdate
  • ToolExecutionContext.kt — narrow runtime context (4 fields; Claude Code's analog has 30+)
  • ToolBuilder.kttool { } DSL (matches Claude Code's buildTool() factory)
  • ToolExecutor.ktrunToolUse returning Flow<ToolUpdate> (Claude Code's entry is an AsyncGenerator)
  • bash/BashTool.kt — first concrete tool, wraps existing CommandExecutionService, includes destructive-command heuristics

Docs (docs/phase2-*):

  • phase2-tools-refactor-brief.md — why (architecture-driven, not user-pain-driven)
  • phase2-tools-design-notes.md — how (Kotlin interface ↔ Claude Code mapping)
  • phase2-mvp-tool-specs.md — what (4 MVP tools: Bash / FileRead / FileEdit / MCPProxy)
  • phase2-milestone-1-summary.md — M1 delivery record + M2 handoff notes

Test plan

  • ./gradlew compileKotlin passes (Corretto 17 — Java 25 crashes the Kotlin compiler, see ~/.claude/projects/.../memory/project_java_home.md)
  • ./gradlew test --tests ToolExecutorTest passes — 7 cases covering happy path, progress forwarding, and validate/permission/execute/parse failure branches
  • Reviewer sanity-check: read phase2-milestone-1-summary.md §"锁定的设计选择" and confirm the trade-offs
  • No regression in existing tests (CI will verify)

Explicitly NOT in scope

  • UI wiring — ChatPanel / BridgeHandler still route through command-mode
  • FileReadTool / FileEditTool / MCPProxyTool → M2-M4
  • Real user-prompt for PermissionResult.Ask (currently auto-approved as placeholder) → M3
  • Parallel batch execution (runToolUseBatch) → M2
  • Tool registry / assembleToolPool equivalent → M2

Relation to existing code

Zero-touch. execution/ package (command-mode) is not modified. BashTool consumes CommandExecutionService via its existing public API; no changes to the service itself.

References

  • Claude Code v2.1.88 reverse-engineered source (for reviewers who want to cross-reference): ~/SourceLib/fishNotExist/claude-code-source/src/Tool.ts, src/tools.ts, src/services/tools/toolExecution.ts

PR Type

Enhancement, Tests


Description

  • Introduce Tool<Input, Output> interface with generic types, 4 required methods (parse/call/mapResult/checkPermissions), and default methods for metadata predicates

  • Add ToolBuilder DSL (tool { } function) for declarative tool construction matching Claude Code's buildTool() pattern

  • Implement Flow-based ToolExecutor (runToolUse returning Flow) for streaming execution instead of suspend functions

  • Create BashTool as first concrete tool wrapping existing CommandExecutionService with destructive-command heuristics

  • Add 7 test cases covering happy path, progress forwarding, validation/permission/execute/parse failure branches


Diagram Walkthrough

flowchart LR
  A[LLM tool_use] --> B[runToolUse Flow]
  B --> C{Parse Input}
  C -->|OK| D{Validate Input}
  C -->|Fail| E[Failed PARSE]
  D -->|OK| F{Check Permissions}
  D -->|Fail| G[Failed VALIDATE]
  F -->|Allow| H[Tool.call]
  F -->|Ask| I[PermissionAsked]
  F -->|Deny| J[Failed PERMISSION]
  I -->|auto-approve| H
  H --> K[Progress events]
  H --> L[ToolResult]
  L --> M[mapResultToApiBlock]
  M --> N[Completed]
Loading

File Walkthrough

Relevant files
Enhancement
6 files
Tool.kt
Define generic Tool interface with lifecycle methods         
+72/-0   
ToolBuilder.kt
Create DSL builder for tool construction                                 
+106/-0 
ToolExecutionContext.kt
Define runtime execution context data class                           
+41/-0   
ToolExecutor.kt
Implement Flow-based tool execution engine                             
+92/-0   
ToolTypes.kt
Define sealed types for validation/permission/results       
+107/-0 
BashTool.kt
Implement first concrete BashTool wrapping service             
+206/-0 
Tests
1 files
ToolExecutorTest.kt
Add 7 test cases for executor flow                                             
+173/-0 
Documentation
4 files
phase2-milestone-1-summary.md
Document M1 delivery record and design choices                     
+77/-0   
phase2-mvp-tool-specs.md
Specify MVP tool requirements and behavior                             
+299/-0 
phase2-tools-design-notes.md
Record architecture design decisions and rationale             
+447/-0 
phase2-tools-refactor-brief.md
Outline refactor motivation and scope                                       
+116/-0 

Introduces a new tool abstraction that runs in parallel to existing
command-mode. M1 is skeleton-only: Tool interface + builder DSL +
Flow-based executor + BashTool wrapping CommandExecutionService.
No UI wiring; command-mode untouched. Cutover planned for M5.

Tests: 7 ToolExecutorTest cases covering happy path, progress
forwarding, and validate/permission/execute/parse failure branches.

Docs:
- phase2-tools-refactor-brief.md  — why (architecture-driven, not user-pain)
- phase2-tools-design-notes.md    — how (Kotlin interface <-> Claude Code map)
- phase2-mvp-tool-specs.md        — what (4 MVP tools across 3 families)
- phase2-milestone-1-summary.md   — M1 delivery record
@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Verify userFacingName default fallback is intentional

The lambda signature is correct, but when not set, the fallback defaults to { _ ->
finalName } which ignores the input. Verify that this behavior is intentional - the
interface allows tools to return different names based on input, but the default
ignores it entirely.

src/main/kotlin/com/github/codeplangui/tools/ToolBuilder.kt [58]

+fun userFacingName(block: (Input?) -> String) { userFacingNameFn = block }
 
-
Suggestion importance[1-10]: 3

__

Why: This is a verification request noting that the default fallback { _ -> finalName } ignores the input parameter entirely, which aligns with the interface default in Tool.kt (line 63: fun userFacingName(input: Input?): String = name). The behavior is consistent but offers a minor maintainability improvement by documenting the design intent.

Low

@genni613 genni613 merged commit d52d810 into phase2 Apr 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants