Repository navigation
acpmux: a Claude Write over an existing file completes with its old text, so Undo never trashes it - #23187
Conversation
… old text, failing first The Claude translator sends every Write as a diff with no old text, so the page's Undo reads an overwrite as a file the turn created and would move the overwritten file to the Trash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d24-cf0a-7373-a03a-75491e4d724a&host=cosmic-gray
…ext, not as a new file The translator sends a Write as a diff with no old text when the call starts, before anyone knows whether the file was there. The page reads a diff with no old text as a file the turn created, so its Undo offered to move an overwritten file to the Trash. At the Write's completion the diff now settles: a Write that created its file (tool_use_result type "create", or "File created successfully") keeps no old text; an overwrite takes tool_use_result.originalFile as its old text, when that result names the same file; without it the diff is marked a fragment (_meta.acpmux.fragment), so Undo writes nothing back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d24-cf0a-7373-a03a-75491e4d724a&host=cosmic-gray
|
Owned by toolbar-diffviewer on cosmic-gray · session |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base 1c367bb; does not block |
|
Found 6 test failures on Blacksmith runners: Failures
|
|
Bead |
|
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
… the file Review of #23187: the safe default came only from a successful live tool_result. A Write whose result never arrived (a crash, a line dropped at the frame limit) and every adopted Claude Write still carried a diff with no old text, which Undo reads as a file the turn created. edit_diffs now marks every Write diff a fragment at the source. The shared settle_write (claude_stdio/mod.rs) removes the mark only on a confirmed create, or on an overwrite whose result names the same file and holds its old text (originalFile, up to 64,000 bytes, adopt's limit; a bigger one stays a fragment rather than doubling a multi-MB update). Adopted turns pass the transcript line's toolUseResult through claude_tool_results into PastTool::settle, which runs the same logic. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d24-cf0a-7373-a03a-75491e4d724a&host=cosmic-gray
|
Review (independent subagent, claude-opus-5-5): SHIP at cbb54ec This re-review covers the PR's five files against merge-base 1c367bb. The base merge 6351b26 is excluded.
Non-blocking follow-ups:
Agent session: cosmic-gray |
acpmux sends a Claude Write as a fragment with no old text until its result confirms a create or carries the old text (#23187). One that stays a fragment (an overwrite without originalFile, an oversized one, no result yet, a cut-off adopted Write) read as a new file: "+N -0" on the Edited card, all added lines in its inline diff and in View changes. model.ts `contentsUnknown` names that diff (oldText null, fragment, not read from the input). `turnFiles` leaves it out, so nothing counts, diffs or undoes it, and `plainEditLabels` lists its path without counts, the path #23161 gives an overwrite built from the input. The card keeps such a path when no listed file is it, and once a checkpoint loads keeps an absolute path none of its files is (a /tmp file), as `outsideRepository` does for diffs. Edit and MultiEdit fragments keep their hunks and counts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d24-cf0a-7373-a03a-75491e4d724a&host=cosmic-gray
…s, chevron or Undo (#23419) * test: a Write whose old contents are unknown lists without counts A translator Write that stays a fragment (oldText null, `_meta.acpmux.fragment`) reads today as a new file, "+9 -0", with a chevron. These cases pin the card, its inline diff, Undo and View changes for it, a fragment Edit beside it, the checkpoint-loaded case outside the repository, and the same Write once its result confirmed a create. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d24-cf0a-7373-a03a-75491e4d724a&host=cosmic-gray * cmux NEXT: a Write whose old contents are unknown lists without counts acpmux sends a Claude Write as a fragment with no old text until its result confirms a create or carries the old text (#23187). One that stays a fragment (an overwrite without originalFile, an oversized one, no result yet, a cut-off adopted Write) read as a new file: "+N -0" on the Edited card, all added lines in its inline diff and in View changes. model.ts `contentsUnknown` names that diff (oldText null, fragment, not read from the input). `turnFiles` leaves it out, so nothing counts, diffs or undoes it, and `plainEditLabels` lists its path without counts, the path #23161 gives an overwrite built from the input. The card keeps such a path when no listed file is it, and once a checkpoint loads keeps an absolute path none of its files is (a /tmp file), as `outsideRepository` does for diffs. Edit and MultiEdit fragments keep their hunks and counts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d24-cf0a-7373-a03a-75491e4d724a&host=cosmic-gray * cmux NEXT: an unknown Write makes its whole file unknown; checkpoint keeps only paths outside the repository Review follow-ups: - A file a Write rewrote with unknown old contents is unknown as a whole: an Edit to the same file no longer lends it counts. - A pending or failed Write lists nothing. - Once the checkpoint loads, the card keeps a path without counts only when it lies outside the checkpoint's repository (turnCheckpoint.ts `outsideRepository`, now shared, with `turnCounts` naming the root), so an in-repo path the checkpoint skipped does not reappear. - The row height estimate counts a path once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d24-cf0a-7373-a03a-75491e4d724a&host=cosmic-gray * cmux NEXT: a checkpoint file a Write rewrote without counts is still the tool calls' change With the unknown Write left out of the turn's files, a loaded checkpoint marked the file it rewrote as changed outside the tool calls ("Includes changes outside tool calls"). diff.ts `turnPathsOnly` names the paths the turn's edits changed without a counted diff (an edit with no diff, as #23161 lists, and a Write whose old contents are unknown); `turnDisplay` and `turnCounts` take them so a checkpoint file there is not `outside`. The pane passes them for View changes, the header's Changes and the card. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d24-cf0a-7373-a03a-75491e4d724a&host=cosmic-gray --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Shipped in:
|
In a Claude chat, a Write that overwrote an existing file could be undone by moving that file to the Trash. The Claude translator sent every Write as an ACP diff with no old text, and the page reads a diff with no old text as a file the turn created. So the edited-files card's Undo offered "trash" for the overwritten file. The host would trash it, since the file still held the Write's text.
Now every Write diff starts out marked as a fragment (
_meta.acpmux.fragment), which the page shows as "Cannot undo this edit" (edit_diffsincmux-tui/crates/acpmux/src/claude_stdio/mod.rs). A sharedsettle_writeremoves the mark only when Claude Code's result confirms what the Write did:typeiscreate, or the result text reads "File created successfully at: …"): the mark is removed. The diff reads as a new file, and Undo may trash it.originalFile, up to 64,000 bytes, the same limit adopt uses): the diff takes that old text and loses the mark. Undo can then put the old text back.Both paths run the same logic:
tool_resultline'stool_use_result, ininbound.rs.toolUseResult, passed throughclaude_tool_resultsintoPastTool::settle(adopt_replay.rs,adopt_tools.rs).Found while reviewing #23161. With that PR's page code, a confirmed create still shows as all added lines with Undo set to trash, and a Write still marked as a fragment shows no Undo. I checked this with a scratch jsdom test against #23161's head
b2f24178bb3e.Changelog
Tests
claude_stdio/edit_diff_tests.rs:claude_edit_write_is_a_whole_file_diff)originalFiletype: "update"withoriginalFile: nulloriginalFileover 64,000 bytesadopt_tools.rs: an adopted Write that was created, overwritten with old text, overwritten with no structured result, and cut off before its result.Fleet runs (
scripts/ci/cmux-tui-rust-check.sh) atcbb54ec99d0:all write(step59d5ecd11f642b2e7da5c97a):cargo fmt --checkand clippy-D warningspass, and 195 of 195 tests pass, including all 9 Write tests.test adopt: 86 run, 85 passed on the first try.durable_agents::an_adopted_claude_chat_still_moves_its_work_after_an_update_restartfailed once and passed on its automatic solo rerun. It covers update restarts, not Write diffs.test claude_edit: 14 of 14 pass.The first round's red run at
3a35056908b(tests only, stepc212bed92cec8193d33a8486): the overwrite and other-file tests fail, and the created-file case passes.Observed / Not run
Agent session: cosmic-gray
🤖 Generated with Claude Code