Skip to content

Commit 5b6262c

Browse files
author
Rishab Motgi
committed
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
1 parent 656baf8 commit 5b6262c

4 files changed

Lines changed: 18 additions & 0 deletions

File tree

src/agents/extensions/memory/async_sqlite_session.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,9 @@ async def get_items(self, limit: int | None = None) -> list[TResponseInputItem]:
119119

120120
session_limit = resolve_session_limit(limit, self.session_settings)
121121

122+
if session_limit is not None and session_limit <= 0:
123+
return []
124+
122125
async with self._locked_connection() as conn:
123126
if session_limit is None:
124127
cursor = await conn.execute(
@@ -259,5 +262,7 @@ async def close(self) -> None:
259262
if self._connection is None:
260263
return
261264
async with self._lock:
265+
if self._connection is None:
266+
return
262267
await self._connection.close()
263268
self._connection = None

src/agents/extensions/memory/sqlalchemy_session.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,9 @@ async def get_items(self, limit: int | None = None) -> list[TResponseInputItem]:
288288

289289
session_limit = resolve_session_limit(limit, self.session_settings)
290290

291+
if session_limit is not None and session_limit <= 0:
292+
return []
293+
291294
async with self._session_factory() as sess:
292295
if session_limit is None:
293296
stmt = (

tests/extensions/memory/test_async_sqlite_session.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,12 @@ async def test_async_sqlite_session_get_items_limit():
138138
none = await session.get_items(limit=0)
139139
assert none == []
140140

141+
# Negative limit must also return [] to match MongoDB/Redis parity.
142+
# SQLite treats LIMIT -1 as "no limit", so without an explicit guard
143+
# a negative limit would incorrectly return all rows instead of [].
144+
neg = await session.get_items(limit=-1)
145+
assert neg == []
146+
141147
await session.close()
142148

143149

tests/extensions/memory/test_sqlalchemy_session.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,10 @@ async def test_get_items_with_limit(agent: Agent):
183183
more_than_all = await session.get_items(limit=10)
184184
assert len(more_than_all) == 4
185185

186+
# Non-positive limits must return [] for parity with MongoDB/Redis backends.
187+
assert await session.get_items(limit=0) == []
188+
assert await session.get_items(limit=-1) == []
189+
186190

187191
async def test_pop_from_empty_session():
188192
"""Test that pop_item returns None on an empty session."""

0 commit comments

Comments
 (0)