From 783aa51c9e28a2ba03daea635b99a3ceddb3209c Mon Sep 17 00:00:00 2001 From: Eliot Hedeman Date: Wed, 22 Apr 2026 16:30:09 -0400 Subject: [PATCH] fix(toolpath-convo): strip leading slash in unified_diff headers `unified_diff(path, ...)` previously emitted `--- a//abs/file.rs` / `+++ b//abs/file.rs` when `path` already started with `/` (common for Claude's `tool_use.input.file_path`, which carries absolute paths). Git-style `a/` and `b/` prefixes already denote the repo root, so the doubled slash breaks `patch(1)` and any other tool that parses the header. Trim a single leading `/` before splicing into the header. Add a regression test in `toolpath-convo::derive::tests` and update the `toolpath-claude::derive::test_derive_path_edit_tool_emits_unified_diff` assertion, which had been canonicalising the broken output. The frontend diff renderer splits on lines and does not parse the header, so no UI breakage. Internal-only; no API change. Closes #36 --- crates/toolpath-claude/src/derive.rs | 6 ++++- crates/toolpath-convo/src/derive.rs | 39 +++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/toolpath-claude/src/derive.rs b/crates/toolpath-claude/src/derive.rs index af656e2..62a15de 100644 --- a/crates/toolpath-claude/src/derive.rs +++ b/crates/toolpath-claude/src/derive.rs @@ -1688,7 +1688,11 @@ mod tests { let tool_step = &path.steps[1]; let ch = &tool_step.change["/src/login.rs"]; let raw = ch.raw.as_deref().expect("edit tool should emit unified diff"); - assert!(raw.contains("--- a//src/login.rs"), "{}", raw); + // Leading `/` is stripped from the header so `a/`/`b/` don't double up + // (git-style prefixes already denote the repo root). See #36. + assert!(raw.contains("--- a/src/login.rs"), "{}", raw); + assert!(raw.contains("+++ b/src/login.rs"), "{}", raw); + assert!(!raw.contains("a//"), "header should not double-slash: {}", raw); assert!(raw.contains("-validate_token()"), "{}", raw); assert!(raw.contains("+validate_token_v2()"), "{}", raw); diff --git a/crates/toolpath-convo/src/derive.rs b/crates/toolpath-convo/src/derive.rs index 5be8eb0..6d9171a 100644 --- a/crates/toolpath-convo/src/derive.rs +++ b/crates/toolpath-convo/src/derive.rs @@ -407,11 +407,17 @@ pub fn file_write_diff( /// /// Always emits a `--- a/{path}` / `+++ b/{path}` header even when one side is /// empty so downstream renderers can anchor the change to the file it touched. +/// +/// Any leading `/` on `path` is stripped before splicing into the header — +/// git-style `a/` and `b/` prefixes already denote the repo root, so an +/// absolute path like `/abs/file.rs` would otherwise emit `--- a//abs/file.rs`, +/// which breaks `patch(1)` and other consumers that parse the header. pub fn unified_diff(path: &str, before: &str, after: &str) -> String { use similar::TextDiff; let diff = TextDiff::from_lines(before, after); + let display = path.trim_start_matches('/'); let mut out = String::new(); - out.push_str(&format!("--- a/{path}\n+++ b/{path}\n")); + out.push_str(&format!("--- a/{display}\n+++ b/{display}\n")); out.push_str( &diff .unified_diff() @@ -712,6 +718,37 @@ mod tests { assert!(!raw.contains("something else entirely")); } + #[test] + fn test_unified_diff_strips_leading_slash_on_absolute_path() { + // Regression for #36: headers for absolute paths must not contain `a//`. + let raw = unified_diff("/abs/path.rs", "a\n", "b\n"); + assert!( + raw.contains("--- a/abs/path.rs\n"), + "missing stripped --- header: {raw}" + ); + assert!( + raw.contains("+++ b/abs/path.rs\n"), + "missing stripped +++ header: {raw}" + ); + assert!( + !raw.contains("a//"), + "header should not contain doubled slash: {raw}" + ); + assert!( + !raw.contains("b//"), + "header should not contain doubled slash: {raw}" + ); + } + + #[test] + fn test_unified_diff_preserves_relative_path() { + // Relative paths (no leading slash) are unchanged — only a single + // leading `/` is stripped. + let raw = unified_diff("src/login.rs", "a\n", "b\n"); + assert!(raw.contains("--- a/src/login.rs\n"), "{raw}"); + assert!(raw.contains("+++ b/src/login.rs\n"), "{raw}"); + } + #[test] fn test_tool_use_multiedit_emits_per_hunk_diff() { let mut turn = base_turn("t1", Role::Assistant);