From 24c41d0347d013e5f2a3fd1e24ed0811836d0050 Mon Sep 17 00:00:00 2001 From: kevin9327 Date: Mon, 31 Aug 2026 19:35:39 +0900 Subject: [PATCH] fix(tools): keep result truncation inside its byte budget and terminating `truncate_text` looks for a fixed point where the kept prefix length equals the byte count printed in its own truncation notice. Two things go wrong when the limit is small, and both are reachable because two call sites pass a *remaining* budget rather than a constant: * `gate_grep_content` -> `truncate_text("Grep", .., budget.content_bytes)` * `gate_mcp` -> `truncate_text("MCP text", .., remaining_text)` 1. The loop can spin forever. `notice.len()` grows with the decimal digit count of `shown`, so `kept.len()` alternates between two values across a power-of- ten boundary and never equals `shown`. With `tool_name = "Grep"` this happens at `limit` 78 and 170; with `"MCP text"` at 82 and 174. The conversation task then spins at 100% CPU and the turn never completes. `truncate_middle` in `model/tool_result_replay.rs` already guards against exactly this. 2. When the notice itself does not fit, `available` saturates to 0 and the function returns the ~68 byte notice alone, i.e. *more* than `limit`. `gate_grep_content` then evaluates `budget.content_bytes -= <68 bytes>` on a budget of at most 68, which panics with "attempt to subtract with overflow" in debug/test builds and wraps in release, silently disabling the 32 KiB content cap for the rest of the result. Both are ordinary Grep results away: 16 matches of ~2 KiB leave a two-digit remainder of the 32 KiB budget, and the next match then hits the small-limit path. Fix: return a plain prefix when the notice cannot fit, and stop as soon as the kept length repeats a previous value. The reported byte count is then off by one at most in that rare oscillating case, and the result is guaranteed to be at most `limit` bytes. Every constant-limit call site is unaffected: their notices always fit and their limits do not oscillate. `budget.content_bytes` now uses `saturating_sub`, matching every other subtraction in this file. Verified against the unfixed function: cargo test -p cursor-server --lib gate::tests::truncate_text_never_exceeds_its_limit -> FAILED: limit 1 produced 67 bytes cargo test -p cursor-server --lib gate::tests::grep_content_gate_survives -> FAILED: panicked at gate.rs:261: attempt to subtract with overflow cargo test -p cursor-server --lib gate::tests::truncate_text_terminates -> never returns (killed after 45s) cargo test -p cursor-server --lib gate::tests::grep_content_gate_terminates -> never returns (killed after 60s) After: cargo test -p cursor-server --lib tools::tool_call_result::gate -> 4 passed --- .../src/cursor/tools/tool_call_result/gate.rs | 100 +++++++++++++++++- 1 file changed, 96 insertions(+), 4 deletions(-) diff --git a/server/src/cursor/tools/tool_call_result/gate.rs b/server/src/cursor/tools/tool_call_result/gate.rs index c174a5c..44d4d9b 100644 --- a/server/src/cursor/tools/tool_call_result/gate.rs +++ b/server/src/cursor/tools/tool_call_result/gate.rs @@ -258,7 +258,9 @@ fn gate_grep_content(content: &mut pb::GrepContentResult, budget: &mut GrepBudge truncated = true; break; } - budget.content_bytes -= next_match.content.len(); + budget.content_bytes = budget + .content_bytes + .saturating_sub(next_match.content.len()); budget.matches -= 1; next.matches.push(next_match); } @@ -630,15 +632,25 @@ fn truncate_text(tool_name: &str, content: &str, limit: usize) -> String { } let original = content.len(); let mut shown = limit; + let mut previous = None; loop { let notice = format!( "\n\n[truncated: {tool_name} result exceeded {limit} bytes; showing {shown} of {original} bytes]" ); - let available = limit.saturating_sub(notice.len()); - let kept = utf8_prefix(content, available); - if kept.len() == shown { + if notice.len() >= limit { + // The notice alone would blow the budget; keep a plain prefix so the + // result never costs more than `limit` bytes. + return utf8_prefix(content, limit).to_string(); + } + let kept = utf8_prefix(content, limit - notice.len()); + // `notice.len()` grows with the digit count of `shown`, so `kept.len()` + // can alternate between two values across a power-of-ten boundary + // instead of reaching a fixed point. Settle on the first repeat; the + // reported count is then off by one at most and the result still fits. + if kept.len() == shown || previous == Some(kept.len()) { return format!("{}{notice}", kept.trim_end_matches('\n')); } + previous = Some(shown); shown = kept.len(); } } @@ -685,3 +697,83 @@ fn utf8_suffix(value: &str, limit: usize) -> &str { } &value[start..] } + +#[cfg(test)] +mod tests { + use super::*; + + fn grep_tool(matches: Vec) -> pb::tool_call::Tool { + pb::tool_call::Tool::GrepToolCall(pb::GrepToolCall { + args: None, + result: Some(pb::GrepResult { + result: Some(pb::grep_result::Result::Success(pb::GrepSuccess { + active_editor_result: Some(pb::GrepUnionResult { + result: Some(pb::grep_union_result::Result::Content( + pb::GrepContentResult { + matches: vec![pb::GrepFileMatch { + file: "src/lib.rs".into(), + matches: matches + .into_iter() + .enumerate() + .map(|(index, content)| pb::GrepContentMatch { + line_number: index as i32 + 1, + content, + ..Default::default() + }) + .collect(), + }], + ..Default::default() + }, + )), + }), + ..Default::default() + })), + }), + }) + } + + #[test] + fn truncate_text_never_exceeds_its_limit() { + let content = "b".repeat(200); + for limit in 1..=250 { + let output = truncate_text("Grep", &content, limit); + assert!( + output.len() <= limit, + "limit {limit} produced {} bytes", + output.len() + ); + } + } + + #[test] + fn truncate_text_terminates_when_the_notice_length_oscillates() { + // `limit` values where the notice grows and shrinks with the digit count + // of the reported byte count, so the fixed point is never reached. + assert!(truncate_text("Grep", &"b".repeat(200), 78).len() <= 78); + assert!(truncate_text("Grep", &"b".repeat(200), 170).len() <= 170); + assert!(truncate_text("MCP text", &"b".repeat(500), 82).len() <= 82); + } + + #[test] + fn grep_content_gate_survives_a_nearly_exhausted_byte_budget() { + // 16 matches leave 16 bytes of the 32 KiB content budget, which is less + // than the truncation notice for the 17th match. + let mut matches = vec!["a".repeat(2047); 16]; + matches.push("b".repeat(100)); + let mut tool = grep_tool(matches); + let mut content = String::new(); + tool_completion("Grep", &mut tool, &mut content); + } + + #[test] + fn grep_content_gate_terminates_on_an_oscillating_remaining_budget() { + // The same path, tuned so the remaining budget lands on a `limit` where + // the truncation notice length oscillates. + let mut matches = vec!["a".repeat(2043); 15]; + matches.push("a".repeat(2045)); + matches.push("b".repeat(200)); + let mut tool = grep_tool(matches); + let mut content = String::new(); + tool_completion("Grep", &mut tool, &mut content); + } +}