fix: server-side diffs and stricter fuzzy splicing for edit_files (#24454)

Fixes three classes of edit_files bugs and adds structured per-file
diff output for tool callers:

- New IncludeDiff flag on FileEditRequest; when set, the agent
  returns FileEditResponse.Files[]{Path, Diff} with unified diffs
  computed via go-udiff v0.4.1 Lines + ToUnified (not Unified,
  which calls log.Fatalf on internal error).
- Fuzzy match comparators split each line into leading whitespace,
  body, trailing whitespace, and ending. The splice substitutes at
  each position: on agreement between search and replace the file's
  bytes win; on disagreement the replacement's bytes are spliced
  verbatim. Carve-outs for empty-body lines, multi-line EOF splices,
  and level-aware indent translation for inserted lines.
- Indent-unit detection (GCD for spaces, tab-priority) lets a 4sp
  LLM search insert correctly into tab or 2sp files. Falls back to
  the previous cLead-inheritance path when units can't be detected
  cleanly.
- Empty search is rejected with "search string must not be empty".
- Duplicate file paths in one request are rejected; symlink aliases
  resolved via api.resolvePath before the dedup check.
- Frontend EditFilesRenderer consumes the structured files array by
  explicit path (no label munging) with per-file synthetic fallback
  for older agents or mismatched paths. On error, no diff is
  rendered so the synthetic fallback doesn't misrepresent a
  rejected edit as applied.

Breaking change: AgentConn.EditFiles changes from (ctx, req) error
to (ctx, req) (FileEditResponse, error) in codersdk/workspacesdk.
Source-breaking for external Go consumers; no compat shim per plan
owner.

Out of scope (tracked in CODAGT-214): level-aware indent for
middle-substituted splice lines. Locked in
TestEditFiles_FuzzyIndent_InsertionLevelAware's Lock_* cases plus
TestEditFiles_ReplaceAll_FuzzyIndentGap.
This commit is contained in:
Mathias Fredriksson
2026-04-18 16:39:34 +03:00
committed by GitHub
parent 23f9e26796
commit 6b0bb02e5d
15 changed files with 3032 additions and 144 deletions
+620 -56
View File
@@ -13,6 +13,7 @@ import (
"strings"
"syscall"
"github.com/aymanbagabas/go-udiff"
"github.com/google/uuid"
"golang.org/x/xerrors"
@@ -44,7 +45,16 @@ type HTTPResponseCode = int
// pendingEdit holds the computed result of a file edit, ready to
// be written to disk.
type pendingEdit struct {
path string
// origPath is the caller-supplied path, pre-symlink-resolution.
// Used for response labels so the caller can match responses to
// their original requests.
origPath string
// path is the symlink-resolved path; what actually gets written.
path string
// oldContent is the file content before edits were applied. Used
// for diff computation when the request asked for diffs.
oldContent string
// content is the file content after all edits.
content string
mode os.FileMode
}
@@ -375,6 +385,37 @@ func (api *API) HandleEditFiles(rw http.ResponseWriter, r *http.Request) {
return
}
// Duplicate entries both read the same file and race to write;
// the first entry's edits are silently lost. Resolve symlinks
// before comparing so two paths that alias the same real file
// (e.g. one via a symlink, one direct) don't slip past as
// distinct keys. prepareFileEdit resolves the path again for
// its own use; the double lstat cost is cheap compared to the
// data-loss risk of silent aliasing.
type seenEntry struct {
caller string
}
seenPaths := make(map[string]seenEntry, len(req.Files))
for _, f := range req.Files {
// On resolve error, use the raw path; phase 1 surfaces
// the error with its proper status code.
key := f.Path
if resolved, err := api.resolvePath(f.Path); err == nil {
key = resolved
}
if prev, dup := seenPaths[key]; dup {
msg := fmt.Sprintf("duplicate file path %q: combine edits into a single entry's \"edits\" list", f.Path)
if prev.caller != f.Path {
msg = fmt.Sprintf("duplicate file path %q aliases %q (same real file): combine edits into a single entry's \"edits\" list", f.Path, prev.caller)
}
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: msg,
})
return
}
seenPaths[key] = seenEntry{caller: f.Path}
}
// Phase 1: compute all edits in memory. If any file fails
// (bad path, search miss, permission error), bail before
// writing anything.
@@ -426,9 +467,29 @@ func (api *API) HandleEditFiles(rw http.ResponseWriter, r *http.Request) {
}
}
httpapi.Write(ctx, rw, http.StatusOK, codersdk.Response{
Message: "Successfully edited file(s)",
})
resp := workspacesdk.FileEditResponse{}
if req.IncludeDiff {
resp.Files = make([]workspacesdk.FileEditResult, 0, len(pending))
for _, p := range pending {
// udiff.Unified calls log.Fatalf on its internal error,
// which would kill the agent process. Route through
// Lines + ToUnified so a library bug yields an empty
// diff plus a log line instead.
edits := udiff.Lines(p.oldContent, p.content)
diff, err := udiff.ToUnified(p.origPath, p.origPath, p.oldContent, edits, udiff.DefaultContextLines)
if err != nil {
api.logger.Warn(ctx, "unified diff computation failed",
slog.F("path", p.origPath),
slog.Error(err))
diff = ""
}
resp.Files = append(resp.Files, workspacesdk.FileEditResult{
Path: p.origPath,
Diff: diff,
})
}
}
httpapi.Write(ctx, rw, http.StatusOK, resp)
}
// prepareFileEdit validates, reads, and computes edits for a single
@@ -450,6 +511,7 @@ func (api *API) prepareFileEdit(path string, edits []workspacesdk.FileEdit) (int
if err != nil {
return http.StatusInternalServerError, nil, xerrors.Errorf("resolve symlink %q: %w", path, err)
}
origPath := path
path = resolved
f, err := api.filesystem.Open(path)
@@ -479,6 +541,7 @@ func (api *API) prepareFileEdit(path string, edits []workspacesdk.FileEdit) (int
return http.StatusInternalServerError, nil, xerrors.Errorf("read %s: %w", path, err)
}
content := string(data)
oldContent := content
for _, edit := range edits {
var err error
@@ -489,9 +552,11 @@ func (api *API) prepareFileEdit(path string, edits []workspacesdk.FileEdit) (int
}
return 0, &pendingEdit{
path: path,
content: content,
mode: stat.Mode(),
origPath: origPath,
path: path,
oldContent: oldContent,
content: content,
mode: stat.Mode(),
}, nil
}
@@ -555,6 +620,458 @@ func (api *API) atomicWrite(ctx context.Context, path string, mode *os.FileMode,
return 0, nil
}
// splitEnding separates a line produced by strings.SplitAfter(s,
// "\n") into its content bytes and its line ending. The ending is
// one of "\r\n", "\n", or "" (the last slice when the input lacks a
// trailing newline).
func splitEnding(line string) (content, ending string) {
if strings.HasSuffix(line, "\r\n") {
return line[:len(line)-2], "\r\n"
}
if strings.HasSuffix(line, "\n") {
return line[:len(line)-1], "\n"
}
return line, ""
}
// endingsMatch decides whether two line endings may pair up during
// fuzzy matching. Identical endings always match. "\n" and "\r\n"
// interchange so LLMs can send LF searches against CRLF content.
// An empty ending (EOF, no terminator) acts as a wildcard and
// matches any ending, which lets the splice later substitute the
// file's actual ending in place of a missing one.
func endingsMatch(a, b string) bool {
// Wildcard: empty ending matches any ending at the matching
// phase. Only valid here, not at the splice phase.
if a == "" || b == "" {
return true
}
if a == b {
return true
}
return isNewlineEnding(a) && isNewlineEnding(b)
}
// isNewlineEnding reports whether s is one of the newline-class
// endings: "\n" or "\r\n". Shared primitive for endingsMatch
// (matching phase) and endingShapeEqual (splice phase) so a new
// ending class added in one predicate can't silently diverge from
// the other.
func isNewlineEnding(s string) bool {
return s == "\n" || s == "\r\n"
}
// internalLineEnding returns the shared line ending used across
// lines. An unterminated last line (EOF-no-newline) is excluded.
// Returns ("", false) if any non-last line has no ending, or if
// endings disagree.
func internalLineEnding(lines []string) (string, bool) {
if len(lines) < 2 {
return "", false
}
var want string
for i, l := range lines {
isLast := i == len(lines)-1
_, e := splitEnding(l)
if isLast && e == "" {
continue
}
if e == "" {
return "", false
}
if want == "" {
want = e
continue
}
if e != want {
return "", false
}
}
return want, want != ""
}
// dominantFileEnding returns CRLF if CRLF endings outnumber LF in
// contentLines, LF otherwise (including ties and ending-less files).
func dominantFileEnding(contentLines []string) string {
var crlf, lf int
for _, l := range contentLines {
switch {
case strings.HasSuffix(l, "\r\n"):
crlf++
case strings.HasSuffix(l, "\n"):
lf++
}
}
if crlf > lf {
return "\r\n"
}
return "\n"
}
// atNoNewlineEOF reports whether the matched region ends at a
// file that lacks a trailing newline. True when no non-empty lines
// follow the match and the last matched line has no ending.
func atNoNewlineEOF(contentLines []string, end int) bool {
if end == 0 {
return false
}
if end < len(contentLines) {
// Anything non-empty after the match disqualifies.
for _, l := range contentLines[end:] {
if l != "" {
return false
}
}
}
// Last matched content line must itself have no ending.
_, e := splitEnding(contentLines[end-1])
return e == ""
}
// leadOnly returns the leading whitespace of line (spaces and
// tabs only), excluding the ending.
func leadOnly(line string) string {
//nolint:dogsled // splitLineParts is the shared decomposer; other parts are genuinely unused here.
lead, _, _, _ := splitLineParts(line)
return lead
}
// alignSearchReplace returns the count of leading and trailing
// lines that match between searchLines and repLines under
// TrimSpace equality. Between the prefix and suffix ranges lies
// the middle: inserted, deleted, or rewritten lines. TrimSpace
// matches what pass 3 uses for matching, so pair identification
// stays consistent with how the region was found.
func alignSearchReplace(searchLines, repLines []string) (prefix, suffix int) {
eq := func(a, b string) bool {
aContent, _ := splitEnding(a)
bContent, _ := splitEnding(b)
return strings.TrimSpace(aContent) == strings.TrimSpace(bContent)
}
maxPrefix := len(searchLines)
if len(repLines) < maxPrefix {
maxPrefix = len(repLines)
}
for prefix < maxPrefix && eq(searchLines[prefix], repLines[prefix]) {
prefix++
}
// Suffix must not overlap prefix on either side.
maxSuffix := maxPrefix - prefix
for suffix < maxSuffix &&
eq(searchLines[len(searchLines)-1-suffix], repLines[len(repLines)-1-suffix]) {
suffix++
}
return prefix, suffix
}
// detectIndentUnit scans leading whitespace across the given lines
// and returns the smallest consistent indentation unit (one tab, or
// N spaces where N is the GCD of observed non-zero lead lengths).
// Returns ("", false) when no useful unit can be detected: no lines
// have indent, indents mix tabs and spaces, or the GCD is zero.
//
// Tabs take priority: any tab-indented line forces unit="\t" and any
// space-only indent on another line marks the sample as mixed.
func detectIndentUnit(lines []string) (string, bool) {
sawTab := false
sawSpace := false
var spaceGCD int
for _, l := range lines {
lead, mid, _, _ := splitLineParts(l)
// Skip body-less lines: a blank line or a line with only
// trailing whitespace has no indent signal. Otherwise a
// 2sp whitespace-only line on a 4sp file would corrupt
// the GCD down to 2sp and emit the wrong unit.
if lead == "" || mid == "" {
continue
}
switch {
case strings.HasPrefix(lead, "\t") && !strings.ContainsAny(lead, " "):
sawTab = true
case !strings.ContainsAny(lead, "\t"):
sawSpace = true
if spaceGCD == 0 {
spaceGCD = len(lead)
} else {
spaceGCD = indentGCD(spaceGCD, len(lead))
}
default:
// Mixed tab+space in a single lead; bail.
return "", false
}
}
if sawTab && sawSpace {
return "", false
}
if sawTab {
return "\t", true
}
if spaceGCD > 0 {
return strings.Repeat(" ", spaceGCD), true
}
return "", false
}
// indentGCD returns the greatest common divisor of a and b. Used
// only by detectIndentUnit on positive space-lead lengths.
func indentGCD(a, b int) int {
for b != 0 {
a, b = b, a%b
}
return a
}
// translateIndentLevel returns the file-side lead for an inserted
// splice line by translating the caller's indent level. rLead is
// the inserted replacement line's lead, sLead is the reference
// search line's lead (the pair the splice would have inherited
// from), cLead is the matched content's lead at that same
// reference slot. Returns ("", false) when any of the leads are
// not clean multiples of their respective units.
func translateIndentLevel(rLead, sLead, cLead, searchUnit, fileUnit string) (string, bool) {
repLevel, ok := indentLevel(rLead, searchUnit)
if !ok {
return "", false
}
searchBase, ok := indentLevel(sLead, searchUnit)
if !ok {
return "", false
}
fileBase, ok := indentLevel(cLead, fileUnit)
if !ok {
return "", false
}
targetLevel := fileBase + (repLevel - searchBase)
if targetLevel < 0 {
return "", false
}
return strings.Repeat(fileUnit, targetLevel), true
}
// indentLevel returns len(lead) / len(unit) when lead is a clean
// multiple of unit. Returns (0, false) when lead doesn't divide
// evenly by unit. Callers must ensure unit is non-empty;
// detectIndentUnit's second return gates this.
func indentLevel(lead, unit string) (int, bool) {
if len(lead)%len(unit) != 0 {
return 0, false
}
// Verify the lead is actually composed of repetitions of unit.
if strings.Repeat(unit, len(lead)/len(unit)) != lead {
return 0, false
}
return len(lead) / len(unit), true
}
// non-last line's ending replaced by ending; the last line keeps
// its original ending. Used before pass 1 splicing to normalize
// the replacement to the file's ending style.
func rewriteInternalEnding(lines []string, ending string) string {
var b strings.Builder
for i, l := range lines {
body, e := splitEnding(l)
_, _ = b.WriteString(body)
isLast := i == len(lines)-1
switch {
case isLast:
_, _ = b.WriteString(e)
case e == "":
// Non-last line without ending is only legal at EOF;
// leave the caller's shape alone.
default:
_, _ = b.WriteString(ending)
}
}
return b.String()
}
// splitLineParts decomposes a line into its leading whitespace
// (spaces and tabs only), middle body, trailing whitespace
// (spaces and tabs only), and line ending. Used by the fuzzy
// splice to substitute the file's whitespace at each position
// when search and replace agree on what that position should be.
func splitLineParts(line string) (lead, middle, trail, ending string) {
body, ending := splitEnding(line)
i := 0
for i < len(body) && (body[i] == ' ' || body[i] == '\t') {
i++
}
lead = body[:i]
rest := body[i:]
j := len(rest)
for j > 0 && (rest[j-1] == ' ' || rest[j-1] == '\t') {
j--
}
middle = rest[:j]
trail = rest[j:]
return lead, middle, trail, ending
}
// endingShapeEqual reports whether two line endings occupy the
// same "position class" for the splice substitution: both empty,
// or both in the newline class ({"\n", "\r\n"}). When this is
// true and the pair matched during matching, the splice uses the
// file's ending. When false, the splice keeps the replacement's
// ending verbatim (the caller is signaling an intentional fold
// or split). Unlike endingsMatch, empty is not a wildcard here:
// the splice phase needs a strict "same class" test so interior
// lines don't silently pick up a missing EOF terminator from the
// reference content.
func endingShapeEqual(a, b string) bool {
if a == b {
return true
}
return isNewlineEnding(a) && isNewlineEnding(b)
}
// buildReplacementLines emits the splice for a fuzzy match by
// per-position substitution at leading-ws, body, trailing-ws, and
// ending. Search and replace agreement at a position -> file's
// bytes win; disagreement -> replacement's bytes are spliced.
// Extra replace lines past the matched region reference the last
// search/content line.
//
// Carve-outs on "file wins on agreement":
// - Empty replacement body: emit the replacement's whitespace
// verbatim so a body-less line doesn't materialize whitespace.
// - Reference content line has no ending and this isn't the
// final replacement line: keep the replacement's newline so a
// multi-line splice at EOF doesn't collapse.
// - Inserted lines (no paired search line) try level-aware
// indent translation: if we can detect both the caller's
// search_unit and the file's fileUnit cleanly, the emitted
// lead is fileUnit * (file_base + (rep_level - search_base)).
// The caller's rep_level is computed from their own indent
// style; output in the file's style so a 4sp LLM inserting
// into a 2sp file emits 2sp indent at the correct depth. If
// detection fails (no indent info, mixed tabs+spaces, or
// a non-unit multiple), fall back to inheriting cLead.
//
// forcedEnding (from internalLineEnding normalization) overrides
// interior endings; the final ending is forced too unless
// atNoNewlineEOF (preserving the file's no-terminator EOF).
// When atNoNewlineEOF is false and the final ending would still
// be empty, force a terminator so unmatched content doesn't
// concatenate onto the splice.
//
// len(matched) == len(searchLines) is the invariant; callers
// slice contentLines before invoking.
//
//nolint:revive // atNoNewlineEOF is a computed match property, not caller control coupling.
func buildReplacementLines(matched, searchLines []string, replace, forcedEnding string, atNoNewlineEOF bool) string {
repLines := strings.SplitAfter(replace, "\n")
// SplitAfter on a string ending in "\n" yields a trailing empty
// element. Drop it so it doesn't pair with a phantom line.
if len(repLines) > 0 && repLines[len(repLines)-1] == "" {
repLines = repLines[:len(repLines)-1]
}
prefix, suffix := alignSearchReplace(searchLines, repLines)
// Combine search and replace so a zero-width search still
// informs the unit from the replacement's inserted depths.
// Fallback for detection failure lives in the inserted branch.
searchUnit, searchUnitOK := detectIndentUnit(append(append([]string(nil), searchLines...), repLines...))
fileUnit, fileUnitOK := detectIndentUnit(matched)
var b strings.Builder
for i, rLine := range repLines {
var refIdx int
inserted := false
searchMiddleLen := len(searchLines) - prefix - suffix
switch {
case i < prefix:
refIdx = i
case i >= len(repLines)-suffix:
refIdx = i - (len(repLines) - len(searchLines))
case i-prefix < searchMiddleLen:
refIdx = prefix + (i - prefix)
default:
// Pure insertion: pick the reference content line by
// the caller's indent signal. An inserted line whose
// lead matches the suffix's first rep line belongs to
// the suffix scope; one matching the prefix's last rep
// line belongs to the prefix scope. Fall back to
// suffix, then prefix, then i-clamped.
inserted = true
rLeadForI := leadOnly(rLine)
switch {
case prefix > 0 && suffix > 0:
prefixRLead := leadOnly(repLines[prefix-1])
suffixRLead := leadOnly(repLines[len(repLines)-suffix])
switch {
case rLeadForI == suffixRLead:
refIdx = len(searchLines) - suffix
case rLeadForI == prefixRLead:
refIdx = prefix - 1
default:
refIdx = len(searchLines) - suffix
}
case suffix > 0:
refIdx = len(searchLines) - suffix
case prefix > 0:
refIdx = prefix - 1
default:
refIdx = min(i, len(searchLines)-1)
}
}
refContent := matched[refIdx]
sLead, _, sTrail, sEnd := splitLineParts(searchLines[refIdx])
rLead, rMid, rTrail, rEnd := splitLineParts(rLine)
cLead, _, cTrail, cEnd := splitLineParts(refContent)
lead := rLead
trail := rTrail
switch {
case rMid == "":
// Body-less: emit the replacement's whitespace verbatim.
case inserted:
// Translate the caller's indent level to the file's
// unit; fall back to cLead when detection fails.
lead = cLead
if searchUnitOK && fileUnitOK {
if translated, ok := translateIndentLevel(rLead, sLead, cLead, searchUnit, fileUnit); ok {
lead = translated
}
}
default:
if sLead == rLead {
lead = cLead
}
if sTrail == rTrail {
trail = cTrail
}
}
ending := rEnd
if !inserted && endingShapeEqual(sEnd, rEnd) {
ending = cEnd
// Interior lines keep their newline when the reference
// content has cEnd="" (no-EOL EOF); only the final
// output line may inherit the empty ending.
if cEnd == "" && i < len(repLines)-1 {
ending = rEnd
}
}
if inserted && i == len(repLines)-1 && atNoNewlineEOF {
ending = ""
}
if forcedEnding != "" && (i < len(repLines)-1 || !atNoNewlineEOF) {
ending = forcedEnding
}
if i == len(repLines)-1 && !atNoNewlineEOF && ending == "" {
if forcedEnding != "" {
ending = forcedEnding
} else {
ending = "\n"
}
}
_, _ = b.WriteString(lead)
_, _ = b.WriteString(rMid)
_, _ = b.WriteString(trail)
_, _ = b.WriteString(ending)
}
return b.String()
}
// fuzzyReplace attempts to find `search` inside `content` and replace it
// with `replace`. It uses a cascading match strategy inspired by
// openai/codex's apply_patch:
@@ -569,17 +1086,67 @@ func (api *API) atomicWrite(ctx context.Context, path string, mode *os.FileMode,
// is returned asking the caller to include more context or set
// replace_all.
//
// When a fuzzy match is found (passes 2 or 3), the replacement is still
// applied at the byte offsets of the original content so that surrounding
// text (including indentation of untouched lines) is preserved.
// When a fuzzy match is found (passes 2 or 3), buildReplacementLines
// emits the spliced output by per-position substitution at
// leading-whitespace, body, trailing-whitespace, and ending: where
// search and replace agree at a position, the file's bytes win. This
// preserves surrounding text (including indentation of untouched
// lines) while letting the caller drive deliberate rewrites of
// leading whitespace or endings.
func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) {
search := edit.Search
replace := edit.Replace
// Pass 1 exact substring match.
// An empty search string has no meaningful interpretation: it
// matches at every byte position, which means the caller has not
// told us what they want to replace. Reject explicitly so
// replace_all=true can't silently inject the replacement between
// every byte.
if search == "" {
return "", xerrors.New("search string must not be empty; include the " +
"text you want to match")
}
// Split up front so the ending-normalization rule can inspect
// all three before any matching pass.
contentLines := strings.SplitAfter(content, "\n")
searchLines := strings.SplitAfter(search, "\n")
// A trailing newline in the search produces an empty final element
// from SplitAfter. Drop it so it doesn't interfere with line
// matching.
if len(searchLines) > 0 && searchLines[len(searchLines)-1] == "" {
searchLines = searchLines[:len(searchLines)-1]
}
replaceLines := strings.SplitAfter(replace, "\n")
if len(replaceLines) > 0 && replaceLines[len(replaceLines)-1] == "" {
replaceLines = replaceLines[:len(replaceLines)-1]
}
// Ending normalization. If replace has a consistent internal
// ending, force every spliced interior line to the file's
// dominant ending. If search also has a consistent internal
// ending and it disagrees with replace's, the caller signaled
// intent to rewrite endings; restrict the match to pass 1 so
// CRLF/LF interchange at pass 2 can't silently bridge a search
// that doesn't actually occur in the file.
var forcedEnding string
searchInternal, searchOK := internalLineEnding(searchLines)
replaceInternal, replaceOK := internalLineEnding(replaceLines)
if replaceOK {
forcedEnding = dominantFileEnding(contentLines)
}
callerEndingIntent := searchOK && replaceOK && searchInternal != replaceInternal
// Pass 1 - exact substring match. Normalize replace's interior
// endings to the file's style unless the caller's search/replace
// disagreement signaled intent to rewrite endings.
pass1Replace := replace
if forcedEnding != "" && !callerEndingIntent && replaceInternal != forcedEnding {
pass1Replace = rewriteInternalEnding(replaceLines, forcedEnding)
}
if strings.Contains(content, search) {
if edit.ReplaceAll {
return strings.ReplaceAll(content, search, replace), nil
return strings.ReplaceAll(content, search, pass1Replace), nil
}
count := strings.Count(content, search)
if count > 1 {
@@ -589,37 +1156,39 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) {
"replace_all to true", count)
}
// Exactly one match.
return strings.Replace(content, search, replace, 1), nil
return strings.Replace(content, search, pass1Replace, 1), nil
}
// For line-level fuzzy matching we split both content and search
// into lines.
contentLines := strings.SplitAfter(content, "\n")
searchLines := strings.SplitAfter(search, "\n")
// A trailing newline in the search produces an empty final element
// from SplitAfter. Drop it so it doesn't interfere with line
// matching.
if len(searchLines) > 0 && searchLines[len(searchLines)-1] == "" {
searchLines = searchLines[:len(searchLines)-1]
if callerEndingIntent {
// Intent signaled but pass 1 missed; reject rather than let
// pass 2's CRLF/LF interchange bridge a mismatched search.
return "", xerrors.New("search string not found in file. Verify the search " +
"string matches the file content exactly, including whitespace, " +
"indentation, and line endings")
}
trimRight := func(a, b string) bool {
return strings.TrimRight(a, " \t\r\n") == strings.TrimRight(b, " \t\r\n")
aContent, aEnding := splitEnding(a)
bContent, bEnding := splitEnding(b)
return endingsMatch(aEnding, bEnding) &&
strings.TrimRight(aContent, " \t") == strings.TrimRight(bContent, " \t")
}
trimAll := func(a, b string) bool {
return strings.TrimSpace(a) == strings.TrimSpace(b)
aContent, aEnding := splitEnding(a)
bContent, bEnding := splitEnding(b)
return endingsMatch(aEnding, bEnding) &&
strings.TrimSpace(aContent) == strings.TrimSpace(bContent)
}
// Pass 2 trim trailing whitespace on each line.
if result, matched, err := fuzzyReplaceLines(contentLines, searchLines, replace, trimRight, edit.ReplaceAll); matched {
if result, matched, err := fuzzyReplaceLines(contentLines, searchLines, replace, trimRight, edit.ReplaceAll, forcedEnding); matched {
return result, err
}
// Pass 3 trim all leading and trailing whitespace
// (indentation-tolerant). The replacement is inserted verbatim;
// callers must provide correctly indented replacement text.
if result, matched, err := fuzzyReplaceLines(contentLines, searchLines, replace, trimAll, edit.ReplaceAll); matched {
if result, matched, err := fuzzyReplaceLines(contentLines, searchLines, replace, trimAll, edit.ReplaceAll, forcedEnding); matched {
return result, err
}
@@ -670,20 +1239,6 @@ outer:
return count
}
// spliceLines replaces contentLines[start:end] with replacement text, returning
// the full content as a single string.
func spliceLines(contentLines []string, start, end int, replacement string) string {
var b strings.Builder
for _, l := range contentLines[:start] {
_, _ = b.WriteString(l)
}
_, _ = b.WriteString(replacement)
for _, l := range contentLines[end:] {
_, _ = b.WriteString(l)
}
return b.String()
}
// fuzzyReplaceLines handles fuzzy matching passes (2 and 3) for
// fuzzyReplace. When replaceAll is false and there are multiple
// matches, an error is returned. When replaceAll is true, all
@@ -699,6 +1254,7 @@ func fuzzyReplaceLines(
replace string,
eq func(a, b string) bool,
replaceAll bool,
forcedEnding string,
) (string, bool, error) {
start, end, ok := seekLines(contentLines, searchLines, eq)
if !ok {
@@ -712,11 +1268,22 @@ func fuzzyReplaceLines(
"context to make the match unique, or set "+
"replace_all to true", count)
}
return spliceLines(contentLines, start, end, replace), true, nil
var b strings.Builder
for _, l := range contentLines[:start] {
_, _ = b.WriteString(l)
}
_, _ = b.WriteString(buildReplacementLines(contentLines[start:end], searchLines, replace, forcedEnding, atNoNewlineEOF(contentLines, end)))
for _, l := range contentLines[end:] {
_, _ = b.WriteString(l)
}
return b.String(), true, nil
}
// Replace all: collect all match positions, then apply from last
// to first to preserve indices.
// Replace all: collect all match positions, then emit the
// output forward, interleaving unmatched spans with spliced
// replacements. Each match runs through the same per-position
// splice as single-replace, using its own matched content
// slice as the reference.
type lineMatch struct{ start, end int }
var matches []lineMatch
for i := 0; i <= len(contentLines)-len(searchLines); {
@@ -735,19 +1302,16 @@ func fuzzyReplaceLines(
}
}
// Apply replacements from last to first.
repLines := strings.SplitAfter(replace, "\n")
for i := len(matches) - 1; i >= 0; i-- {
m := matches[i]
newLines := make([]string, 0, m.start+len(repLines)+(len(contentLines)-m.end))
newLines = append(newLines, contentLines[:m.start]...)
newLines = append(newLines, repLines...)
newLines = append(newLines, contentLines[m.end:]...)
contentLines = newLines
}
var b strings.Builder
for _, l := range contentLines {
prev := 0
for _, m := range matches {
for _, l := range contentLines[prev:m.start] {
_, _ = b.WriteString(l)
}
_, _ = b.WriteString(buildReplacementLines(contentLines[m.start:m.end], searchLines, replace, forcedEnding, atNoNewlineEOF(contentLines, m.end)))
prev = m.end
}
for _, l := range contentLines[prev:] {
_, _ = b.WriteString(l)
}
return b.String(), true, nil
@@ -0,0 +1,298 @@
package agentfiles
import (
"testing"
"github.com/stretchr/testify/require"
)
// Direct unit tests for the indent-splice helpers. These test the
// functions in isolation so a helper bug surfaces here with a
// descriptive failure instead of as a rendered-file mismatch deep
// in an integration test.
func TestDetectIndentUnit(t *testing.T) {
t.Parallel()
tests := []struct {
name string
lines []string
wantUnit string
wantOK bool
}{
{
name: "Empty",
lines: nil,
wantUnit: "",
wantOK: false,
},
{
name: "NoIndent",
lines: []string{"foo\n", "bar\n"},
wantUnit: "",
wantOK: false,
},
{
name: "TabOnly",
lines: []string{"\tfoo\n", "\t\tbar\n"},
wantUnit: "\t",
wantOK: true,
},
{
name: "FourSpaceUniform",
lines: []string{" foo\n", " bar\n"},
wantUnit: " ",
wantOK: true,
},
{
name: "TwoSpaceUniform",
lines: []string{" foo\n", " bar\n"},
wantUnit: " ",
wantOK: true,
},
{
name: "GCDReducesFourAndSixToTwo",
lines: []string{" foo\n", " bar\n"},
wantUnit: " ",
wantOK: true,
},
{
name: "MixedAcrossLinesTabAndSpace",
lines: []string{"\tfoo\n", " bar\n"},
wantUnit: "",
wantOK: false,
},
{
name: "MixedWithinLeadTabThenSpace",
lines: []string{"\t foo\n"},
wantUnit: "",
wantOK: false,
},
{
name: "MixedWithinLeadSpaceThenTab",
lines: []string{" \tfoo\n"},
wantUnit: "",
wantOK: false,
},
{
// DEREM-33 regression: a 2sp whitespace-only line in
// a 4sp-indented region must not pull the GCD down.
name: "WhitespaceOnlyLineSkipped",
lines: []string{" foo\n", " \n", " bar\n"},
wantUnit: " ",
wantOK: true,
},
{
name: "OnlyWhitespaceOnlyLines",
lines: []string{" \n", " \n"},
wantUnit: "",
wantOK: false,
},
{
name: "BlankLineIgnored",
lines: []string{"\n", " foo\n"},
wantUnit: " ",
wantOK: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
gotUnit, gotOK := detectIndentUnit(tc.lines)
require.Equal(t, tc.wantUnit, gotUnit)
require.Equal(t, tc.wantOK, gotOK)
})
}
}
func TestIndentGCD(t *testing.T) {
t.Parallel()
tests := []struct {
name string
a, b int
want int
}{
{"BothZero", 0, 0, 0},
{"AZero", 0, 4, 4},
{"BZero", 4, 0, 4},
{"Equal", 4, 4, 4},
{"Coprime", 3, 5, 1},
{"CommonFactorTwo", 4, 6, 2},
{"CommonFactorFour", 8, 12, 4},
{"TwoSpaceAndFourSpace", 2, 4, 2},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tc.want, indentGCD(tc.a, tc.b))
})
}
}
func TestIndentLevel(t *testing.T) {
t.Parallel()
tests := []struct {
name string
lead string
unit string
wantLevel int
wantOK bool
}{
{
name: "EmptyLead",
lead: "",
unit: " ",
wantLevel: 0,
wantOK: true,
},
{
name: "CleanMultipleOne",
lead: " ",
unit: " ",
wantLevel: 1,
wantOK: true,
},
{
name: "CleanMultipleThreeTwoSp",
lead: " ",
unit: " ",
wantLevel: 3,
wantOK: true,
},
{
name: "CleanMultipleTwoTab",
lead: "\t\t",
unit: "\t",
wantLevel: 2,
wantOK: true,
},
{
name: "NonMultipleLength",
lead: " ",
unit: " ",
wantLevel: 0,
wantOK: false,
},
{
// Even when the length divides evenly, the lead must
// be composed of repetitions of the unit.
name: "LengthDividesButCompositionMismatches",
lead: "\t ",
unit: " ",
wantLevel: 0,
wantOK: false,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
gotLevel, gotOK := indentLevel(tc.lead, tc.unit)
require.Equal(t, tc.wantLevel, gotLevel)
require.Equal(t, tc.wantOK, gotOK)
})
}
}
func TestTranslateIndentLevel(t *testing.T) {
t.Parallel()
tests := []struct {
name string
rLead string
sLead string
cLead string
searchUnit string
fileUnit string
want string
wantOK bool
}{
{
// Caller sends a 4sp search; inserted line is 8sp
// (one level deeper). File uses tabs, matched at
// 1-tab depth. Expected: 2 tabs.
name: "PositiveDeltaWrap",
rLead: " ",
sLead: " ",
cLead: "\t",
searchUnit: " ",
fileUnit: "\t",
want: "\t\t",
wantOK: true,
},
{
// Inserted line at the same level as its reference.
name: "ZeroDeltaSameLevel",
rLead: " ",
sLead: " ",
cLead: "\t",
searchUnit: " ",
fileUnit: "\t",
want: "\t",
wantOK: true,
},
{
// Inserted line shallower than the reference's
// level by more than the file_base: target goes
// negative, helper bails.
name: "NegativeDeltaBelowFileBase",
rLead: "",
sLead: " ",
cLead: "\t",
searchUnit: " ",
fileUnit: "\t",
want: "",
wantOK: false,
},
{
// Malformed rLead (3 spaces under a 4sp unit).
name: "MalformedRLead",
rLead: " ",
sLead: " ",
cLead: "\t",
searchUnit: " ",
fileUnit: "\t",
want: "",
wantOK: false,
},
{
// 4sp LLM into a 2sp file at matched-4sp baseline.
// rep_level=2, search_base=1, file_base=2,
// target=3, emit " " (6sp).
name: "CrossStyle4spTo2sp",
rLead: " ",
sLead: " ",
cLead: " ",
searchUnit: " ",
fileUnit: " ",
want: " ",
wantOK: true,
},
{
// 2sp LLM into a tab file.
// rep_level=2, search_base=1, file_base=1,
// target=2, emit "\t\t".
name: "CrossStyle2spToTab",
rLead: " ",
sLead: " ",
cLead: "\t",
searchUnit: " ",
fileUnit: "\t",
want: "\t\t",
wantOK: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
got, gotOK := translateIndentLevel(tc.rLead, tc.sLead, tc.cLead, tc.searchUnit, tc.fileUnit)
require.Equal(t, tc.want, got)
require.Equal(t, tc.wantOK, gotOK)
})
}
}
File diff suppressed because it is too large Load Diff
+9 -2
View File
@@ -96,8 +96,15 @@ func executeEditFilesTool(
}
}
if err := conn.EditFiles(ctx, workspacesdk.FileEditRequest{Files: args.Files}); err != nil {
resp, err := conn.EditFiles(ctx, workspacesdk.FileEditRequest{
Files: args.Files,
IncludeDiff: true,
})
if err != nil {
return fantasy.NewTextErrorResponse(err.Error()), nil
}
return toolResponse(map[string]any{"ok": true}), nil
return toolResponse(map[string]any{
"ok": true,
"files": resp.Files,
}), nil
}
+123 -49
View File
@@ -2,6 +2,7 @@ package chattool_test
import (
"context"
"encoding/json"
"net/http"
"testing"
@@ -85,14 +86,17 @@ func TestEditFiles(t *testing.T) {
planPath := "/home/coder/.coder/plans/PLAN-test-uuid.md"
resolvePlanPathCalls := 0
mockConn.EXPECT().ResolvePath(gomock.Any(), planPath).Return(planPath, nil)
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
Path: planPath,
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
request := workspacesdk.FileEditRequest{
Files: []workspacesdk.FileEdits{{
Path: planPath,
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
}},
}},
}}}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
IncludeDiff: true,
}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(workspacesdk.FileEditResponse{}, nil)
tool := chattool.EditFiles(chattool.EditFilesOptions{
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
@@ -123,14 +127,17 @@ func TestEditFiles(t *testing.T) {
mockConn.EXPECT().
ResolvePath(gomock.Any(), planPath).
Return("", statusError{statusCode: http.StatusNotFound, message: "missing resolve-path endpoint"})
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
Path: planPath,
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
request := workspacesdk.FileEditRequest{
Files: []workspacesdk.FileEdits{{
Path: planPath,
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
}},
}},
}}}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
IncludeDiff: true,
}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(workspacesdk.FileEditResponse{}, nil)
tool := chattool.EditFiles(chattool.EditFilesOptions{
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
@@ -290,14 +297,17 @@ func TestEditFiles(t *testing.T) {
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
chatPlanPath := "/home/coder/.coder/plans/PLAN-123e4567-e89b-12d3-a456-426614174000.md"
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
Path: chatPlanPath,
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
request := workspacesdk.FileEditRequest{
Files: []workspacesdk.FileEdits{{
Path: chatPlanPath,
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
}},
}},
}}}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
IncludeDiff: true,
}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(workspacesdk.FileEditResponse{}, nil)
resolvePlanPathCalled := false
tool := chattool.EditFiles(chattool.EditFilesOptions{
@@ -324,14 +334,17 @@ func TestEditFiles(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
Path: "/home/coder/myproject/plan.md",
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
request := workspacesdk.FileEditRequest{
Files: []workspacesdk.FileEdits{{
Path: "/home/coder/myproject/plan.md",
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
}},
}},
}}}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
IncludeDiff: true,
}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(workspacesdk.FileEditResponse{}, nil)
tool := chattool.EditFiles(chattool.EditFilesOptions{
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
@@ -355,14 +368,17 @@ func TestEditFiles(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
Path: "/home/coder/myproject/plan.md",
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
request := workspacesdk.FileEditRequest{
Files: []workspacesdk.FileEdits{{
Path: "/home/coder/myproject/plan.md",
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
}},
}},
}}}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
IncludeDiff: true,
}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(workspacesdk.FileEditResponse{}, nil)
planPathCalled := false
tool := chattool.EditFiles(chattool.EditFilesOptions{
@@ -389,14 +405,17 @@ func TestEditFiles(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
Path: "/home/dev/my-plan.md",
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
request := workspacesdk.FileEditRequest{
Files: []workspacesdk.FileEdits{{
Path: "/home/dev/my-plan.md",
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
}},
}},
}}}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
IncludeDiff: true,
}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(workspacesdk.FileEditResponse{}, nil)
resolvePlanPathCalled := false
tool := chattool.EditFiles(chattool.EditFilesOptions{
@@ -423,14 +442,17 @@ func TestEditFiles(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
Path: chattool.LegacySharedPlanPath,
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
request := workspacesdk.FileEditRequest{
Files: []workspacesdk.FileEdits{{
Path: chattool.LegacySharedPlanPath,
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
}},
}},
}}}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
IncludeDiff: true,
}
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(workspacesdk.FileEditResponse{}, nil)
tool := chattool.EditFiles(chattool.EditFilesOptions{
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
@@ -447,3 +469,55 @@ func TestEditFiles(t *testing.T) {
assert.False(t, resp.IsError)
})
}
func TestEditFiles_ToolResponseCarriesFileResults(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
targetPath := "/home/coder/target.txt"
expectedFiles := []workspacesdk.FileEditResult{
{
Path: targetPath,
Diff: "--- " + targetPath + "\n+++ " + targetPath + "\n@@ -1 +1 @@\n-old\n+new\n",
},
}
// The tool must opt into diffs (IncludeDiff: true) and forward
// the agent's per-file results through to its response.
mockConn.EXPECT().
EditFiles(gomock.Any(), workspacesdk.FileEditRequest{
Files: []workspacesdk.FileEdits{{
Path: targetPath,
Edits: []workspacesdk.FileEdit{{
Search: "old",
Replace: "new",
}},
}},
IncludeDiff: true,
}).
Return(workspacesdk.FileEditResponse{Files: expectedFiles}, nil)
tool := chattool.EditFiles(chattool.EditFilesOptions{
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
return mockConn, nil
},
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "edit_files",
Input: `{"files":[{"path":"` + targetPath + `","edits":[{"search":"old","replace":"new"}]}]}`,
})
require.NoError(t, err)
assert.False(t, resp.IsError)
var decoded struct {
OK bool `json:"ok"`
Files []workspacesdk.FileEditResult `json:"files"`
}
require.NoError(t, json.Unmarshal([]byte(resp.Content), &decoded))
assert.True(t, decoded.OK)
require.Len(t, decoded.Files, 1)
assert.Equal(t, targetPath, decoded.Files[0].Path)
assert.Equal(t, expectedFiles[0].Diff, decoded.Files[0].Diff)
}
+30 -12
View File
@@ -1665,7 +1665,19 @@ type WorkspaceEditFileArgs struct {
Edits []workspacesdk.FileEdit `json:"edits"`
}
var WorkspaceEditFile = Tool[WorkspaceEditFileArgs, codersdk.Response]{
// WorkspaceEditFilesResponse is the response shape for the edit-file
// and edit-files tools. Message preserves the existing success text.
// Files carries the per-file results returned by the agent
// (populated when the agent-side IncludeDiff flag was set). The
// field is named Files (matching the agent's FileEditResponse.Files)
// so future per-file error or status fields can be added without a
// second wire break.
type WorkspaceEditFilesResponse struct {
Message string `json:"message"`
Files []workspacesdk.FileEditResult `json:"files,omitempty"`
}
var WorkspaceEditFile = Tool[WorkspaceEditFileArgs, WorkspaceEditFilesResponse]{
Tool: aisdk.Tool{
Name: ToolNameWorkspaceEditFile,
Description: `Edit a file in a workspace.`,
@@ -1703,27 +1715,29 @@ var WorkspaceEditFile = Tool[WorkspaceEditFileArgs, codersdk.Response]{
},
MCPAnnotations: mcpDestructiveAnnotations,
UserClientOptional: true,
Handler: func(ctx context.Context, deps Deps, args WorkspaceEditFileArgs) (codersdk.Response, error) {
Handler: func(ctx context.Context, deps Deps, args WorkspaceEditFileArgs) (WorkspaceEditFilesResponse, error) {
conn, err := newAgentConn(ctx, deps.coderClient, args.Workspace)
if err != nil {
return codersdk.Response{}, err
return WorkspaceEditFilesResponse{}, err
}
defer conn.Close()
err = conn.EditFiles(ctx, workspacesdk.FileEditRequest{
resp, err := conn.EditFiles(ctx, workspacesdk.FileEditRequest{
Files: []workspacesdk.FileEdits{
{
Path: args.Path,
Edits: args.Edits,
},
},
IncludeDiff: true,
})
if err != nil {
return codersdk.Response{}, err
return WorkspaceEditFilesResponse{}, err
}
return codersdk.Response{
return WorkspaceEditFilesResponse{
Message: "File edited successfully.",
Files: resp.Files,
}, nil
},
}
@@ -1733,7 +1747,7 @@ type WorkspaceEditFilesArgs struct {
Files []workspacesdk.FileEdits `json:"files"`
}
var WorkspaceEditFiles = Tool[WorkspaceEditFilesArgs, codersdk.Response]{
var WorkspaceEditFiles = Tool[WorkspaceEditFilesArgs, WorkspaceEditFilesResponse]{
Tool: aisdk.Tool{
Name: ToolNameWorkspaceEditFiles,
Description: `Edit one or more files in a workspace.`,
@@ -1785,20 +1799,24 @@ var WorkspaceEditFiles = Tool[WorkspaceEditFilesArgs, codersdk.Response]{
},
MCPAnnotations: mcpDestructiveAnnotations,
UserClientOptional: true,
Handler: func(ctx context.Context, deps Deps, args WorkspaceEditFilesArgs) (codersdk.Response, error) {
Handler: func(ctx context.Context, deps Deps, args WorkspaceEditFilesArgs) (WorkspaceEditFilesResponse, error) {
conn, err := newAgentConn(ctx, deps.coderClient, args.Workspace)
if err != nil {
return codersdk.Response{}, err
return WorkspaceEditFilesResponse{}, err
}
defer conn.Close()
err = conn.EditFiles(ctx, workspacesdk.FileEditRequest{Files: args.Files})
resp, err := conn.EditFiles(ctx, workspacesdk.FileEditRequest{
Files: args.Files,
IncludeDiff: true,
})
if err != nil {
return codersdk.Response{}, err
return WorkspaceEditFilesResponse{}, err
}
return codersdk.Response{
return WorkspaceEditFilesResponse{
Message: "File(s) edited successfully.",
Files: resp.Files,
}, nil
},
}
+36 -8
View File
@@ -86,7 +86,7 @@ type AgentConn interface {
ReadFile(ctx context.Context, path string, offset, limit int64) (io.ReadCloser, string, error)
ReadFileLines(ctx context.Context, path string, offset, limit int64, limits ReadFileLinesLimits) (ReadFileLinesResponse, error)
WriteFile(ctx context.Context, path string, reader io.Reader) error
EditFiles(ctx context.Context, edits FileEditRequest) error
EditFiles(ctx context.Context, edits FileEditRequest) (FileEditResponse, error)
SSH(ctx context.Context) (*gonet.TCPConn, error)
SSHClient(ctx context.Context) (*ssh.Client, error)
SSHClientOnPort(ctx context.Context, port uint16) (*ssh.Client, error)
@@ -1043,6 +1043,32 @@ type FileEdits struct {
type FileEditRequest struct {
Files []FileEdits `json:"files"`
// IncludeDiff asks the agent to compute a unified diff per file
// and return it in FileEditResponse.Files[i].Diff. When false
// (default) the agent skips diff computation and Files is nil.
IncludeDiff bool `json:"include_diff,omitempty"`
}
// FileEditResponse is the success response for the edit-files endpoint.
// When the request's IncludeDiff flag is set, Files contains one entry
// per edited file in request order. Each entry's Path matches the
// caller-supplied path (pre-symlink resolution).
//
// The slice is named Files (rather than Diffs) so future work can
// hang per-file errors or status off each element without a second
// wire break.
type FileEditResponse struct {
Files []FileEditResult `json:"files,omitempty"`
}
// FileEditResult carries the outcome of editing one file. Path is
// the original caller-supplied path, not any symlink-resolved
// target. Diff is the unified-diff string produced when the
// caller set FileEditRequest.IncludeDiff; it is empty for no-op
// edits or when diffs were not requested.
type FileEditResult struct {
Path string `json:"path"`
Diff string `json:"diff"`
}
// ListMCPToolsResponse is the response from the agent's
@@ -1219,24 +1245,26 @@ func (c *agentConn) SignalProcess(ctx context.Context, id string, signal string)
}
// EditFiles performs search and replace edits on one or more files.
func (c *agentConn) EditFiles(ctx context.Context, edits FileEditRequest) error {
// When edits.IncludeDiff is true, the returned FileEditResponse
// carries a unified diff per edited file.
func (c *agentConn) EditFiles(ctx context.Context, edits FileEditRequest) (FileEditResponse, error) {
ctx, span := tracing.StartSpan(ctx)
defer span.End()
res, err := c.apiRequest(ctx, http.MethodPost, "/api/v0/edit-files", edits)
if err != nil {
return xerrors.Errorf("do request: %w", err)
return FileEditResponse{}, xerrors.Errorf("do request: %w", err)
}
defer res.Body.Close()
if res.StatusCode != http.StatusOK {
return codersdk.ReadBodyAsError(res)
return FileEditResponse{}, codersdk.ReadBodyAsError(res)
}
var m codersdk.Response
if err := json.NewDecoder(res.Body).Decode(&m); err != nil {
return xerrors.Errorf("decode response body: %w", err)
var resp FileEditResponse
if err := json.NewDecoder(res.Body).Decode(&resp); err != nil {
return FileEditResponse{}, xerrors.Errorf("decode response body: %w", err)
}
return nil
return resp, nil
}
func agentAPIPath(path string, query neturl.Values) string {
@@ -204,11 +204,12 @@ func (mr *MockAgentConnMockRecorder) DialContext(ctx, network, addr any) *gomock
}
// EditFiles mocks base method.
func (m *MockAgentConn) EditFiles(ctx context.Context, edits workspacesdk.FileEditRequest) error {
func (m *MockAgentConn) EditFiles(ctx context.Context, edits workspacesdk.FileEditRequest) (workspacesdk.FileEditResponse, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "EditFiles", ctx, edits)
ret0, _ := ret[0].(error)
return ret0
ret0, _ := ret[0].(workspacesdk.FileEditResponse)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// EditFiles indicates an expected call of EditFiles.
+1
View File
@@ -496,6 +496,7 @@ require (
require (
charm.land/fantasy v0.8.1
github.com/anthropics/anthropic-sdk-go v1.19.0
github.com/aymanbagabas/go-udiff v0.4.1
github.com/brianvoe/gofakeit/v7 v7.14.0
github.com/coder/agentapi-sdk-go v0.0.0-20250505131810-560d1d88d225
github.com/coder/aibridge v1.1.2
+2 -2
View File
@@ -203,8 +203,8 @@ github.com/aws/smithy-go v1.24.2 h1:FzA3bu/nt/vDvmnkg+R8Xl46gmzEDam6mZ1hzmwXFng=
github.com/aws/smithy-go v1.24.2/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc=
github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k=
github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8=
github.com/aymanbagabas/go-udiff v0.3.1 h1:LV+qyBQ2pqe0u42ZsUEtPiCaUoqgA9gYRDs3vj1nolY=
github.com/aymanbagabas/go-udiff v0.3.1/go.mod h1:G0fsKmG+P6ylD0r6N/KgQD/nWzgfnl8ZBcNLgcbrw8E=
github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ12Gv5o=
github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w=
github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk=
github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4=
github.com/bahlo/generic-list-go v0.2.0 h1:5sz/EEAK+ls5wF+NeqDpk5+iNdMDXrh3z3nPnH1Wvgk=
@@ -85,6 +85,7 @@ export const EditFilesTool: React.FC<{
diff ? (
<ScrollArea
key={files[i].path}
data-testid="edit-file-diff"
className="rounded-md border border-solid border-border-default text-2xs"
viewportClassName="max-h-64"
scrollBarClassName="w-1.5"
@@ -896,6 +896,160 @@ export const EditFilesError: Story = {
},
result: { error: "File not found" },
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
expect(canvas.getByText(/Edited missing\.ts/)).toBeInTheDocument();
// On error, no diff body: the synthetic fallback would
// misrepresent a rejected edit as applied.
expect(canvas.queryAllByTestId("edit-file-diff")).toHaveLength(0);
},
};
export const EditFilesServerDiffMultiFile: Story = {
args: {
name: "edit_files",
status: "completed",
args: {
files: [
{
path: "src/config.ts",
edits: [
{
search: "const timeout = 30;",
replace: "const timeout = 60;",
},
],
},
{
path: "src/server.ts",
edits: [
{
search: 'const host = "localhost";',
replace: 'const host = "0.0.0.0";',
},
],
},
],
},
result: {
ok: true,
files: [
{
path: "src/config.ts",
diff: "--- src/config.ts\n+++ src/config.ts\n@@ -1,3 +1,3 @@\n export const settings = {\n-\tconst timeout = 30;\n+\tconst timeout = 60;\n };\n",
},
{
path: "src/server.ts",
diff: '--- src/server.ts\n+++ src/server.ts\n@@ -1,3 +1,3 @@\n export const server = {\n-\tconst host = "localhost";\n+\tconst host = "0.0.0.0";\n };\n',
},
],
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
expect(canvas.getByText(/Edited 2 files/)).toBeInTheDocument();
// Both server diffs must render. FileDiff's internals aren't
// queryable from jsdom; count the testid'd wrappers instead.
expect(canvas.getAllByTestId("edit-file-diff")).toHaveLength(2);
},
};
export const EditFilesServerDiffNoOp: Story = {
args: {
name: "edit_files",
status: "completed",
args: {
files: [
{
path: "src/unchanged.ts",
edits: [
{
search: "same",
replace: "same",
},
],
},
],
},
result: {
ok: true,
files: [{ path: "src/unchanged.ts", diff: "" }],
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
expect(canvas.getByText(/Edited unchanged\.ts/)).toBeInTheDocument();
expect(canvas.queryAllByTestId("edit-file-diff")).toHaveLength(0);
},
};
export const EditFilesFallbackToSynthetic: Story = {
args: {
name: "edit_files",
status: "completed",
args: {
files: [
{
path: "src/legacy.ts",
edits: [
{
search: "const timeout = 30;",
replace: "const timeout = 60;",
},
],
},
],
},
result: { ok: true },
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
expect(canvas.getByText(/Edited legacy\.ts/)).toBeInTheDocument();
expect(canvas.getAllByTestId("edit-file-diff")).toHaveLength(1);
},
};
export const EditFilesServerDiffPartialFallback: Story = {
args: {
name: "edit_files",
status: "completed",
args: {
files: [
{
path: "src/config.ts",
edits: [
{
search: "const timeout = 30;",
replace: "const timeout = 60;",
},
],
},
{
path: "src/server.ts",
edits: [
{
search: 'const host = "localhost";',
replace: 'const host = "0.0.0.0";',
},
],
},
],
},
result: {
ok: true,
files: [
{
path: "src/config.ts",
diff: "--- src/config.ts\n+++ src/config.ts\n@@ -1,3 +1,3 @@\n export const settings = {\n-\tconst timeout = 30;\n+\tconst timeout = 60;\n };\n",
},
],
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
expect(canvas.getByText(/Edited 2 files/)).toBeInTheDocument();
expect(canvas.getAllByTestId("edit-file-diff")).toHaveLength(2);
},
};
// ---------------------------------------------------------------------------
@@ -50,10 +50,13 @@ import {
mapSubagentStatusToToolStatus,
parseArgs,
parseEditFilesArgs,
parseServerEditDiffText,
parseServerEditResults,
stripNoNewline,
type ToolStatus,
toProviderLabel,
} from "./utils";
import { WriteFileTool } from "./WriteFileTool";
interface ToolProps extends Omit<ComponentPropsWithRef<"div">, "children"> {
@@ -383,9 +386,17 @@ const EditFilesRenderer: FC<ToolRendererProps> = ({
}) => {
const rec = asRecord(result);
const editFiles = parseEditFilesArgs(args);
const editDiffs = editFiles.map((file) =>
buildEditDiff(file.path, file.edits),
);
// On error, render no diff: the agent rejected the edit, so a
// synthetic args-derived diff would misrepresent it as applied.
const serverResults = parseServerEditResults(result);
const editDiffs = isError
? editFiles.map(() => null)
: editFiles.map((file) => {
const entry = serverResults?.find((d) => d.path === file.path);
return entry
? parseServerEditDiffText(entry.diff)
: buildEditDiff(file.path, file.edits);
});
return (
<EditFilesTool
@@ -22,6 +22,8 @@ import {
normalizeStatus,
parseArgs,
parseEditFilesArgs,
parseServerEditDiffText,
parseServerEditResults,
shortDurationMs,
stripSvnIndexHeaders,
toProviderLabel,
@@ -854,3 +856,68 @@ describe("constants", () => {
expect(diffViewerCSS).toContain("border-left");
});
});
describe("parseServerEditResults", () => {
it("returns null when result is not a record", () => {
expect(parseServerEditResults(null)).toBeNull();
expect(parseServerEditResults(undefined)).toBeNull();
expect(parseServerEditResults("foo")).toBeNull();
});
it("returns null when files field is absent", () => {
expect(parseServerEditResults({ ok: true })).toBeNull();
});
it("returns null when files is explicitly null (older agents)", () => {
expect(parseServerEditResults({ files: null })).toBeNull();
});
it("returns an empty array when files is an empty array", () => {
expect(parseServerEditResults({ files: [] })).toEqual([]);
});
it("parses well-formed entries", () => {
const result = parseServerEditResults({
files: [
{
path: "/abs/a.txt",
diff: "--- /abs/a.txt\n+++ /abs/a.txt\n@@ -1 +1 @@\n-a\n+A\n",
},
{ path: "/abs/b.txt", diff: "" },
],
});
expect(result).toEqual([
{
path: "/abs/a.txt",
diff: "--- /abs/a.txt\n+++ /abs/a.txt\n@@ -1 +1 @@\n-a\n+A\n",
},
{ path: "/abs/b.txt", diff: "" },
]);
});
it("skips malformed entries without dropping the rest", () => {
const result = parseServerEditResults({
files: [
null,
{ diff: "orphan" },
{ path: "", diff: "empty-path" },
{ path: "/ok", diff: "--- /ok\n+++ /ok\n" },
],
});
expect(result).toEqual([{ path: "/ok", diff: "--- /ok\n+++ /ok\n" }]);
});
});
describe("parseServerEditDiffText", () => {
it("returns null for an empty string (no-op edit)", () => {
expect(parseServerEditDiffText("")).toBeNull();
});
it("parses a unified diff into a FileDiffMetadata", () => {
const diff = parseServerEditDiffText(
"--- /abs/a.txt\n+++ /abs/a.txt\n@@ -1 +1 @@\n-hello\n+HELLO\n",
);
expect(diff).not.toBeNull();
expect(diff?.name).toBe("/abs/a.txt");
});
});
@@ -476,6 +476,20 @@ export const getFileContentForViewer = (
return { path, content };
};
/**
* Parses a unified-diff string (with an optional SVN `Index:`
* banner) into the first FileDiffMetadata it contains. Returns
* null when the input is empty or the parser produces no files.
* Shared between the write_file diff builder, the synthetic
* edit_files diff builder, and the server-supplied diff parser.
*/
const parseSingleFileDiff = (raw: string): FileDiffMetadata | null => {
if (!raw) return null;
const parsed = parsePatchFiles(stripSvnIndexHeaders(raw));
if (!parsed.length || !parsed[0].files.length) return null;
return parsed[0].files[0];
};
/**
* Builds a FileDiffMetadata representing a new-file diff (all lines
* are additions) from the content written by a write_file tool call.
@@ -486,10 +500,7 @@ export const buildWriteFileDiff = (
content: string,
): FileDiffMetadata | null => {
if (!content) return null;
const patch = Diff.createPatch(path, "", content, "", "");
const parsed = parsePatchFiles(stripSvnIndexHeaders(patch));
if (!parsed.length || !parsed[0].files.length) return null;
return parsed[0].files[0];
return parseSingleFileDiff(Diff.createPatch(path, "", content, "", ""));
};
/**
@@ -568,11 +579,54 @@ export const buildEditDiff = (
patches.push(`--- ${diffPath}\n+++ ${diffPath}\n`);
}
const parsed = parsePatchFiles(stripSvnIndexHeaders(patches.join("")));
if (!parsed.length || !parsed[0].files.length) return null;
return parsed[0].files[0];
return parseSingleFileDiff(patches.join(""));
};
/**
* Per-file result from the agent's FileEditResponse. `path` matches
* the caller-supplied path (pre-symlink resolution). `diff` is a
* unified-diff string, possibly empty for no-op edits.
*/
interface ServerEditResult {
path: string;
diff: string;
}
/**
* Parses the structured `files` array from an edit_files tool
* response. The field is only populated when the agent observed the
* request's `include_diff` flag; older agents omit it entirely.
* Returns null when no per-file result array is present (callers
* should fall back to the synthetic client-side diff path). Returns
* an empty array when the field is explicitly present but empty.
*/
export const parseServerEditResults = (
result: unknown,
): ServerEditResult[] | null => {
const rec = asRecord(result);
if (!rec) return null;
const raw = rec.files;
if (raw === undefined || raw === null) return null;
if (!Array.isArray(raw)) return null;
const results: ServerEditResult[] = [];
for (const entry of raw) {
const entryRec = asRecord(entry);
if (!entryRec) continue;
const path = asString(entryRec.path).trim();
if (!path) continue;
results.push({ path, diff: asString(entryRec.diff) });
}
return results;
};
/**
* Parses a server-supplied unified diff. Returns null for empty
* strings (no-op edits) or when the parser produces no file entries.
*/
export const parseServerEditDiffText = (
diff: string,
): FileDiffMetadata | null => parseSingleFileDiff(diff);
/**
* Converts an MCP-prefixed tool name into a human-readable label.
* E.g. "linear__list_issues" with slug "linear" "List issues"