fix(sqlite): close state.db connections explicitly to stop FD leak in sidebar polling (#1494)

Production WebUI on macOS launchd reproduced an HTTP-unhealthy wedge after
#1483 closed the bootstrap supervisor double-fork: process alive, port
listening, every HTTP request reset by peer before a response. The reporter
(@insecurejezza) traced it to FD exhaustion — 366 open FDs on the wedged
process, 238 of them `~/.hermes/state.db`, `state.db-wal`, and `state.db-shm`.

Root cause: four sqlite callsites use `with sqlite3.connect(...) as conn:`.
Python's sqlite3 connection context manager only commits or rolls back on
exit; it does NOT close the connection. `/api/sessions` polling calls these
on every sidebar refresh, so each poll leaked one or more open state.db FDs
until the process hit macOS's soft FD limit and new sqlite3.connect() calls
inside fresh request handlers raised before any response bytes were written.

Fix: wrap each `sqlite3.connect(...)` in `contextlib.closing(...)` so the
connection is explicitly closed on scope exit, in addition to the auto-
commit / rollback semantics that `Connection.__exit__` already provides.

Callsites patched:
- api/agent_sessions.py:read_importable_agent_session_rows
- api/agent_sessions.py:read_session_lineage_metadata
- api/models.py:get_cli_session_messages
- api/models.py:delete_cli_session

Reporter's verification (post-patch, 100-request stress loop against
/api/sessions and /api/projects):

  batch=1 fd=92 state_handles=0
  batch=2 fd=92 state_handles=0
  ...
  batch=5 fd=92 state_handles=0

Pre-patch the same loop made FD count and state.db handle count climb
monotonically.

4 regression tests in tests/test_issue1494_state_db_fd_leak.py monkeypatch
sqlite3.connect with a tracking wrapper that records .close() calls and
assert every connection opened by each of the four functions is explicitly
closed. Verified to fail (catching the original bug) when the closing()
wrap is reverted: "leaked 5 of 5 sqlite connection(s) — context-manager-
only `with sqlite3.connect()` does not close. Wrap in contextlib.closing()."

This addresses Bug #2 of the umbrella issue #1458. Bug #3 (HTTP-unhealthy
wedge in the absence of FD exhaustion) remains open pending separate
diagnostic data — explicit scope discipline.

Closes #1494
Refs #1458 (Bug #2 of 3)

Co-authored-by: insecurejezza <70424851+insecurejezza@users.noreply.github.com>

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Hermes Web UI -- Changelog
 
+## [Unreleased]
+
+### Fixed (1 PR)
+
+- **`state.db` connection FD leak in sidebar polling** (#1494, fix shape provided by @insecurejezza, closes #1494, addresses Bug #2 of #1458) — production WebUI on macOS launchd reproduced this after #1483 fixed the bootstrap supervisor double-fork: process alive, port listening, every HTTP request reset before a response. Investigation traced it to FD exhaustion from `~/.hermes/state.db` handles (366 total FDs, 238 of them `state.db*` on a wedged process). Root cause: four sqlite callsites used `with sqlite3.connect(...) as conn:`, but Python's `sqlite3.Connection` context manager only commits or rolls back on exit — it does **not** close the connection. `/api/sessions` polling calls these on every sidebar refresh, so each poll leaked one or more open `state.db` / `state.db-wal` / `state.db-shm` FDs until the process hit the macOS soft limit (256) and new connections RST'd before any handler bytes were written. **Fix:** wrap each `sqlite3.connect(...)` call in `contextlib.closing(...)` at: `api/agent_sessions.py:read_importable_agent_session_rows`, `api/agent_sessions.py:read_session_lineage_metadata`, `api/models.py:get_cli_session_messages`, `api/models.py:delete_cli_session`. The reporter verified the fix in production (FD count flat at 92 across a 100-request stress loop against `/api/sessions` and `/api/projects`, vs. monotonic growth before). 4 regression tests in `tests/test_issue1494_state_db_fd_leak.py` monkeypatch `sqlite3.connect` with a tracking wrapper that records `.close()` calls and assert every connection opened by each function is explicitly closed — verified to fail (catching the original bug) when the `closing()` wrap is reverted. Bug #3 from #1458 (HTTP-unhealthy wedge in absence of FD exhaustion) remains open pending separate diagnostic data. (`api/agent_sessions.py`, `api/models.py`, `tests/test_issue1494_state_db_fd_leak.py`)
+
 ## [v0.50.271] — 2026-05-02
 
 ### Changed (1 self-built PR)

api/agent_sessions.py

@@ -1,6 +1,7 @@
 """Shared helpers for reading Hermes Agent sessions from state.db."""
 import logging
 import sqlite3
+from contextlib import closing
 from pathlib import Path
 
 logger = logging.getLogger(__name__)
@@ -234,7 +235,7 @@ def read_importable_agent_session_rows(
         return []
 
     log = log or logger
-    with sqlite3.connect(str(db_path)) as conn:
+    with closing(sqlite3.connect(str(db_path))) as conn:
         conn.row_factory = sqlite3.Row
         cur = conn.cursor()
 
@@ -306,7 +307,7 @@ def read_session_lineage_metadata(db_path: Path, session_ids: list[str] | set[st
         return {}
 
     try:
-        with sqlite3.connect(str(db_path)) as conn:
+        with closing(sqlite3.connect(str(db_path))) as conn:
             conn.row_factory = sqlite3.Row
             cur = conn.cursor()
             cur.execute("PRAGMA table_info(sessions)")

api/models.py

@@ -6,6 +6,7 @@ import os
 import threading
 import time
 import uuid
+from contextlib import closing
 from pathlib import Path
 
 import api.config as _cfg
@@ -1101,7 +1102,7 @@ def get_cli_session_messages(sid) -> list:
         return []
 
     try:
-        with sqlite3.connect(str(db_path)) as conn:
+        with closing(sqlite3.connect(str(db_path))) as conn:
             conn.row_factory = sqlite3.Row
             cur = conn.cursor()
             cur.execute("""
@@ -1142,7 +1143,7 @@ def delete_cli_session(sid) -> bool:
         return False
 
     try:
-        with sqlite3.connect(str(db_path)) as conn:
+        with closing(sqlite3.connect(str(db_path))) as conn:
             cur = conn.cursor()
             cur.execute("DELETE FROM messages WHERE session_id = ?", (sid,))
             cur.execute("DELETE FROM sessions WHERE id = ?", (sid,))

tests/test_issue1494_state_db_fd_leak.py

@@ -0,0 +1,234 @@
+"""Regression test for #1494: state.db connection FD leak via context-manager use.
+
+The bug: Python's sqlite3 connection context manager (`with sqlite3.connect(...) as
+conn:`) commits or rolls back on exit. It does NOT close the connection. In a
+long-running server with sidebar polling (`/api/sessions` calls
+`read_importable_agent_session_rows` and `read_session_lineage_metadata` on every
+poll), every poll leaked one or more open file descriptors against `~/.hermes/state.db`.
+
+In production this drove the WebUI process past macOS's 256-FD soft limit, after
+which new requests reset before producing a response (see #1458, #1494) — the
+process stayed alive, the port stayed listening, but every connection RST'd because
+sqlite3.connect() in a freshly accepted handler raised on FD exhaustion before any
+bytes were written.
+
+The fix wraps each `sqlite3.connect(...)` in `contextlib.closing(...)` so the
+connection is explicitly closed on scope exit (in addition to the auto-commit /
+rollback semantics).
+
+This file pins all four production callsites the issue reporter (insecurejezza)
+audited as still leaking on master @ 7fddc33:
+
+  * api/agent_sessions.py:read_importable_agent_session_rows
+  * api/agent_sessions.py:read_session_lineage_metadata
+  * api/models.py:get_cli_session_messages
+  * api/models.py:delete_cli_session
+
+Each test monkeypatches sqlite3.connect to track every connection the function
+opens, then asserts every connection is .close()'d after the call returns.
+"""
+import sqlite3
+
+import pytest
+
+
+def _make_state_db(path):
+    """Minimal state.db schema sufficient for the four functions under test.
+
+    Bypasses sqlite3.connect (uses sqlite3.Connection directly) so the
+    seed-data setup is not counted by the _TrackingConn monkeypatch — only
+    connections opened by the function under test should appear in
+    `_TrackingConn.instances`.
+    """
+    conn = sqlite3.Connection(str(path))
+    conn.executescript(
+        """
+        CREATE TABLE sessions (
+            id TEXT PRIMARY KEY,
+            title TEXT,
+            model TEXT,
+            message_count INTEGER DEFAULT 0,
+            started_at TEXT,
+            source TEXT,
+            parent_session_id TEXT,
+            ended_at TEXT,
+            end_reason TEXT
+        );
+        CREATE INDEX idx_sessions_parent ON sessions(parent_session_id);
+        CREATE TABLE messages (
+            id INTEGER PRIMARY KEY AUTOINCREMENT,
+            session_id TEXT,
+            role TEXT,
+            content TEXT,
+            timestamp TEXT
+        );
+        INSERT INTO sessions (id, title, model, message_count, started_at, source)
+        VALUES ('s1', 'cli session', 'gpt-x', 2, '2026-01-01T00:00:00Z', 'cli');
+        INSERT INTO messages (session_id, role, content, timestamp)
+        VALUES ('s1', 'user', 'hi', '2026-01-01T00:00:01Z'),
+               ('s1', 'assistant', 'hello', '2026-01-01T00:00:02Z');
+        """
+    )
+    conn.commit()
+    conn.close()
+
+
+class _TrackingConn:
+    """Wraps a real sqlite3.Connection to record open/close lifecycle.
+
+    Mirrors the lightweight wrapper pattern already in
+    test_pr1370_lineage_metadata_perf_and_orphan.py — keeping it inline here so
+    this regression test stays self-contained and survives refactors there.
+    """
+
+    instances: list = []
+
+    def __init__(self, *args, **kwargs):
+        self._real = sqlite3.Connection(*args, **kwargs)
+        self.closed = False
+        _TrackingConn.instances.append(self)
+
+    # Connection-shaped delegation
+    def cursor(self):
+        return self._real.cursor()
+
+    def execute(self, *a, **kw):
+        return self._real.execute(*a, **kw)
+
+    def executescript(self, *a, **kw):
+        return self._real.executescript(*a, **kw)
+
+    def commit(self):
+        return self._real.commit()
+
+    def rollback(self):
+        return self._real.rollback()
+
+    def close(self):
+        self.closed = True
+        return self._real.close()
+
+    # row_factory needs to round-trip onto the real connection
+    @property
+    def row_factory(self):
+        return self._real.row_factory
+
+    @row_factory.setter
+    def row_factory(self, value):
+        self._real.row_factory = value
+
+    # Context-manager protocol — the bug only triggers if a caller relies on
+    # __exit__ to close. Defer to the real Connection's CM (commit/rollback)
+    # so we faithfully reproduce the leak shape that prompted the fix.
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc, tb):
+        return self._real.__exit__(exc_type, exc, tb)
+
+
+@pytest.fixture
+def tracking_sqlite(monkeypatch):
+    """Monkeypatch sqlite3.connect to use _TrackingConn and reset the instance log."""
+    _TrackingConn.instances = []
+
+    def _connect(*args, **kwargs):
+        return _TrackingConn(*args, **kwargs)
+
+    monkeypatch.setattr(sqlite3, "connect", _connect)
+    return _TrackingConn
+
+
+def _assert_all_closed(tracking, fn_name):
+    assert tracking.instances, (
+        f"{fn_name}: no sqlite connections were opened — test setup is wrong"
+    )
+    leaked = [c for c in tracking.instances if not c.closed]
+    assert not leaked, (
+        f"{fn_name} leaked {len(leaked)} of {len(tracking.instances)} sqlite "
+        f"connection(s) — context-manager-only `with sqlite3.connect()` does "
+        f"not close. Wrap in contextlib.closing(). See #1494."
+    )
+
+
+def test_read_importable_agent_session_rows_closes_connection(tmp_path, tracking_sqlite):
+    """`read_importable_agent_session_rows` must close every sqlite connection."""
+    db = tmp_path / "state.db"
+    _make_state_db(db)
+
+    from api.agent_sessions import read_importable_agent_session_rows
+
+    # Call repeatedly — under the bug each call leaked a connection.
+    for _ in range(5):
+        read_importable_agent_session_rows(db)
+
+    _assert_all_closed(tracking_sqlite, "read_importable_agent_session_rows")
+    assert len(tracking_sqlite.instances) == 5
+
+
+def test_read_session_lineage_metadata_closes_connection(tmp_path, tracking_sqlite):
+    """`read_session_lineage_metadata` must close every sqlite connection."""
+    db = tmp_path / "state.db"
+    _make_state_db(db)
+
+    from api.agent_sessions import read_session_lineage_metadata
+
+    for _ in range(5):
+        read_session_lineage_metadata(db, ["s1"])
+
+    _assert_all_closed(tracking_sqlite, "read_session_lineage_metadata")
+    assert len(tracking_sqlite.instances) == 5
+
+
+def test_get_cli_session_messages_closes_connection(tmp_path, tracking_sqlite, monkeypatch):
+    """`get_cli_session_messages` must close every sqlite connection."""
+    db = tmp_path / "state.db"
+    _make_state_db(db)
+
+    monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+    # Stub get_active_hermes_home so the tmp_path is used regardless of profile state.
+    import api.profiles
+    monkeypatch.setattr(api.profiles, "get_active_hermes_home", lambda: str(tmp_path))
+
+    from api.models import get_cli_session_messages
+
+    for _ in range(5):
+        rows = get_cli_session_messages("s1")
+        # Sanity: the seeded messages are returned (proves we hit the real query path).
+        assert len(rows) == 2
+
+    _assert_all_closed(tracking_sqlite, "get_cli_session_messages")
+    assert len(tracking_sqlite.instances) == 5
+
+
+def test_delete_cli_session_closes_connection(tmp_path, tracking_sqlite, monkeypatch):
+    """`delete_cli_session` must close its sqlite connection (also keeps explicit commit working)."""
+    db = tmp_path / "state.db"
+    _make_state_db(db)
+
+    monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+    import api.profiles
+    monkeypatch.setattr(api.profiles, "get_active_hermes_home", lambda: str(tmp_path))
+
+    from api.models import delete_cli_session
+
+    deleted = delete_cli_session("s1")
+    assert deleted is True, "delete_cli_session should report the row was removed"
+
+    # Second call: row gone, nothing to delete — connection must still close cleanly.
+    deleted_again = delete_cli_session("s1")
+    assert deleted_again is False
+
+    _assert_all_closed(tracking_sqlite, "delete_cli_session")
+    # First call commits; second call short-circuits but still opens+closes a connection.
+    assert len(tracking_sqlite.instances) == 2
+
+    # Verify the commit semantics survived the closing() change — row really is gone.
+    real = sqlite3.Connection(str(db))
+    try:
+        cur = real.execute("SELECT COUNT(*) FROM sessions WHERE id = ?", ("s1",))
+        assert cur.fetchone()[0] == 0
+        cur = real.execute("SELECT COUNT(*) FROM messages WHERE session_id = ?", ("s1",))
+        assert cur.fetchone()[0] == 0
+    finally:
+        real.close()