fix(go-adk): truncate A2A session name on rune boundaries#2234
Open
anxkhn wants to merge 1 commit into
Open
Conversation
extractSessionName derived the session name from the first text part of an inbound A2A message and capped it with a byte-wise slice (tp.Text[:sessionNameMaxLength]). Because len() and the slice operate on bytes, a first message longer than the 20-byte cap that begins with multi-byte UTF-8 (CJK, emoji, accented Latin) was cut in the middle of a rune, yielding an invalid-UTF-8 name. That value is persisted via CreateSession: Postgres rejects the invalid text and fails the turn, while SQLite stores mojibake. Count the cap in runes and slice on a rune boundary so the result is always valid UTF-8. Pure-ASCII behavior is unchanged. Adds a table-driven test covering ASCII, at-limit, and multi-byte truncation. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes UTF-8 corruption in A2A session name derivation by ensuring truncation happens on rune boundaries (preventing invalid UTF-8 that can break Postgres session creation), and adds focused unit tests to lock in correct behavior for ASCII and multibyte inputs.
Changes:
- Update
extractSessionNameto truncate based on rune count rather than byte count. - Add a table-driven
TestExtractSessionNamevalidating exact outputs andutf8.ValidString(...)across ASCII + multibyte scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| go/adk/pkg/a2a/executor.go | Makes session-name truncation UTF-8 rune-aware to avoid invalid strings on multibyte input. |
| go/adk/pkg/a2a/executor_test.go | Adds unit coverage ensuring truncation correctness and UTF-8 validity for multibyte and ASCII cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+437
to
440
| if utf8.RuneCountInString(tp.Text) > sessionNameMaxLength { | ||
| return string([]rune(tp.Text)[:sessionNameMaxLength]) + "..." | ||
| } | ||
| return tp.Text |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
extractSessionNameingo/adk/pkg/a2a/executor.goderives a session name from thefirst text part of an inbound A2A message and caps it at
sessionNameMaxLength(20).The cap used a byte-wise slice:
Both
len()and the[:20]slice operate on bytes, not runes. A first message longerthan 20 bytes that begins with multi-byte UTF-8 (CJK, emoji, accented Latin) gets cut
in the middle of a rune, producing an invalid-UTF-8 string.
This is only ~6-7 non-ASCII characters, so it is easy to hit for non-English users.
For example
こんにちは世界(7 CJK runes = 21 bytes) truncates toこんにちは世\xe7\x95..., which is not valid UTF-8.Why it matters
The derived name flows into
state[StateKeySessionName]and is persisted viaCreateSession:textcolumn rejects invalid UTF-8, so thesession-create call fails and the whole turn is aborted with an error, a valid
non-English first message can break the conversation.
The fix
Count the cap in runes and slice on a rune boundary, so the result is always valid
UTF-8:
Pure-ASCII behavior is unchanged (for ASCII, byte count == rune count), and the
"..."suffix is kept. The only additions are the
unicode/utf8import and a short doc commentexplaining why truncation is rune-aware.
Tests
Adds a table-driven
TestExtractSessionNamecolocated ingo/adk/pkg/a2a/executor_test.gocovering:...)Each case asserts both the exact result and
utf8.ValidString(got). The multi-bytecases fail against the old byte-slice code (they return invalid UTF-8) and pass with the
fix; the ASCII cases pass either way.
Verified locally:
This is a small (<100 LoC) bug fix with unit coverage and no CRD/API change, so no E2E
change is needed.