fix(agent/agentfiles): merge duplicate file paths instead of rejecting (#25767)

When a caller sends multiple entries for the same literal path, merge
their edits into a single entry rather than returning 400. Symlink
aliases (different paths, same real file) are still rejected.

agent/agentfiles/files.go

@@ -387,17 +387,17 @@ 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.
+	// Merge duplicate entries that refer to the same literal path
+	// so callers don't have to pre-coalesce. Two different paths
+	// that resolve to the same real file via symlinks are still
+	// rejected: silently merging edits the caller addressed to
+	// different paths would hide accidental aliasing.
 	type seenEntry struct {
 		caller string
+		index  int // position in merged slice
 	}
 	seenPaths := make(map[string]seenEntry, len(req.Files))
+	var merged []workspacesdk.FileEdits
 	for _, f := range req.Files {
 		// On resolve error, use the raw path; phase 1 surfaces
 		// the error with its proper status code.
@@ -406,17 +406,22 @@ func (api *API) HandleEditFiles(rw http.ResponseWriter, r *http.Request) {
 			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)
+			// Same literal path: merge edits.
+			if filepath.Clean(prev.caller) == filepath.Clean(f.Path) {
+				merged[prev.index].Edits = append(merged[prev.index].Edits, f.Edits...)
+				continue
 			}
+			// Different paths, same real file (symlink alias).
+			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}
+		seenPaths[key] = seenEntry{caller: f.Path, index: len(merged)}
+		merged = append(merged, f)
 	}
+	req.Files = merged
 
 	// Phase 1: compute all edits in memory. If any file fails
 	// (bad path, search miss, permission error), bail before

agent/agentfiles/files_test.go

@@ -2622,11 +2622,10 @@ func TestFuzzyReplace_Rejects(t *testing.T) {
 	}
 }
 
-// TestEditFiles_DuplicatePath_Rejects pins that duplicate paths in
-// one request are rejected with 400 and the file on disk is
-// unchanged. The pre-fix behavior silently dropped the first
-// entry's edits while reporting success (last write wins).
-func TestEditFiles_DuplicatePath_Rejects(t *testing.T) {
+// TestEditFiles_DuplicatePath_Merges verifies that duplicate paths in
+// one request are merged: edits from all entries for the same path are
+// concatenated and applied in order.
+func TestEditFiles_DuplicatePath_Merges(t *testing.T) {
 	t.Parallel()
 
 	tmpdir := os.TempDir()
@@ -2637,10 +2636,12 @@ func TestEditFiles_DuplicatePath_Rejects(t *testing.T) {
 	original := "one\ntwo\nthree\n"
 	require.NoError(t, afero.WriteFile(fs, path, []byte(original), 0o644))
 
+	// Entry 2 searches for the output of entry 1, proving edits
+	// are applied in the order they appear across entries.
 	req := workspacesdk.FileEditRequest{
 		Files: []workspacesdk.FileEdits{
-			{Path: path, Edits: []workspacesdk.FileEdit{{Search: "one", Replace: "ONE"}}},
-			{Path: path, Edits: []workspacesdk.FileEdit{{Search: "three", Replace: "THREE"}}},
+			{Path: path, Edits: []workspacesdk.FileEdit{{Search: "one", Replace: "CHANGED"}}},
+			{Path: path, Edits: []workspacesdk.FileEdit{{Search: "CHANGED", Replace: "FINAL"}}},
 		},
 	}
 
@@ -2653,15 +2654,49 @@ func TestEditFiles_DuplicatePath_Rejects(t *testing.T) {
 	r := httptest.NewRequestWithContext(ctx, http.MethodPost, "/edit-files", buf)
 	api.Routes().ServeHTTP(w, r)
 
-	require.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String())
-	got := &codersdk.Error{}
-	require.NoError(t, json.NewDecoder(w.Body).Decode(got))
-	require.ErrorContains(t, got, "duplicate file path")
+	require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
 
-	// File on disk must be untouched: no partial edits.
 	data, err := afero.ReadFile(fs, path)
 	require.NoError(t, err)
-	require.Equal(t, original, string(data))
+	require.Equal(t, "FINAL\ntwo\nthree\n", string(data))
+}
+
+// TestEditFiles_DuplicatePath_NonCanonicalMerges verifies that
+// non-canonical paths normalizing to the same file are merged,
+// not rejected as symlink aliases.
+func TestEditFiles_DuplicatePath_NonCanonicalMerges(t *testing.T) {
+	t.Parallel()
+
+	tmpdir := os.TempDir()
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
+	fs := afero.NewMemMapFs()
+	api := agentfiles.NewAPI(logger, fs, nil)
+	canonical := filepath.Join(tmpdir, "noncanon")
+	nonCanonical := canonical[:len(tmpdir)] + "/./noncanon"
+	original := "one\ntwo\nthree\n"
+	require.NoError(t, afero.WriteFile(fs, canonical, []byte(original), 0o644))
+
+	req := workspacesdk.FileEditRequest{
+		Files: []workspacesdk.FileEdits{
+			{Path: canonical, Edits: []workspacesdk.FileEdit{{Search: "one", Replace: "ONE"}}},
+			{Path: nonCanonical, Edits: []workspacesdk.FileEdit{{Search: "three", Replace: "THREE"}}},
+		},
+	}
+
+	ctx := testutil.Context(t, testutil.WaitShort)
+	buf := bytes.NewBuffer(nil)
+	enc := json.NewEncoder(buf)
+	enc.SetEscapeHTML(false)
+	require.NoError(t, enc.Encode(req))
+	w := httptest.NewRecorder()
+	r := httptest.NewRequestWithContext(ctx, http.MethodPost, "/edit-files", buf)
+	api.Routes().ServeHTTP(w, r)
+
+	require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
+
+	data, err := afero.ReadFile(fs, canonical)
+	require.NoError(t, err)
+	require.Equal(t, "ONE\ntwo\nTHREE\n", string(data))
 }
 
 // TestEditFiles_DuplicatePath_SymlinkAliasRejects pins that two