From 713afdb39b817060fce7d29f4e54065c6c2f7089 Mon Sep 17 00:00:00 2001
From: Albrecht Degering <albrecht@degering.name>
Date: Tue, 21 Jul 2026 16:44:20 +0200
Subject: [PATCH] fix(installer): enlist table retirement transaction

---
 src/govoplan_core/core/module_guards.py | 12 +++++++----
 tests/test_module_system.py             | 28 ++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/govoplan_core/core/module_guards.py b/src/govoplan_core/core/module_guards.py
index 1574cf3..52cc6a4 100644
--- a/src/govoplan_core/core/module_guards.py
+++ b/src/govoplan_core/core/module_guards.py
@@ -79,15 +79,19 @@ def drop_table_retirement_provider(
             warnings.append("Tables not present and therefore skipped: " + ", ".join(missing_names))
 
         def executor(execute_session: object, _module_id: str) -> None:
-            if not hasattr(execute_session, "get_bind"):
+            if not hasattr(execute_session, "connection"):
                 raise RuntimeError("No database session is available for destructive table retirement.")
-            execute_bind = execute_session.get_bind()  # type: ignore[attr-defined]
-            live_inspector = inspect(execute_bind)
+            # Enlist schema retirement in the caller's active transaction.  An
+            # Engine returned by Session.get_bind() may acquire a second
+            # connection, separating the DROP from module-specific secret
+            # scrubbing/audit writes and deadlocking on their uncommitted locks.
+            execute_connection = execute_session.connection()  # type: ignore[attr-defined]
+            live_inspector = inspect(execute_connection)
             live_tables = [table for table in tables if live_inspector.has_table(table.name)]
             if not live_tables:
                 return
             metadata = live_tables[0].metadata
-            metadata.drop_all(bind=execute_bind, tables=live_tables, checkfirst=True)
+            metadata.drop_all(bind=execute_connection, tables=live_tables, checkfirst=True)
 
         return MigrationRetirementPlan(
             supported=True,
diff --git a/tests/test_module_system.py b/tests/test_module_system.py
index 4105d41..5141705 100644
--- a/tests/test_module_system.py
+++ b/tests/test_module_system.py
@@ -20,7 +20,8 @@ from pathlib import Path
 from types import SimpleNamespace
 from unittest.mock import patch
 
-from sqlalchemy import Column, Integer, MetaData, Table, inspect
+from sqlalchemy import Column, Integer, MetaData, Table, create_engine, insert, inspect
+from sqlalchemy.orm import Session
 
 # Keep the default app import side effect from bootstrapping a development DB.
 _TEST_ROOT = Path(tempfile.mkdtemp(prefix="govoplan-module-tests-"))
@@ -2370,6 +2371,31 @@ finally:
         self.assertEqual("example", record["retirements"][0]["module_id"])
         self.assertIn("database_backup", record["snapshot"])
 
+    def test_drop_table_retirement_is_atomic_with_session_writes(self) -> None:
+        root = Path(tempfile.mkdtemp(prefix="govoplan-retirement-transaction-", dir=_TEST_ROOT))
+        engine = create_engine(
+            f"sqlite:///{root / 'retirement.db'}",
+            connect_args={"timeout": 0.1},
+        )
+        metadata = MetaData()
+        audit_table = Table("retirement_audit", metadata, Column("id", Integer, primary_key=True))
+        owned_table = Table("retirement_owned", metadata, Column("id", Integer, primary_key=True))
+        metadata.create_all(bind=engine)
+        owned_model = type("OwnedModel", (), {"__table__": owned_table})
+
+        with Session(engine) as session:
+            session.execute(insert(audit_table).values(id=1))
+            plan = drop_table_retirement_provider(owned_model, label="Owned")(session, "owned")
+            self.assertIsNotNone(plan.destroy_data_executor)
+            plan.destroy_data_executor(session, "owned")  # type: ignore[misc]
+            self.assertFalse(inspect(session.connection()).has_table("retirement_owned"))
+            session.rollback()
+
+        self.assertTrue(inspect(engine).has_table("retirement_owned"))
+        with engine.connect() as connection:
+            self.assertEqual([], connection.execute(audit_table.select()).all())
+        engine.dispose()
+
     def test_module_installer_dry_run_writes_run_record_without_applying(self) -> None:
         root = Path(tempfile.mkdtemp(prefix="govoplan-installer-dry-run-", dir=_TEST_ROOT))
         settings = _settings(root)