diff --git a/internal/core/runtime/internal_command_apply_patch.go b/internal/core/runtime/internal_command_apply_patch.go index 25e569c..b3aa18a 100644 --- a/internal/core/runtime/internal_command_apply_patch.go +++ b/internal/core/runtime/internal_command_apply_patch.go @@ -14,6 +14,9 @@ import ( const ( applyPatchCommandName = "apply_patch" + patchOpAdd = "add" + patchOpUpdate = "update" + patchOpDelete = "delete" ) type applyPatchOptions struct { @@ -24,6 +27,7 @@ type applyPatchOptions struct { type patchOperation struct { opType string path string + moveTo string hunks []patchHunk } @@ -262,7 +266,7 @@ func parsePatch(input string) ([]patchOperation, error) { if err := flushHunk(); err != nil { return err } - if len(currentOp.hunks) == 0 { + if currentOp.opType == patchOpUpdate && len(currentOp.hunks) == 0 { return fmt.Errorf("no hunks provided for %s", currentOp.path) } operations = append(operations, *currentOp) @@ -291,17 +295,41 @@ func parsePatch(input string) ([]patchOperation, error) { } if strings.HasPrefix(line, "*** ") { + if currentOp != nil { + if moveTarget, ok := strings.CutPrefix(line, "*** Move to: "); ok { + if currentOp.opType != patchOpUpdate { + return nil, fmt.Errorf("unsupported patch directive: %s", line) + } + if currentHunk != nil && len(currentHunk.lines) > 0 { + return nil, fmt.Errorf("move directive must appear before diff hunks for %s", currentOp.path) + } + if currentOp.moveTo != "" { + return nil, fmt.Errorf("duplicate move directive for %s", currentOp.path) + } + target := strings.TrimSpace(moveTarget) + if target == "" { + return nil, fmt.Errorf("move directive missing destination for %s", currentOp.path) + } + currentOp.moveTo = target + continue + } + } if err := flushOp(); err != nil { return nil, err } if updatePath, ok := strings.CutPrefix(line, "*** Update File: "); ok { path := strings.TrimSpace(updatePath) - currentOp = &patchOperation{opType: "update", path: path} + currentOp = &patchOperation{opType: patchOpUpdate, path: path} continue } if addPath, ok := strings.CutPrefix(line, "*** Add File: "); ok { path := strings.TrimSpace(addPath) - currentOp = &patchOperation{opType: "add", path: path} + currentOp = &patchOperation{opType: patchOpAdd, path: path} + continue + } + if deletePath, ok := strings.CutPrefix(line, "*** Delete File: "); ok { + path := strings.TrimSpace(deletePath) + currentOp = &patchOperation{opType: patchOpDelete, path: path} continue } return nil, fmt.Errorf("unsupported patch directive: %s", line) @@ -372,184 +400,258 @@ func splitLines(input string) []string { } func applyPatchOperations(ctx context.Context, operations []patchOperation, opts applyPatchOptions) ([]fileResult, *patchError) { - states := make(map[string]*fileState) + var results []fileResult - ensureState := func(relativePath string, create bool) (*fileState, error) { - rel := strings.TrimSpace(relativePath) - if rel == "" { - return nil, fmt.Errorf("invalid patch path") + for _, op := range operations { + if ctx.Err() != nil { + return nil, &patchError{Message: ctx.Err().Error()} } - var abs string - if filepath.IsAbs(rel) { - abs = filepath.Clean(rel) - } else { - abs = filepath.Clean(filepath.Join(opts.workingDir, rel)) - } + switch op.opType { + case patchOpAdd: + state, err := prepareFileState(op.path, opts, true, true) + if err != nil { + return nil, &patchError{Message: err.Error()} + } + state.cursor = 0 + state.hunkStatuses = nil + if len(op.hunks) == 0 { + state.touched = true + } + for index, hunk := range op.hunks { + if ctx.Err() != nil { + return nil, &patchError{Message: ctx.Err().Error()} + } + hunkNumber := index + 1 + if err := applyHunk(state, hunk); err != nil { + return nil, enhanceHunkError(err, state, hunk, hunkNumber) + } + state.hunkStatuses = append(state.hunkStatuses, hunkStatus{Number: hunkNumber, Status: "applied"}) + state.touched = true + } + if err := writePatchedFile(state, state.path, state.relativePath); err != nil { + return nil, err + } + results = append(results, fileResult{status: "A", path: state.relativePath}) - if state, ok := states[abs]; ok { - state.options = opts - if opts.ignoreWhitespace { - state.normalizedLines = ensureNormalizedLines(state) - } else { - state.normalizedLines = nil + case patchOpUpdate: + state, err := prepareFileState(op.path, opts, false, false) + if err != nil { + return nil, &patchError{Message: err.Error()} + } + state.cursor = 0 + state.hunkStatuses = nil + for index, hunk := range op.hunks { + if ctx.Err() != nil { + return nil, &patchError{Message: ctx.Err().Error()} + } + hunkNumber := index + 1 + if err := applyHunk(state, hunk); err != nil { + return nil, enhanceHunkError(err, state, hunk, hunkNumber) + } + state.hunkStatuses = append(state.hunkStatuses, hunkStatus{Number: hunkNumber, Status: "applied"}) + state.touched = true } - return state, nil - } - info, err := os.Stat(abs) - switch { - case err == nil && create: - return nil, fmt.Errorf("cannot add %s because it already exists", rel) - case err == nil: - if info.IsDir() { - return nil, fmt.Errorf("cannot patch directory %s", rel) + destAbs := state.path + destDisplay := state.relativePath + if strings.TrimSpace(op.moveTo) != "" { + moveAbs, moveDisplay, err := resolvePatchPath(op.moveTo, opts) + if err != nil { + return nil, &patchError{Message: err.Error()} + } + destAbs = moveAbs + destDisplay = moveDisplay } - content, readErr := os.ReadFile(abs) - if readErr != nil { - return nil, fmt.Errorf("failed to read %s: %v", rel, readErr) + + if err := writePatchedFile(state, destAbs, destDisplay); err != nil { + return nil, err } - normalized := strings.ReplaceAll(string(content), "\r\n", "\n") - normalized = strings.ReplaceAll(normalized, "\r", "\n") - lines := strings.Split(normalized, "\n") - ends := strings.HasSuffix(normalized, "\n") - state := &fileState{ - path: abs, - relativePath: rel, - lines: lines, - normalizedLines: nil, - originalContent: string(content), - originalEndsWithNewline: &ends, - originalMode: info.Mode(), - touched: false, - cursor: 0, - hunkStatuses: nil, - isNew: false, - options: opts, + + if destAbs != state.path { + if err := os.Remove(state.path); err != nil { + return nil, &patchError{Message: fmt.Sprintf("failed to remove original %s: %v", state.relativePath, err)} + } } - if opts.ignoreWhitespace { - state.normalizedLines = ensureNormalizedLines(state) + + results = append(results, fileResult{status: "M", path: destDisplay}) + + case patchOpDelete: + abs, rel, err := resolvePatchPath(op.path, opts) + if err != nil { + return nil, &patchError{Message: err.Error()} } - states[abs] = state - return state, nil - case errors.Is(err, fs.ErrNotExist): - if !create { - return nil, fmt.Errorf("failed to read %s: file does not exist", rel) + info, statErr := os.Stat(abs) + if statErr != nil { + if errors.Is(statErr, fs.ErrNotExist) { + return nil, &patchError{Message: fmt.Sprintf("Failed to delete file %s", rel)} + } + return nil, &patchError{Message: fmt.Sprintf("failed to stat %s: %v", rel, statErr)} } - state := &fileState{ - path: abs, - relativePath: rel, - lines: []string{}, - normalizedLines: nil, - originalContent: "", - originalEndsWithNewline: nil, - originalMode: 0, - touched: false, - cursor: 0, - hunkStatuses: nil, - isNew: true, - options: opts, + if info.IsDir() { + return nil, &patchError{Message: fmt.Sprintf("Failed to delete file %s", rel)} } - if opts.ignoreWhitespace { - state.normalizedLines = []string{} + if err := os.Remove(abs); err != nil { + return nil, &patchError{Message: fmt.Sprintf("Failed to delete file %s", rel)} } - states[abs] = state - return state, nil + results = append(results, fileResult{status: "D", path: rel}) + default: - return nil, fmt.Errorf("failed to stat %s: %v", rel, err) + return nil, &patchError{Message: fmt.Sprintf("unsupported patch operation for %s: %s", op.path, op.opType)} } } - for _, op := range operations { - if ctx.Err() != nil { - return nil, &patchError{Message: ctx.Err().Error()} - } - if op.opType != "update" && op.opType != "add" { - return nil, &patchError{Message: fmt.Sprintf("unsupported patch operation for %s: %s", op.path, op.opType)} + return results, nil +} + +func resolvePatchPath(path string, opts applyPatchOptions) (string, string, error) { + rel := strings.TrimSpace(path) + if rel == "" { + return "", "", fmt.Errorf("invalid patch path") + } + if filepath.IsAbs(rel) { + return filepath.Clean(rel), rel, nil + } + abs := filepath.Clean(filepath.Join(opts.workingDir, rel)) + return abs, rel, nil +} + +func prepareFileState(relativePath string, opts applyPatchOptions, allowCreate, treatAsNew bool) (*fileState, error) { + abs, rel, err := resolvePatchPath(relativePath, opts) + if err != nil { + return nil, err + } + + info, statErr := os.Stat(abs) + switch { + case statErr == nil: + if info.IsDir() { + return nil, fmt.Errorf("cannot patch directory %s", rel) } - state, err := ensureState(op.path, op.opType == "add") - if err != nil { - return nil, &patchError{Message: err.Error()} + state := &fileState{ + path: abs, + relativePath: rel, + lines: []string{}, + normalizedLines: nil, + originalContent: "", + originalEndsWithNewline: nil, + originalMode: info.Mode(), + touched: false, + cursor: 0, + hunkStatuses: nil, + isNew: treatAsNew, + options: opts, } - state.cursor = 0 - state.hunkStatuses = nil - for index, hunk := range op.hunks { - hunkNumber := index + 1 - if ctx.Err() != nil { - return nil, &patchError{Message: ctx.Err().Error()} - } - if err := applyHunk(state, hunk); err != nil { - return nil, enhanceHunkError(err, state, hunk, hunkNumber) + if treatAsNew { + if opts.ignoreWhitespace { + state.normalizedLines = []string{} } - state.hunkStatuses = append(state.hunkStatuses, hunkStatus{Number: hunkNumber, Status: "applied"}) - state.touched = true + return state, nil } + + content, readErr := os.ReadFile(abs) + if readErr != nil { + return nil, fmt.Errorf("Failed to read file to update %s: %v", rel, readErr) + } + normalized := strings.ReplaceAll(string(content), "\r\n", "\n") + normalized = strings.ReplaceAll(normalized, "\r", "\n") + lines := strings.Split(normalized, "\n") + ends := strings.HasSuffix(normalized, "\n") + state.lines = lines + state.originalContent = string(content) + state.originalEndsWithNewline = &ends + if opts.ignoreWhitespace { + state.normalizedLines = ensureNormalizedLines(state) + } + return state, nil + + case errors.Is(statErr, fs.ErrNotExist): + if !allowCreate { + return nil, fmt.Errorf("Failed to read file to update %s: file does not exist", rel) + } + state := &fileState{ + path: abs, + relativePath: rel, + lines: []string{}, + normalizedLines: nil, + originalContent: "", + originalEndsWithNewline: nil, + originalMode: 0, + touched: false, + cursor: 0, + hunkStatuses: nil, + isNew: true, + options: opts, + } + if opts.ignoreWhitespace { + state.normalizedLines = []string{} + } + return state, nil + + default: + return nil, fmt.Errorf("failed to stat %s: %v", rel, statErr) } +} - var results []fileResult - for _, state := range states { - if !state.touched { - continue +func writePatchedFile(state *fileState, destinationPath, displayPath string) *patchError { + if state == nil { + return &patchError{Message: "missing file state"} + } + + newContent := strings.Join(state.lines, "\n") + if state.originalEndsWithNewline != nil { + if *state.originalEndsWithNewline && !strings.HasSuffix(newContent, "\n") { + newContent += "\n" } - newContent := strings.Join(state.lines, "\n") - if state.originalEndsWithNewline != nil { - if *state.originalEndsWithNewline && !strings.HasSuffix(newContent, "\n") { - newContent += "\n" - } - if !*state.originalEndsWithNewline && strings.HasSuffix(newContent, "\n") { - newContent = strings.TrimSuffix(newContent, "\n") - } + if !*state.originalEndsWithNewline && strings.HasSuffix(newContent, "\n") { + newContent = strings.TrimSuffix(newContent, "\n") } + } else if len(state.lines) > 0 && !strings.HasSuffix(newContent, "\n") { + newContent += "\n" + } - if err := os.MkdirAll(filepath.Dir(state.path), 0o755); err != nil { - return nil, &patchError{Message: fmt.Sprintf("failed to create directory for %s: %v", state.relativePath, err)} - } + if err := os.MkdirAll(filepath.Dir(destinationPath), 0o755); err != nil { + return &patchError{Message: fmt.Sprintf("failed to create directory for %s: %v", displayPath, err)} + } - perm := state.originalMode & fs.ModePerm - if perm == 0 { - perm = 0o644 - } + perm := state.originalMode & fs.ModePerm + if perm == 0 { + perm = 0o644 + } - if err := os.WriteFile(state.path, []byte(newContent), perm); err != nil { - return nil, &patchError{Message: fmt.Sprintf("failed to write %s: %v", state.relativePath, err)} - } + if err := os.WriteFile(destinationPath, []byte(newContent), perm); err != nil { + return &patchError{Message: fmt.Sprintf("failed to write %s: %v", displayPath, err)} + } - if state.originalMode != 0 { - desired := (state.originalMode & fs.ModePerm) | (state.originalMode & (fs.ModeSetuid | fs.ModeSetgid | fs.ModeSticky)) - if desired == 0 { - desired = perm - } + if state.originalMode != 0 { + desired := (state.originalMode & fs.ModePerm) | (state.originalMode & (fs.ModeSetuid | fs.ModeSetgid | fs.ModeSticky)) + if desired == 0 { + desired = perm + } - specialBits := state.originalMode & (fs.ModeSetuid | fs.ModeSetgid | fs.ModeSticky) - needsChmod := specialBits != 0 - if !needsChmod { - info, statErr := os.Stat(state.path) - if statErr != nil { - return nil, &patchError{Message: fmt.Sprintf("failed to stat %s after write: %v", state.relativePath, statErr)} - } - current := info.Mode() & (fs.ModePerm | fs.ModeSetuid | fs.ModeSetgid | fs.ModeSticky) - if current != desired { - needsChmod = true - } + specialBits := state.originalMode & (fs.ModeSetuid | fs.ModeSetgid | fs.ModeSticky) + needsChmod := specialBits != 0 + if !needsChmod { + info, statErr := os.Stat(destinationPath) + if statErr != nil { + return &patchError{Message: fmt.Sprintf("failed to stat %s after write: %v", displayPath, statErr)} } - - if needsChmod { - if err := os.Chmod(state.path, desired); err != nil { - return nil, &patchError{Message: fmt.Sprintf("failed to restore permissions for %s: %v", state.relativePath, err)} - } + current := info.Mode() & (fs.ModePerm | fs.ModeSetuid | fs.ModeSetgid | fs.ModeSticky) + if current != desired { + needsChmod = true } } - status := "M" - if state.isNew { - status = "A" + if needsChmod { + if err := os.Chmod(destinationPath, desired); err != nil { + return &patchError{Message: fmt.Sprintf("failed to restore permissions for %s: %v", displayPath, err)} + } } - results = append(results, fileResult{status: status, path: state.relativePath}) } - return results, nil + return nil } func normalizeLine(line string) string { diff --git a/internal/core/runtime/internal_command_apply_patch_test.go b/internal/core/runtime/internal_command_apply_patch_test.go index 9dba9e3..aa38585 100644 --- a/internal/core/runtime/internal_command_apply_patch_test.go +++ b/internal/core/runtime/internal_command_apply_patch_test.go @@ -2,6 +2,7 @@ package runtime import ( "context" + "errors" "io/fs" "os" "path/filepath" @@ -167,7 +168,7 @@ func TestApplyPatchAddsFile(t *testing.T) { if err != nil { t.Fatalf("failed to read new file: %v", err) } - if got, want := string(data), "hello\nworld"; got != want { + if got, want := string(data), "hello\nworld\n"; got != want { t.Fatalf("new file mismatch: got %q want %q", got, want) } } @@ -226,3 +227,190 @@ func TestApplyPatchWhitespaceOptions(t *testing.T) { t.Fatalf("stderr missing hunk message: %q", payload.Stderr) } } + +func TestApplyPatchDeletesFile(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + target := filepath.Join(dir, "obsolete.txt") + if err := os.WriteFile(target, []byte("gone soon\n"), 0o644); err != nil { + t.Fatalf("failed to seed file: %v", err) + } + + run := strings.Join([]string{ + "apply_patch", + "*** Begin Patch", + "*** Delete File: obsolete.txt", + "*** End Patch", + }, "\n") + + step := PlanStep{ID: "delete", Command: CommandDraft{Shell: agentShell, Run: run, Cwd: dir}} + req := InternalCommandRequest{Name: applyPatchCommandName, Raw: run, Step: step} + + payload, err := newApplyPatchCommand()(context.Background(), req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if payload.ExitCode == nil || *payload.ExitCode != 0 { + t.Fatalf("expected exit code 0, got %+v", payload.ExitCode) + } + if _, err := os.Stat(target); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("expected file to be deleted, stat err=%v", err) + } + if !strings.Contains(payload.Stdout, "D obsolete.txt") { + t.Fatalf("stdout missing delete summary: %q", payload.Stdout) + } +} + +func TestApplyPatchDeleteMissingFileFails(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + run := strings.Join([]string{ + "apply_patch", + "*** Begin Patch", + "*** Delete File: missing.txt", + "*** End Patch", + }, "\n") + + step := PlanStep{ID: "delete-missing", Command: CommandDraft{Shell: agentShell, Run: run, Cwd: dir}} + req := InternalCommandRequest{Name: applyPatchCommandName, Raw: run, Step: step} + + payload, err := newApplyPatchCommand()(context.Background(), req) + if err == nil { + t.Fatalf("expected error when deleting missing file") + } + if payload.ExitCode == nil || *payload.ExitCode == 0 { + t.Fatalf("expected non-zero exit code") + } + if got, want := payload.Stderr, "Failed to delete file missing.txt"; !strings.Contains(got, want) { + t.Fatalf("stderr mismatch: got %q want substring %q", got, want) + } +} + +func TestApplyPatchMovesFile(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + original := filepath.Join(dir, "old", "name.txt") + if err := os.MkdirAll(filepath.Dir(original), 0o755); err != nil { + t.Fatalf("failed to create directory: %v", err) + } + if err := os.WriteFile(original, []byte("old content\n"), 0o644); err != nil { + t.Fatalf("failed to seed file: %v", err) + } + + run := strings.Join([]string{ + "apply_patch", + "*** Begin Patch", + "*** Update File: old/name.txt", + "*** Move to: renamed/dir/name.txt", + "@@", + "-old content", + "+new content", + "*** End Patch", + }, "\n") + + step := PlanStep{ID: "move", Command: CommandDraft{Shell: agentShell, Run: run, Cwd: dir}} + req := InternalCommandRequest{Name: applyPatchCommandName, Raw: run, Step: step} + + payload, err := newApplyPatchCommand()(context.Background(), req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if payload.ExitCode == nil || *payload.ExitCode != 0 { + t.Fatalf("expected exit code 0, got %+v", payload.ExitCode) + } + if strings.Contains(payload.Stdout, "old/name.txt") { + t.Fatalf("stdout reported old path: %q", payload.Stdout) + } + if !strings.Contains(payload.Stdout, "M renamed/dir/name.txt") { + t.Fatalf("stdout missing move summary: %q", payload.Stdout) + } + if _, err := os.Stat(original); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("expected original file removed, stat err=%v", err) + } + moved := filepath.Join(dir, "renamed", "dir", "name.txt") + data, err := os.ReadFile(moved) + if err != nil { + t.Fatalf("failed to read moved file: %v", err) + } + if got, want := string(data), "new content\n"; got != want { + t.Fatalf("moved file content mismatch: got %q want %q", got, want) + } +} + +func TestApplyPatchAddOverwritesExistingFile(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + target := filepath.Join(dir, "duplicate.txt") + if err := os.WriteFile(target, []byte("old content\n"), 0o644); err != nil { + t.Fatalf("failed to seed file: %v", err) + } + + run := strings.Join([]string{ + "apply_patch", + "*** Begin Patch", + "*** Add File: duplicate.txt", + "+new content", + "*** End Patch", + }, "\n") + + step := PlanStep{ID: "overwrite", Command: CommandDraft{Shell: agentShell, Run: run, Cwd: dir}} + req := InternalCommandRequest{Name: applyPatchCommandName, Raw: run, Step: step} + + payload, err := newApplyPatchCommand()(context.Background(), req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if payload.ExitCode == nil || *payload.ExitCode != 0 { + t.Fatalf("expected exit code 0, got %+v", payload.ExitCode) + } + data, err := os.ReadFile(target) + if err != nil { + t.Fatalf("failed to read overwritten file: %v", err) + } + if got, want := string(data), "new content\n"; got != want { + t.Fatalf("overwritten content mismatch: got %q want %q", got, want) + } + if !strings.Contains(payload.Stdout, "A duplicate.txt") { + t.Fatalf("stdout missing add summary: %q", payload.Stdout) + } +} + +func TestApplyPatchPartialSuccessLeavesChanges(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + run := strings.Join([]string{ + "apply_patch", + "*** Begin Patch", + "*** Add File: created.txt", + "+hello", + "*** Update File: missing.txt", + "@@", + "-old", + "+new", + "*** End Patch", + }, "\n") + + step := PlanStep{ID: "partial", Command: CommandDraft{Shell: agentShell, Run: run, Cwd: dir}} + req := InternalCommandRequest{Name: applyPatchCommandName, Raw: run, Step: step} + + payload, err := newApplyPatchCommand()(context.Background(), req) + if err == nil { + t.Fatalf("expected error from missing update target") + } + if payload.ExitCode == nil || *payload.ExitCode == 0 { + t.Fatalf("expected non-zero exit code") + } + created := filepath.Join(dir, "created.txt") + data, readErr := os.ReadFile(created) + if readErr != nil { + t.Fatalf("expected created file to persist, read err=%v", readErr) + } + if got, want := string(data), "hello\n"; got != want { + t.Fatalf("created file content mismatch: got %q want %q", got, want) + } +}