fix: Make EUC request args JSON-serializable#6007
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.
adk-bot
added
the
core
|
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
|
create_auth_request_event in _workflow_hitl_utils.py dumped AuthToolArguments with the default mode="python", the same hazard fixed in functions.py: auth_scheme.type stays a live enum, so any consumer that json.dumps the in-memory event before it round-trips the session DB raises TypeError. Use mode="json" there too. - Add regression test asserting create_auth_request_event args are JSON-serializable (fails before, passes after) - Trim prior commit's multi-line test comment to a one-liner Closes google#6006
|
I have completed a thorough, read-only architectural and style compliance analysis of PR #6007 (which addresses the serialization crash reported in Issue #6006). The Google 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
|
rohityan
added
the
request clarification
|
Hi @doughayden, Thank you for your contribution! We appreciate you taking the time to submit this pull request. Please fix formatting errors before we can proceed with the review. |
|
Thanks @rohityan. The failing check is the check-file-contents hardcoded endpoints scan. It's a clean fix for I want your call on |
|
Not pushing changes to tests following #6006 (comment) |
|
Hi @doughayden , please fix the failing mypy-diff tests. |
|
Hi @rohityan, the mypy failure in https://github.com/google/adk-python/actions/runs/27711711919/job/81974523407 is in a file this PR did not touch. The PR branch is behind |
Merge #6007 ### Link to Issue or Description of Change - Closes: #6006 **Problem:** Two call sites dump `AuthToolArguments` with the default `mode="python"`, leaving the auth scheme's `type` field as a live `SecuritySchemeType` enum inside the `adk_request_credential` FunctionCall args: - `build_auth_request_event` in `flows/llm_flows/functions.py` (the LLM-flow EUC path) - `create_auth_request_event` in `workflow/utils/_workflow_hitl_utils.py` (the v2 workflow HITL path) The in-memory `Event.content` is therefore not JSON-serializable: any consumer that `json.dumps` it before it round-trips the session DB raises `TypeError: 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"` at both sites, which coerces the enum to its `"oauth2"` string while preserving `by_alias`/`exclude_none`. Parse-back is unaffected: pydantic validation already accepts the string form, exactly what DB-read events carry. Two one-line changes, each with a regression test. ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. Both tests build an EUC/auth request, assert the args are JSON-serializable, and assert the auth scheme type is the string `"oauth2"`. Each fails before its change and passes after. - `test_function_request_euc_args_are_json_serializable` in `tests/unittests/flows/llm_flows/test_functions_request_euc.py` (LLM-flow path) - `test_args_are_json_serializable` in `tests/unittests/workflow/utils/test_workflow_hitl_utils.py` (workflow HITL path) ```text $ pytest tests/unittests/flows/llm_flows/test_functions_request_euc.py -q 4 passed, 24 warnings in 1.25s $ pytest tests/unittests/workflow/utils/test_workflow_hitl_utils.py -q 12 passed, 5 warnings in 0.67s ``` **Manual End-to-End (E2E) Tests:** The inline repro in the linked issue is the manual check: `json.dumps` on the args dict raises `TypeError` before the change and passes after. Verified against google-adk 2.2.0 (== current `main`). ### Checklist - [x] I have read the CONTRIBUTING.md document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [ ] Any dependent changes have been merged and published in downstream modules. ### Additional context The workflow HITL site was folded in per @surajksharma07's suggestion on the issue, so both auth-request builders are now covered. The remaining sibling `model_dump(exclude_none=True, by_alias=True)` call sites in `functions.py` (the tool-confirmation args near `:368`/`:371` and the event-actions dump near `:1241`) follow the same python-mode pattern and may carry the same hazard if their models contain enums, but they are a different feature path with no confirmed repro, so I left them out of scope. Happy to fold them in if you'd prefer consistency across the file. Co-authored-by: George Weale <gweale@google.com> COPYBARA_INTEGRATE_REVIEW=#6007 from doughayden:fix/euc-request-args-json-serializable 71f02d9 PiperOrigin-RevId: 933934084
|
Thank you @doughayden for your contribution! 🎉 Your changes have been successfully imported and merged via Copybara in commit a181a39. Closing this PR as the changes are now in the main branch. |
3 participants
Link to Issue or Description of Change
Problem:
Two call sites dump
AuthToolArgumentswith the defaultmode="python", leaving the auth scheme'stypefield as a liveSecuritySchemeTypeenum inside theadk_request_credentialFunctionCall args:build_auth_request_eventinflows/llm_flows/functions.py(the LLM-flow EUC path)create_auth_request_eventinworkflow/utils/_workflow_hitl_utils.py(the v2 workflow HITL path)The in-memory
Event.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"at both sites, 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. Two one-line changes, each with a regression test.Testing Plan
Unit Tests:
Both tests build an EUC/auth request, assert the args are JSON-serializable, and assert the auth scheme type is the string
"oauth2". Each fails before its change and passes after.test_function_request_euc_args_are_json_serializableintests/unittests/flows/llm_flows/test_functions_request_euc.py(LLM-flow path)test_args_are_json_serializableintests/unittests/workflow/utils/test_workflow_hitl_utils.py(workflow HITL path)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
The workflow HITL site was folded in per @surajksharma07's suggestion on the issue, so both auth-request builders are now covered. The remaining sibling
model_dump(exclude_none=True, by_alias=True)call sites infunctions.py(the tool-confirmation args near: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, but they are a different feature path with no confirmed repro, so I left them out of scope. Happy to fold them in if you'd prefer consistency across the file.