Skip to content

fix: prevent panics when truncating non-ASCII strings in agent log helpers#8024

Draft
cgarrett-square wants to merge 5 commits intoblock:mainfrom
cgarrett-square:fix/utf8-slice-panic-agent-log
Draft

fix: prevent panics when truncating non-ASCII strings in agent log helpers#8024
cgarrett-square wants to merge 5 commits intoblock:mainfrom
cgarrett-square:fix/utf8-slice-panic-agent-log

Conversation

@cgarrett-square
Copy link

Summary

Two log helper functions in agent.rs used byte-index slicing (&s[..N]) to truncate strings, which panics at runtime if the cutoff byte falls in the middle of a multi-byte UTF-8 character (e.g. emoji, accented characters, CJK text).

This caused tokio-runtime-worker thread panics that killed the agent mid-session whenever a user message or tool response contained non-ASCII content near the truncation boundary.

Affected sites:

  • derive_call_reason — truncated user message preview at byte 100
  • format_messages_for_log — truncated tool response body at byte 2000

Fix

Replace byte-index slicing with char_indices().nth(N) to find a safe UTF-8 character boundary before slicing. This is the standard Rust idiom for character-count-based truncation.

Test plan

  • Added unit tests for both sites covering: ASCII truncation, short string (no truncation), and multi-byte input (emoji / CJK) that would have panicked before the fix
  • Manually send a message containing emoji or non-ASCII characters and confirm the agent no longer panics mid-session

Clay Garrett and others added 5 commits March 12, 2026 11:50
…response merges

After a tool executes and the subsequent LLM call fails with a provider
error (network, credits, context, etc.), the error message was yielded
to the frontend but never added to messages_to_add. The session would
end with user(tool_response) as the last message. On the next user
turn, fix_conversation's merge_consecutive_messages would combine that
tool_response user message with the new user message into a single
mixed-content message, which is invalid for OpenAI-format providers.
This caused an irrecoverable loop: every new message the user sent
triggered the same provider rejection.

Fix 1 (agent.rs): add the error message to messages_to_add before
breaking in all provider error branches so the session always ends
with an assistant message after a failed LLM call.

Fix 2 (conversation/mod.rs): do not merge consecutive user messages
when either contains ToolResponse content — tool results must stay
in their own message so providers can correlate them with the
corresponding tool requests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lpers

Byte-index slicing (`&s[..N]`) panics if the Nth byte falls in the
middle of a multi-byte UTF-8 character (e.g. emoji). Two sites in
agent.rs were affected:

- `derive_call_reason`: truncating user message preview at byte 100
- `format_messages_for_log`: truncating tool response body at byte 2000

Replace both with `char_indices().nth(N)` to find a safe char boundary
before slicing, eliminating the runtime panic.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers the two panic sites fixed in the previous commit:
- derive_call_reason: truncation of user message preview at char 100
- format_messages_for_log: truncation of tool response body at char 2000

Each site gets three cases: ASCII truncation, short (no truncation),
and a multi-byte input (emoji / CJK) that would have panicked before
the fix.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…austive error

- Replace char_indices byte-boundary slicing with chars().take(N).collect()
  to satisfy the clippy::string_slice lint (no byte-index slicing at all)
- Construct CallToolResult via serde_json deserialization in test to avoid
  the #[non_exhaustive] restriction on struct literals outside rmcp crate

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my agent went ahead and was a bit too eager; still though the PR description talks about an issue introduced in this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants