diff --git a/src/google/adk/sessions/_session_util.py b/src/google/adk/sessions/_session_util.py index 2fc0c811d8..6f7335c6fa 100644 --- a/src/google/adk/sessions/_session_util.py +++ b/src/google/adk/sessions/_session_util.py @@ -44,6 +44,17 @@ def decode_model(data: object | None, model_cls: type[M]) -> M | None: return model_cls.model_validate(data) +def normalize_session_id(session_id: str | None) -> str | None: + """Normalizes a session id so that writes and reads agree on the key. + + Ids often arrive from files, environment variables or spreadsheet cells with + surrounding whitespace. Every entry point must apply the same normalization, + otherwise a session created with a padded id cannot be read or deleted with + that same id. + """ + return session_id.strip() if session_id else session_id + + def extract_state_delta( state: dict[str, Any], ) -> dict[str, dict[str, Any]]: diff --git a/src/google/adk/sessions/in_memory_session_service.py b/src/google/adk/sessions/in_memory_session_service.py index e7613e8f50..ee07ab302b 100644 --- a/src/google/adk/sessions/in_memory_session_service.py +++ b/src/google/adk/sessions/in_memory_session_service.py @@ -114,7 +114,7 @@ def _create_session_impl( state: Optional[dict[str, Any]] = None, session_id: Optional[str] = None, ) -> Session: - session_id = session_id.strip() if session_id else None + session_id = _session_util.normalize_session_id(session_id) if session_id and self._get_session_impl( app_name=app_name, user_id=user_id, session_id=session_id ): @@ -188,6 +188,7 @@ def _get_session_impl( session_id: str, config: Optional[GetSessionConfig] = None, ) -> Optional[Session]: + session_id = _session_util.normalize_session_id(session_id) if app_name not in self.sessions: return None if user_id not in self.sessions[app_name]: @@ -303,6 +304,7 @@ def delete_session_sync( def _delete_session_impl( self, *, app_name: str, user_id: str, session_id: str ) -> None: + session_id = _session_util.normalize_session_id(session_id) if ( self._get_session_impl( app_name=app_name, user_id=user_id, session_id=session_id diff --git a/src/google/adk/sessions/sqlite_session_service.py b/src/google/adk/sessions/sqlite_session_service.py index e40eb08768..0922d2b9f0 100644 --- a/src/google/adk/sessions/sqlite_session_service.py +++ b/src/google/adk/sessions/sqlite_session_service.py @@ -206,8 +206,7 @@ async def create_session( state: Optional[dict[str, Any]] = None, session_id: Optional[str] = None, ) -> Session: - if session_id: - session_id = session_id.strip() + session_id = _session_util.normalize_session_id(session_id) if not session_id: session_id = platform_uuid.new_uuid() now = platform_time.get_time() @@ -280,6 +279,7 @@ async def get_session( session_id: str, config: Optional[GetSessionConfig] = None, ) -> Optional[Session]: + session_id = _session_util.normalize_session_id(session_id) async with self._get_db_connection() as db: async with db.execute( "SELECT state, update_time FROM sessions WHERE app_name=? AND" @@ -403,6 +403,7 @@ async def list_sessions( async def delete_session( self, *, app_name: str, user_id: str, session_id: str ) -> None: + session_id = _session_util.normalize_session_id(session_id) async with self._get_db_connection() as db: await db.execute( "DELETE FROM sessions WHERE app_name=? AND user_id=? AND id=?", diff --git a/tests/unittests/sessions/test_session_service.py b/tests/unittests/sessions/test_session_service.py index 9148eab8e0..df8205cd51 100644 --- a/tests/unittests/sessions/test_session_service.py +++ b/tests/unittests/sessions/test_session_service.py @@ -1227,6 +1227,46 @@ async def test_create_session_with_padded_duplicate_id_raises_error(): assert session.state['keep'] == 'original' +@pytest.mark.asyncio +@pytest.mark.parametrize( + 'service_type', + [SessionServiceType.IN_MEMORY, SessionServiceType.SQLITE], +) +async def test_padded_session_id_reads_and_deletes(service_type, tmp_path): + """Tests that the id normalization applied on create also applies on read + and delete, so a caller passing one padded id throughout keeps reaching the + session it created.""" + service = get_session_service(service_type, tmp_path) + app_name = 'my_app' + user_id = 'test_user' + padded_id = 'order-42\n' + + created = await service.create_session( + app_name=app_name, + user_id=user_id, + session_id=padded_id, + state={'cart': 'book'}, + ) + assert created.id == 'order-42' + + session = await service.get_session( + app_name=app_name, user_id=user_id, session_id=padded_id + ) + assert session is not None + assert session.id == 'order-42' + assert session.state['cart'] == 'book' + + await service.delete_session( + app_name=app_name, user_id=user_id, session_id=padded_id + ) + assert ( + await service.get_session( + app_name=app_name, user_id=user_id, session_id='order-42' + ) + is None + ) + + @pytest.mark.asyncio async def test_create_session_with_blank_id_generates_one(): """Tests that a whitespace-only session id is treated the same as no id diff --git a/tests/unittests/sessions/test_session_util.py b/tests/unittests/sessions/test_session_util.py index 28b79e9eb2..d753653baa 100644 --- a/tests/unittests/sessions/test_session_util.py +++ b/tests/unittests/sessions/test_session_util.py @@ -25,6 +25,7 @@ from google.adk.sessions._session_util import decode_model from google.adk.sessions._session_util import extract_state_delta from google.adk.sessions._session_util import make_json_safe_state +from google.adk.sessions._session_util import normalize_session_id from google.genai import types from pydantic import BaseModel import pytest @@ -62,6 +63,22 @@ class _SampleModel(BaseModel): decode_model({"name": "foo"}, _SampleModel) +class TestNormalizeSessionId: + """Tests for normalize_session_id utility.""" + + def test_strips_surrounding_whitespace(self): + """An id padded by a file or CSV cell normalizes to its trimmed form.""" + assert normalize_session_id(" order-42\n") == "order-42" + + def test_maps_whitespace_only_id_to_a_falsy_value(self): + """A whitespace-only id is as good as no id, so callers can generate one.""" + assert not normalize_session_id(" ") + + def test_passes_none_through(self): + """A missing id stays missing rather than becoming an empty string.""" + assert normalize_session_id(None) is None + + class TestExtractStateDelta: """Tests for extract_state_delta utility."""