fix(cli): keep doctor's read-only opens URI-safe and holder-gated
- _session_count: back to main's raw sqlite mode=ro COUNT(*) via as_uri() — routing it through SessionDB(read_only=True) both re-introduced the raw f-string URI ('?'/'#' in the home path truncate it) and queries columns (s.archived) an unmigrated store lacks, so doctor would report a healthy DB as broken.
- _write_health_reason: snapshot source URI built with as_uri() for the same reason; the --fix live probe (_db_opens_cleanly runs BEGIN IMMEDIATE) now falls back to the snapshot unless live_writer_holds_db proves the store quiet, matching _state_db_wal — hermes doctor --fix never becomes a second writer against a gateway's state.db (#103339).
- SessionDB._connect_read_only: same as_uri() form so every read-only opener is safe in a home containing '?' or '#'.
- test_sessions_export_output_dir: fixture accepts the read_only kwarg the PR introduced.
- Drop the two doctor tests that pinned the SessionDB factory kwargs; main's URI-reserved-chars test covers _session_count.
Co-authored-by: Ahmett101 <Ahmett101@users.noreply.github.com>
This commit is contained in:
@@ -149,26 +149,31 @@ def _check_directory_structure(should_fix: bool, f: Finding) -> None:
|
||||
|
||||
|
||||
def _session_count(state_db_path: Path):
|
||||
"""COUNT(*) through a read-only SessionDB — never a writer, so schema auto-repair cannot run."""
|
||||
from hermes_state import SessionDB
|
||||
db = SessionDB(db_path=state_db_path, read_only=True)
|
||||
import sqlite3
|
||||
# mode=ro: doctor is a reader; a writable open of a gateway-held WAL DB is the second-writer class (#103339).
|
||||
# as_uri() percent-encodes '?' / '#' in the home path; a raw f-string URI truncates there.
|
||||
conn = sqlite3.connect(Path(state_db_path).resolve().as_uri() + "?mode=ro", uri=True)
|
||||
try:
|
||||
return db.session_count()
|
||||
return conn.execute("SELECT COUNT(*) FROM sessions").fetchone()[0]
|
||||
finally:
|
||||
db.close()
|
||||
conn.close()
|
||||
|
||||
|
||||
def _write_health_reason(state_db_path: Path, *, isolate: bool):
|
||||
"""FTS/write-health probe. Isolated copies never join the live store WAL lifecycle."""
|
||||
from hermes_state_repair import _db_opens_cleanly
|
||||
if not isolate:
|
||||
from hermes_state_repair import _connect_repair_durable, _db_opens_cleanly
|
||||
from hermes_state_holders import live_writer_holds_db
|
||||
# Even under --fix the probe's BEGIN IMMEDIATE is a second writer against a gateway-held DB (#103339):
|
||||
# only probe the live file when the holder scan proves it quiet, else fall back to a snapshot.
|
||||
if not isolate and not live_writer_holds_db(state_db_path, connect_repair_durable=_connect_repair_durable):
|
||||
return _db_opens_cleanly(state_db_path)
|
||||
import sqlite3
|
||||
import tempfile
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
snapshot = Path(tmp) / "state.db"
|
||||
try:
|
||||
src = sqlite3.connect(f"file:{state_db_path}?mode=ro", uri=True, timeout=1.0)
|
||||
# as_uri() percent-encodes '?' / '#' in the home path; a raw f-string URI truncates there.
|
||||
src = sqlite3.connect(Path(state_db_path).resolve().as_uri() + "?mode=ro", uri=True, timeout=1.0)
|
||||
except sqlite3.Error as exc:
|
||||
return str(exc)
|
||||
try:
|
||||
|
||||
+2
-1
@@ -642,8 +642,9 @@ class SessionDB(
|
||||
def _connect_read_only(self, timeout: float) -> sqlite3.Connection:
|
||||
"""``mode=ro`` tracked connection with Row factory. check_same_thread=False: pooled connections
|
||||
are borrowed by whichever thread reads next; exclusive ownership is enforced by pool checkout."""
|
||||
# as_uri() percent-encodes '?' / '#' in the home path; a raw f-string URI truncates there.
|
||||
conn = _connect_tracked_db(
|
||||
f"file:{self.db_path}?mode=ro", tracking_path=self.db_path, uri=True,
|
||||
Path(self.db_path).resolve().as_uri() + "?mode=ro", tracking_path=self.db_path, uri=True,
|
||||
check_same_thread=False, timeout=timeout, isolation_level=None,
|
||||
)
|
||||
conn.row_factory = sqlite3.Row
|
||||
|
||||
@@ -20,42 +20,6 @@ def test_status_session_summary_opens_a_read_only_store(monkeypatch):
|
||||
db.close.assert_called_once()
|
||||
|
||||
|
||||
def test_doctor_without_fix_counts_sessions_through_read_only_store(monkeypatch, tmp_path):
|
||||
from hermes_cli.doctor_report import Finding
|
||||
from hermes_cli.doctor_state import _state_db_health
|
||||
|
||||
db_path = tmp_path / "state.db"
|
||||
db_path.touch()
|
||||
db = MagicMock()
|
||||
db.session_count.return_value = 3
|
||||
factory = MagicMock(return_value=db)
|
||||
monkeypatch.setattr("hermes_state.SessionDB", factory)
|
||||
monkeypatch.setattr("hermes_state_repair._db_opens_cleanly", lambda _path: None)
|
||||
|
||||
_state_db_health(Finding(), False, db_path, "~/hermes")
|
||||
|
||||
factory.assert_called_once_with(db_path=db_path, read_only=True)
|
||||
db.close.assert_called_once()
|
||||
|
||||
|
||||
def test_doctor_with_fix_also_counts_through_read_only_store(monkeypatch, tmp_path):
|
||||
from hermes_cli.doctor_report import Finding
|
||||
from hermes_cli.doctor_state import _state_db_health
|
||||
|
||||
db_path = tmp_path / "state.db"
|
||||
db_path.touch()
|
||||
db = MagicMock()
|
||||
db.session_count.return_value = 3
|
||||
factory = MagicMock(return_value=db)
|
||||
monkeypatch.setattr("hermes_state.SessionDB", factory)
|
||||
monkeypatch.setattr("hermes_state_repair._db_opens_cleanly", lambda _path: None)
|
||||
|
||||
_state_db_health(Finding(), True, db_path, "~/hermes")
|
||||
|
||||
factory.assert_called_once_with(db_path=db_path, read_only=True)
|
||||
db.close.assert_called_once()
|
||||
|
||||
|
||||
def test_sessions_list_stats_and_pinned_open_a_read_only_store(monkeypatch):
|
||||
factory = MagicMock()
|
||||
monkeypatch.setattr("hermes_state.SessionDB", factory)
|
||||
|
||||
@@ -19,7 +19,7 @@ class _FakeDB:
|
||||
|
||||
|
||||
def _export(monkeypatch, *argv):
|
||||
monkeypatch.setattr(hermes_state, "SessionDB", lambda: _FakeDB())
|
||||
monkeypatch.setattr(hermes_state, "SessionDB", lambda *args, **kwargs: _FakeDB())
|
||||
monkeypatch.setattr(sys, "argv", ["hermes", "sessions", "export", "--session-id", "sess", *argv])
|
||||
main_mod.main()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user