fix: exclude temp: state keys from Firestore session writes#5841
fix: exclude temp: state keys from Firestore session writes#5841Tinnitussen wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
adk-bot
added
the
services
|
Response from ADK Triaging Agent Hello @Tinnitussen, thank you for your contribution to the ADK project! To help us review and merge your pull request more efficiently, please ensure it complies with our Contribution Guidelines. Currently, the following items are missing or require action:
Thank you again for your PR! Let us know if you have any questions. |
rohityan
added
the
request clarification
|
Hi @Tinnitussen , Thank you for your contribution! It appears you haven't yet signed the Contributor License Agreement (CLA). Please visit https://cla.developers.google.com/ to complete the signing process. Once the CLA is signed, we'll be able to proceed with the review of your PR. Thank you! |
FirestoreSessionService.append_event() writes the full session.state dict to Firestore, filtering app: and user: prefixed keys but not temp: prefixed keys. This causes crashes when temp state contains non-serializable objects (e.g. GroundingMetadata from GoogleSearchTool via AgentTool's propagate_grounding_metadata). The DatabaseSessionService and SqliteSessionService don't have this issue because they only persist state_deltas extracted via extract_state_delta(), which already filters temp: keys. Fix: add State.TEMP_PREFIX to the exclusion filter in session_only_state. Test: extend existing test_append_event_with_temp_state to assert temp keys are absent from the persisted session state dict.
Tinnitussen
force-pushed
the
fix/firestore-temp-state-leak
branch
from
9a95b05 to
2ab8af3
Compare
|
Hi @rohityan, thanks for the heads up! I've signed the CLA and updated my commit email to match. The CLA check is showing green now. |
3 participants
FirestoreSessionService.append_event() writes full session state to Firestore but only filters out app: and user: prefixed keys - not temp: keys. This causes a crash when GoogleSearchTool is used, because AgentTool stores a raw GroundingMetadata object under temp:_adk_grounding_metadata which Firestore cannot serialize.
DatabaseSessionService and SqliteSessionService don't have this issue because they persist only the output of extract_state_delta(), which already excludes temp: keys.
Fix: Add
and not k.startswith(State.TEMP_PREFIX)to the session_only_state filter.Fixes #5840