diff --git a/cmux-tui/crates/acpmux/src/adopt_replay.rs b/cmux-tui/crates/acpmux/src/adopt_replay.rs index dd818a9a56b1..df74597dcea1 100644 --- a/cmux-tui/crates/acpmux/src/adopt_replay.rs +++ b/cmux-tui/crates/acpmux/src/adopt_replay.rs @@ -159,7 +159,7 @@ fn claude_steps(record: &Value, at: Option) -> Vec { blocks.iter().any(|b| b.get("type").and_then(Value::as_str) == Some("tool_result")) }) => { - crate::adopt_tools::claude_tool_results(content) + crate::adopt_tools::claude_tool_results(content, record.get("toolUseResult")) .into_iter() .map(Step::ToolResult) .collect() diff --git a/cmux-tui/crates/acpmux/src/adopt_tools.rs b/cmux-tui/crates/acpmux/src/adopt_tools.rs index a86dec5ce742..d0c7e2aa1924 100644 --- a/cmux-tui/crates/acpmux/src/adopt_tools.rs +++ b/cmux-tui/crates/acpmux/src/adopt_tools.rs @@ -32,6 +32,9 @@ pub struct PastTool { pub struct ToolResult { pub id: String, pub done: Value, + /// Claude Code's structured result beside it (the transcript line's `toolUseResult`): for a + /// Write, whether it created its file and the file's text before it (`settle_write`). + pub structured: Option, } impl PastTool { @@ -39,7 +42,14 @@ impl PastTool { pub fn settle(&mut self, result: &ToolResult, at: Option) { let failed = result.done["status"] == "failed"; if let (Some(proposed), false) = (self.proposed.take(), failed) { - self.done["content"] = proposed; + // A Write's diff is a fragment until its result says it created the file or holds + // the text it replaced; otherwise Undo would read the file as one the turn created. + let text = result.done.pointer("/content/0/content/text").and_then(Value::as_str); + self.done["content"] = crate::claude_stdio::settle_write( + proposed, + text.unwrap_or_default(), + result.structured.as_ref(), + ); } if let (Some(done), Some(update)) = (self.done.as_object_mut(), result.done.as_object()) { for (key, value) in update { @@ -473,7 +483,7 @@ pub fn codex_tool(record: &Value, at: Option) -> Option<(Result { let id = s(payload, "call_id")?; @@ -531,8 +541,9 @@ fn oversize(content: &Value) -> bool { }) } -/// Claude Code's `tool_result` blocks in a user message. -pub fn claude_tool_results(content: &Value) -> Vec { +/// Claude Code's `tool_result` blocks in a user message; `structured` is the line's +/// `toolUseResult`, which `settle_write` checks against each Write's own path. +pub fn claude_tool_results(content: &Value, structured: Option<&Value>) -> Vec { content .as_array() .into_iter() @@ -546,6 +557,7 @@ pub fn claude_tool_results(content: &Value) -> Vec { "status": if failed { "failed" } else { "completed" }, "content": result_text(block.get("content")), }), + structured: structured.cloned(), }) }) .collect() @@ -562,7 +574,7 @@ mod tests { fn result(id: &str, is_error: bool) -> ToolResult { let block = json!([{"type": "tool_result", "tool_use_id": id, "content": "ok", "is_error": is_error}]); - claude_tool_results(&block).remove(0) + claude_tool_results(&block, None).remove(0) } /// A Claude Code edit shows its diff only once a result that did not fail confirms it: one @@ -583,4 +595,46 @@ mod tests { assert_eq!(applied.done["content"][0]["type"], "diff", "{}", applied.done); assert_eq!(applied.done["content"][0]["newText"], "b"); } + + fn write(id: &str) -> Value { + json!([{"type": "tool_use", "id": id, "name": "Write", + "input": {"file_path": "/p/notes.md", "content": "new\n"}}]) + } + + /// An adopted Write as the transcript line records it: the tool_result and, beside the + /// message, `toolUseResult`. + fn write_settled(id: &str, text: &str, structured: Option) -> Value { + let [mut tool] = claude_tool_uses(&write(id), Some(1)).try_into().unwrap(); + let block = json!([{"type": "tool_result", "tool_use_id": id, "content": text}]); + let [result] = claude_tool_results(&block, structured.as_ref()).try_into().unwrap(); + tool.settle(&result, Some(2)); + assert_eq!(tool.done["status"], "completed"); + tool.done["content"][0].clone() + } + + /// An adopted Claude Write reads as a new file (which Undo may trash) only when its result + /// says it created the file; over a file that was there it carries the old text, or stays a + /// fragment, so Undo never trashes an overwritten file. + #[test] + fn an_adopted_claude_write_is_new_only_when_its_result_says_it_created_the_file() { + let fragment = |diff: &Value| diff.pointer("/_meta/acpmux/fragment") == Some(&json!(true)); + + let created = json!({"type": "create", "filePath": "/p/notes.md", "originalFile": null}); + let diff = write_settled("w1", "File created successfully at: /p/notes.md", Some(created)); + assert_eq!(diff["oldText"], Value::Null); + assert!(!fragment(&diff), "{diff}"); + + let updated = json!({"type": "update", "filePath": "/p/notes.md", "originalFile": "old\n"}); + let diff = write_settled("w2", "The file /p/notes.md has been updated.", Some(updated)); + assert_eq!(diff["oldText"], "old\n"); + assert!(!fragment(&diff), "{diff}"); + + let diff = write_settled("w3", "The file /p/notes.md has been updated.", None); + assert_eq!(diff["oldText"], Value::Null); + assert!(fragment(&diff), "{diff}"); + + // Cut off before its result: no diff at all, as for any edit. + let [cut_off] = claude_tool_uses(&write("w4"), Some(1)).try_into().unwrap(); + assert_eq!(cut_off.done, json!({"status": "failed"})); + } } diff --git a/cmux-tui/crates/acpmux/src/claude_stdio/edit_diff_tests.rs b/cmux-tui/crates/acpmux/src/claude_stdio/edit_diff_tests.rs index e5e206788345..6d497c98e902 100644 --- a/cmux-tui/crates/acpmux/src/claude_stdio/edit_diff_tests.rs +++ b/cmux-tui/crates/acpmux/src/claude_stdio/edit_diff_tests.rs @@ -82,6 +82,9 @@ async fn claude_edit_multiedit_is_one_diff_per_edit() { ); } +// A Write goes out as the whole file, marked a fragment until its result says +// whether it created the file: a call whose result never comes (a crash, a +// dropped line) must not read as a new file Undo could trash. #[tokio::test] async fn claude_edit_write_is_a_whole_file_diff() { let t = Translator::new("acp-1".into(), "default", "haiku", "default"); @@ -89,10 +92,40 @@ async fn claude_edit_write_is_a_whole_file_diff() { let out = updates(&t.inbound(&tool_use("toolu_1", "Write", input)).await); assert_eq!( out[0]["content"], - json!([{"type": "diff", "path": "/repo/new.md", "oldText": null, "newText": "# New\n"}]) + json!([{"type": "diff", "path": "/repo/new.md", "oldText": null, "newText": "# New\n", + "_meta": {"acpmux": {"fragment": true}}}]) ); } +#[tokio::test] +async fn claude_edit_write_with_no_result_stays_a_fragment() { + let t = Translator::new("acp-1".into(), "default", "haiku", "default"); + let input = json!({"file_path": "/repo/notes.md", "content": "new\n"}); + let out = updates(&t.inbound(&tool_use("toolu_1", "Write", input)).await); + assert_eq!(out[0]["status"], "in_progress"); + assert_eq!(out[0]["content"][0]["oldText"], Value::Null); + assert_eq!(out[0]["content"][0]["_meta"]["acpmux"]["fragment"], true); + // Its permission request shows the same fragment. + let ask = json!({"type": "control_request", "request_id": "r1", "request": { + "subtype": "can_use_tool", "tool_name": "Write", "tool_use_id": "toolu_2", + "input": {"file_path": "/repo/notes.md", "content": "new\n"} + }}); + let call = t + .inbound(&ask) + .await + .into_iter() + .find_map(|m| match m { + Message::Request { method: m, params: Some(p), .. } + if m == method::SESSION_REQUEST_PERMISSION => + { + Some(p["toolCall"].clone()) + } + _ => None, + }) + .expect("a permission request"); + assert_eq!(call["content"][0]["_meta"]["acpmux"]["fragment"], true); +} + #[tokio::test] async fn claude_edit_permission_request_shows_the_diff() { let t = Translator::new("acp-1".into(), "default", "haiku", "default"); @@ -114,3 +147,107 @@ async fn claude_edit_permission_request_shows_the_diff() { assert_eq!(call["content"], edit_diff()); assert_eq!(call["locations"], json!([{"path": "/repo/src/lib.rs"}])); } + +/// A Write's result line as Claude Code sends it: the tool_result text and, +/// beside the message, `tool_use_result` (`type` create or update, and the +/// file's `originalFile` before an update). +fn write_result(id: &str, text: &str, tool_use_result: Option) -> Value { + let mut line = tool_result(id, false, text); + if let Some(result) = tool_use_result { + line["tool_use_result"] = result; + } + line +} + +async fn finished_write(result: Value) -> Value { + let t = Translator::new("acp-1".into(), "default", "haiku", "default"); + let input = json!({"file_path": "/repo/notes.md", "content": "new\n"}); + t.inbound(&tool_use("toolu_1", "Write", input)).await; + let out = updates(&t.inbound(&result).await); + assert_eq!(out[0]["status"], "completed", "{out:?}"); + out[0]["content"].clone() +} + +// Undo trashes a file whose diff has no old text (the turn created it). A +// Write over a file that was there must not read that way: its completion +// carries the file's text before the Write, or says the diff is a fragment. +#[tokio::test] +async fn claude_edit_write_over_a_file_completes_with_its_old_text() { + let result = json!({"type": "update", "filePath": "/repo/notes.md", "content": "new\n", "originalFile": "old\n"}); + let content = finished_write(write_result( + "toolu_1", + "The file /repo/notes.md has been updated. Here's the result of running `cat -n`:", + Some(result), + )) + .await; + assert_eq!( + content, + json!([{"type": "diff", "path": "/repo/notes.md", "oldText": "old\n", "newText": "new\n"}]) + ); +} + +#[tokio::test] +async fn claude_edit_write_over_a_file_without_its_old_text_is_a_fragment() { + let content = + finished_write(write_result("toolu_1", "The file /repo/notes.md has been updated.", None)) + .await; + assert_eq!( + content, + json!([{"type": "diff", "path": "/repo/notes.md", "oldText": null, "newText": "new\n", + "_meta": {"acpmux": {"fragment": true}}}]) + ); +} + +#[tokio::test] +async fn claude_edit_write_that_created_its_file_completes_as_a_new_file() { + let new_file = + json!([{"type": "diff", "path": "/repo/notes.md", "oldText": null, "newText": "new\n"}]); + let created = json!({"type": "create", "filePath": "/repo/notes.md", "content": "new\n", "originalFile": null}); + let text = "File created successfully at: /repo/notes.md"; + assert_eq!(finished_write(write_result("toolu_1", text, Some(created))).await, new_file); + // Without the structured result, its text says so. + assert_eq!(finished_write(write_result("toolu_1", text, None)).await, new_file); +} + +// A result for another file (two results in one line) does not lend its old text. +#[tokio::test] +async fn claude_edit_write_ignores_another_files_result() { + let other = json!({"type": "update", "filePath": "/repo/other.md", "originalFile": "other\n"}); + let content = finished_write(write_result( + "toolu_1", + "The file /repo/notes.md has been updated.", + Some(other), + )) + .await; + assert_eq!(content[0]["oldText"], Value::Null); + assert_eq!(content[0]["_meta"]["acpmux"]["fragment"], true); +} + +#[tokio::test] +async fn claude_edit_write_update_with_no_old_text_stays_a_fragment() { + let update = json!({"type": "update", "filePath": "/repo/notes.md", "originalFile": null}); + let content = finished_write(write_result( + "toolu_1", + "The file /repo/notes.md has been updated.", + Some(update), + )) + .await; + assert_eq!(content[0]["oldText"], Value::Null); + assert_eq!(content[0]["_meta"]["acpmux"]["fragment"], true); +} + +// A file too big to carry twice stays a fragment rather than doubling a +// multi-megabyte update. +#[tokio::test] +async fn claude_edit_write_over_a_huge_file_stays_a_fragment() { + let original = "x".repeat(64_001); + let update = json!({"type": "update", "filePath": "/repo/notes.md", "originalFile": original}); + let content = finished_write(write_result( + "toolu_1", + "The file /repo/notes.md has been updated.", + Some(update), + )) + .await; + assert_eq!(content[0]["oldText"], Value::Null); + assert_eq!(content[0]["_meta"]["acpmux"]["fragment"], true); +} diff --git a/cmux-tui/crates/acpmux/src/claude_stdio/inbound.rs b/cmux-tui/crates/acpmux/src/claude_stdio/inbound.rs index bb77aa1af25d..6eca3dfdc0fc 100644 --- a/cmux-tui/crates/acpmux/src/claude_stdio/inbound.rs +++ b/cmux-tui/crates/acpmux/src/claude_stdio/inbound.rs @@ -331,7 +331,9 @@ impl Translator { // ACP content replaces the call's content: a finished edit // keeps its diff, and a failed one says why. let content = match self.edits.lock().await.remove(tool) { - Some(diffs) if !is_err => diffs, + Some(diffs) if !is_err => { + settle_write(diffs, &text, line.get("tool_use_result")) + } _ => { json!([{"type": "content", "content": {"type": "text", "text": text}}]) } diff --git a/cmux-tui/crates/acpmux/src/claude_stdio/mod.rs b/cmux-tui/crates/acpmux/src/claude_stdio/mod.rs index a8e65f0b5731..b35e304208a0 100644 --- a/cmux-tui/crates/acpmux/src/claude_stdio/mod.rs +++ b/cmux-tui/crates/acpmux/src/claude_stdio/mod.rs @@ -658,7 +658,10 @@ fn tool_kind(name: &str) -> &'static str { /// A file edit's ACP `diff` content: one diff per Edit or MultiEdit /// replacement, and Write as the whole file (no old text). None for any other -/// tool, or input without a path. +/// tool, or input without a path. A Write is marked a fragment until its result +/// says what it did (`settle_write`): a diff with no old text reads as a file the +/// turn created, which Undo moves to the Trash, and a Write over a file that was +/// there, or one whose result never arrives, must not read that way. fn edit_diffs(name: &str, input: &Value) -> Option { let path = input.get("file_path").and_then(Value::as_str)?; let diff = |old: Option<&Value>, new: Option<&Value>| json!({"type": "diff", "path": path, "oldText": old.cloned().unwrap_or(Value::Null), "newText": new.cloned().unwrap_or_else(|| json!(""))}); @@ -670,12 +673,57 @@ fn edit_diffs(name: &str, input: &Value) -> Option { .iter() .map(|e| diff(e.get("old_string"), e.get("new_string"))) .collect(), - "Write" => vec![diff(None, input.get("content"))], + "Write" => { + let mut write = diff(None, input.get("content")); + write["_meta"] = json!({"acpmux": {"fragment": true}}); + vec![write] + } _ => return None, }; Some(Value::Array(diffs)) } +/// The most of a Write's old text a settled diff carries: a bigger file stays a +/// fragment rather than doubling a multi-megabyte update (as adopt's limit). +const MAX_WRITE_OLD_TEXT: usize = 64_000; + +/// A finished Write's diff, sent as a fragment with no old text (`edit_diffs`) +/// before Claude Code said whether the file was there. `result` is Claude Code's +/// structured result (`tool_use_result` live, `toolUseResult` in a transcript), +/// `text` the tool_result's text. A Write that created its file loses the +/// fragment mark: it reads as new, and Undo may trash it. An overwrite whose +/// result names this file and holds its old text (`originalFile`, within the +/// limit) takes that text and loses the mark. Anything else stays a fragment, so +/// nothing is written back from it. +pub(crate) fn settle_write(mut diffs: Value, text: &str, result: Option<&Value>) -> Value { + let field = |key: &str| result.and_then(|r| r.get(key)).and_then(Value::as_str); + let Some(items) = diffs.as_array_mut() else { return diffs }; + let held = |d: &Value| { + d.get("oldText").is_some_and(Value::is_null) + && d.pointer("/_meta/acpmux/fragment") == Some(&json!(true)) + }; + for diff in items.iter_mut().filter(|d| held(d)) { + let ours = field("filePath").is_some() + && field("filePath") == diff.get("path").and_then(Value::as_str); + let created = if ours && field("type").is_some() { + field("type") == Some("create") + } else { + text.trim_start().starts_with("File created successfully") + }; + let original = field("originalFile").filter(|o| ours && o.len() <= MAX_WRITE_OLD_TEXT); + if !created && original.is_none() { + continue; + } + if let Some(original) = original.filter(|_| !created) { + diff["oldText"] = json!(original); + } + if let Some(fields) = diff.as_object_mut() { + fields.remove("_meta"); + } + } + diffs +} + /// The file a tool reads or writes, as ACP `locations`. fn tool_locations(input: &Value) -> Option { let path = input.get("file_path").and_then(Value::as_str)?;