fix: stop leaking DB connections when persisting session mode (#64)
chat_routes.py persisted a session's "mode" in three best-effort spots —
reading the current mode, writing the effective mode, and setting
research_pending on the stream path. Each opened a session with SessionLocal()
and called .close() as the LAST statement inside a try/except, so if anything
before close() raised (e.g. a SQLite "database is locked" under concurrent chat
streams) the except only logged and the connection was never returned to the
pool.
DATABASE_URL defaults to file-backed SQLite, whose engine uses SQLAlchemy's
default QueuePool (5 connections + 10 overflow). Repeated leaks on these hot
paths exhaust the pool; later requests then block for pool_timeout and fail
with "QueuePool limit ... reached", taking the app down until restart.
Move the logic into two best-effort helpers in core.database, next to the
existing session helpers (update_session_last_accessed, get_session_by_id):
- get_session_mode(session_id) -> Optional[str]
- set_session_mode(session_id, mode) -> bool
Both route through the existing get_db_session() context manager, which commits
on success, rolls back on error, and always closes in a finally, so the
connection is returned to the pool on every path. chat_routes.py now calls
these instead of hand-rolling sessions, also removing three copies of the same
try/except.
Add tests/test_session_mode_helpers.py: the helpers commit+close on success
and, on a mid-operation DB error, swallow + roll back + close (no leak). The
error-path tests fail against the old close()-inside-try pattern.
2026-06-01 00:57:48 -04:00
|
|
|
"""Pin the leak-safety of the session-mode DB helpers.
|
|
|
|
|
|
|
|
|
|
chat_routes.py persists a session's "mode" in three best-effort spots (read
|
|
|
|
|
current mode, persist the effective mode, set research_pending). Those spots
|
|
|
|
|
previously hand-rolled `SessionLocal()` with `.close()` as the LAST statement
|
|
|
|
|
inside a try/except — so any error before close() (e.g. a SQLite "database is
|
|
|
|
|
locked" under concurrent streams) leaked the connection. With the default
|
|
|
|
|
QueuePool for file SQLite (5 + 10 overflow), accumulated leaks exhaust the
|
|
|
|
|
pool and the app can no longer obtain a DB session until restart.
|
|
|
|
|
|
|
|
|
|
The logic now lives in core.database.{get,set}_session_mode, which route
|
|
|
|
|
through get_db_session() (commit/rollback + guaranteed close). These tests pin
|
|
|
|
|
that a mid-operation DB error neither raises out of the helper nor leaks the
|
|
|
|
|
connection. The error-path cases fail against the old close()-inside-try
|
|
|
|
|
pattern.
|
|
|
|
|
"""
|
2026-06-01 00:55:09 -07:00
|
|
|
import ast
|
|
|
|
|
from contextlib import contextmanager
|
|
|
|
|
from pathlib import Path
|
|
|
|
|
from types import SimpleNamespace
|
|
|
|
|
from typing import Generator
|
fix: stop leaking DB connections when persisting session mode (#64)
chat_routes.py persisted a session's "mode" in three best-effort spots —
reading the current mode, writing the effective mode, and setting
research_pending on the stream path. Each opened a session with SessionLocal()
and called .close() as the LAST statement inside a try/except, so if anything
before close() raised (e.g. a SQLite "database is locked" under concurrent chat
streams) the except only logged and the connection was never returned to the
pool.
DATABASE_URL defaults to file-backed SQLite, whose engine uses SQLAlchemy's
default QueuePool (5 connections + 10 overflow). Repeated leaks on these hot
paths exhaust the pool; later requests then block for pool_timeout and fail
with "QueuePool limit ... reached", taking the app down until restart.
Move the logic into two best-effort helpers in core.database, next to the
existing session helpers (update_session_last_accessed, get_session_by_id):
- get_session_mode(session_id) -> Optional[str]
- set_session_mode(session_id, mode) -> bool
Both route through the existing get_db_session() context manager, which commits
on success, rolls back on error, and always closes in a finally, so the
connection is returned to the pool on every path. chat_routes.py now calls
these instead of hand-rolling sessions, also removing three copies of the same
try/except.
Add tests/test_session_mode_helpers.py: the helpers commit+close on success
and, on a mid-operation DB error, swallow + roll back + close (no leak). The
error-path tests fail against the old close()-inside-try pattern.
2026-06-01 00:57:48 -04:00
|
|
|
from unittest.mock import MagicMock
|
|
|
|
|
|
2026-06-01 00:55:09 -07:00
|
|
|
|
|
|
|
|
def _load_db_helpers():
|
|
|
|
|
"""Load only the helper bodies under test, without importing SQLAlchemy."""
|
|
|
|
|
db_path = Path(__file__).parents[1] / "core" / "database.py"
|
|
|
|
|
tree = ast.parse(db_path.read_text(encoding="utf-8"), filename=str(db_path))
|
|
|
|
|
wanted = {"get_db_session", "get_session_mode", "set_session_mode"}
|
|
|
|
|
helper_nodes = [
|
|
|
|
|
node for node in tree.body
|
|
|
|
|
if isinstance(node, ast.FunctionDef) and node.name in wanted
|
|
|
|
|
]
|
|
|
|
|
namespace = {
|
|
|
|
|
"contextmanager": contextmanager,
|
|
|
|
|
"Generator": Generator,
|
|
|
|
|
"Session": MagicMock(),
|
|
|
|
|
"SessionLocal": MagicMock(),
|
|
|
|
|
"logger": MagicMock(),
|
|
|
|
|
}
|
|
|
|
|
exec(compile(ast.Module(helper_nodes, type_ignores=[]), str(db_path), "exec"), namespace)
|
|
|
|
|
return SimpleNamespace(**namespace, _namespace=namespace)
|
fix: stop leaking DB connections when persisting session mode (#64)
chat_routes.py persisted a session's "mode" in three best-effort spots —
reading the current mode, writing the effective mode, and setting
research_pending on the stream path. Each opened a session with SessionLocal()
and called .close() as the LAST statement inside a try/except, so if anything
before close() raised (e.g. a SQLite "database is locked" under concurrent chat
streams) the except only logged and the connection was never returned to the
pool.
DATABASE_URL defaults to file-backed SQLite, whose engine uses SQLAlchemy's
default QueuePool (5 connections + 10 overflow). Repeated leaks on these hot
paths exhaust the pool; later requests then block for pool_timeout and fail
with "QueuePool limit ... reached", taking the app down until restart.
Move the logic into two best-effort helpers in core.database, next to the
existing session helpers (update_session_last_accessed, get_session_by_id):
- get_session_mode(session_id) -> Optional[str]
- set_session_mode(session_id, mode) -> bool
Both route through the existing get_db_session() context manager, which commits
on success, rolls back on error, and always closes in a finally, so the
connection is returned to the pool on every path. chat_routes.py now calls
these instead of hand-rolling sessions, also removing three copies of the same
try/except.
Add tests/test_session_mode_helpers.py: the helpers commit+close on success
and, on a mid-operation DB error, swallow + roll back + close (no leak). The
error-path tests fail against the old close()-inside-try pattern.
2026-06-01 00:57:48 -04:00
|
|
|
|
|
|
|
|
|
|
|
|
|
def _mock_session(monkeypatch):
|
|
|
|
|
"""Make get_db_session() hand out a MagicMock session (no real DB)."""
|
2026-06-01 00:55:09 -07:00
|
|
|
db = _load_db_helpers()
|
fix: stop leaking DB connections when persisting session mode (#64)
chat_routes.py persisted a session's "mode" in three best-effort spots —
reading the current mode, writing the effective mode, and setting
research_pending on the stream path. Each opened a session with SessionLocal()
and called .close() as the LAST statement inside a try/except, so if anything
before close() raised (e.g. a SQLite "database is locked" under concurrent chat
streams) the except only logged and the connection was never returned to the
pool.
DATABASE_URL defaults to file-backed SQLite, whose engine uses SQLAlchemy's
default QueuePool (5 connections + 10 overflow). Repeated leaks on these hot
paths exhaust the pool; later requests then block for pool_timeout and fail
with "QueuePool limit ... reached", taking the app down until restart.
Move the logic into two best-effort helpers in core.database, next to the
existing session helpers (update_session_last_accessed, get_session_by_id):
- get_session_mode(session_id) -> Optional[str]
- set_session_mode(session_id, mode) -> bool
Both route through the existing get_db_session() context manager, which commits
on success, rolls back on error, and always closes in a finally, so the
connection is returned to the pool on every path. chat_routes.py now calls
these instead of hand-rolling sessions, also removing three copies of the same
try/except.
Add tests/test_session_mode_helpers.py: the helpers commit+close on success
and, on a mid-operation DB error, swallow + roll back + close (no leak). The
error-path tests fail against the old close()-inside-try pattern.
2026-06-01 00:57:48 -04:00
|
|
|
sess = MagicMock()
|
2026-06-01 00:55:09 -07:00
|
|
|
monkeypatch.setitem(db._namespace, "SessionLocal", lambda: sess)
|
|
|
|
|
return db, sess
|
fix: stop leaking DB connections when persisting session mode (#64)
chat_routes.py persisted a session's "mode" in three best-effort spots —
reading the current mode, writing the effective mode, and setting
research_pending on the stream path. Each opened a session with SessionLocal()
and called .close() as the LAST statement inside a try/except, so if anything
before close() raised (e.g. a SQLite "database is locked" under concurrent chat
streams) the except only logged and the connection was never returned to the
pool.
DATABASE_URL defaults to file-backed SQLite, whose engine uses SQLAlchemy's
default QueuePool (5 connections + 10 overflow). Repeated leaks on these hot
paths exhaust the pool; later requests then block for pool_timeout and fail
with "QueuePool limit ... reached", taking the app down until restart.
Move the logic into two best-effort helpers in core.database, next to the
existing session helpers (update_session_last_accessed, get_session_by_id):
- get_session_mode(session_id) -> Optional[str]
- set_session_mode(session_id, mode) -> bool
Both route through the existing get_db_session() context manager, which commits
on success, rolls back on error, and always closes in a finally, so the
connection is returned to the pool on every path. chat_routes.py now calls
these instead of hand-rolling sessions, also removing three copies of the same
try/except.
Add tests/test_session_mode_helpers.py: the helpers commit+close on success
and, on a mid-operation DB error, swallow + roll back + close (no leak). The
error-path tests fail against the old close()-inside-try pattern.
2026-06-01 00:57:48 -04:00
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_set_session_mode_commits_and_closes_on_success(monkeypatch):
|
2026-06-01 00:55:09 -07:00
|
|
|
db, sess = _mock_session(monkeypatch)
|
fix: stop leaking DB connections when persisting session mode (#64)
chat_routes.py persisted a session's "mode" in three best-effort spots —
reading the current mode, writing the effective mode, and setting
research_pending on the stream path. Each opened a session with SessionLocal()
and called .close() as the LAST statement inside a try/except, so if anything
before close() raised (e.g. a SQLite "database is locked" under concurrent chat
streams) the except only logged and the connection was never returned to the
pool.
DATABASE_URL defaults to file-backed SQLite, whose engine uses SQLAlchemy's
default QueuePool (5 connections + 10 overflow). Repeated leaks on these hot
paths exhaust the pool; later requests then block for pool_timeout and fail
with "QueuePool limit ... reached", taking the app down until restart.
Move the logic into two best-effort helpers in core.database, next to the
existing session helpers (update_session_last_accessed, get_session_by_id):
- get_session_mode(session_id) -> Optional[str]
- set_session_mode(session_id, mode) -> bool
Both route through the existing get_db_session() context manager, which commits
on success, rolls back on error, and always closes in a finally, so the
connection is returned to the pool on every path. chat_routes.py now calls
these instead of hand-rolling sessions, also removing three copies of the same
try/except.
Add tests/test_session_mode_helpers.py: the helpers commit+close on success
and, on a mid-operation DB error, swallow + roll back + close (no leak). The
error-path tests fail against the old close()-inside-try pattern.
2026-06-01 00:57:48 -04:00
|
|
|
assert db.set_session_mode("s1", "agent") is True
|
|
|
|
|
sess.query.return_value.filter.return_value.update.assert_called_once_with({"mode": "agent"})
|
|
|
|
|
sess.commit.assert_called_once()
|
|
|
|
|
sess.close.assert_called_once()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_set_session_mode_does_not_leak_on_error(monkeypatch):
|
2026-06-01 00:55:09 -07:00
|
|
|
db, sess = _mock_session(monkeypatch)
|
fix: stop leaking DB connections when persisting session mode (#64)
chat_routes.py persisted a session's "mode" in three best-effort spots —
reading the current mode, writing the effective mode, and setting
research_pending on the stream path. Each opened a session with SessionLocal()
and called .close() as the LAST statement inside a try/except, so if anything
before close() raised (e.g. a SQLite "database is locked" under concurrent chat
streams) the except only logged and the connection was never returned to the
pool.
DATABASE_URL defaults to file-backed SQLite, whose engine uses SQLAlchemy's
default QueuePool (5 connections + 10 overflow). Repeated leaks on these hot
paths exhaust the pool; later requests then block for pool_timeout and fail
with "QueuePool limit ... reached", taking the app down until restart.
Move the logic into two best-effort helpers in core.database, next to the
existing session helpers (update_session_last_accessed, get_session_by_id):
- get_session_mode(session_id) -> Optional[str]
- set_session_mode(session_id, mode) -> bool
Both route through the existing get_db_session() context manager, which commits
on success, rolls back on error, and always closes in a finally, so the
connection is returned to the pool on every path. chat_routes.py now calls
these instead of hand-rolling sessions, also removing three copies of the same
try/except.
Add tests/test_session_mode_helpers.py: the helpers commit+close on success
and, on a mid-operation DB error, swallow + roll back + close (no leak). The
error-path tests fail against the old close()-inside-try pattern.
2026-06-01 00:57:48 -04:00
|
|
|
sess.query.return_value.filter.return_value.update.side_effect = RuntimeError("database is locked")
|
|
|
|
|
# Best-effort: the error is swallowed and False returned...
|
|
|
|
|
assert db.set_session_mode("s1", "agent") is False
|
|
|
|
|
# ...and crucially the connection is still returned to the pool.
|
|
|
|
|
sess.rollback.assert_called_once()
|
|
|
|
|
sess.close.assert_called_once()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_get_session_mode_reads_and_closes(monkeypatch):
|
2026-06-01 00:55:09 -07:00
|
|
|
db, sess = _mock_session(monkeypatch)
|
fix: stop leaking DB connections when persisting session mode (#64)
chat_routes.py persisted a session's "mode" in three best-effort spots —
reading the current mode, writing the effective mode, and setting
research_pending on the stream path. Each opened a session with SessionLocal()
and called .close() as the LAST statement inside a try/except, so if anything
before close() raised (e.g. a SQLite "database is locked" under concurrent chat
streams) the except only logged and the connection was never returned to the
pool.
DATABASE_URL defaults to file-backed SQLite, whose engine uses SQLAlchemy's
default QueuePool (5 connections + 10 overflow). Repeated leaks on these hot
paths exhaust the pool; later requests then block for pool_timeout and fail
with "QueuePool limit ... reached", taking the app down until restart.
Move the logic into two best-effort helpers in core.database, next to the
existing session helpers (update_session_last_accessed, get_session_by_id):
- get_session_mode(session_id) -> Optional[str]
- set_session_mode(session_id, mode) -> bool
Both route through the existing get_db_session() context manager, which commits
on success, rolls back on error, and always closes in a finally, so the
connection is returned to the pool on every path. chat_routes.py now calls
these instead of hand-rolling sessions, also removing three copies of the same
try/except.
Add tests/test_session_mode_helpers.py: the helpers commit+close on success
and, on a mid-operation DB error, swallow + roll back + close (no leak). The
error-path tests fail against the old close()-inside-try pattern.
2026-06-01 00:57:48 -04:00
|
|
|
sess.query.return_value.filter.return_value.scalar.return_value = "research_pending"
|
|
|
|
|
assert db.get_session_mode("s1") == "research_pending"
|
|
|
|
|
sess.close.assert_called_once()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_get_session_mode_does_not_leak_on_error(monkeypatch):
|
2026-06-01 00:55:09 -07:00
|
|
|
db, sess = _mock_session(monkeypatch)
|
fix: stop leaking DB connections when persisting session mode (#64)
chat_routes.py persisted a session's "mode" in three best-effort spots —
reading the current mode, writing the effective mode, and setting
research_pending on the stream path. Each opened a session with SessionLocal()
and called .close() as the LAST statement inside a try/except, so if anything
before close() raised (e.g. a SQLite "database is locked" under concurrent chat
streams) the except only logged and the connection was never returned to the
pool.
DATABASE_URL defaults to file-backed SQLite, whose engine uses SQLAlchemy's
default QueuePool (5 connections + 10 overflow). Repeated leaks on these hot
paths exhaust the pool; later requests then block for pool_timeout and fail
with "QueuePool limit ... reached", taking the app down until restart.
Move the logic into two best-effort helpers in core.database, next to the
existing session helpers (update_session_last_accessed, get_session_by_id):
- get_session_mode(session_id) -> Optional[str]
- set_session_mode(session_id, mode) -> bool
Both route through the existing get_db_session() context manager, which commits
on success, rolls back on error, and always closes in a finally, so the
connection is returned to the pool on every path. chat_routes.py now calls
these instead of hand-rolling sessions, also removing three copies of the same
try/except.
Add tests/test_session_mode_helpers.py: the helpers commit+close on success
and, on a mid-operation DB error, swallow + roll back + close (no leak). The
error-path tests fail against the old close()-inside-try pattern.
2026-06-01 00:57:48 -04:00
|
|
|
sess.query.return_value.filter.return_value.scalar.side_effect = RuntimeError("database is locked")
|
|
|
|
|
assert db.get_session_mode("s1") is None
|
|
|
|
|
sess.close.assert_called_once()
|