Skip to content

Commit dfb7f91

Browse files
committed
Fix: RedisSessionService.list_sessions() state loss bug
## Problem RedisSessionService.list_sessions() was clearing session state by setting `session.state = {}` at line 178, causing loss of app-level and user-level state data when listing sessions. This differed from InMemorySessionService.list_sessions() which correctly preserves state by calling `self._merge_state()`. ## Impact - Session state (including custom fields like 'title', '_ag_ui_thread_id') was lost when listing sessions via the API - Frontend applications couldn't display session metadata in session lists - State was only available when fetching individual sessions via get_session() ## Root Cause The list_sessions() method was directly clearing state instead of merging app/user state from Redis hashes, breaking the state management contract. ## Solution Replace `session.state = {}` with `session = await self._merge_state(app_name, user_id, session)` to match the behavior of InMemorySessionService.list_sessions() ## Changes - Line 178: Changed from `session.state = {}` to merge_state() call - Ensures consistent state handling across all session operations - Maintains backward compatibility (events still cleared as intended) ## Testing - Updated test_create_and_list_sessions to verify state is preserved - Added test_list_sessions_preserves_app_user_state for explicit state verification - All 16 tests pass
1 parent cc56a5e commit dfb7f91

2 files changed

Lines changed: 54 additions & 4 deletions

File tree

src/google/adk_community/sessions/redis_session_service.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ async def list_sessions(
175175
for session_data in sessions.values():
176176
session = Session.model_validate(session_data)
177177
session.events = []
178-
session.state = {}
178+
session = await self._merge_state(app_name, user_id, session)
179179
sessions_without_events.append(session)
180180

181181
return ListSessionsResponse(sessions=sessions_without_events)

tests/unittests/sessions/test_redis_session_service.py

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ async def test_create_and_list_sessions(self, redis_service):
178178
"""Test creating multiple sessions and listing them.
179179
180180
list_sessions() is expected to return lightweight session summaries,
181-
i.e., with events and state stripped for performance.
181+
i.e., with events stripped for performance but state preserved (from app/user state).
182182
"""
183183
app_name = "test_app"
184184
user_id = "test_user"
@@ -212,9 +212,59 @@ async def test_create_and_list_sessions(self, redis_service):
212212
assert returned_session_ids == set(session_ids)
213213

214214
for s in sessions:
215-
# list_sessions returns summaries: events and state removed for perf.
215+
# list_sessions returns summaries: events stripped but state preserved via _merge_state().
216216
assert len(s.events) == 0
217-
assert s.state == {}
217+
# State is preserved (though empty in this test since no app/user state was set)
218+
assert isinstance(s.state, dict)
219+
220+
@pytest.mark.asyncio
221+
async def test_list_sessions_preserves_app_user_state(self, redis_service):
222+
"""Test that list_sessions preserves app and user state via _merge_state."""
223+
app_name = "test_app"
224+
user_id = "test_user"
225+
226+
self._setup_redis_mocks(redis_service)
227+
228+
# Create a session
229+
session = await redis_service.create_session(
230+
app_name=app_name,
231+
user_id=user_id,
232+
session_id="test_session",
233+
state={"title": "My Session Title", "_ag_ui_thread_id": "thread123"},
234+
)
235+
236+
# Mock app and user state in Redis
237+
redis_service.cache.hgetall = AsyncMock(
238+
side_effect=[
239+
{b"shared_config": orjson.dumps("app_level_value")}, # app state
240+
{b"user_pref": orjson.dumps("user_level_value")}, # user state
241+
]
242+
)
243+
244+
sessions_data = {session.id: session.model_dump()}
245+
self._setup_redis_mocks(redis_service, sessions_data)
246+
247+
# Override hgetall mock again after _setup_redis_mocks
248+
redis_service.cache.hgetall = AsyncMock(
249+
side_effect=[
250+
{b"shared_config": orjson.dumps("app_level_value")}, # app state
251+
{b"user_pref": orjson.dumps("user_level_value")}, # user state
252+
]
253+
)
254+
255+
list_sessions_response = await redis_service.list_sessions(
256+
app_name=app_name, user_id=user_id
257+
)
258+
sessions = list_sessions_response.sessions
259+
260+
assert len(sessions) == 1
261+
# Events are stripped
262+
assert len(sessions[0].events) == 0
263+
# But state is preserved via _merge_state (app/user state merged)
264+
assert "app:shared_config" in sessions[0].state
265+
assert sessions[0].state["app:shared_config"] == "app_level_value"
266+
assert "user:user_pref" in sessions[0].state
267+
assert sessions[0].state["user:user_pref"] == "user_level_value"
218268

219269
@pytest.mark.asyncio
220270
async def test_session_state_management(self, redis_service):

0 commit comments

Comments
 (0)