From 382b7f89cc32be33fda2b5c06c9dcb017fa26278 Mon Sep 17 00:00:00 2001 From: Kujtim Hoxha Date: Sat, 14 Jun 2025 13:33:09 +0200 Subject: [PATCH] fix: remove broken patch tool --- internal/exp/diffview/diffview.go | 7 +- internal/llm/agent/tools.go | 1 - internal/llm/tools/patch.go | 372 ------------------ .../tui/components/chat/messages/renderer.go | 35 -- .../dialogs/permissions/permissions.go | 16 - internal/tui/tui.go | 11 - 6 files changed, 1 insertion(+), 441 deletions(-) delete mode 100644 internal/llm/tools/patch.go diff --git a/internal/exp/diffview/diffview.go b/internal/exp/diffview/diffview.go index 37c126513bff31efd2409902aecc3720ff6a1346..92dca75aaf59f26b959ad12506a6d7f414b80c98 100644 --- a/internal/exp/diffview/diffview.go +++ b/internal/exp/diffview/diffview.go @@ -3,7 +3,6 @@ package diffview import ( "fmt" "image/color" - "os" "strconv" "strings" @@ -70,11 +69,7 @@ func New() *DiffView { lineNumbers: true, tabWidth: 8, } - if lipgloss.HasDarkBackground(os.Stdin, os.Stdout) { - dv.style = DefaultDarkStyle - } else { - dv.style = DefaultLightStyle - } + dv.style = DefaultDarkStyle return dv } diff --git a/internal/llm/agent/tools.go b/internal/llm/agent/tools.go index 763f53ea6f2246f2acae3f8c2907abf8be34a1d0..0fe2c530ca6dc30916fd2dfa094ad6303bf39443 100644 --- a/internal/llm/agent/tools.go +++ b/internal/llm/agent/tools.go @@ -33,7 +33,6 @@ func CoderAgentTools( tools.NewLsTool(), tools.NewSourcegraphTool(), tools.NewViewTool(lspClients), - tools.NewPatchTool(lspClients, permissions, history), tools.NewWriteTool(lspClients, permissions, history), NewAgentTool(sessions, messages, lspClients), }, otherTools..., diff --git a/internal/llm/tools/patch.go b/internal/llm/tools/patch.go deleted file mode 100644 index f66017e25cd647190421eda40c5628b24bd1b58c..0000000000000000000000000000000000000000 --- a/internal/llm/tools/patch.go +++ /dev/null @@ -1,372 +0,0 @@ -package tools - -import ( - "context" - "encoding/json" - "fmt" - "os" - "path/filepath" - "time" - - "github.com/charmbracelet/crush/internal/config" - "github.com/charmbracelet/crush/internal/diff" - "github.com/charmbracelet/crush/internal/history" - "github.com/charmbracelet/crush/internal/logging" - "github.com/charmbracelet/crush/internal/lsp" - "github.com/charmbracelet/crush/internal/permission" -) - -type PatchParams struct { - PatchText string `json:"patch_text"` -} - -type PatchResponseMetadata struct { - FilesChanged []string `json:"files_changed"` - Additions int `json:"additions"` - Removals int `json:"removals"` -} - -type patchTool struct { - lspClients map[string]*lsp.Client - permissions permission.Service - files history.Service -} - -const ( - PatchToolName = "patch" - patchDescription = `Applies a patch to multiple files in one operation. This tool is useful for making coordinated changes across multiple files. - -The patch text must follow this format: -*** Begin Patch -*** Update File: /path/to/file -@@ Context line (unique within the file) - Line to keep --Line to remove -+Line to add - Line to keep -*** Add File: /path/to/new/file -+Content of the new file -+More content -*** Delete File: /path/to/file/to/delete -*** End Patch - -Before using this tool: -1. Use the FileRead tool to understand the files' contents and context -2. Verify all file paths are correct (use the LS tool) - -CRITICAL REQUIREMENTS FOR USING THIS TOOL: - -1. UNIQUENESS: Context lines MUST uniquely identify the specific sections you want to change -2. PRECISION: All whitespace, indentation, and surrounding code must match exactly -3. VALIDATION: Ensure edits result in idiomatic, correct code -4. PATHS: Always use absolute file paths (starting with /) - -The tool will apply all changes in a single atomic operation.` -) - -func NewPatchTool(lspClients map[string]*lsp.Client, permissions permission.Service, files history.Service) BaseTool { - return &patchTool{ - lspClients: lspClients, - permissions: permissions, - files: files, - } -} - -func (p *patchTool) Info() ToolInfo { - return ToolInfo{ - Name: PatchToolName, - Description: patchDescription, - Parameters: map[string]any{ - "patch_text": map[string]any{ - "type": "string", - "description": "The full patch text that describes all changes to be made", - }, - }, - Required: []string{"patch_text"}, - } -} - -func (p *patchTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error) { - var params PatchParams - if err := json.Unmarshal([]byte(call.Input), ¶ms); err != nil { - return NewTextErrorResponse("invalid parameters"), nil - } - - if params.PatchText == "" { - return NewTextErrorResponse("patch_text is required"), nil - } - - // Identify all files needed for the patch and verify they've been read - filesToRead := diff.IdentifyFilesNeeded(params.PatchText) - for _, filePath := range filesToRead { - absPath := filePath - if !filepath.IsAbs(absPath) { - wd := config.WorkingDirectory() - absPath = filepath.Join(wd, absPath) - } - - if getLastReadTime(absPath).IsZero() { - return NewTextErrorResponse(fmt.Sprintf("you must read the file %s before patching it. Use the FileRead tool first", filePath)), nil - } - - fileInfo, err := os.Stat(absPath) - if err != nil { - if os.IsNotExist(err) { - return NewTextErrorResponse(fmt.Sprintf("file not found: %s", absPath)), nil - } - return ToolResponse{}, fmt.Errorf("failed to access file: %w", err) - } - - if fileInfo.IsDir() { - return NewTextErrorResponse(fmt.Sprintf("path is a directory, not a file: %s", absPath)), nil - } - - modTime := fileInfo.ModTime() - lastRead := getLastReadTime(absPath) - if modTime.After(lastRead) { - return NewTextErrorResponse( - fmt.Sprintf("file %s has been modified since it was last read (mod time: %s, last read: %s)", - absPath, modTime.Format(time.RFC3339), lastRead.Format(time.RFC3339), - )), nil - } - } - - // Check for new files to ensure they don't already exist - filesToAdd := diff.IdentifyFilesAdded(params.PatchText) - for _, filePath := range filesToAdd { - absPath := filePath - if !filepath.IsAbs(absPath) { - wd := config.WorkingDirectory() - absPath = filepath.Join(wd, absPath) - } - - _, err := os.Stat(absPath) - if err == nil { - return NewTextErrorResponse(fmt.Sprintf("file already exists and cannot be added: %s", absPath)), nil - } else if !os.IsNotExist(err) { - return ToolResponse{}, fmt.Errorf("failed to check file: %w", err) - } - } - - // Load all required files - currentFiles := make(map[string]string) - for _, filePath := range filesToRead { - absPath := filePath - if !filepath.IsAbs(absPath) { - wd := config.WorkingDirectory() - absPath = filepath.Join(wd, absPath) - } - - content, err := os.ReadFile(absPath) - if err != nil { - return ToolResponse{}, fmt.Errorf("failed to read file %s: %w", absPath, err) - } - currentFiles[filePath] = string(content) - } - - // Process the patch - patch, fuzz, err := diff.TextToPatch(params.PatchText, currentFiles) - if err != nil { - return NewTextErrorResponse(fmt.Sprintf("failed to parse patch: %s", err)), nil - } - - if fuzz > 3 { - return NewTextErrorResponse(fmt.Sprintf("patch contains fuzzy matches (fuzz level: %d). Please make your context lines more precise", fuzz)), nil - } - - // Convert patch to commit - commit, err := diff.PatchToCommit(patch, currentFiles) - if err != nil { - return NewTextErrorResponse(fmt.Sprintf("failed to create commit from patch: %s", err)), nil - } - - // Get session ID and message ID - sessionID, messageID := GetContextValues(ctx) - if sessionID == "" || messageID == "" { - return ToolResponse{}, fmt.Errorf("session ID and message ID are required for creating a patch") - } - - // Request permission for all changes - for path, change := range commit.Changes { - switch change.Type { - case diff.ActionAdd: - dir := filepath.Dir(path) - patchDiff, _, _ := diff.GenerateDiff("", *change.NewContent, path) - p := p.permissions.Request( - permission.CreatePermissionRequest{ - SessionID: sessionID, - Path: dir, - ToolName: PatchToolName, - Action: "create", - Description: fmt.Sprintf("Create file %s", path), - Params: EditPermissionsParams{ - FilePath: path, - Diff: patchDiff, - }, - }, - ) - if !p { - return ToolResponse{}, permission.ErrorPermissionDenied - } - case diff.ActionUpdate: - currentContent := "" - if change.OldContent != nil { - currentContent = *change.OldContent - } - newContent := "" - if change.NewContent != nil { - newContent = *change.NewContent - } - patchDiff, _, _ := diff.GenerateDiff(currentContent, newContent, path) - dir := filepath.Dir(path) - p := p.permissions.Request( - permission.CreatePermissionRequest{ - SessionID: sessionID, - Path: dir, - ToolName: PatchToolName, - Action: "update", - Description: fmt.Sprintf("Update file %s", path), - Params: EditPermissionsParams{ - FilePath: path, - Diff: patchDiff, - }, - }, - ) - if !p { - return ToolResponse{}, permission.ErrorPermissionDenied - } - case diff.ActionDelete: - dir := filepath.Dir(path) - patchDiff, _, _ := diff.GenerateDiff(*change.OldContent, "", path) - p := p.permissions.Request( - permission.CreatePermissionRequest{ - SessionID: sessionID, - Path: dir, - ToolName: PatchToolName, - Action: "delete", - Description: fmt.Sprintf("Delete file %s", path), - Params: EditPermissionsParams{ - FilePath: path, - Diff: patchDiff, - }, - }, - ) - if !p { - return ToolResponse{}, permission.ErrorPermissionDenied - } - } - } - - // Apply the changes to the filesystem - err = diff.ApplyCommit(commit, func(path string, content string) error { - absPath := path - if !filepath.IsAbs(absPath) { - wd := config.WorkingDirectory() - absPath = filepath.Join(wd, absPath) - } - - // Create parent directories if needed - dir := filepath.Dir(absPath) - if err := os.MkdirAll(dir, 0o755); err != nil { - return fmt.Errorf("failed to create parent directories for %s: %w", absPath, err) - } - - return os.WriteFile(absPath, []byte(content), 0o644) - }, func(path string) error { - absPath := path - if !filepath.IsAbs(absPath) { - wd := config.WorkingDirectory() - absPath = filepath.Join(wd, absPath) - } - return os.Remove(absPath) - }) - if err != nil { - return NewTextErrorResponse(fmt.Sprintf("failed to apply patch: %s", err)), nil - } - - // Update file history for all modified files - changedFiles := []string{} - totalAdditions := 0 - totalRemovals := 0 - - for path, change := range commit.Changes { - absPath := path - if !filepath.IsAbs(absPath) { - wd := config.WorkingDirectory() - absPath = filepath.Join(wd, absPath) - } - changedFiles = append(changedFiles, absPath) - - oldContent := "" - if change.OldContent != nil { - oldContent = *change.OldContent - } - - newContent := "" - if change.NewContent != nil { - newContent = *change.NewContent - } - - // Calculate diff statistics - _, additions, removals := diff.GenerateDiff(oldContent, newContent, path) - totalAdditions += additions - totalRemovals += removals - - // Update history - file, err := p.files.GetByPathAndSession(ctx, absPath, sessionID) - if err != nil && change.Type != diff.ActionAdd { - // If not adding a file, create history entry for existing file - _, err = p.files.Create(ctx, sessionID, absPath, oldContent) - if err != nil { - logging.Debug("Error creating file history", "error", err) - } - } - - if err == nil && change.Type != diff.ActionAdd && file.Content != oldContent { - // User manually changed content, store intermediate version - _, err = p.files.CreateVersion(ctx, sessionID, absPath, oldContent) - if err != nil { - logging.Debug("Error creating file history version", "error", err) - } - } - - // Store new version - if change.Type == diff.ActionDelete { - _, err = p.files.CreateVersion(ctx, sessionID, absPath, "") - } else { - _, err = p.files.CreateVersion(ctx, sessionID, absPath, newContent) - } - if err != nil { - logging.Debug("Error creating file history version", "error", err) - } - - // Record file operations - recordFileWrite(absPath) - recordFileRead(absPath) - } - - // Run LSP diagnostics on all changed files - for _, filePath := range changedFiles { - waitForLspDiagnostics(ctx, filePath, p.lspClients) - } - - result := fmt.Sprintf("Patch applied successfully. %d files changed, %d additions, %d removals", - len(changedFiles), totalAdditions, totalRemovals) - - diagnosticsText := "" - for _, filePath := range changedFiles { - diagnosticsText += getDiagnostics(filePath, p.lspClients) - } - - if diagnosticsText != "" { - result += "\n\nDiagnostics:\n" + diagnosticsText - } - - return WithResponseMetadata( - NewTextResponse(result), - PatchResponseMetadata{ - FilesChanged: changedFiles, - Additions: totalAdditions, - Removals: totalRemovals, - }), nil -} diff --git a/internal/tui/components/chat/messages/renderer.go b/internal/tui/components/chat/messages/renderer.go index 339aa51b299d368a5d8f3c31b0c10d6d00a8a784..12e34c5d11486859df5d22cfe23d646fff456c91 100644 --- a/internal/tui/components/chat/messages/renderer.go +++ b/internal/tui/components/chat/messages/renderer.go @@ -148,7 +148,6 @@ func init() { registry.register(tools.GrepToolName, func() renderer { return grepRenderer{} }) registry.register(tools.LSToolName, func() renderer { return lsRenderer{} }) registry.register(tools.SourcegraphToolName, func() renderer { return sourcegraphRenderer{} }) - registry.register(tools.PatchToolName, func() renderer { return patchRenderer{} }) registry.register(tools.DiagnosticsToolName, func() renderer { return diagnosticsRenderer{} }) registry.register(agent.AgentToolName, func() renderer { return agentRenderer{} }) } @@ -446,38 +445,6 @@ func (sr sourcegraphRenderer) Render(v *toolCallCmp) string { }) } -// ----------------------------------------------------------------------------- -// Patch renderer -// ----------------------------------------------------------------------------- - -// patchRenderer handles multi-file patches with change summaries -type patchRenderer struct { - baseRenderer -} - -// Render displays patch summary with file count and change statistics -func (pr patchRenderer) Render(v *toolCallCmp) string { - var params tools.PatchParams - if err := pr.unmarshalParams(v.call.Input, ¶ms); err != nil { - return pr.renderError(v, "Invalid patch parameters") - } - - args := newParamBuilder().addMain("multiple files").build() - - return pr.renderWithParams(v, "Patch", args, func() string { - var meta tools.PatchResponseMetadata - if err := pr.unmarshalParams(v.result.Metadata, &meta); err != nil { - return renderPlainContent(v, v.result.Content) - } - - summary := fmt.Sprintf("Changed %d files (%d+ %d-)", - len(meta.FilesChanged), meta.Additions, meta.Removals) - filesList := strings.Join(meta.FilesChanged, "\n") - - return renderPlainContent(v, summary+"\n\n"+filesList) - }) -} - // ----------------------------------------------------------------------------- // Diagnostics renderer // ----------------------------------------------------------------------------- @@ -711,8 +678,6 @@ func prettifyToolName(name string) string { return "View" case tools.WriteToolName: return "Write" - case tools.PatchToolName: - return "Patch" default: return name } diff --git a/internal/tui/components/dialogs/permissions/permissions.go b/internal/tui/components/dialogs/permissions/permissions.go index 0e69eaccc89237bc6ca4bf7fe694a9f48b8d524c..5a4680d9409310c089965efa6801a70633ef5317 100644 --- a/internal/tui/components/dialogs/permissions/permissions.go +++ b/internal/tui/components/dialogs/permissions/permissions.go @@ -279,20 +279,6 @@ func (p *permissionDialogCmp) renderEditContent() string { return "" } -func (p *permissionDialogCmp) renderPatchContent() string { - if pr, ok := p.permission.Params.(tools.EditPermissionsParams); ok { - diff := p.GetOrSetDiff(p.permission.ID, func() (string, error) { - return diff.FormatDiff(pr.Diff, diff.WithTotalWidth(p.contentViewPort.Width())) - }) - - contentHeight := min(p.height-9, lipgloss.Height(diff)) - p.contentViewPort.SetHeight(contentHeight) - p.contentViewPort.SetContent(diff) - return p.styleViewport() - } - return "" -} - func (p *permissionDialogCmp) renderWriteContent() string { if pr, ok := p.permission.Params.(tools.WritePermissionsParams); ok { // Use the cache for diff rendering @@ -381,8 +367,6 @@ func (p *permissionDialogCmp) render() string { contentFinal = p.renderBashContent() case tools.EditToolName: contentFinal = p.renderEditContent() - case tools.PatchToolName: - contentFinal = p.renderPatchContent() case tools.WriteToolName: contentFinal = p.renderWriteContent() case tools.FetchToolName: diff --git a/internal/tui/tui.go b/internal/tui/tui.go index e2c037a586aaad777412372b79b5596f22d569f4..cc5a2003d85d1207b6490592d858ae3e5d553837 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -8,7 +8,6 @@ import ( "github.com/charmbracelet/crush/internal/app" "github.com/charmbracelet/crush/internal/config" "github.com/charmbracelet/crush/internal/llm/agent" - "github.com/charmbracelet/crush/internal/llm/tools" "github.com/charmbracelet/crush/internal/logging" "github.com/charmbracelet/crush/internal/permission" "github.com/charmbracelet/crush/internal/pubsub" @@ -220,16 +219,6 @@ func (a *appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return a, tea.Batch(cmds...) // Key Press Messages case tea.KeyPressMsg: - if msg.String() == "ctrl+t" { - go a.app.Permissions.Request(permission.CreatePermissionRequest{ - SessionID: "123", - ToolName: "bash", - Action: "execute", - Params: tools.BashPermissionsParams{ - Command: "ls -la", - }, - }) - } return a, a.handleKeyPressMsg(msg) } s, _ := a.status.Update(msg)