From 5b6262c4ebdcfba3ee77693063ce03fc5415f258 Mon Sep 17 00:00:00 2001 From: Rishab Motgi Date: Thu, 14 May 2026 16:38:05 -0700 Subject: [PATCH] fix: guard non-positive session limits and close() race in SQLite sessions AsyncSQLiteSession.get_items() and SQLAlchemySession.get_items() passed the resolved session_limit directly to SQL LIMIT without checking whether the value is non-positive. MongoDB and Redis already guard this: if session_limit is not None and session_limit <= 0: return [] Without the guard, SQLite treats LIMIT -1 as 'no limit' and returns all rows, while MongoDB/Redis would return []. This inconsistency means the same SessionSettings(limit=-1) produces different histories depending on which backend is used. Also fix a TOCTOU race in AsyncSQLiteSession.close(): the None check happened outside the asyncio lock, so two concurrent close() calls could both pass the guard and the second one would call .close() on a None connection. Add a second check inside the lock (standard double-checked pattern). Changes: - src/agents/extensions/memory/async_sqlite_session.py: add non-positive limit guard in get_items(); add inside-lock None re-check in close() - src/agents/extensions/memory/sqlalchemy_session.py: add non-positive limit guard in get_items() - tests: extend existing limit tests with limit=-1 assertions for both AsyncSQLiteSession and SQLAlchemySession --- src/agents/extensions/memory/async_sqlite_session.py | 5 +++++ src/agents/extensions/memory/sqlalchemy_session.py | 3 +++ tests/extensions/memory/test_async_sqlite_session.py | 6 ++++++ tests/extensions/memory/test_sqlalchemy_session.py | 4 ++++ 4 files changed, 18 insertions(+) diff --git a/src/agents/extensions/memory/async_sqlite_session.py b/src/agents/extensions/memory/async_sqlite_session.py index 27a23b1cbe..d2b2c5b44e 100644 --- a/src/agents/extensions/memory/async_sqlite_session.py +++ b/src/agents/extensions/memory/async_sqlite_session.py @@ -119,6 +119,9 @@ async def get_items(self, limit: int | None = None) -> list[TResponseInputItem]: session_limit = resolve_session_limit(limit, self.session_settings) + if session_limit is not None and session_limit <= 0: + return [] + async with self._locked_connection() as conn: if session_limit is None: cursor = await conn.execute( @@ -259,5 +262,7 @@ async def close(self) -> None: if self._connection is None: return async with self._lock: + if self._connection is None: + return await self._connection.close() self._connection = None diff --git a/src/agents/extensions/memory/sqlalchemy_session.py b/src/agents/extensions/memory/sqlalchemy_session.py index fd2502e24b..306d322ef9 100644 --- a/src/agents/extensions/memory/sqlalchemy_session.py +++ b/src/agents/extensions/memory/sqlalchemy_session.py @@ -288,6 +288,9 @@ async def get_items(self, limit: int | None = None) -> list[TResponseInputItem]: session_limit = resolve_session_limit(limit, self.session_settings) + if session_limit is not None and session_limit <= 0: + return [] + async with self._session_factory() as sess: if session_limit is None: stmt = ( diff --git a/tests/extensions/memory/test_async_sqlite_session.py b/tests/extensions/memory/test_async_sqlite_session.py index 7269951829..4332df7dfb 100644 --- a/tests/extensions/memory/test_async_sqlite_session.py +++ b/tests/extensions/memory/test_async_sqlite_session.py @@ -138,6 +138,12 @@ async def test_async_sqlite_session_get_items_limit(): none = await session.get_items(limit=0) assert none == [] + # Negative limit must also return [] to match MongoDB/Redis parity. + # SQLite treats LIMIT -1 as "no limit", so without an explicit guard + # a negative limit would incorrectly return all rows instead of []. + neg = await session.get_items(limit=-1) + assert neg == [] + await session.close() diff --git a/tests/extensions/memory/test_sqlalchemy_session.py b/tests/extensions/memory/test_sqlalchemy_session.py index fe30993699..28043543d9 100644 --- a/tests/extensions/memory/test_sqlalchemy_session.py +++ b/tests/extensions/memory/test_sqlalchemy_session.py @@ -183,6 +183,10 @@ async def test_get_items_with_limit(agent: Agent): more_than_all = await session.get_items(limit=10) assert len(more_than_all) == 4 + # Non-positive limits must return [] for parity with MongoDB/Redis backends. + assert await session.get_items(limit=0) == [] + assert await session.get_items(limit=-1) == [] + async def test_pop_from_empty_session(): """Test that pop_item returns None on an empty session."""