From e44692c14c78b47ef8dc9a77bcb019d87cef4892 Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Wed, 21 Oct 2026 22:15:29 -0500 Subject: [PATCH 1/3] fix(sessions): list only the empty user's sessions in FirestoreSessionService list_sessions checked `if user_id:`, so user_id="" fell through to the all-users collection_group query and returned every user's sessions. Check `is not None` instead, as the other session services do. Fixes #7662 --- .../firestore/firestore_session_service.py | 4 +-- .../test_firestore_session_service.py | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/google/adk/integrations/firestore/firestore_session_service.py b/src/google/adk/integrations/firestore/firestore_session_service.py index 7371b51a96a..a8736c5febb 100644 --- a/src/google/adk/integrations/firestore/firestore_session_service.py +++ b/src/google/adk/integrations/firestore/firestore_session_service.py @@ -368,7 +368,7 @@ async def list_sessions( self, *, app_name: str, user_id: Optional[str] = None ) -> ListSessionsResponse: """Lists sessions from Firestore.""" - if user_id: + if user_id is not None: query = self._get_sessions_ref(app_name, user_id).where( filter=FieldFilter("appName", "==", app_name) ) @@ -392,7 +392,7 @@ def _iter_sessions_data() -> Iterator[dict[str, Any]]: app_state = app_doc.to_dict() if app_doc.exists else {} user_states_map = {} - if user_id: + if user_id is not None: user_ref = ( self.client.collection(self.user_state_collection) .document(app_name) diff --git a/tests/unittests/integrations/firestore/test_firestore_session_service.py b/tests/unittests/integrations/firestore/test_firestore_session_service.py index ec90d3c3809..8b2ec6fe653 100644 --- a/tests/unittests/integrations/firestore/test_firestore_session_service.py +++ b/tests/unittests/integrations/firestore/test_firestore_session_service.py @@ -961,6 +961,32 @@ async def mock_get_all(refs): assert _where_filter(where) == ("appName", "==", app_name) +@pytest.mark.asyncio +async def test_list_sessions_with_empty_user_id_lists_only_that_user( + mock_firestore_client, +): + service = FirestoreSessionService(client=mock_firestore_client) + root_coll = mock_firestore_client.collection.return_value + users_coll = root_coll.document.return_value.collection.return_value + user_doc_ref = users_coll.document.return_value + user_doc_ref.collection.return_value.where.return_value.get = mock.AsyncMock( + return_value=[] + ) + user_doc_ref.get = mock.AsyncMock(return_value=_stored_snapshot(None)) + root_coll.document.return_value.get = mock.AsyncMock( + return_value=_stored_snapshot(None) + ) + mock_firestore_client.collection_group.return_value.where.return_value.get = ( + mock.AsyncMock(return_value=[]) + ) + + response = await service.list_sessions(app_name="test_app", user_id="") + + assert response.sessions == [] + mock_firestore_client.collection_group.assert_not_called() + users_coll.document.assert_any_call("") + + @pytest.mark.asyncio async def test_get_user_state_returns_stored_user_state(mock_firestore_client): """get_user_state returns the user_states document for the app and user.""" From af160d652ed577f16667feb3a95dcbb2135f554f Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Thu, 22 Oct 2026 00:01:00 -0500 Subject: [PATCH 2/3] fix(sessions): return no sessions for an empty user id without querying The Firestore backend rejects a document path with an empty segment (400 InvalidArgument, checked against the emulator), so list_sessions with user_id="" raised instead of returning an empty list, and create_session cannot store a session for that user either. --- .../firestore/firestore_session_service.py | 4 ++++ .../firestore/test_firestore_session_service.py | 14 ++------------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/google/adk/integrations/firestore/firestore_session_service.py b/src/google/adk/integrations/firestore/firestore_session_service.py index a8736c5febb..ece0aae20dd 100644 --- a/src/google/adk/integrations/firestore/firestore_session_service.py +++ b/src/google/adk/integrations/firestore/firestore_session_service.py @@ -368,6 +368,10 @@ async def list_sessions( self, *, app_name: str, user_id: Optional[str] = None ) -> ListSessionsResponse: """Lists sessions from Firestore.""" + if user_id == "": + # Firestore rejects a document path with an empty segment, so no session + # can be stored for this user, and querying for one would fail. + return ListSessionsResponse() if user_id is not None: query = self._get_sessions_ref(app_name, user_id).where( filter=FieldFilter("appName", "==", app_name) diff --git a/tests/unittests/integrations/firestore/test_firestore_session_service.py b/tests/unittests/integrations/firestore/test_firestore_session_service.py index 8b2ec6fe653..f3055aa8c02 100644 --- a/tests/unittests/integrations/firestore/test_firestore_session_service.py +++ b/tests/unittests/integrations/firestore/test_firestore_session_service.py @@ -962,20 +962,11 @@ async def mock_get_all(refs): @pytest.mark.asyncio -async def test_list_sessions_with_empty_user_id_lists_only_that_user( +async def test_list_sessions_with_empty_user_id_returns_no_sessions( mock_firestore_client, ): + """user_id="" must not fall through to the all-users query.""" service = FirestoreSessionService(client=mock_firestore_client) - root_coll = mock_firestore_client.collection.return_value - users_coll = root_coll.document.return_value.collection.return_value - user_doc_ref = users_coll.document.return_value - user_doc_ref.collection.return_value.where.return_value.get = mock.AsyncMock( - return_value=[] - ) - user_doc_ref.get = mock.AsyncMock(return_value=_stored_snapshot(None)) - root_coll.document.return_value.get = mock.AsyncMock( - return_value=_stored_snapshot(None) - ) mock_firestore_client.collection_group.return_value.where.return_value.get = ( mock.AsyncMock(return_value=[]) ) @@ -984,7 +975,6 @@ async def test_list_sessions_with_empty_user_id_lists_only_that_user( assert response.sessions == [] mock_firestore_client.collection_group.assert_not_called() - users_coll.document.assert_any_call("") @pytest.mark.asyncio From d300b2fed94e49cf369c6c5d998c17148733d465 Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Fri, 23 Oct 2026 11:18:18 -0500 Subject: [PATCH 3/3] test(sessions): assert list_sessions(user_id="") makes no Firestore query --- .../integrations/firestore/test_firestore_session_service.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unittests/integrations/firestore/test_firestore_session_service.py b/tests/unittests/integrations/firestore/test_firestore_session_service.py index f3055aa8c02..7f244466171 100644 --- a/tests/unittests/integrations/firestore/test_firestore_session_service.py +++ b/tests/unittests/integrations/firestore/test_firestore_session_service.py @@ -975,6 +975,7 @@ async def test_list_sessions_with_empty_user_id_returns_no_sessions( assert response.sessions == [] mock_firestore_client.collection_group.assert_not_called() + mock_firestore_client.collection.assert_not_called() @pytest.mark.asyncio