From a266155cc440cacf68faf891eca3e0885a1a8829 Mon Sep 17 00:00:00 2001
From: Jeremy <peace@trippyogi.com>
Date: Fri, 31 Jul 2026 13:54:41 -0700
Subject: [PATCH] fix(cli): untrack sqlite connections only after close
 succeeds

A failed close left the FD open while the byte-probe guard thought
nothing was live. Keep the registry entry until close actually works.
---
 hermes_cli/sqlite_safe_read.py            | 14 ++++++---
 tests/test_sqlite_lock_safe_inspection.py | 38 +++++++++++++++++++++++
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/hermes_cli/sqlite_safe_read.py b/hermes_cli/sqlite_safe_read.py
index 352228ed52..be58ab9dd8 100644
--- a/hermes_cli/sqlite_safe_read.py
+++ b/hermes_cli/sqlite_safe_read.py
@@ -40,7 +40,7 @@ lifecycle**. ``_live_lock`` is therefore held across three critical sections,
 each of which spans the syscall *and* the registry mutation:
 
 * open + register (:func:`connect_tracked`)
-* unregister + close (:meth:`TrackedConnection.close`)
+* close + unregister (:meth:`TrackedConnection.close`)
 * check + ``open``/``read``/``close`` (:func:`read_header_bytes_preopen`)
 
 Without that, a thread could pass the "no live connection" check, a second
@@ -154,10 +154,14 @@ class _TrackingMixin:
     def close(self) -> None:  # type: ignore[misc]
         with _live_lock:
             path = getattr(self, "_hermes_tracked_path", None)
+            # Close first; untrack only once the descriptor is actually gone.
+            # Untracking before a failing close (e.g. cross-thread
+            # ProgrammingError) leaves the FD open while the byte-probe
+            # guard thinks nothing is live — see #75629.
+            super().close()  # type: ignore[misc]
             if path is not None:
                 self._hermes_tracked_path = None
                 untrack_connection(path)
-            super().close()  # type: ignore[misc]
 
 
 class TrackedConnection(_TrackingMixin, sqlite3.Connection):
@@ -169,9 +173,11 @@ class TrackedConnection(_TrackingMixin, sqlite3.Connection):
     method every close path must go through — keeps the registry from
     drifting upward and permanently disabling byte-probes.
 
-    The unregister and the real ``close()`` happen together under
+    The real ``close()`` and the unregister happen together under
     ``_live_lock`` so a concurrent probe can never observe "no live
-    connection" while this descriptor is still open.
+    connection" while this descriptor is still open. Unregister runs only
+    after ``close()`` succeeds; a raising close leaves the connection
+    tracked so the byte-probe guard keeps refusing.
 
     Note ``with conn:`` does NOT close a sqlite3 connection (it only commits or
     rolls back), so this hook is not fired spuriously by transaction scopes.
diff --git a/tests/test_sqlite_lock_safe_inspection.py b/tests/test_sqlite_lock_safe_inspection.py
index 3e3787e82d..45a2d05b82 100644
--- a/tests/test_sqlite_lock_safe_inspection.py
+++ b/tests/test_sqlite_lock_safe_inspection.py
@@ -162,6 +162,44 @@ def test_tracking_registry_does_not_leak_across_close_paths(tmp_path, clean_regi
     assert not has_live_connection(db)
 
 
+def test_failed_close_keeps_connection_tracked(tmp_path, clean_registry):
+    """A raising close must not release the registry (#75629).
+
+    Untracking before ``super().close()`` leaves the descriptor open while
+    ``has_live_connection`` reports false, so the byte-probe guard permits
+    ``open``/``close`` on a live database — cancelling POSIX advisory locks.
+    """
+    from hermes_cli.sqlite_safe_read import connect_tracked
+
+    class ControllableConnection(sqlite3.Connection):
+        def close(self):
+            if getattr(self, "_hermes_fail_close", False):
+                raise sqlite3.ProgrammingError(
+                    "SQLite objects created in a thread can only be used in "
+                    "that same thread"
+                )
+            return super().close()
+
+    db = tmp_path / "state.db"
+    _make_db(db, "WAL")
+
+    conn = connect_tracked(db, factory=ControllableConnection)
+    assert has_live_connection(db)
+    assert read_header_bytes_preopen(db, length=16) is None
+
+    conn._hermes_fail_close = True
+    with pytest.raises(sqlite3.ProgrammingError):
+        conn.close()
+
+    assert has_live_connection(db), "failed close must leave the registry entry"
+    assert read_header_bytes_preopen(db, length=16) is None
+
+    conn._hermes_fail_close = False
+    conn.close()
+    assert not has_live_connection(db)
+    assert read_header_bytes_preopen(db, length=16) is not None
+
+
 def test_probe_and_connect_do_not_race(tmp_path, clean_registry, monkeypatch):
     """The check and the raw read must be atomic w.r.t. connection lifecycle.