fix(db): release read transactions — no more idle-in-transaction on Postgres

.github/workflows/ci.yml

@@ -138,3 +138,8 @@ jobs:
         env:
           LAMBDA_ERP_TEST_DB: postgresql://postgres:postgres@localhost:5432/lambda_test
         run: python -m tests.test_adjacent
+
+      - name: Transaction release (no idle-in-transaction) — PostgreSQL
+        env:
+          LAMBDA_ERP_TEST_DB: postgresql://postgres:postgres@localhost:5432/lambda_test
+        run: python -m tests.test_txn_release

CHANGELOG.md

@@ -13,6 +13,23 @@ semver-governed public surface — a breaking change to a seam is a major bump.
 
 ## [Unreleased]
 
+## [0.6.5] - 2026-07-28
+
+### Fixed
+- **Postgres connections no longer sit "idle in transaction" after reads.**
+  With `autocommit=False`, even a plain SELECT opens a transaction, and nothing
+  ended it — so every pooled request thread (and the chat websocket) left an
+  open transaction pinning `ACCESS SHARE` locks on each table it had read.
+  Those idle locks starved the lock-safe plugin migrations (`ensure_column`/
+  `drop_column`) of their `ACCESS EXCLUSIVE` lock on **every** boot: the ALTERs
+  timed out forever, columns were never added, and — because `_persist()` only
+  writes columns that exist — the UI's saves returned 200 while silently
+  dropping edits to the missing fields (observed in production). `_PgConn` now
+  rolls back immediately after a read when no write is pending and no explicit
+  transaction (submit/cancel) is active, so connections never idle inside a
+  transaction. Write atomicity is unchanged. Regression-tested in
+  `tests/test_txn_release.py`, including the migration-starvation scenario.
+
 ## [0.6.4] - 2026-07-28
 
 ### Added

lambda_erp/database.py

@@ -99,10 +99,38 @@ def make(values):
 class _PgConn:
     """Adapts a psycopg (v3) connection to the sqlite3 connection API the code
     uses. autocommit is off so the existing `_in_transaction` + single commit()
-    / rollback() flow gives the same atomicity as SQLite's implicit transaction."""
-
-    def __init__(self, raw):
+    / rollback() flow gives the same atomicity as SQLite's implicit transaction.
+
+    With autocommit off, even a plain SELECT opens a transaction that stays
+    open until commit/rollback — and these thread-local connections live as
+    long as their (pooled) thread, so a request whose last statement was a read
+    left the connection 'idle in transaction' forever, pinning ACCESS SHARE
+    locks on every table it touched. That starves any ALTER TABLE of its
+    ACCESS EXCLUSIVE lock, which made lock-safe plugin migrations time out on
+    every boot in production. The fix: after a read, when there is no pending
+    write and no explicit transaction, end the implicit transaction right away
+    (_maybe_release), so connections never idle inside one."""
+
+    def __init__(self, raw, in_explicit_txn=None):
         self._raw = raw
+        self._dirty = False  # uncommitted write on this connection
+        self._in_explicit_txn = in_explicit_txn or (lambda: False)
+
+    @staticmethod
+    def _is_read(sql):
+        s = sql.lstrip().lower()
+        return s.startswith("select") and "for update" not in s and "for share" not in s
+
+    def _maybe_release(self, sql):
+        if self._in_explicit_txn():
+            return
+        if self._is_read(sql):
+            # psycopg's default cursor has already buffered the full result set
+            # client-side, so the caller can still fetch after the rollback.
+            if not self._dirty:
+                self._safe_rollback()
+        else:
+            self._dirty = True
 
     def execute(self, sql, params=None):
         try:
@@ -114,36 +142,41 @@ def execute(self, sql, params=None):
                 cur.execute(sql.replace("%", "%%").replace("?", "%s"), list(params))
             else:
                 cur.execute(sql.replace("?", "%s"))
-            return cur
         except Exception:
             # A failed statement leaves the connection in an aborted transaction
             # (autocommit=False); every later query on this pooled/thread-local
             # connection would then fail with InFailedSqlTransaction. Roll back
             # so the connection stays usable, then re-raise the real error.
             self._safe_rollback()
             raise
+        self._maybe_release(sql)
+        return cur
 
     def executemany(self, sql, seq_params):
         try:
             cur = self._raw.cursor()
             cur.executemany(sql.replace("%", "%%").replace("?", "%s"),
                             [list(p) for p in seq_params])
-            return cur
         except Exception:
             self._safe_rollback()
             raise
+        self._dirty = True
+        return cur
 
     def _safe_rollback(self):
         try:
             self._raw.rollback()
+            self._dirty = False
         except Exception:
             pass
 
     def commit(self):
         self._raw.commit()
+        self._dirty = False
 
     def rollback(self):
         self._raw.rollback()
+        self._dirty = False
 
     def close(self):
         self._raw.close()
@@ -224,9 +257,11 @@ def _open_pg_conn(self):
 
         # autocommit off: matches SQLite's implicit transaction so the existing
         # _in_transaction + commit()/rollback() atomicity (GL submit/cancel) is
-        # preserved exactly.
+        # preserved exactly. in_explicit_txn lets the wrapper see the
+        # thread-local flag so reads inside submit/cancel keep their snapshot.
         return _PgConn(psycopg.connect(self.db_path, autocommit=False,
-                                       row_factory=_pg_row_factory))
+                                       row_factory=_pg_row_factory),
+                       in_explicit_txn=lambda: self._in_transaction)
 
     @property
     def conn(self):

tests/test_txn_release.py

@@ -0,0 +1,106 @@
+#!/usr/bin/env python3
+"""Tests that Postgres connections never idle inside an implicit transaction.
+
+With autocommit off, a plain SELECT opens a transaction; before 0.6.5 nothing
+ended it, so thread-local connections sat 'idle in transaction' after every
+read-only request, pinning ACCESS SHARE locks that starved lock-safe plugin
+migrations (ALTER TABLE) on every boot. _PgConn._maybe_release now rolls back
+right after a read when no write is pending and no explicit transaction is
+active. Postgres-only semantics — a no-op on SQLite.
+
+Run:  LAMBDA_ERP_TEST_DB=postgresql://... python -m tests.test_txn_release
+"""
+import os
+import sys
+
+
+def main():
+    url = os.environ.get("LAMBDA_ERP_TEST_DB")
+    if not url:
+        print("SKIP: transaction-release semantics are Postgres-only "
+              "(set LAMBDA_ERP_TEST_DB=postgresql://...)")
+        return
+
+    import psycopg
+    from psycopg.pq import TransactionStatus
+
+    with psycopg.connect(url, autocommit=True) as conn:
+        conn.execute("DROP SCHEMA public CASCADE")
+        conn.execute("CREATE SCHEMA public")
+
+    from lambda_erp.database import Database
+
+    db = Database(url)
+    status = lambda: db.conn._raw.info.transaction_status
+
+    # Schema setup commits; the connection must start outside a transaction.
+    assert status() == TransactionStatus.IDLE, status()
+
+    # 1. A read releases its implicit transaction immediately — and the rows
+    #    are still fetchable (psycopg buffers the result set client-side).
+    rows = db.sql('SELECT key FROM "Settings"')
+    assert status() == TransactionStatus.IDLE, f"read left txn open: {status()}"
+    assert isinstance(rows, list)
+
+    # Read helpers funnel through the same path (Customer has the standard
+    # name/creation columns; Settings is a bare key/value table).
+    db.exists("Customer", "CUST-does-not-exist")
+    db.get_all("Customer", limit=1)
+    assert status() == TransactionStatus.IDLE, f"helper left txn open: {status()}"
+
+    # 2. A write keeps its transaction open (atomicity with the later commit),
+    #    and interleaved reads must NOT roll it back.
+    db.sql('INSERT INTO "Settings" (key, value) VALUES (?, ?)', ["txn-test", "1"])
+    assert status() == TransactionStatus.INTRANS, f"write did not open txn: {status()}"
+    got = db.sql('SELECT value FROM "Settings" WHERE key = ?', ["txn-test"])
+    assert status() == TransactionStatus.INTRANS, "read rolled back a pending write"
+    assert got and got[0]["value"] == "1"
+    db.conn.commit()
+    assert status() == TransactionStatus.IDLE
+    got = db.sql('SELECT value FROM "Settings" WHERE key = ?', ["txn-test"])
+    assert got and got[0]["value"] == "1", "committed write lost"
+
+    # 3. Inside an explicit transaction (submit/cancel flow) reads keep the
+    #    transaction — and its snapshot — open.
+    db._in_transaction = True
+    try:
+        db.sql('SELECT 1 FROM "Settings"')
+        assert status() == TransactionStatus.INTRANS, "explicit txn was released"
+        db.conn.rollback()
+    finally:
+        db._in_transaction = False
+    assert status() == TransactionStatus.IDLE
+
+    # 4. The payoff — the production failure mode: a second thread (its own
+    #    thread-local connection) does a read and then goes quiet, exactly like
+    #    a pooled request thread between requests. Before the fix its idle
+    #    transaction held ACCESS SHARE on "Settings" and this ALTER timed out
+    #    after 8 retries; now the read released the lock and the ALTER wins.
+    import threading
+
+    read_done, release = threading.Event(), threading.Event()
+
+    def quiet_reader():
+        db.sql('SELECT * FROM "Settings"')
+        read_done.set()
+        release.wait(timeout=60)  # keep the thread (and its connection) alive
+
+    t = threading.Thread(target=quiet_reader, daemon=True)
+    t.start()
+    assert read_done.wait(timeout=10), "reader thread never ran"
+    try:
+        db.ensure_column("Settings", "txn_release_probe", "TEXT")
+    finally:
+        release.set()
+        t.join(timeout=10)
+    assert "txn_release_probe" in db._get_table_columns("Settings")
+
+    print("OK: reads release their transaction; writes stay atomic")
+
+
+if __name__ == "__main__":
+    try:
+        main()
+    except AssertionError as e:
+        print(f"FAIL: {e}")
+        sys.exit(1)