From 4f2d25cf95763461d86006cb6454e0e2802620a6 Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Tue, 26 May 2026 09:57:46 -0700 Subject: [PATCH] fix(advanced-sqlite-session): re-raise structure metadata write failures AdvancedSQLiteSession.add_items() commits the base messages table write, then writes message_structure rows. If the structure write fails, the previous implementation logged the error and returned successfully without raising. Subsequent get_items() calls join through message_structure and see nothing, so the caller could observe a 'successful' add_items() followed by missing reads with no way to know whether to retry. Re-raise the original exception after the orphan cleanup attempt so the caller learns add_items() did not succeed. The cleanup path that deletes the orphaned base rows still runs first, so the on-disk state stays internally consistent. Fixes #3348. --- .../memory/advanced_sqlite_session.py | 4 +++ .../memory/test_advanced_sqlite_session.py | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/agents/extensions/memory/advanced_sqlite_session.py b/src/agents/extensions/memory/advanced_sqlite_session.py index 83c289bdf8..6337871862 100644 --- a/src/agents/extensions/memory/advanced_sqlite_session.py +++ b/src/agents/extensions/memory/advanced_sqlite_session.py @@ -153,6 +153,10 @@ def _add_items_sync(): except Exception as cleanup_error: conn.rollback() self._logger.error(f"Failed to cleanup orphaned messages: {cleanup_error}") + # Re-raise so callers see the failure. Without this, add_items returns + # successfully even though the message_structure rows that get_items joins + # through are missing, and the new items become invisible to later reads. + raise await asyncio.to_thread(_add_items_sync) diff --git a/tests/extensions/memory/test_advanced_sqlite_session.py b/tests/extensions/memory/test_advanced_sqlite_session.py index ad4b5c4d86..1f6c26c7eb 100644 --- a/tests/extensions/memory/test_advanced_sqlite_session.py +++ b/tests/extensions/memory/test_advanced_sqlite_session.py @@ -1422,3 +1422,32 @@ async def test_output_tokens_details_persisted_when_input_details_missing(): assert turn_usage["output_tokens_details"] == {"reasoning_tokens": 42} assert turn_usage["input_tokens_details"] is None session.close() + + +async def test_add_items_propagates_structure_metadata_errors(): + # Regression test for openai/openai-agents-python#3348. + # If _insert_structure_metadata raises, add_items used to log the error + # and return without raising, so callers could not tell that the new + # items were missing from later get_items() reads. Verify the error is + # now propagated to the caller and that the orphaned base rows are + # cleaned up (so the session stays internally consistent). + import sqlite3 + + class BrokenMetadataSession(AdvancedSQLiteSession): + def _insert_structure_metadata( + self, conn: sqlite3.Connection, items: list[TResponseInputItem] + ) -> None: + raise RuntimeError("simulated structure metadata write failure") + + session = BrokenMetadataSession(session_id="propagation_test", create_tables=True) + try: + items: list[TResponseInputItem] = [{"role": "user", "content": "hi"}] + with pytest.raises(RuntimeError, match="simulated structure metadata write failure"): + await session.add_items(items) + + # The orphan cleanup path runs in the except block, so subsequent + # reads see an empty session rather than rows the caller cannot reach. + retrieved = await session.get_items() + assert retrieved == [] + finally: + session.close()