feat: keep successful edits even if some fail (#1327)

internal/agent/tools/multiedit.go

@@ -37,12 +37,19 @@ type MultiEditPermissionsParams struct {
 	NewContent string `json:"new_content,omitempty"`
 }
 
+type FailedEdit struct {
+	Index int                `json:"index"`
+	Error string             `json:"error"`
+	Edit  MultiEditOperation `json:"edit"`
+}
+
 type MultiEditResponseMetadata struct {
-	Additions    int    `json:"additions"`
-	Removals     int    `json:"removals"`
-	OldContent   string `json:"old_content,omitempty"`
-	NewContent   string `json:"new_content,omitempty"`
-	EditsApplied int    `json:"edits_applied"`
+	Additions    int          `json:"additions"`
+	Removals     int          `json:"removals"`
+	OldContent   string       `json:"old_content,omitempty"`
+	NewContent   string       `json:"new_content,omitempty"`
+	EditsApplied int          `json:"edits_applied"`
+	EditsFailed  []FailedEdit `json:"edits_failed,omitempty"`
 }
 
 const MultiEditToolName = "multiedit"
@@ -102,9 +109,6 @@ func NewMultiEditTool(lspClients *csync.Map[string, *lsp.Client], permissions pe
 
 func validateEdits(edits []MultiEditOperation) error {
 	for i, edit := range edits {
-		if edit.OldString == edit.NewString {
-			return fmt.Errorf("edit %d: old_string and new_string are identical", i+1)
-		}
 		// Only the first edit can have empty old_string (for file creation)
 		if i > 0 && edit.OldString == "" {
 			return fmt.Errorf("edit %d: only the first edit can have empty old_string (for file creation)", i+1)
@@ -136,12 +140,18 @@ func processMultiEditWithCreation(edit editContext, params MultiEditParams, call
 	// Start with the content from the first edit
 	currentContent := firstEdit.NewString
 
-	// Apply remaining edits to the content
+	// Apply remaining edits to the content, tracking failures
+	var failedEdits []FailedEdit
 	for i := 1; i < len(params.Edits); i++ {
 		edit := params.Edits[i]
 		newContent, err := applyEditToContent(currentContent, edit)
 		if err != nil {
-			return fantasy.NewTextErrorResponse(fmt.Sprintf("edit %d failed: %s", i+1, err.Error())), nil
+			failedEdits = append(failedEdits, FailedEdit{
+				Index: i + 1,
+				Error: err.Error(),
+				Edit:  edit,
+			})
+			continue
 		}
 		currentContent = newContent
 	}
@@ -192,14 +202,23 @@ func processMultiEditWithCreation(edit editContext, params MultiEditParams, call
 	recordFileWrite(params.FilePath)
 	recordFileRead(params.FilePath)
 
+	editsApplied := len(params.Edits) - len(failedEdits)
+	var message string
+	if len(failedEdits) > 0 {
+		message = fmt.Sprintf("File created with %d of %d edits: %s (%d edit(s) failed)", editsApplied, len(params.Edits), params.FilePath, len(failedEdits))
+	} else {
+		message = fmt.Sprintf("File created with %d edits: %s", len(params.Edits), params.FilePath)
+	}
+
 	return fantasy.WithResponseMetadata(
-		fantasy.NewTextResponse(fmt.Sprintf("File created with %d edits: %s", len(params.Edits), params.FilePath)),
+		fantasy.NewTextResponse(message),
 		MultiEditResponseMetadata{
 			OldContent:   "",
 			NewContent:   currentContent,
 			Additions:    additions,
 			Removals:     removals,
-			EditsApplied: len(params.Edits),
+			EditsApplied: editsApplied,
+			EditsFailed:  failedEdits,
 		},
 	), nil
 }
@@ -242,17 +261,33 @@ func processMultiEditExistingFile(edit editContext, params MultiEditParams, call
 	oldContent, isCrlf := fsext.ToUnixLineEndings(string(content))
 	currentContent := oldContent
 
-	// Apply all edits sequentially
+	// Apply all edits sequentially, tracking failures
+	var failedEdits []FailedEdit
 	for i, edit := range params.Edits {
 		newContent, err := applyEditToContent(currentContent, edit)
 		if err != nil {
-			return fantasy.NewTextErrorResponse(fmt.Sprintf("edit %d failed: %s", i+1, err.Error())), nil
+			failedEdits = append(failedEdits, FailedEdit{
+				Index: i + 1,
+				Error: err.Error(),
+				Edit:  edit,
+			})
+			continue
 		}
 		currentContent = newContent
 	}
 
 	// Check if content actually changed
 	if oldContent == currentContent {
+		// If we have failed edits, report them
+		if len(failedEdits) > 0 {
+			return fantasy.WithResponseMetadata(
+				fantasy.NewTextErrorResponse(fmt.Sprintf("no changes made - all %d edit(s) failed", len(failedEdits))),
+				MultiEditResponseMetadata{
+					EditsApplied: 0,
+					EditsFailed:  failedEdits,
+				},
+			), nil
+		}
 		return fantasy.NewTextErrorResponse("no changes made - all edits resulted in identical content"), nil
 	}
 
@@ -316,14 +351,23 @@ func processMultiEditExistingFile(edit editContext, params MultiEditParams, call
 	recordFileWrite(params.FilePath)
 	recordFileRead(params.FilePath)
 
+	editsApplied := len(params.Edits) - len(failedEdits)
+	var message string
+	if len(failedEdits) > 0 {
+		message = fmt.Sprintf("Applied %d of %d edits to file: %s (%d edit(s) failed)", editsApplied, len(params.Edits), params.FilePath, len(failedEdits))
+	} else {
+		message = fmt.Sprintf("Applied %d edits to file: %s", len(params.Edits), params.FilePath)
+	}
+
 	return fantasy.WithResponseMetadata(
-		fantasy.NewTextResponse(fmt.Sprintf("Applied %d edits to file: %s", len(params.Edits), params.FilePath)),
+		fantasy.NewTextResponse(message),
 		MultiEditResponseMetadata{
 			OldContent:   oldContent,
 			NewContent:   currentContent,
 			Additions:    additions,
 			Removals:     removals,
-			EditsApplied: len(params.Edits),
+			EditsApplied: editsApplied,
+			EditsFailed:  failedEdits,
 		},
 	), nil
 }

internal/agent/tools/multiedit.md

@@ -17,7 +17,8 @@ Makes multiple edits to a single file in one operation. Built on Edit tool for e
 <operation>
 - Edits applied sequentially in provided order.
 - Each edit operates on result of previous edit.
-- ATOMIC: If any single edit fails, the entire operation fails and no changes are applied.
+- PARTIAL SUCCESS: If some edits fail, successful edits are still applied. Failed edits are returned in the response.
+- File is modified if at least one edit succeeds.
 - Ideal for several changes to different parts of same file.
 </operation>
 
@@ -31,9 +32,10 @@ Use the same level of precision as Edit. Multiedit often fails due to formatting
 
 <critical_requirements>
 1. Apply Edit tool rules to EACH edit (see edit.md).
-2. Edits are atomic—either all succeed or none are applied.
+2. Edits are applied in order; successful edits are kept even if later edits fail.
 3. Plan sequence carefully: earlier edits change the file content that later edits must match.
 4. Ensure each old_string is unique at its application time (after prior edits).
+5. Check the response for failed edits and retry them if needed.
 </critical_requirements>
 
 <verification_before_using>
@@ -45,26 +47,27 @@ Use the same level of precision as Edit. Multiedit often fails due to formatting
 </verification_before_using>
 
 <warnings>
-- Operation fails if any old_string doesn’t match exactly (including whitespace) or equals new_string.
+- Operation continues even if some edits fail; check response for failed edits.
 - Earlier edits can invalidate later matches (added/removed spaces, lines, or reordered text).
 - Mixed tabs/spaces, trailing spaces, or missing blank lines commonly cause failures.
 - replace_all may affect unintended regions—use carefully or provide more context.
 </warnings>
 
 <recovery_steps>
-If the operation fails:
-1. Identify the first failing edit (start from top; test subsets to isolate).
-2. View the file again and copy more surrounding context for that edit.
-3. Recalculate later old_string values based on the file state AFTER preceding edits.
-4. Reduce the batch (apply earlier stable edits first), then follow up with the rest.
+If some edits fail:
+1. Check the response metadata for the list of failed edits with their error messages.
+2. View the file again to see the current state after successful edits.
+3. Adjust the failed edits based on the new file content.
+4. Retry the failed edits with corrected old_string values.
+5. Consider breaking complex batches into smaller, independent operations.
 </recovery_steps>
 
 <best_practices>
-- Ensure all edits result in correct, idiomatic code; don’t leave code broken.
+- Ensure all edits result in correct, idiomatic code; don't leave code broken.
 - Use absolute file paths (starting with /).
-- Use replace_all only when you’re certain; otherwise provide unique context.
+- Use replace_all only when you're certain; otherwise provide unique context.
 - Match existing style exactly (spaces, tabs, blank lines).
-- Test after the operation; if it fails, fix and retry in smaller chunks.
+- Review failed edits in the response and retry with corrections.
 </best_practices>
 
 <whitespace_checklist>
@@ -109,4 +112,14 @@ edits: [
   },
 ]
 ```
+
+✅ Correct: Handling partial success
+
+```
+// If edit 2 fails, edit 1 is still applied
+// Response will indicate:
+// - edits_applied: 1
+// - edits_failed: [{index: 2, error: "...", edit: {...}}]
+// You can then retry edit 2 with corrected context
+```
 </examples>

internal/agent/tools/multiedit_test.go

@@ -0,0 +1,225 @@
+package tools
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/charmbracelet/crush/internal/csync"
+	"github.com/charmbracelet/crush/internal/history"
+	"github.com/charmbracelet/crush/internal/lsp"
+	"github.com/charmbracelet/crush/internal/permission"
+	"github.com/charmbracelet/crush/internal/pubsub"
+	"github.com/stretchr/testify/require"
+)
+
+type mockPermissionService struct {
+	*pubsub.Broker[permission.PermissionRequest]
+}
+
+func (m *mockPermissionService) Request(req permission.CreatePermissionRequest) bool {
+	return true
+}
+
+func (m *mockPermissionService) Grant(req permission.PermissionRequest) {}
+
+func (m *mockPermissionService) Deny(req permission.PermissionRequest) {}
+
+func (m *mockPermissionService) GrantPersistent(req permission.PermissionRequest) {}
+
+func (m *mockPermissionService) AutoApproveSession(sessionID string) {}
+
+func (m *mockPermissionService) SetSkipRequests(skip bool) {}
+
+func (m *mockPermissionService) SkipRequests() bool {
+	return false
+}
+
+func (m *mockPermissionService) SubscribeNotifications(ctx context.Context) <-chan pubsub.Event[permission.PermissionNotification] {
+	return make(<-chan pubsub.Event[permission.PermissionNotification])
+}
+
+type mockHistoryService struct {
+	*pubsub.Broker[history.File]
+}
+
+func (m *mockHistoryService) Create(ctx context.Context, sessionID, path, content string) (history.File, error) {
+	return history.File{Path: path, Content: content}, nil
+}
+
+func (m *mockHistoryService) CreateVersion(ctx context.Context, sessionID, path, content string) (history.File, error) {
+	return history.File{}, nil
+}
+
+func (m *mockHistoryService) GetByPathAndSession(ctx context.Context, path, sessionID string) (history.File, error) {
+	return history.File{Path: path, Content: ""}, nil
+}
+
+func (m *mockHistoryService) Get(ctx context.Context, id string) (history.File, error) {
+	return history.File{}, nil
+}
+
+func (m *mockHistoryService) ListBySession(ctx context.Context, sessionID string) ([]history.File, error) {
+	return nil, nil
+}
+
+func (m *mockHistoryService) ListLatestSessionFiles(ctx context.Context, sessionID string) ([]history.File, error) {
+	return nil, nil
+}
+
+func (m *mockHistoryService) Delete(ctx context.Context, id string) error {
+	return nil
+}
+
+func (m *mockHistoryService) DeleteSessionFiles(ctx context.Context, sessionID string) error {
+	return nil
+}
+
+func TestApplyEditToContentPartialSuccess(t *testing.T) {
+	t.Parallel()
+
+	content := "line 1\nline 2\nline 3\n"
+
+	// Test successful edit.
+	newContent, err := applyEditToContent(content, MultiEditOperation{
+		OldString: "line 1",
+		NewString: "LINE 1",
+	})
+	require.NoError(t, err)
+	require.Contains(t, newContent, "LINE 1")
+	require.Contains(t, newContent, "line 2")
+
+	// Test failed edit (string not found).
+	_, err = applyEditToContent(content, MultiEditOperation{
+		OldString: "line 99",
+		NewString: "LINE 99",
+	})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "not found")
+}
+
+func TestMultiEditSequentialApplication(t *testing.T) {
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+	testFile := filepath.Join(tmpDir, "test.txt")
+
+	// Create test file.
+	content := "line 1\nline 2\nline 3\nline 4\n"
+	err := os.WriteFile(testFile, []byte(content), 0o644)
+	require.NoError(t, err)
+
+	// Mock components.
+	lspClients := csync.NewMap[string, *lsp.Client]()
+	permissions := &mockPermissionService{Broker: pubsub.NewBroker[permission.PermissionRequest]()}
+	files := &mockHistoryService{Broker: pubsub.NewBroker[history.File]()}
+
+	// Create multiedit tool.
+	_ = NewMultiEditTool(lspClients, permissions, files, tmpDir)
+
+	// Simulate reading the file first.
+	recordFileRead(testFile)
+
+	// Manually test the sequential application logic.
+	currentContent := content
+
+	// Apply edits sequentially, tracking failures.
+	edits := []MultiEditOperation{
+		{OldString: "line 1", NewString: "LINE 1"},   // Should succeed
+		{OldString: "line 99", NewString: "LINE 99"}, // Should fail - doesn't exist
+		{OldString: "line 3", NewString: "LINE 3"},   // Should succeed
+		{OldString: "line 2", NewString: "LINE 2"},   // Should succeed - still exists
+	}
+
+	var failedEdits []FailedEdit
+	successCount := 0
+
+	for i, edit := range edits {
+		newContent, err := applyEditToContent(currentContent, edit)
+		if err != nil {
+			failedEdits = append(failedEdits, FailedEdit{
+				Index: i + 1,
+				Error: err.Error(),
+				Edit:  edit,
+			})
+			continue
+		}
+		currentContent = newContent
+		successCount++
+	}
+
+	// Verify results.
+	require.Equal(t, 3, successCount, "Expected 3 successful edits")
+	require.Len(t, failedEdits, 1, "Expected 1 failed edit")
+
+	// Check failed edit details.
+	require.Equal(t, 2, failedEdits[0].Index)
+	require.Contains(t, failedEdits[0].Error, "not found")
+
+	// Verify content changes.
+	require.Contains(t, currentContent, "LINE 1")
+	require.Contains(t, currentContent, "LINE 2")
+	require.Contains(t, currentContent, "LINE 3")
+	require.Contains(t, currentContent, "line 4") // Original unchanged
+	require.NotContains(t, currentContent, "LINE 99")
+}
+
+func TestMultiEditAllEditsSucceed(t *testing.T) {
+	t.Parallel()
+
+	content := "line 1\nline 2\nline 3\n"
+
+	edits := []MultiEditOperation{
+		{OldString: "line 1", NewString: "LINE 1"},
+		{OldString: "line 2", NewString: "LINE 2"},
+		{OldString: "line 3", NewString: "LINE 3"},
+	}
+
+	currentContent := content
+	successCount := 0
+
+	for _, edit := range edits {
+		newContent, err := applyEditToContent(currentContent, edit)
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+		currentContent = newContent
+		successCount++
+	}
+
+	require.Equal(t, 3, successCount)
+	require.Contains(t, currentContent, "LINE 1")
+	require.Contains(t, currentContent, "LINE 2")
+	require.Contains(t, currentContent, "LINE 3")
+}
+
+func TestMultiEditAllEditsFail(t *testing.T) {
+	t.Parallel()
+
+	content := "line 1\nline 2\n"
+
+	edits := []MultiEditOperation{
+		{OldString: "line 99", NewString: "LINE 99"},
+		{OldString: "line 100", NewString: "LINE 100"},
+	}
+
+	currentContent := content
+	var failedEdits []FailedEdit
+
+	for i, edit := range edits {
+		newContent, err := applyEditToContent(currentContent, edit)
+		if err != nil {
+			failedEdits = append(failedEdits, FailedEdit{
+				Index: i + 1,
+				Error: err.Error(),
+				Edit:  edit,
+			})
+			continue
+		}
+		currentContent = newContent
+	}
+
+	require.Len(t, failedEdits, 2)
+	require.Equal(t, content, currentContent, "Content should be unchanged")
+}

internal/tui/components/chat/messages/renderer.go

@@ -364,6 +364,17 @@ func (mer multiEditRenderer) Render(v *toolCallCmp) string {
 				Render(fmt.Sprintf("… (%d lines)", len(contentLines)-responseContextHeight))
 			formatted = strings.Join(contentLines[:responseContextHeight], "\n") + "\n" + truncateMessage
 		}
+
+		// Add failed edits warning if any exist
+		if len(meta.EditsFailed) > 0 {
+			noteTag := t.S().Base.Padding(0, 2).Background(t.Info).Foreground(t.White).Render("Note")
+			noteMsg := fmt.Sprintf("%d of %d edits succeeded", meta.EditsApplied, len(params.Edits))
+			note := t.S().Base.
+				Width(v.textWidth() - 2).
+				Render(fmt.Sprintf("%s %s", noteTag, t.S().Muted.Render(noteMsg)))
+			formatted = lipgloss.JoinVertical(lipgloss.Left, formatted, "", note)
+		}
+
 		return formatted
 	})
 }