Skip to content

fix(memory): roll back a failed SQLiteSession insert - #4163

Merged
seratch merged 1 commit into
openai:mainfrom
LeSingh1:fix/sqlite-session-rollback-on-insert-failure
Aug 4, 2026
Merged

fix(memory): roll back a failed SQLiteSession insert#4163
seratch merged 1 commit into
openai:mainfrom
LeSingh1:fix/sqlite-session-rollback-on-insert-failure

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

_locked_connection() does not manage transactions, and SQLiteSession.add_items had no rollback. When _insert_items raised partway — for example json.dumps() failing on a non-serializable item, which happens after the sessions-table upsert has already opened a write transaction — that transaction was left open on a cached connection (shared for :memory:, thread-local for file DBs).

SQLite then held the write lock for the lifetime of that connection, so every later writer against the same file failed with database is locked, including the same session called from another thread. The wedge outlives the failed call:

control (successful add_items): probe write OK
add_items raised: TypeError Object of type object is not JSON serializable
after failed add_items: probe write FAILED -> database is locked
later add_items FAILED -> OperationalError: database is locked

Roll back explicitly on failure, matching what the AdvancedSQLiteSession subclass already does for its own multi-statement writes.

Test plan

Added test_sqlite_session_failed_add_items_releases_write_lock, which triggers a mid-insert failure and then writes from an independent sqlite3 connection opened with timeout=0 — the busy handler is disabled so a retained lock fails immediately rather than after a 5s wait, keeping the test fast and non-flaky. It fails on main with sqlite3.OperationalError: database is locked and passes with the change. It also asserts the partial sessions-table row was rolled back and that the session remains usable afterwards.

Full file: 41 passed. .agents/skills/code-change-verification/scripts/run.sh passed.

Issue number

N/A — found while auditing session backends for transaction handling on failure paths.

Checks

  • I've added new tests, if relevant
  • I've run .agents/skills/code-change-verification/scripts/run.sh
  • I've confirmed all verification steps pass
  • If using Codex, I've run /review before submitting this PR

_locked_connection() does not manage transactions, so when _insert_items
raised partway through add_items the write transaction opened by the
sessions-table upsert was left open on a cached connection. SQLite then
kept the write lock for the lifetime of that connection, so every later
writer against the same database file — including the same session from
another thread — failed with "database is locked" for the rest of the
process.

Roll back explicitly on failure, the same way AdvancedSQLiteSession
already does for its own multi-statement writes.
@seratch seratch added this to the 0.19.x milestone Aug 4, 2026
@seratch
seratch enabled auto-merge (squash) August 4, 2026 02:53
@seratch
seratch merged commit 7b75874 into openai:main Aug 4, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants