From cc4f0cd91eebc40f8f5ba37568f2b87d82cb2175 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 29 Jul 2026 16:03:58 +0800 Subject: [PATCH 1/3] =?UTF-8?q?feat(fork):=20append=5Ffile=20=E8=BE=93?= =?UTF-8?q?=E5=87=BA=20inline=20unified=20diff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit append_file 此前只返回字节摘要,前端无法像 write_file 一样渲染 diff 预览。 追加前快照旧内容,输出 unified diff(尾部 context + 追加行),字节摘要保留在末尾; 已存文件超过 512KB(APPEND_FILE_INLINE_DIFF_LIMIT_BYTES)或非 UTF-8 时回退 [diff omitted] / 纯字节摘要。归属 T2 工具面主题,附 forkguard 结果式回归。 --- crates/tui/src/tools/file.rs | 118 ++++++++++++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 2 deletions(-) diff --git a/crates/tui/src/tools/file.rs b/crates/tui/src/tools/file.rs index d1d4585c44..13f7c5ad2f 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,38 @@ 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 isn't readable UTF-8 — +/// then there is nothing trustworthy to diff against, so keep the plain +/// summary. Oversized existing files get the `[diff omitted]` note instead. +fn append_file_result_body( + path: &str, + prior_contents: Option<&str>, + append_content: &str, + summary: &str, +) -> String { + let Some(prior) = prior_contents else { + return summary.to_string(); + }; + 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 +875,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 +936,14 @@ 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). + // A non-UTF-8 existing file can't be diffed as text → skip the diff. + let prior_contents = if existed_before { + fs::read_to_string(&file_path).ok() + } else { + Some(String::new()) + }; let mut file = fs::OpenOptions::new() .create(true) @@ -917,13 +967,19 @@ 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(), + append_content, + &summary, + ); let diag_block = lsp_diagnostics_for_paths(context, &[file_path]).await; let full_body = if diag_block.is_empty() { @@ -2167,6 +2223,64 @@ 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 test_write_file_rejects_oversized_content() { let tmp = tempdir().expect("tempdir"); From a1728d6518ff71c1a3331a06de82f5d16b4774a4 Mon Sep 17 00:00:00 2001 From: asto Date: Thu, 30 Jul 2026 10:28:34 +0800 Subject: [PATCH 2/3] =?UTF-8?q?fix(tools):=20append=5Ffile=20=E5=BF=AB?= =?UTF-8?q?=E7=85=A7=E6=97=A7=E5=86=85=E5=AE=B9=E5=89=8D=E6=8C=89=E6=96=87?= =?UTF-8?q?=E4=BB=B6=E5=A4=A7=E5=B0=8F=E9=97=A8=E6=8E=A7=E8=AF=BB=E5=8F=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 审阅发现:追加前快照旧内容的 read_to_string 在 512KB 上限判断之前执行, 向超大文件(如数 GB 日志)追加会把整个文件读入内存。改为先用 metadata 的 before_len 门控,超限直接走 [diff omitted] 说明,不再读取;prior.len() 检查保留作为 metadata 与读取之间文件增长的兜底。 同时补齐非 UTF-8 旧文件回退纯字节摘要的 forkguard 回归测试。 Signed-off-by: asto --- crates/tui/src/tools/file.rs | 47 ++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/crates/tui/src/tools/file.rs b/crates/tui/src/tools/file.rs index 13f7c5ad2f..f67072111f 100644 --- a/crates/tui/src/tools/file.rs +++ b/crates/tui/src/tools/file.rs @@ -833,18 +833,31 @@ 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 isn't readable UTF-8 — -/// then there is nothing trustworthy to diff against, so keep the plain -/// summary. Oversized existing files get the `[diff omitted]` note instead. +/// `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\ @@ -938,11 +951,15 @@ impl ToolSpec for AppendFileTool { 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 { + 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 { - Some(String::new()) + None }; let mut file = fs::OpenOptions::new() @@ -977,6 +994,7 @@ impl ToolSpec for AppendFileTool { let body = append_file_result_body( &file_path.display().to_string(), prior_contents.as_deref(), + before_len, append_content, &summary, ); @@ -2281,6 +2299,25 @@ mod tests { 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"); From 7efe2c0ad17a0855008bb3125f18c0a95392c1b0 Mon Sep 17 00:00:00 2001 From: hexin <372726039@qq.com> Date: Thu, 30 Jul 2026 13:42:11 +0800 Subject: [PATCH 3/3] =?UTF-8?q?style(fork):=20=E4=BF=AE=E5=A4=8D=20append?= =?UTF-8?q?=5Ffile=20=E6=B5=8B=E8=AF=95=E6=A0=BC=E5=BC=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: hexin <372726039@qq.com> --- crates/tui/src/tools/file.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/tui/src/tools/file.rs b/crates/tui/src/tools/file.rs index f67072111f..ba60e19425 100644 --- a/crates/tui/src/tools/file.rs +++ b/crates/tui/src/tools/file.rs @@ -2294,9 +2294,17 @@ mod tests { .await .expect("append"); assert!(result.success); - assert!(result.content.contains("[diff omitted]"), "{}", result.content); + assert!( + result.content.contains("[diff omitted]"), + "{}", + result.content + ); assert!(!result.content.contains("--- a/"), "{}", result.content); - assert!(result.content.contains("Appended 5 bytes"), "{}", result.content); + assert!( + result.content.contains("Appended 5 bytes"), + "{}", + result.content + ); } #[tokio::test] @@ -2314,8 +2322,16 @@ mod tests { .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); + assert!( + !result.content.contains("[diff omitted]"), + "{}", + result.content + ); + assert!( + result.content.contains("Appended 5 bytes"), + "{}", + result.content + ); } #[tokio::test]