From e27656ee7ee7867599c32e1644b7c4d861a56373 Mon Sep 17 00:00:00 2001
From: npeter83 <npeter83+claude@gmail.com>
Date: Tue, 20 Oct 2026 00:45:20 +0200
Subject: [PATCH] test: the request commit check sees Core DML and row locks

A db.execute(update/delete) fires no flush and stages nothing, so a handler whose
only write was Core DML could forget its commit and stay green. The check now
asks Postgres whether the request opened a writing transaction. It found one
real case: a repeated issue send returned with the session row still locked.
---
 backend/app/routes/debug_sessions.py          |  1 +
 backend/tests/conftest.py                     | 13 +++++++-
 backend/tests/test_override_dep_convention.py | 31 +++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/backend/app/routes/debug_sessions.py b/backend/app/routes/debug_sessions.py
index a4288c51..4a0a7a20 100644
--- a/backend/app/routes/debug_sessions.py
+++ b/backend/app/routes/debug_sessions.py
@@ -220,6 +220,7 @@ def send_issue(sid: str, user: User = Depends(current_user), db: Session = Depen
         raise refused(409, "issue_sending")
     new = session.issue_url is None
     if not new and not debugsession.unfinished(session):
+        db.commit()  # nothing to send: the lock ends here, not when the request's session closes
         return {"ok": True, "session": _view(db, session)}
     key = str(user.id)
     if new:
diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py
index c00ea44f..c1654ee2 100644
--- a/backend/tests/conftest.py
+++ b/backend/tests/conftest.py
@@ -105,6 +105,11 @@ def pytest_configure(config):
         )
 
 
+def _write_xact(db) -> str | None:
+    """The id of the open transaction if it has written anything, else None (a read never takes one)."""
+    return db.scalar(text("SELECT pg_current_xact_id_if_assigned()::text"))
+
+
 def _request_dep(db, unfinished: list[str]):
     """`get_db` for a test request: the test's own session, plus the check a shared session hides.
 
@@ -124,6 +129,7 @@ def _request_dep(db, unfinished: list[str]):
     def dep(conn: HTTPConnection):
         db.flush()
         state = {"dirty": False}
+        before = _write_xact(db)
 
         def flushed(session, ctx):
             state["dirty"] = True
@@ -138,7 +144,12 @@ def _request_dep(db, unfinished: list[str]):
         try:
             yield db
             staged = db.new or db.deleted or any(db.is_modified(o) for o in db.dirty)
-            if state["dirty"] or staged:
+            # Core DML (`db.execute(update(...))`) fires no flush and stages nothing; the database
+            # itself says whether a write is open — a transaction that wrote and was not committed
+            # holds an id it did not hold when the request began (see `release_read_connection`). A
+            # row lock (`FOR UPDATE`) takes one too: the handler ends what it locked.
+            core_dml = (after := _write_xact(db)) is not None and after != before
+            if state["dirty"] or staged or core_dml:
                 unfinished.append(f"{conn.scope.get('method', 'WS')} {conn.url.path}")
         except Exception:
             db.rollback()
diff --git a/backend/tests/test_override_dep_convention.py b/backend/tests/test_override_dep_convention.py
index 89aab606..41135240 100644
--- a/backend/tests/test_override_dep_convention.py
+++ b/backend/tests/test_override_dep_convention.py
@@ -82,3 +82,34 @@ def test_a_request_that_raises_leaves_nothing_it_did_not_commit(db):
         request.throw(RuntimeError("the handler failed"))
     assert db.scalar(select(User).where(User.email == "rolled-back-probe@example.com")) is None
     assert db.info["unfinished"] == []  # a failing request is rolled back, not reported
+
+
+def test_the_commit_check_sees_a_core_dml_write(db, make_user):
+    """`db.execute(update(...))` flushes nothing and stages nothing, so the session's own bookkeeping
+    never sees it: a handler whose only write is Core DML and that forgot its commit passed. The
+    database's transaction id is what tells. A committed one must still pass."""
+    import pytest
+    from sqlalchemy import update
+    from starlette.requests import HTTPConnection
+
+    from app.models import User
+
+    user = make_user()
+
+    def drive(commit: bool) -> None:
+        conn = HTTPConnection(
+            {"type": "http", "method": "POST", "path": "/probe", "headers": [], "query_string": b""}
+        )
+        request = db.info["request"](conn)
+        session = next(request)
+        session.execute(update(User).where(User.id == user.id).values(display_name="dml"))
+        if commit:
+            session.commit()
+        with pytest.raises(StopIteration):
+            next(request)
+
+    drive(commit=True)
+    assert db.info["unfinished"] == []
+    drive(commit=False)
+    assert db.info["unfinished"] == ["POST /probe"]
+    db.info["unfinished"].clear()  # recorded as it should be — not this test's failure