-
Notifications
You must be signed in to change notification settings - Fork 657
feat: agentic tool op #1307
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
Merged
Merged
feat: agentic tool op #1307
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
6138354
feat: add edit_file tool
zerob13 13a4a60
feat(tool-permission): add batch permission pre-check for concurrent …
zerob13 e119497
\"feat(agent): batch tool permission and session auto-execute\"
zerob13 835e0b7
feat(agent): improve tool descriptions
zerob13 0dd8827
feat: permission loop
zerob13 4d3c244
refactor(agent): permission flow stabilization
zerob13 f1a6aab
feat: refactor permission system to preserve extended payload fields
zerob13 8678b30
fix(agent): harden permission resume flow
zerob13 c728dcb
fix(agent): flush resumed tool updates
zerob13 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,228 @@ | ||
| # edit_file Tool Implementation Plan | ||
|
|
||
| ## Architecture Overview | ||
|
|
||
| The `edit_file` tool will be integrated into the existing agent filesystem tool infrastructure, following the established three-layer architecture: | ||
|
|
||
| ``` | ||
| ┌─────────────────────────────────────────────────────────────┐ | ||
| │ Layer 1: Tool Definition (agentToolManager.ts) │ | ||
| │ - Zod schema for parameter validation │ | ||
| │ - Tool metadata (name, description, parameters) │ | ||
| │ - Registration in filesystem tool registry │ | ||
| └─────────────────────────────────────────────────────────────┘ | ||
| │ | ||
| ▼ | ||
| ┌─────────────────────────────────────────────────────────────┐ | ||
| │ Layer 2: Tool Routing (agentToolManager.ts) │ | ||
| │ - Parameter normalization (alias handling) │ | ||
| │ - Permission checks (assertWritePermission) │ | ||
| │ - Dispatch to filesystem handler │ | ||
| └─────────────────────────────────────────────────────────────┘ | ||
| │ | ||
| ▼ | ||
| ┌─────────────────────────────────────────────────────────────┐ | ||
| │ Layer 3: File Operation (agentFileSystemHandler.ts) │ | ||
| │ - Path validation and resolution │ | ||
| │ - File content reading │ | ||
| │ - Exact text replacement │ | ||
| │ - Diff generation and response formatting │ | ||
| └─────────────────────────────────────────────────────────────┘ | ||
| ``` | ||
|
|
||
| ## Design Decisions | ||
|
|
||
| ### 1. Parameter Alias Handling | ||
|
|
||
| **Decision**: Normalize parameter names at the routing layer (agentToolManager.callFileSystemTool) rather than in Zod schema. | ||
|
|
||
| **Rationale**: | ||
| - Zod does not natively support parameter aliases | ||
| - Normalizing early allows schema to use canonical names | ||
| - Keeps handler implementation clean | ||
|
|
||
| **Implementation**: | ||
| ```typescript | ||
| // Normalize parameter aliases before schema validation | ||
| if (toolName === 'edit_file') { | ||
| args = { | ||
| ...args, | ||
| path: args.path ?? args.file_path, | ||
| oldText: args.oldText ?? args.old_string, | ||
| newText: args.newText ?? args.new_string, | ||
| base_directory: args.base_directory, | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### 2. Text Matching Strategy | ||
|
|
||
| **Decision**: Case-sensitive exact string matching, replace ALL occurrences. | ||
|
|
||
| **Rationale**: | ||
| - Consistent with `edit_text` tool's `edit_lines` operation | ||
| - Simple mental model for LLMs: "find this exact text, replace with that" | ||
| - Replacing all occurrences prevents partial updates which could leave code in inconsistent state | ||
|
|
||
| ### 3. Response Format | ||
|
|
||
| **Decision**: JSON response with diff preview, matching existing filesystem tools. | ||
|
|
||
| **Structure**: | ||
| ```typescript | ||
| interface EditFileSuccessResponse { | ||
| success: true | ||
| originalCode: string // Truncated for large files | ||
| updatedCode: string // Truncated for large files | ||
| language: string // Detected from file extension | ||
| replacements: number // Number of replacements made | ||
| } | ||
|
|
||
| interface EditFileErrorResponse { | ||
| success: false | ||
| error: string | ||
| } | ||
| ``` | ||
|
|
||
| ## File Changes | ||
|
|
||
| ### Modified Files | ||
|
|
||
| | File | Purpose | Lines Added/Modified | | ||
| |------|---------|----------------------| | ||
| | `src/main/presenter/agentPresenter/acp/agentToolManager.ts` | Schema, definition, routing | ~80 lines | | ||
| | `src/main/presenter/agentPresenter/acp/agentFileSystemHandler.ts` | Handler implementation | ~50 lines | | ||
|
|
||
| ### No New Files Required | ||
|
|
||
| The implementation integrates into existing infrastructure without creating new files. | ||
|
|
||
| ## Implementation Details | ||
|
|
||
| ### Schema Definition (agentToolManager.ts) | ||
|
|
||
| ```typescript | ||
| edit_file: z.object({ | ||
| path: z.string().describe('Path to the file to edit'), | ||
| oldText: z | ||
| .string() | ||
| .max(10000) | ||
| .describe('The exact text to find and replace (case-sensitive)'), | ||
| newText: z.string().max(10000).describe('The replacement text'), | ||
| base_directory: z | ||
| .string() | ||
| .optional() | ||
| .describe('Base directory for resolving relative paths') | ||
| }) | ||
| ``` | ||
|
|
||
| ### Tool Definition | ||
|
|
||
| ```typescript | ||
| { | ||
| type: 'function', | ||
| function: { | ||
| name: 'edit_file', | ||
| description: | ||
| 'Make precise edits to files by replacing exact text strings. Use this for simple text replacements when you know the exact content to replace. For regex or complex operations, use edit_text instead.', | ||
| parameters: zodToJsonSchema(schemas.edit_file) | ||
| }, | ||
| server: { | ||
| name: 'agent-filesystem', | ||
| icons: '📁', | ||
| description: 'Agent FileSystem tools' | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Handler Implementation (agentFileSystemHandler.ts) | ||
|
|
||
| ```typescript | ||
| async editFile(args: unknown, baseDirectory?: string): Promise<string> { | ||
| const parsed = EditFileArgsSchema.safeParse(args) | ||
| if (!parsed.success) { | ||
| throw new Error(`Invalid arguments: ${parsed.error}`) | ||
| } | ||
|
|
||
| const { path: filePath, oldText, newText } = parsed.data | ||
| const validPath = await this.validatePath(filePath, baseDirectory) | ||
|
|
||
| const content = await fs.readFile(validPath, 'utf-8') | ||
|
|
||
| if (!content.includes(oldText)) { | ||
| throw new Error(`Cannot find the specified text to replace. The exact text was not found in the file.`) | ||
| } | ||
|
|
||
| let replacementCount = 0 | ||
| const modifiedContent = content.replaceAll(oldText, () => { | ||
| replacementCount++ | ||
| return newText | ||
| }) | ||
|
|
||
| await fs.writeFile(validPath, modifiedContent, 'utf-8') | ||
|
|
||
| const { originalCode, updatedCode } = this.buildTruncatedDiff(content, modifiedContent, 3) | ||
| const language = getLanguageFromFilename(validPath) | ||
|
|
||
| return JSON.stringify({ | ||
| success: true, | ||
| originalCode, | ||
| updatedCode, | ||
| language, | ||
| replacements: replacementCount | ||
| }) | ||
| } | ||
| ``` | ||
|
|
||
| ## Test Strategy | ||
|
|
||
| ### Unit Tests (test/main/presenter/agentPresenter/acp/) | ||
|
|
||
| Create `agentFileSystemHandler.editFile.test.ts`: | ||
|
|
||
| - **Happy Path**: Replace single occurrence, replace multiple occurrences | ||
| - **Error Cases**: File not found, oldText not found, path outside allowed directories | ||
| - **Edge Cases**: Empty oldText, empty newText, large text content | ||
|
|
||
| ### Integration Tests | ||
|
|
||
| - Verify tool registration and routing | ||
| - Verify permission system integration | ||
| - Verify response format consistency | ||
|
|
||
| ## Security Considerations | ||
|
|
||
| - Path traversal prevention via existing `validatePath()` method | ||
| - Write permission enforcement via `assertWritePermission()` | ||
| - Allowed directory restriction via `isPathAllowed()` check | ||
| - Maximum text length limits (10,000 chars for oldText/newText) | ||
|
|
||
| ## Migration & Compatibility | ||
|
|
||
| - No breaking changes to existing tools | ||
| - New tool is additive only | ||
| - No database or configuration migrations required | ||
| - No impact on existing agent workflows | ||
|
|
||
| ## Rollback Plan | ||
|
|
||
| If issues are discovered: | ||
| 1. Remove tool from `isFileSystemTool()` list to disable | ||
| 2. Remove tool definition from `getFileSystemToolDefinitions()` | ||
| 3. No data changes to revert (file changes are atomic writes) | ||
|
|
||
| ## Performance Considerations | ||
|
|
||
| - File reads are limited by available memory | ||
| - `replaceAll()` is O(n*m) where n=content length, m=pattern length | ||
| - For very large files (>1MB), consider warning or limiting | ||
| - Diff truncation limits output size for large changes | ||
|
|
||
| ## Success Metrics | ||
|
|
||
| - Tool is available in agent tool list | ||
| - Tool accepts all parameter variants (camelCase and snake_case) | ||
| - Tool successfully edits files with exact text matching | ||
| - Tool returns proper error messages for invalid inputs | ||
| - All tests pass | ||
| - Lint and typecheck pass | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| # edit_file Tool Specification | ||
|
|
||
| ## Overview | ||
|
|
||
| Add a new `edit_file` tool to the agent filesystem tools that enables AI agents to make precise text-based edits to files. This tool provides a simplified interface for exact string replacement, complementing the existing `edit_text` tool which supports regex and line-based operations. | ||
|
|
||
| ## User Stories | ||
|
|
||
| ### Primary User Story | ||
|
|
||
| As an AI agent interacting with DeepChat, I want to edit specific portions of a file using exact text matching so that I can make precise modifications without worrying about regex escaping or complex operations. | ||
|
|
||
| ### Use Cases | ||
|
|
||
| 1. **Simple Text Replacement** - Replace a specific function implementation with an updated version | ||
| 2. **Configuration Updates** - Modify specific configuration values in config files | ||
| 3. **Bug Fixes** - Replace buggy code snippets with corrected versions | ||
| 4. **Refactoring** - Update variable names or function calls across files | ||
|
|
||
| ## Acceptance Criteria | ||
|
|
||
| ### Functional Requirements | ||
|
|
||
| - [ ] Tool accepts `path` (or `file_path`) parameter for target file | ||
| - [ ] Tool accepts `oldText` (or `old_string`) parameter for text to find | ||
| - [ ] Tool accepts `newText` (or `new_string`) parameter for replacement text | ||
| - [ ] Tool optionally accepts `base_directory` for resolving relative paths | ||
| - [ ] Tool performs exact string matching (case-sensitive) | ||
| - [ ] Tool replaces ALL occurrences of oldText when multiple matches exist | ||
| - [ ] Tool returns a JSON response with diff preview (original vs updated) | ||
| - [ ] Tool returns clear error message when oldText is not found | ||
| - [ ] Tool requires write permission before modifying files | ||
| - [ ] Tool validates paths are within allowed directories | ||
|
|
||
| ### Non-Functional Requirements | ||
|
|
||
| - [ ] Tool follows existing filesystem tool patterns (schema, handler, registration) | ||
| - [ ] Tool response format is consistent with other filesystem tools | ||
| - [ ] Error messages are user-friendly and actionable | ||
| - [ ] Tool is registered under `agent-filesystem` server namespace | ||
|
|
||
| ## Non-Goals | ||
|
|
||
| - Support for regex patterns (use `edit_text` or `text_replace` instead) | ||
| - Support for partial/line-based matching (use `edit_text` with `edit_lines` instead) | ||
| - Support for dry-run mode (can be added in future if needed) | ||
| - Support for multiple file editing in single call | ||
| - Support for backup creation (handled at system level if needed) | ||
|
|
||
| ## Constraints | ||
|
|
||
| - Maximum text length for `oldText` and `newText` should be reasonable (suggest 10,000 chars) | ||
| - Path resolution follows existing `base_directory` pattern | ||
| - Must integrate with existing permission system for write operations | ||
|
|
||
| ## Open Questions | ||
|
|
||
| None. All clarifications resolved: | ||
|
|
||
| - **Parameter naming**: Support both camelCase (`path`, `oldText`, `newText`) and snake_case (`file_path`, `old_string`, `new_string`) variants for LLM compatibility | ||
| - **Multiple matches**: Replace all occurrences (not just first) to match common editing expectations | ||
| - **Case sensitivity**: Exact matching is case-sensitive (consistent with `edit_text` behavior) | ||
|
|
||
| ## UI/UX Considerations | ||
|
|
||
| - Tool icon should match other filesystem tools (📁) | ||
| - Tool description: "Make precise edits to files by replacing exact text strings" | ||
| - Error display should highlight what text was not found | ||
|
|
||
| ## Related Tools | ||
|
|
||
| | Tool | Purpose | When to Use | | ||
| |------|---------|-------------| | ||
| | `write_file` | Overwrite entire file | Creating new files or full rewrites | | ||
| | `edit_text` | Pattern/line-based editing | Regex replacement or complex edits | | ||
| | `text_replace` | Regex-based replacement | Pattern-based text replacement | | ||
| | `edit_file` | Exact string replacement | Simple, precise text modifications | |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.