From d527d13570a2ecf3a7530fb670777c8d6a4347c6 Mon Sep 17 00:00:00 2001
From: Joshua King <joshuaking.mba@gmail.com>
Date: Sun, 13 Sep 2026 21:12:33 -0400
Subject: [PATCH] fix: roll back on commit failure and record connect thread in
 loop-safety test

---
 custom_components/nestquest/db.py | 21 +++++++++++--
 tests/test_db.py                  | 49 +++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/custom_components/nestquest/db.py b/custom_components/nestquest/db.py
index 3fcf6bc..65163e8 100644
--- a/custom_components/nestquest/db.py
+++ b/custom_components/nestquest/db.py
@@ -199,7 +199,10 @@ class NestQuestDatabase:
 
         The wrapper lock is held for the entire BEGIN..COMMIT span, so no
         other statement can interleave inside the transaction.  Entering a
-        second transaction while one is active raises RuntimeError.
+        second transaction while one is active raises RuntimeError.  A
+        failing COMMIT (e.g. a deferred foreign-key violation) also triggers
+        a ROLLBACK before the commit error propagates, so the real
+        connection is never left inside a stray transaction.
         """
         if self._in_transaction:
             raise RuntimeError(
@@ -229,7 +232,21 @@ class NestQuestDatabase:
                 def _commit() -> None:
                     conn.execute("COMMIT")
 
-                await self._hass.async_add_executor_job(_commit)
+                try:
+                    await self._hass.async_add_executor_job(_commit)
+                except BaseException as commit_error:
+
+                    def _rollback() -> None:
+                        conn.execute("ROLLBACK")
+
+                    try:
+                        await self._hass.async_add_executor_job(_rollback)
+                    except BaseException as rollback_error:
+                        raise RuntimeError(
+                            "COMMIT failed and the follow-up ROLLBACK also "
+                            "raised; the transaction may need manual cleanup"
+                        ) from rollback_error
+                    raise
             finally:
                 self._in_transaction = False
                 self._tx_task = None
diff --git a/tests/test_db.py b/tests/test_db.py
index 0ef0ea8..fbd75a1 100644
--- a/tests/test_db.py
+++ b/tests/test_db.py
@@ -200,8 +200,11 @@ async def test_real_executor_never_touches_loop_thread(tmp_path, monkeypatch) ->
             return _wrapped
 
     real_connect = sqlite3.connect
+    connect_threads: list[int] = []
 
     def _traced_connect(*args, **kwargs):
+        _note()
+        connect_threads.append(threading.get_ident())
         return _Proxy(real_connect(*args, **kwargs))
 
     monkeypatch.setattr(db_module.sqlite3, "connect", _traced_connect)
@@ -219,6 +222,9 @@ async def test_real_executor_never_touches_loop_thread(tmp_path, monkeypatch) ->
     await database.close()
 
     assert recorded, "expected at least one recorded sqlite3 call"
+    assert len(connect_threads) == 1, (
+        f"expected exactly one traced sqlite3.connect, got {len(connect_threads)}"
+    )
     assert loop_thread not in recorded, (
         f"sqlite3 call ran on the event-loop thread {loop_thread}; "
         f"recorded threads: {sorted(set(recorded))}"
@@ -357,6 +363,49 @@ def test_transaction_rolls_back_on_exception(tmp_path) -> None:
     _run(database.close())
 
 
+async def test_commit_failure_rolls_back_and_next_transaction_works(
+    tmp_path,
+) -> None:
+    """A COMMIT failure rolls back and leaves the wrapper reusable.
+
+    With PRAGMA defer_foreign_keys=ON a violating insert succeeds inside
+    the transaction and the foreign-key violation only surfaces at COMMIT.
+    The commit error must propagate, the real connection must end up
+    outside any transaction, and the next transaction() plus statement
+    must run cleanly.
+    """
+    hass, _registry = make_hass()
+    database = NestQuestDatabase(hass)
+    await database.open(tmp_path / "deferred.db")
+    await database.execute("CREATE TABLE parent (id INTEGER PRIMARY KEY)")
+    await database.execute(
+        "CREATE TABLE child ("
+        "id INTEGER PRIMARY KEY, "
+        "parent_id INTEGER NOT NULL REFERENCES parent(id))"
+    )
+
+    with pytest.raises(sqlite3.IntegrityError, match="FOREIGN KEY"):
+        async with database.transaction():
+            await database.execute("PRAGMA defer_foreign_keys=ON")
+            await database.execute(
+                "INSERT INTO child (parent_id) VALUES (?)", (999,)
+            )
+
+    assert database._in_transaction is False
+    assert database._tx_task is None
+    assert database._conn is not None
+    assert database._conn.in_transaction is False
+    assert await database.fetch_all("SELECT COUNT(*) FROM child") == [(0,)]
+
+    async with database.transaction():
+        await database.execute("INSERT INTO parent (id) VALUES (?)", (1,))
+        await database.execute(
+            "INSERT INTO child (parent_id) VALUES (?)", (1,)
+        )
+    assert await database.fetch_all("SELECT COUNT(*) FROM child") == [(1,)]
+    await database.close()
+
+
 async def test_transaction_persists_across_connections(tmp_path) -> None:
     """A committed transaction is visible to a brand-new connection."""
     hass, _registry = make_hass()