fix(sessions): deduplicate events in InMemorySessionService.append_event#5815
Open
devteamaegis wants to merge 2 commits into
Open
fix(sessions): deduplicate events in InMemorySessionService.append_event#5815devteamaegis wants to merge 2 commits into
devteamaegis wants to merge 2 commits into
Conversation
Collaborator
|
Response from ADK Triaging Agent Hello @devteamaegis, thank you for creating this PR! This PR looks great and has been labeled with To help speed up the review process, could you please update your PR description to include:
For more details on these requirements, you can check our contribution guidelines. Thank you! |
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.
Problem
When an orchestrator broadcasts a shared-state delta to several concurrent session references,
append_eventcan be called more than once with the same event object (sameevent.id). The current implementation has no guard against this, so:storage_session.eventsmultiple times.state_deltais applied multiple times, corrupting session/user/app state.This is reproducible by holding two references to the same session (e.g.
session_a = get_session(...)thensession_b = get_session(...)) and callingawait service.append_event(session_a, event)followed byawait service.append_event(session_b, event).Closes #5723.
Fix
Fetch
storage_sessionbefore callingsuper().append_event()and short-circuit early if the event ID is already present:This is a pure in-memory lookup (O(n) over events, negligible for typical session sizes) and has no effect on the happy path.
Tests
Two regression tests added to
tests/unittests/sessions/test_session_service.py:test_append_event_is_idempotent_for_same_event_id— appends the same event twice via two different session references; asserts exactly one event is stored andstate_deltais applied only once.test_append_different_events_not_deduplicated— appends two events with distinct IDs; asserts both are stored (confirms the guard doesn't over-suppress).