diff --git a/crates/tui/src/tools/file.rs b/crates/tui/src/tools/file.rs index d1d4585c44..ba60e19425 100644 --- a/crates/tui/src/tools/file.rs +++ b/crates/tui/src/tools/file.rs @@ -41,6 +41,16 @@ const WRITE_FILE_INLINE_DIFF_LIMIT_BYTES: usize = 32 * 1024; const WRITE_FILE_MAX_CONTENT_BYTES: usize = 64 * 1024; const APPEND_FILE_MAX_CONTENT_BYTES: usize = 64 * 1024; +/// Cap on the *existing* file size for `append_file`'s inline diff. +/// +/// Much larger than `WRITE_FILE_INLINE_DIFF_LIMIT_BYTES` on purpose: append is +/// the chunked-artifact path, so the prior contents routinely pass 32KB after a +/// couple of ≤16KB chunks. The appended chunk itself is already hard-capped at +/// 64KB, and `similar` trims the shared prefix before diffing, so the emitted +/// diff stays tiny (tail context + added lines) even for a few hundred KB of +/// prior content. Beyond this cap we fall back to the byte-count summary. +const APPEND_FILE_INLINE_DIFF_LIMIT_BYTES: usize = 512 * 1024; + // === ReadFileTool === fn canonical_path_for_credential_guard(path: &Path) -> PathBuf { @@ -821,6 +831,51 @@ fn write_file_result_body( } } +/// Build the `append_file` result body: unified diff of the appended chunk on +/// top (same renderer path as `write_file`, #505), byte-count summary last. +/// `prior_contents` is `None` when the existing file was skipped at the call +/// site: either it exceeds the inline-diff size cap (gated on metadata, so a +/// huge file is never read into memory — those get the `[diff omitted]` note) +/// or it isn't readable UTF-8 (nothing trustworthy to diff against, so keep +/// the plain summary). +fn append_file_result_body( + path: &str, + prior_contents: Option<&str>, + old_len: u64, + append_content: &str, + summary: &str, +) -> String { + let Some(prior) = prior_contents else { + if old_len > APPEND_FILE_INLINE_DIFF_LIMIT_BYTES as u64 { + return format!( + "{summary}\n\ + [diff omitted] {path} is too large for an inline append_file diff \ + (old={old_len} bytes, limit={APPEND_FILE_INLINE_DIFF_LIMIT_BYTES} bytes). \ + Use read_file with line ranges to inspect it." + ); + } + return summary.to_string(); + }; + // Belt and braces for the file growing between the metadata check and the + // snapshot read. + if prior.len() > APPEND_FILE_INLINE_DIFF_LIMIT_BYTES { + return format!( + "{summary}\n\ + [diff omitted] {path} is too large for an inline append_file diff \ + (old={} bytes, limit={} bytes). Use read_file with line ranges to inspect it.", + prior.len(), + APPEND_FILE_INLINE_DIFF_LIMIT_BYTES + ); + } + let new_contents = format!("{prior}{append_content}"); + let diff = make_unified_diff(path, prior, &new_contents); + if diff.is_empty() { + summary.to_string() + } else { + format!("{diff}\n{summary}") + } +} + // === AppendFileTool === /// Tool for appending UTF-8 content to files in the workspace. @@ -833,7 +888,7 @@ impl ToolSpec for AppendFileTool { } fn description(&self) -> &'static str { - "Append UTF-8 content to a file in the workspace. Use this for large generated artifacts after creating a small skeleton with write_file, instead of trying to send one huge write_file content argument. **Recommended chunk size ≤ 16KB, hard limit 64KB per call** to keep tool-arg size small and avoid SSE-timeout-on-slow-decoders failures. Creates parent directories and the target file if needed. Returns a compact byte-count summary, not a full diff." + "Append UTF-8 content to a file in the workspace. Use this for large generated artifacts after creating a small skeleton with write_file, instead of trying to send one huge write_file content argument. **Recommended chunk size ≤ 16KB, hard limit 64KB per call** to keep tool-arg size small and avoid SSE-timeout-on-slow-decoders failures. Creates parent directories and the target file if needed. Returns a compact unified diff of the appended chunk (omitted when the existing file is very large) plus a byte-count summary." } fn input_schema(&self) -> Value { @@ -894,6 +949,18 @@ impl ToolSpec for AppendFileTool { let existed_before = file_path.exists(); let before_len = fs::metadata(&file_path).map(|m| m.len()).unwrap_or(0); + // Snapshot the existing contents before appending — used to render an + // inline diff in the tool result (same renderer path as write_file). + // Gate on the metadata size first: appending to a huge file must not + // pull the whole file into memory just to decide the diff is omitted. + // A non-UTF-8 existing file can't be diffed as text → skip the diff. + let prior_contents = if !existed_before { + Some(String::new()) + } else if before_len <= APPEND_FILE_INLINE_DIFF_LIMIT_BYTES as u64 { + fs::read_to_string(&file_path).ok() + } else { + None + }; let mut file = fs::OpenOptions::new() .create(true) @@ -917,13 +984,20 @@ impl ToolSpec for AppendFileTool { } else { "Created and appended" }; - let body = format!( + let summary = format!( "{action} {} bytes to {} ({} -> {} bytes)", append_content.len(), file_path.display(), before_len, after_len ); + let body = append_file_result_body( + &file_path.display().to_string(), + prior_contents.as_deref(), + before_len, + append_content, + &summary, + ); let diag_block = lsp_diagnostics_for_paths(context, &[file_path]).await; let full_body = if diag_block.is_empty() { @@ -2167,6 +2241,99 @@ mod tests { assert_eq!(written, "\n\n"); } + #[tokio::test] + async fn forkguard_append_file_emits_inline_diff() { + // pinvou3's DiffView routes on the unified-diff shape (`--- ` / `+++ ` + // headers + `@@` hunks). append_file must emit the same inline diff + + // trailing byte summary as write_file so appended chunks render as a + // diff preview instead of a bare byte count. + let tmp = tempdir().expect("tempdir"); + let ctx = ToolContext::new(tmp.path().to_path_buf()); + let tool = AppendFileTool; + + let first = tool + .execute(json!({"path": "deck.html", "content": "\n"}), &ctx) + .await + .expect("first append"); + assert!(first.content.contains("--- a/"), "{}", first.content); + assert!(first.content.contains("+++ b/"), "{}", first.content); + assert!(first.content.contains("+"), "{}", first.content); + assert!( + first.content.contains("Created and appended"), + "{}", + first.content + ); + + let second = tool + .execute(json!({"path": "deck.html", "content": "\n"}), &ctx) + .await + .expect("second append"); + assert!(second.content.contains("@@"), "{}", second.content); + assert!(second.content.contains("+"), "{}", second.content); + // The byte-count summary stays as the trailing line (old sessions and + // non-diff fallbacks rely on it). + assert!( + second.content.contains("Appended 8 bytes"), + "{}", + second.content + ); + } + + #[tokio::test] + async fn forkguard_append_file_omits_inline_diff_for_huge_prior_contents() { + // Beyond APPEND_FILE_INLINE_DIFF_LIMIT_BYTES the append falls back to + // the summary + [diff omitted] note instead of diffing a huge file. + let tmp = tempdir().expect("tempdir"); + let ctx = ToolContext::new(tmp.path().to_path_buf()); + let huge = "x".repeat(APPEND_FILE_INLINE_DIFF_LIMIT_BYTES + 1); + fs::write(tmp.path().join("big.txt"), huge).expect("seed big file"); + + let tool = AppendFileTool; + let result = tool + .execute(json!({"path": "big.txt", "content": "tail\n"}), &ctx) + .await + .expect("append"); + assert!(result.success); + assert!( + result.content.contains("[diff omitted]"), + "{}", + result.content + ); + assert!(!result.content.contains("--- a/"), "{}", result.content); + assert!( + result.content.contains("Appended 5 bytes"), + "{}", + result.content + ); + } + + #[tokio::test] + async fn forkguard_append_file_falls_back_to_summary_for_non_utf8_prior() { + // A non-UTF-8 existing file can't be diffed as text → plain byte-count + // summary, no diff headers, and the append itself still succeeds. + let tmp = tempdir().expect("tempdir"); + let ctx = ToolContext::new(tmp.path().to_path_buf()); + fs::write(tmp.path().join("bin.dat"), [0xff, 0xfe, 0x00, 0x01]).expect("seed binary"); + + let tool = AppendFileTool; + let result = tool + .execute(json!({"path": "bin.dat", "content": "tail\n"}), &ctx) + .await + .expect("append"); + assert!(result.success); + assert!(!result.content.contains("--- a/"), "{}", result.content); + assert!( + !result.content.contains("[diff omitted]"), + "{}", + result.content + ); + assert!( + result.content.contains("Appended 5 bytes"), + "{}", + result.content + ); + } + #[tokio::test] async fn test_write_file_rejects_oversized_content() { let tmp = tempdir().expect("tempdir");