fix(a2a): percent-encode context id fields so ids with separators round-trip#6407
Open
anxkhn wants to merge 1 commit into
Open
fix(a2a): percent-encode context id fields so ids with separators round-trip#6407anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
…nd-trip _to_a2a_context_id joins [ADK, app_name, user_id, session_id] on "/" and _from_a2a_context_id splits on "/" and requires exactly four parts. When any of app_name, user_id or session_id contains "/" (for example fully-qualified resource paths used as ids), the join produces more than four segments and the inverse silently returns (None, None, None), so A2A session resolution fails for those ids. Percent-encode each field on encode and decode it on decode. This keeps the exact four-part structure, remains invertible for a separator in any field, and leaves ids without special characters byte-identical to the previous format. Also fix the stale docstring that still described the old "$"-delimited format and drop the unreachable except ValueError (str.split never raises). Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
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.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
Problem:
The A2A context id codec in
src/google/adk/a2a/converters/utils.pyis notinvertible when
app_name,user_id, orsession_idcontains the/separator.
_to_a2a_context_idjoins[ADK, app_name, user_id, session_id]onADK_CONTEXT_ID_SEPARATOR("/"), and_from_a2a_context_idsplits on"/"and requires exactly four parts, otherwise it returns
(None, None, None). Sowhen any field contains
/, the join produces more than four segments and theinverse silently fails:
A returned
(None, None, None)means A2A session resolution silently fails forthose ids. Fully-qualified resource paths are a realistic source of embedded
/(for example resource paths used as auser_id, or an Agent Engine sessionpath used as a
session_id).The decode path also had a stale docstring that still described the old
ADK$app_name$user_id$session_idformat, and an unreachableexcept ValueErroraround
str.split(which never raises), which masked the failure mode.Solution:
Percent-encode each field on encode (
quote(field, safe="")) and decode it ondecode (
unquote(...)). This is the smallest change that:ADK/<app>/<user>/<session>structure (so theexisting "too many parts" rejection still holds),
split(sep, 3)wouldonly fix
session_id, notuser_id/app_name, and would break the existingtoo-many-parts test),
so there is no breaking change on the wire for the common case.
I also fixed the stale
$-format docstring and removed the deadexcept ValueError.Testing Plan
Unit Tests:
Added a parametrized regression test
test_roundtrip_context_id_with_separator_in_idscovering/insession_id,in
user_id, inapp_name, and a multi-slash session id. It fails on thecurrent code (each case returns
(None, None, None)) and passes with the fix.Also updated
test_to_a2a_context_id_special_charactersto expect thepercent-encoded form (
%40for@).pre-commit run --files src/google/adk/a2a/converters/utils.py tests/unittests/a2a/converters/test_utils.pypasses (ruff, isort, pyink, addlicense, ADK compliance checks) with no
reformatting.
Manual End-to-End (E2E) Tests:
This is a self-contained, pure-function correctness fix in the A2A context id
codec with no runtime/UI surface, so it is exercised by the unit tests above
rather than an
adk web/runner flow. The snippet in the Problem section is aminimal manual repro (returns
(None, None, None)before the fix, the originalids after).
Checklist
Additional context
The percent-encode approach was chosen over a bounded left split because it fixes
every field (not just
session_id) and preserves the existing four-partvalidation (including the "too many parts" rejection test). Ids without special
characters are unchanged on the wire.