diff --git a/hermes_state.py b/hermes_state.py index 9fa861541e..c9399c7087 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -3733,11 +3733,13 @@ def repair_state_db_schema(db_path: Path, *, backup: bool = True) -> Dict[str, A # database.journal_mode setting is the restore target. before_mode = _probe_journal_mode_for_repair(db_path) result = _repair_state_db_schema_locked( - db_path, backup=backup, report=report + db_path, + backup=backup, + report=report, + journal_mode_before=before_mode, ) if result.get("repaired"): result["journal_mode_before"] = before_mode - _restore_journal_mode_after_repair(db_path, before_mode) # Environmental aborts happen before a strategy gets to mutate the # isolated snapshot. They are retriable operating conditions, not # proof that the damaged database exhausted a repair strategy. @@ -3773,7 +3775,9 @@ def _probe_journal_mode_for_repair(db_path: Path) -> Optional[str]: return None -def _restore_journal_mode_after_repair(db_path: Path, before_mode: Optional[str]) -> None: +def _restore_journal_mode_after_repair( + db_path: Path, before_mode: Optional[str], *, conn=None +) -> None: """Re-apply the journal mode after schema surgery (#89674). A repaired/rebuilt SQLite file comes back in the default journal mode @@ -3798,12 +3802,15 @@ def _restore_journal_mode_after_repair(db_path: Path, before_mode: Optional[str] Best-effort by design: the repair itself already succeeded, so failures to re-apply are logged at WARNING, never raised. """ + owned_conn = conn is None try: - conn = _connect_repair_durable(db_path) + if owned_conn: + conn = _connect_repair_durable(db_path) try: after = apply_wal_with_fallback(conn, db_label=db_path.name) finally: - conn.close() + if owned_conn: + conn.close() if before_mode and after != before_mode: logger.warning( "state.db repair changed journal_mode %r -> %r " @@ -3821,7 +3828,11 @@ def _restore_journal_mode_after_repair(db_path: Path, before_mode: Optional[str] def _repair_state_db_schema_locked( - db_path: Path, *, backup: bool, report: Dict[str, Any] + db_path: Path, + *, + backup: bool, + report: Dict[str, Any], + journal_mode_before: Optional[str] = None, ) -> Dict[str, Any]: """Repair strategies for :func:`repair_state_db_schema`. @@ -3963,6 +3974,11 @@ def _repair_state_db_schema_locked( report.get("strategy"), db_path, ) + _restore_journal_mode_after_repair( + db_path, + journal_mode_before, + conn=live_guard, + ) if not report.get("repaired"): # Logged HERE, not inside the strategies: they run against the # scratch copy, and naming that throwaway path in the one diff --git a/tests/state/test_state_db_wal_unlink_race.py b/tests/state/test_state_db_wal_unlink_race.py new file mode 100644 index 0000000000..a9b46125f7 --- /dev/null +++ b/tests/state/test_state_db_wal_unlink_race.py @@ -0,0 +1,29 @@ +"""Regression coverage for WAL restoration during state.db repair.""" + +import sqlite3 + +import pytest + +import hermes_state + + +def test_wal_restoration_reuses_exclusive_repair_connection(tmp_path, monkeypatch): + """WAL must be restored before the repair guard releases the live DB.""" + db_path = tmp_path / "state.db" + conn = sqlite3.connect(db_path, isolation_level=None) + conn.execute("CREATE TABLE marker (value TEXT)") + conn.execute("PRAGMA journal_mode=DELETE") + + def fail_if_reopened(_path): + pytest.fail("WAL restoration reopened state.db outside the repair guard") + + monkeypatch.setattr(hermes_state, "_connect_repair_durable", fail_if_reopened) + + hermes_state._restore_journal_mode_after_repair( + db_path, + "delete", + conn=conn, + ) + + assert conn.execute("PRAGMA journal_mode").fetchone()[0].lower() == "wal" + conn.close()