fix: re-read schema version inside each migration transaction; spy on close in setup-failure test

- apply_migrations re-reads the version under the transaction lock
  before applying, so two interleaved runners on one connection cannot
  both apply a non-idempotent migration from a stale read
- setup-failure test now spies on NestQuestDatabase.close and asserts
  it ran and no runtime record was stored (was vacuous)

custom_components/nestquest/migrations.py

@@ -131,6 +131,13 @@ async def apply_migrations(
     migrations (database already current) is a no-op that logs at debug
     level and opens no write transaction.
 
+    The version is re-read inside each migration transaction before
+    deciding to apply it: the outer read is advisory only (loop bounds,
+    no-op fast path), and on one shared connection two interleaved
+    runners would otherwise both read the same version and both apply
+    a non-idempotent migration.  Under the transaction's lock the
+    second runner sees the first runner's stamp and skips.
+
     Raises whatever the failing migration raised, with the database
     rolled back to its prior version and prior rows untouched.
     """
@@ -153,25 +160,46 @@ async def apply_migrations(
         return current
 
     for target in range(current + 1, latest + 1):
-        statements = migrations[target - 1]
+        applied = False
         async with database.transaction():
-            for sql in statements:
-                await database.execute(sql)
-            # UPDATE keyed on the singleton row (id = 1): a fresh file
-            # has no row yet, but migration 1 is the only migration a
-            # version-0 database can run, so the row is seeded by the
-            # same statement batch when needed.  UPDATE alone would
-            # silently match zero rows there, so seed-if-absent first.
-            await database.execute(
-                f"INSERT INTO {VERSION_TABLE} (id, version) VALUES (1, ?) "
-                "ON CONFLICT (id) DO UPDATE SET version = excluded.version",
-                (target,),
+            # Re-read the version INSIDE the write transaction.  The
+            # version read before the loop is advisory (it sizes the
+            # loop and enables the byte-identical no-op fast path), but
+            # two runners on one connection interleave between their
+            # reads: the wrapper serializes individual statements, not
+            # the read-decide-apply decision.  Re-reading under the
+            # transaction's lock means the loser of the race sees the
+            # winner's stamp and skips instead of re-applying a
+            # non-idempotent migration with a stale version.
+            in_tx_version = await read_schema_version(database)
+            if in_tx_version > latest:
+                raise RuntimeError(
+                    f"Database schema version {in_tx_version} is newer "
+                    f"than this integration understands (latest known: "
+                    f"{latest}); downgrades are not supported"
+                )
+            if in_tx_version < target:
+                for sql in migrations[target - 1]:
+                    await database.execute(sql)
+                # INSERT keyed on the singleton row (id = 1): a fresh
+                # file has no row yet, and migration 1 is the only
+                # migration a version-0 database can run, so the row is
+                # seeded by the same statement batch when needed.
+                # UPDATE alone would silently match zero rows there.
+                await database.execute(
+                    f"INSERT INTO {VERSION_TABLE} (id, version) "
+                    "VALUES (1, ?) "
+                    "ON CONFLICT (id) DO UPDATE SET version = "
+                    "excluded.version",
+                    (target,),
+                )
+                applied = True
+        if applied:
+            LOGGER.info(
+                "NestQuest schema migrated to version %d (%d statement%s)",
+                target,
+                len(migrations[target - 1]),
+                "s" if len(migrations[target - 1]) != 1 else "",
             )
-        LOGGER.info(
-            "NestQuest schema migrated to version %d (%d statement%s)",
-            target,
-            len(statements),
-            "s" if len(statements) != 1 else "",
-        )
 
     return latest

tests/test_migrations.py

@@ -574,9 +574,16 @@ async def test_options_reload_reopens_and_rechecks_version(
 async def test_setup_entry_failure_closes_database(
     hass, make_entry, monkeypatch
 ) -> None:
+    """A failing migration closes the opened connection before re-raising.
+
+    Spies on the real wrapper instance: records whether close() ran and
+    whether the connection is left disconnected, rather than inferring
+    from hass.data (a failed setup never stores a runtime record).
+    """
     import pytest as pytest_module
 
     import custom_components.nestquest as nestquest_module
+    from custom_components.nestquest.db import NestQuestDatabase
     from custom_components.nestquest.migrations import (
         MIGRATIONS as _MIGRATIONS,
     )
@@ -587,12 +594,22 @@ async def test_setup_entry_failure_closes_database(
     monkeypatch.setattr(
         nestquest_module, "apply_migrations", _failing, raising=True
     )
+
+    close_calls: list[bool] = []
+    original_close = NestQuestDatabase.close
+
+    async def _spy_close(self):
+        close_calls.append(True)
+        return await original_close(self)
+
+    monkeypatch.setattr(NestQuestDatabase, "close", _spy_close)
+
     entry = make_entry()
     with pytest_module.raises(RuntimeError, match="boom"):
         await _setup_entry(hass, entry, hass.registry)
-    # Setup closed the connection before re-raising: no leaked handle.
-    # The record only lands in hass.data after the listener is added, so
-    # on failure the connection must be gone from the wrapper itself.
+
+    assert close_calls == [True], (
+        "failed setup must close the connection it opened"
+    )
     record = hass.data["nestquest"].get(entry.entry_id)
-    if record is not None:
-        assert not record.database.connected
+    assert record is None, "failed setup must not store runtime data"