fix: Make EUC request args JSON-serializable#6007
Open
doughayden wants to merge 1 commit into
Open
Conversation
build_auth_request_event dumped AuthToolArguments with the default mode="python", leaving auth_scheme.type as a live SecuritySchemeType enum in the adk_request_credential FunctionCall args. Any consumer that json.dumps the in-memory event before it round-trips the session DB raises TypeError; memory ingestion is a concrete trigger. Use mode="json" to coerce the enum to its string value. Parse-back is unaffected: pydantic validation accepts the string form, exactly what DB-read events already carry.
Collaborator
|
I have completed a thorough, read-only architectural and QA compliance analysis of PR #6007 (addressing the serialization crash reported in Issue #6006). The Contributor License Agreement (CLA) verification for contributor I have generated a detailed PR Analysis Report artifact, which you can read directly. 💡 Key Findings & Highlights
Recommended Next Step
|
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.
Link to Issue or Description of Change
Problem:
build_auth_request_eventdumpsAuthToolArgumentswith the defaultmode="python", leaving the auth scheme'stypefield as a liveSecuritySchemeTypeenum inside theadk_request_credentialFunctionCall args. The in-memoryEvent.contentis therefore not JSON-serializable: any consumer thatjson.dumpsit before it round-trips the session DB raisesTypeError: Object of type SecuritySchemeType is not JSON serializable. Memory ingestion (VertexAiMemoryBankService) is a concrete trigger. It stays hidden in most flows because the session DB launders enums to strings on write and the auth preprocessor re-validates from either form.Solution:
Dump with
mode="json", which coerces the enum to its"oauth2"string while preservingby_alias/exclude_none. Parse-back is unaffected: pydantic validation already accepts the string form, exactly what DB-read events carry. One line plus a regression test.Testing Plan
Unit Tests:
Added
test_function_request_euc_args_are_json_serializableintests/unittests/flows/llm_flows/test_functions_request_euc.py: runs the EUC flow and asserts theadk_request_credentialargs are JSON-serializable and the auth scheme type is the string"oauth2". Fails before the change, passes after.Manual End-to-End (E2E) Tests:
The inline repro in the linked issue is the manual check:
json.dumpson the args dict raisesTypeErrorbefore the change and passes after. Verified against google-adk 2.2.0 (== currentmain).Checklist
Additional context
Sibling
model_dump(exclude_none=True, by_alias=True)call sites in the same file (the tool-confirmation args nearfunctions.py:368/:371and the event-actions dump near:1241) follow the same python-mode pattern and may carry the same hazard if their models contain enums. I scoped this PR to the confirmed, reproducible auth path; happy to fold the siblings in if you'd prefer consistency across the file.