Skip to content

fix(acp): stop echoing edited files to the client - #52466

Merged
nexxeln merged 3 commits into
v2from
acp-write-echo
Oct 1, 2026
Merged

nexxeln merged 3 commits into
v2from
acp-write-echo

Conversation

@nexxeln

@nexxeln nexxeln commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

#35457 | #50740

Fixes #50740

Why the change

Core has already written the file by the time an ACP edit, write, or patch tool succeeds, so sending the whole file back to the client with fs/write_text_file did nothing useful, and in Zed it rewrote the line endings.

Special things to note

  • Zed's source (zed-industries/zed @ f2cdf3d) shows the echo is harmful. AcpThread::write_text_file diffs the new content into a buffer and saves it again (crates/acp_thread/src/acp_thread.rs:6421-6513). Buffers normalise line endings to LF (crates/text/src/text.rs:48-49) and save with one guessed line ending (crates/project/src/buffer_store.rs:399-413), which breaks files with mixed or stray \r endings. The echo also races Zed's reload of the file core already wrote (crates/language/src/buffer.rs:~1795, 1722-1751). The review log gains almost nothing, because tracking starts from the already-written buffer (crates/action_log/src/action_log.rs:185).
  • Diff previews in permission prompts and diff content in tool calls are unchanged, and Zed's inline diff cards come from that diff content. We never relied on client fs for reads, and we now ignore clientCapabilities.fs.writeTextFile too.
  • Routing writes through the client so it can review them first would be a separate feature. This PR does not add it.

Change outline

What happens when an edit tool succeeds:

 core edit/write/patch tool
   writes the file to disk
 session.tool.success
   streamTurn (event.ts)
-    syncEditedFiles (permission.ts)
-      read each edited file from disk
-      fs/write_text_file → client rewrites the file again
     tool_call_update { status: completed, content: [text, diff] }

Files changed:

 packages/cli/
 ├── src/acp/
 │   ├── connection.ts   # drop writeTextFile request
 │   ├── event.ts        # drop the syncEditedFiles call and the writeTextFile turn input
 │   ├── permission.ts   # drop syncEditedFiles
 │   └── service.ts      # stop capturing clientCapabilities.fs.writeTextFile
 └── test/acp/
     └── permission.test.ts  # no fs/write_text_file even when the client advertises it; preview tests no longer assert writes

@nexxeln
nexxeln merged commit 966e718 into v2 Oct 1, 2026
10 checks passed
@nexxeln
nexxeln deleted the acp-write-echo branch October 1, 2026 10:50
Ichinose-Kazuki pushed a commit to Ichinose-Kazuki/opencode that referenced this pull request Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant