From d2c3924dc9ca5a2473a2ea32c70019cc600c149b Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:16:21 -0700 Subject: [PATCH] =?UTF-8?q?refactor(state):=20repair/wal=20=E2=80=94=20onc?= =?UTF-8?q?e-log=20table,=20strategy=20table,=20phase=20helpers,=20unified?= =?UTF-8?q?=20disk/offline-read/exclusive=20helpers,=20compact=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_state_repair.py | 1226 +++++++++++++++++----------------------- hermes_state_wal.py | 613 ++++++++------------ 2 files changed, 754 insertions(+), 1085 deletions(-) diff --git a/hermes_state_repair.py b/hermes_state_repair.py index a4ec8dbcfc..39039327c2 100644 --- a/hermes_state_repair.py +++ b/hermes_state_repair.py @@ -8,10 +8,12 @@ time so monkeypatches there still intercept. from __future__ import annotations import contextlib +import datetime import hashlib import json import logging import os +import shutil import sqlite3 import time from contextlib import contextmanager @@ -21,10 +23,7 @@ from typing import Any, Dict, List, Optional, Tuple from hermes_constants import get_hermes_home from hermes_startup_watchdog import report_startup_progress from hermes_state_common import ( - _acquire_db_flock, - _clear_lock_holder_record, - _describe_lock_holder, - _read_lock_holder_record, + _acquire_db_flock, _clear_lock_holder_record, _describe_lock_holder, _read_lock_holder_record, is_advisory_lock_contention, ) @@ -32,32 +31,27 @@ from hermes_state_common import ( logger = logging.getLogger("hermes_state") _REPAIR_LOCK_POLL_SECONDS = 0.1 -# Snapshot copies are data transfer, not inter-process locking: bound them -# separately at 10 MiB/s, with the historical two-minute floor. +# Snapshot copies are data transfer, not locking: bounded separately at 10 MiB/s +# with the historical two-minute floor. _REPAIR_SNAPSHOT_MIN_THROUGHPUT_BYTES_PER_SECOND = 10 * 1024 * 1024 _MAX_PERSISTENT_REPAIR_ATTEMPTS = 3 _MAX_MALFORMED_BACKUPS = 3 -# Sidecars copied alongside a damaged DB and pruned with it. ``-journal`` -# matters because rollback-journal (DELETE) mode — the fallback on NFS/SMB/ -# FUSE/ZFS and WAL-reset-vulnerable builds — leaves a hot journal whenever a -# transaction was open; without it the forensic copy cannot be rolled back. +# Sidecars copied with a damaged DB and pruned with it. ``-journal``: DELETE mode +# (the NFS/SMB/FUSE/ZFS and WAL-reset-bug fallback) leaves a hot journal whenever +# a transaction was open; without it the forensic copy cannot be rolled back. _DB_SIDECAR_SUFFIXES = ("-wal", "-shm", "-journal") # Head/tail bytes sampled by ``_db_fingerprint``: changes on any genuine # repair/truncation/restore while staying O(1) on a multi-GB file. _FINGERPRINT_SAMPLE_BYTES = 65536 -# Header ranges that move on ordinary commits rather than on repair, masked -# out of the content sample: file change counter (24-27) and version-valid-for -# (92-95). In DELETE mode a commit writes the main file directly and a -# malformed-SCHEMA DB still accepts writes, so without the mask any live write -# re-keys the ledger and the repair budget resets to 1 forever. The page-1 -# sqlite_master b-tree — what repair identity depends on — sits after byte 100. -# (WAL mode routes commits to the -wal sidecar; masking is harmless there.) +# Header ranges that move on ordinary commits, not on repair — file change counter +# (24-27), version-valid-for (92-95) — masked out of the sample. A malformed-SCHEMA +# DB still accepts writes and DELETE mode writes the main file directly, so without +# the mask any live write re-keys the ledger and the repair budget resets to 1 +# forever. The page-1 sqlite_master b-tree (repair identity) sits after byte 100. _FINGERPRINT_VOLATILE_HEADER_RANGES = ((24, 28), (92, 96)) -# Free-space headroom for the pre-repair forensic backup (a full raw copy of -# the damaged DB plus sidecars; a repair loop on a large state.db is a disk -# amplifier). Proportional, not a flat multi-GB floor: a refused backup is a -# HARD STOP, and a large absolute reserve would turn "repair loops" into -# "repair never runs" on small container/VM volumes. +# Headroom for the forensic backup (a full raw copy; a repair loop on a large +# state.db is a disk amplifier). Proportional, not a flat multi-GB floor: a refused +# backup is a HARD STOP, and a big reserve would make repair never run on small volumes. _REPAIR_BACKUP_MIN_FREE_BYTES = 256 * 1024 * 1024 # 256 MiB absolute floor _REPAIR_BACKUP_FREE_FRACTION = 0.02 # plus 2% of the volume _FTS_TABLES = ("messages_fts", "messages_fts_trigram", "messages_fts_cjk") @@ -76,36 +70,34 @@ def _bundle_bytes(db_path: Path) -> int: def _unlink_quiet(path: Path) -> None: """Best-effort unlink; a failure here is never the caller's error.""" - try: + with contextlib.suppress(OSError): path.unlink(missing_ok=True) - except OSError: - pass -def _offline_access_tools(): - """``(offline_file_access, LiveConnectionError)`` from hermes_cli, or inert - stand-ins on scaffold/embed installs (no tracked connections exist there, - so a raw read is safe).""" +def _read_offline(db_path: Path, what: str, reader) -> Optional[str]: + """Run *reader()* under ``hermes_cli.sqlite_safe_read.offline_file_access``. + + ``close()`` on ANY raw descriptor cancels every POSIX advisory lock this + process holds on the file, including a peer connection's RESERVED lock + (``sqlite_safe_read`` rule 1), so a raw read is only safe with no live + connection; ``None`` when that makes it unsafe or the file is unreadable. + Scaffold/embed installs without hermes_cli have no tracked connections. + """ try: from hermes_cli.sqlite_safe_read import LiveConnectionError, offline_file_access - return offline_file_access, LiveConnectionError except ImportError: - @contextmanager - def offline_file_access(_path, **_kw): - yield - - class LiveConnectionError(Exception): - pass - - return offline_file_access, LiveConnectionError + offline_file_access, LiveConnectionError = (lambda _p, **_k: contextlib.nullcontext()), OSError + try: + with offline_file_access(db_path, what=what): + return reader() + except (LiveConnectionError, OSError): + return None def _claim_repair_attempt(db_path: Path) -> bool: - """Claim the one-shot per-process repair attempt for *db_path*. - - True for the first caller, False afterwards: bounds the repair/reopen loop - and stops concurrent callers racing surgery on one file. - """ + """Claim the one-shot per-process repair attempt for *db_path*: True for the + first caller, False afterwards (bounds the repair/reopen loop and stops + concurrent callers racing surgery on one file).""" from hermes_state import _repair_attempt_lock, _repair_attempted_paths key = str(db_path) with _repair_attempt_lock: @@ -151,29 +143,42 @@ def _release_lock_handle(handle, *, clear_record: bool = False) -> None: handle.close() +def _acquire_repair_lock_windows(lock_path: Path, handle, timeout: float): + """Windows counterpart of ``_acquire_db_flock``: True / False (timed out) / None (non-contention error).""" + deadline = time.monotonic() + timeout + while True: + try: + _msvcrt_lock(handle, "LK_NBLCK") + return True + except (BlockingIOError, OSError) as exc: + if not is_advisory_lock_contention(exc): + logger.warning( + "Could not acquire state.db repair lock %s (%s) — " + "skipping schema surgery on a non-contention error.", lock_path, exc, + ) + return None + if time.monotonic() >= deadline: + return False + time.sleep(_REPAIR_LOCK_POLL_SECONDS) + + @contextlib.contextmanager def _cross_process_repair_lock(db_path: Path): """Serialize state.db schema surgery across processes. - Yields True when this process holds the repair lock for *db_path*, False - when the bounded acquire timed out or the lock file could not be opened. - Running surgery unlocked IS the unsafe interleaving this prevents: a caller - that gets False must NOT do surgery. - - ``flock`` because the kernel drops it when the holder dies (a pidfile - would wedge every future repair); a forked child that inherited the fd is - the exception, so the acquire records the holder's pid + start time and - breaks the lock when that holder is provably dead (``_acquire_db_flock``). - The acquire is bounded because a *live* repairer can sit in ``VACUUM`` - for minutes, and an unbounded wait would hang the caller's open silently. - An unopenable lock file (out of space/inodes/descriptors) fails closed - too: a sibling that opened ITS handle before the disk filled may still be - inside surgery. + Yields True when this process holds the repair lock, False when the bounded + acquire timed out or the lock file could not be opened; on False the caller + must NOT do surgery (unlocked surgery IS the interleaving this prevents). + ``flock``: the kernel drops it when the holder dies (a pidfile would wedge + every future repair); a forked child inheriting the fd is the exception, so + the acquire records pid + start time and breaks a provably dead holder's + lock. Bounded because a live repairer can sit in ``VACUUM`` for minutes. An + unopenable lock file (no space/inodes/descriptors) fails closed too: a + sibling that opened ITS handle before the disk filled may be inside surgery. """ from hermes_state import _IS_WINDOWS, _REPAIR_LOCK_TIMEOUT_SECONDS lock_path, handle = _open_lock_file( - db_path, ".repair.lock", "repair", - "skipping schema surgery rather than running it without cross-process authority.", + db_path, ".repair.lock", "repair", "skipping schema surgery rather than running it without cross-process authority.", ) if handle is None: yield False @@ -182,37 +187,18 @@ def _cross_process_repair_lock(db_path: Path): acquired = False try: if _IS_WINDOWS: - deadline = time.monotonic() + _REPAIR_LOCK_TIMEOUT_SECONDS - while True: - try: - _msvcrt_lock(handle, "LK_NBLCK") - acquired = True - break - except (BlockingIOError, OSError) as exc: - if not is_advisory_lock_contention(exc): - logger.warning( - "Could not acquire state.db repair lock %s (%s) — " - "skipping schema surgery on a non-contention error.", - lock_path, exc, - ) - acquired = None - break - if time.monotonic() >= deadline: - break - time.sleep(_REPAIR_LOCK_POLL_SECONDS) + acquired = _acquire_repair_lock_windows(lock_path, handle, _REPAIR_LOCK_TIMEOUT_SECONDS) else: acquired, handle = _acquire_db_flock( - str(lock_path), handle, _REPAIR_LOCK_TIMEOUT_SECONDS, - _REPAIR_LOCK_POLL_SECONDS, "state.db repair lock", + str(lock_path), handle, _REPAIR_LOCK_TIMEOUT_SECONDS, _REPAIR_LOCK_POLL_SECONDS, "state.db repair lock", ) if acquired is None: acquired = False # non-contention failure already logged with its errno elif not acquired: record = None if _IS_WINDOWS else _read_lock_holder_record(handle) logger.warning( - "state.db repair lock %s held by another process for more " - "than %.0fs — skipping schema surgery in this process to " - "avoid racing the repairer. Recorded holder: %s.", + "state.db repair lock %s held by another process for more than %.0fs — skipping schema surgery in " + "this process to avoid racing the repairer. Recorded holder: %s.", lock_path, _REPAIR_LOCK_TIMEOUT_SECONDS, _describe_lock_holder(record), ) yield acquired @@ -224,16 +210,11 @@ def _cross_process_repair_lock(db_path: Path): def _try_acquire_auto_maintenance_lock(db_path: Path) -> Optional[Any]: - """Non-blocking cross-process lock for one auto-maintenance pass. - - Advisory lock the kernel releases if the holder exits. A caller that cannot - acquire it must skip the pass: otherwise two startups both pass the interval - check and the second prunes a row the first has only just closed recoverably. - """ + """Non-blocking cross-process lock for one auto-maintenance pass (None = skip + the pass: otherwise two startups both pass the interval check and the second + prunes a row the first has only just closed recoverably).""" from hermes_state import _IS_WINDOWS - _lock_path, handle = _open_lock_file( - db_path, ".auto-maintenance.lock", "auto-maintenance", "skipping automatic maintenance." - ) + _lock_path, handle = _open_lock_file(db_path, ".auto-maintenance.lock", "auto-maintenance", "skipping automatic maintenance.") if handle is None: return None try: @@ -249,20 +230,16 @@ def _try_acquire_auto_maintenance_lock(db_path: Path) -> Optional[Any]: return handle -def _release_auto_maintenance_lock(handle: Any) -> None: - """Release a handle returned by :func:`_try_acquire_auto_maintenance_lock`.""" - _release_lock_handle(handle) +_release_auto_maintenance_lock = _release_lock_handle # release a _try_acquire_auto_maintenance_lock handle def _bump_schema_cookie(conn: sqlite3.Connection) -> None: """Increment the schema cookie after direct ``sqlite_master`` surgery. - Ordinary DDL bumps this counter and other connections compare it before - running a prepared statement — that is how they discard a cached schema. - Editing ``sqlite_master`` under ``writable_schema=ON`` does NOT bump it, - so live connections elsewhere keep compiling against the old schema (e.g. - firing triggers into ``messages_fts*`` shadow tables that no longer - exist). Best-effort, never raises: a failed bump leaves the status quo. + Ordinary DDL bumps it and peers compare it before running a prepared + statement (how they discard a cached schema); ``writable_schema=ON`` edits + do NOT, so live connections would keep firing triggers into ``messages_fts*`` + shadow tables that no longer exist. Best-effort, never raises. """ try: current = conn.execute("PRAGMA schema_version").fetchone()[0] @@ -287,57 +264,73 @@ def _repair_backup_headroom_bytes(total_bytes: int) -> int: return max(_REPAIR_BACKUP_MIN_FREE_BYTES, int(total_bytes * _REPAIR_BACKUP_FREE_FRACTION)) -def _repair_scratch_space_error(db_path: Path) -> Optional[str]: - """Return an error unless snapshot, VACUUM and promotion can fit safely.""" - import shutil - +def _disk_budget(db_path: Path, refusal: str): + """``(bundle_bytes, free_bytes, headroom_bytes)`` for *db_path*'s volume, or an + error string. Fails CLOSED on stat()/disk_usage() errors: the nearly-full + volume these guards exist for is exactly where they are most likely to fail.""" try: - snapshot_bytes = _bundle_bytes(db_path) + need = _bundle_bytes(db_path) usage = shutil.disk_usage(db_path.parent) - headroom = _repair_backup_headroom_bytes(usage.total) - # Strategy 2 runs VACUUM on the staged DB, which SQLite documents may - # need up to 2x the database size in extra space; the same reserve then - # covers transactional promotion into the live DB. - if usage.free >= snapshot_bytes + (2 * snapshot_bytes) + headroom: - return None - return ( - f"only {usage.free / 1e9:.2f}GB free on {db_path.parent}; the " - f"repair snapshot needs up to {snapshot_bytes / 1e9:.2f}GB, " - f"VACUUM may need another {(2 * snapshot_bytes) / 1e9:.2f}GB, and " - f"{headroom / 1e9:.2f}GB must remain as headroom. Free disk space, " - "then retry." - ) except OSError as exc: return ( f"could not determine free space on {db_path.parent} ({exc}); " - "refusing the repair snapshot rather than risk filling the volume" + f"refusing the {refusal} rather than risk filling the volume" ) + return need, usage.free, _repair_backup_headroom_bytes(usage.total) + + +def _repair_scratch_space_error(db_path: Path) -> Optional[str]: + """Return an error unless snapshot, VACUUM and promotion can fit safely.""" + budget = _disk_budget(db_path, "repair snapshot") + if isinstance(budget, str): + return budget + snapshot_bytes, free, headroom = budget + # VACUUM on the staged DB may need up to 2x the database size (SQLite docs); + # the same reserve then covers transactional promotion into the live DB. + if free >= snapshot_bytes + (2 * snapshot_bytes) + headroom: + return None + return ( + f"only {free / 1e9:.2f}GB free on {db_path.parent}; the repair snapshot needs up to " + f"{snapshot_bytes / 1e9:.2f}GB, VACUUM may need another {(2 * snapshot_bytes) / 1e9:.2f}GB, and " + f"{headroom / 1e9:.2f}GB must remain as headroom. Free disk space, then retry." + ) + + +def _backup_free_space_error(db_path: Path) -> Optional[str]: + """Disk guard for the forensic copy: reason to refuse, or None. A full raw + copy on a nearly-full volume (which a preceding repair loop may itself have + caused) can finish off the disk and every process on the machine.""" + hint = _MANUAL_RECOVER_HINT.format(db_path=db_path) + budget = _disk_budget(db_path, "forensic copy") + if isinstance(budget, str): + return f"{budget}. {hint}" + need, free, headroom = budget + if free - need >= headroom: + return None + return ( + f"only {free / 1e9:.2f}GB free on {db_path.parent}; copying the damaged DB needs {need / 1e9:.2f}GB and must " + f"leave {headroom / 1e9:.2f}GB headroom. {hint}" + ) def _repair_snapshot_timeout_seconds(source_path: Path) -> float: - """Bound one SQLite snapshot by source size, including live sidecars. - - A WAL can hold committed rows not yet in the main file; count it so a - healthy large-database copy is not cut off by the repair-lock timeout. - """ + """Bound one SQLite snapshot by source size incl. sidecars (a WAL can hold + committed rows not yet in the main file), so a healthy large-database copy + is not cut off by the repair-lock timeout.""" from hermes_state import _REPAIR_LOCK_TIMEOUT_SECONDS, _REPAIR_SNAPSHOT_MIN_THROUGHPUT_BYTES_PER_SECOND source_bytes = 0 for candidate in (source_path, *_sidecars(source_path)): - try: + with contextlib.suppress(FileNotFoundError): source_bytes += candidate.stat().st_size - except FileNotFoundError: - continue return max(_REPAIR_LOCK_TIMEOUT_SECONDS, source_bytes / _REPAIR_SNAPSHOT_MIN_THROUGHPUT_BYTES_PER_SECOND) def _repair_failure_consumes_attempt(exc: BaseException) -> bool: """Whether a pre-strategy SQLite failure proves deterministic corruption. - Lock contention, timeouts, disk-full, I/O and filesystem failures are - environmental — a retry may succeed, so they must not burn the repair - ledger. Only SQLite's corruption/image result codes prove deterministic - damage, even when SQLite cannot stage a snapshot far enough to run a - named strategy. + Lock contention, timeouts, disk-full and I/O failures are environmental — a + retry may succeed, so they must not burn the repair ledger. Only SQLite's + corruption/image result codes prove deterministic damage. """ if not isinstance(exc, sqlite3.DatabaseError): return False @@ -358,87 +351,63 @@ def _repair_ledger_path(db_path: Path) -> Path: def _db_fingerprint(db_path: Path) -> "Optional[str]": """Cheap identity for a damaged DB file: size + a bounded content sample. - Deliberately EXCLUDES mtime: the malformed-schema class still accepts - writes, so live writers, WAL checkpoints and the strategies themselves move - mtime between passes; keyed on mtime, every pass looked like a NEW file and - the attempt counter reset to 1 forever. Hashing a multi-GB file on every - open is the cost this ledger exists to avoid, so sample the head/tail - slices any real repair, truncation or restore necessarily changes. + EXCLUDES mtime: a malformed-schema DB still accepts writes, so live writers, + checkpoints and the strategies move mtime between passes; keyed on mtime + every pass looked like a NEW file and the attempt counter reset forever. + Hashing a multi-GB file per open is the cost this ledger avoids, so sample + the head/tail slices any real repair, truncation or restore must change. - Runs under ``offline_file_access``: ``close()`` on ANY raw descriptor - cancels every POSIX advisory lock this process holds on the file, including - a peer connection's RESERVED lock (``hermes_cli.sqlite_safe_read`` rule 1), - and a live peer is the expected case here (this runs BEFORE - ``_backup_db_file``'s ``has_live_connection`` guard). Returns ``None`` - ("identity unavailable") when that makes the read unsafe. Callers MUST NOT - substitute a differently-shaped key: the ledger compares keys for equality, - so alternating shapes never matches and the unbounded loop returns. + ``None`` = identity unavailable (:func:`_read_offline`; a live peer is expected + here since this runs BEFORE ``_backup_db_file``'s live-connection guard). + Callers MUST NOT substitute a differently-shaped key: the ledger compares for + equality, so alternating shapes never match and the unbounded loop returns. """ - try: + def _sample() -> str: st = db_path.stat() - offline_file_access, LiveConnectionError = _offline_access_tools() - try: - with offline_file_access(db_path, what="fingerprint"): - with open(db_path, "rb") as fh: - head = fh.read(_FINGERPRINT_SAMPLE_BYTES) - tail = b"" - if st.st_size > _FINGERPRINT_SAMPLE_BYTES: - fh.seek(max(0, st.st_size - _FINGERPRINT_SAMPLE_BYTES)) - tail = fh.read(_FINGERPRINT_SAMPLE_BYTES) - except LiveConnectionError: - return None - digest = hashlib.sha256(_mask_volatile_header(head) + tail).hexdigest()[:32] - return f"{st.st_size}:{digest}" - except OSError: - return None + with open(db_path, "rb") as fh: + head = fh.read(_FINGERPRINT_SAMPLE_BYTES) + tail = b"" + if st.st_size > _FINGERPRINT_SAMPLE_BYTES: + fh.seek(max(0, st.st_size - _FINGERPRINT_SAMPLE_BYTES)) + tail = fh.read(_FINGERPRINT_SAMPLE_BYTES) + return f"{st.st_size}:{hashlib.sha256(_mask_volatile_header(head) + tail).hexdigest()[:32]}" + + return _read_offline(db_path, "fingerprint", _sample) def _backup_content_identity(db_path: Path) -> "Optional[str]": """Recovery-image identity for forensic-backup dedupe: whole file + sidecars. - A DIFFERENT equivalence relation from :func:`_db_fingerprint`; never - conflate them. The fingerprint answers "same repair epoch?" and samples - only head+tail; a live writer can commit rows into an *interior* page while - preserving size and both 64 KiB slices, so two materially different - recovery images share one fingerprint, and reusing a backup on that basis - hands the operator a snapshot predating real user data. A forensic copy - must claim byte identity, so this digests the ENTIRE main file plus every - present sidecar (the WAL can hold uncheckpointed committed frames). Runs - under ``offline_file_access`` (same POSIX-lock reason); ``None`` when a - live connection makes the read unsafe — the caller then takes a fresh - backup, never a false reuse. + A DIFFERENT relation from :func:`_db_fingerprint` (never conflate them): + the fingerprint answers "same repair epoch?" from head+tail only, and a live + writer can commit into an *interior* page while preserving size and both + 64 KiB slices — reusing a backup on that basis hands the operator a snapshot + predating real user data. A forensic copy must claim byte identity, so this + digests the ENTIRE main file plus every present sidecar (the WAL can hold + uncheckpointed frames). ``None`` when a live connection makes the read unsafe + (:func:`_read_offline`) — the caller then takes a fresh backup, never a false reuse. """ - offline_file_access, LiveConnectionError = _offline_access_tools() - - def _hash_whole(path: Path, hasher: "Any") -> None: - with open(path, "rb") as fh: - for chunk in iter(lambda: fh.read(1024 * 1024), b""): - hasher.update(chunk) - - try: + def _digest() -> str: hasher = hashlib.sha256() - with offline_file_access(db_path, what="backup-identity"): - # Length-delimit every member (main file included) so the - # concatenation is prefix-free; otherwise a main-file tail could - # coincide with a main+sidecar split and dedupe two images together. - hasher.update(f"\0main:{db_path.stat().st_size}\0".encode()) - _hash_whole(db_path, hasher) - for suffix, sidecar in zip(_DB_SIDECAR_SUFFIXES, _sidecars(db_path)): - if sidecar.exists(): - hasher.update(f"\0{suffix}:{sidecar.stat().st_size}\0".encode()) - _hash_whole(sidecar, hasher) + members = [("main", db_path), *((sfx, p) for sfx, p in zip(_DB_SIDECAR_SUFFIXES, _sidecars(db_path)) if p.exists())] + for label, path in members: + # Length-delimit every member (main file included) so the concatenation + # is prefix-free; otherwise a main-file tail could coincide with a + # main+sidecar split and dedupe two images together. + hasher.update(f"\0{label}:{path.stat().st_size}\0".encode()) + with open(path, "rb") as fh: + for chunk in iter(lambda: fh.read(1024 * 1024), b""): + hasher.update(chunk) return hasher.hexdigest() - except (LiveConnectionError, OSError): - return None + + return _read_offline(db_path, "backup-identity", _digest) def _read_repair_ledger(db_path: Path) -> "Dict[str, Any]": - try: + with contextlib.suppress(OSError, ValueError): raw = json.loads(_repair_ledger_path(db_path).read_text(encoding="utf-8")) if isinstance(raw, dict): return raw - except (OSError, ValueError): - pass return {} @@ -448,9 +417,9 @@ def _persistent_repair_attempts_exhausted(db_path: Path) -> bool: True only when the ledger records ``_MAX_PERSISTENT_REPAIR_ATTEMPTS`` failures against the CURRENT fingerprint. Never raises; a missing/corrupt ledger or unstatable DB reads as "not exhausted" (the in-process claim and - cross-process lock still bound one run). When a live connection makes the - fingerprint unavailable, fall back to the SIZE the ledger recorded — - otherwise a peer connection hides an exhausted budget on every pass. + cross-process lock still bound one run). Fingerprint unavailable (live + connection) -> compare the SIZE the ledger recorded, otherwise a peer + connection hides an exhausted budget on every pass. """ ledger = _read_repair_ledger(db_path) recorded = ledger.get("fingerprint") @@ -471,23 +440,21 @@ def _persistent_repair_attempts_exhausted(db_path: Path) -> bool: def _persistent_repair_exhausted_error(db_path: Path) -> str: """The stable operator-facing diagnostic for an exhausted repair budget.""" return ( - f"automatic repair has already failed " - f"{_MAX_PERSISTENT_REPAIR_ATTEMPTS} times on this exact file — " - "the corruption is beyond the schema/FTS repair strategies " - "(likely b-tree page damage). Manual recovery required: restore " + f"automatic repair has already failed {_MAX_PERSISTENT_REPAIR_ATTEMPTS} times on this exact file — the " + f"corruption is beyond the schema/FTS repair strategies (likely b-tree page damage). Manual recovery " + f"required: restore " f"a backup, or salvage with `sqlite3 {db_path} \".recover\"`. " - f"Delete {_repair_ledger_path(db_path).name} to force another " - "automatic attempt." + f"Delete {_repair_ledger_path(db_path).name} to force another automatic attempt." ) def _record_repair_outcome(db_path: Path, *, repaired: bool, fingerprint: "Optional[str]" = None) -> None: """Update the persistent attempt ledger after a repair pass. Never raises. - Defaults to the post-attempt fingerprint (what the NEXT exhaustion probe - observes). When a live connection makes it unavailable, keep the recorded - key and still increment — dropping the pass would let a peer connection - reset the budget every time. Never write a differently shaped key. + Keys on the post-attempt fingerprint (what the NEXT exhaustion probe sees). + If a live connection makes it unavailable, keep the recorded key and still + increment — dropping the pass lets a peer reset the budget every time. + Never write a differently shaped key. """ ledger_path = _repair_ledger_path(db_path) try: @@ -504,15 +471,9 @@ def _record_repair_outcome(db_path: Path, *, repaired: bool, fingerprint: "Optio return fp = recorded attempts = int(ledger.get("failed_attempts", 0)) + 1 if recorded == fp else 1 - import datetime - + stamp = datetime.datetime.now().isoformat(timespec="seconds") ledger_path.write_text( - json.dumps({ - "fingerprint": fp, - "failed_attempts": attempts, - "last_attempt": datetime.datetime.now().isoformat(timespec="seconds"), - }), - encoding="utf-8", + json.dumps({"fingerprint": fp, "failed_attempts": attempts, "last_attempt": stamp}), encoding="utf-8", ) except Exception as exc: # pragma: no cover - best effort logger.warning("Could not update state.db repair ledger: %s", exc) @@ -522,10 +483,7 @@ def _existing_malformed_backups(db_path: Path) -> "List[Path]": """Timestamped forensic backups of *db_path*, newest first.""" prefix = f"{db_path.name}.malformed-backup-" try: - found = [ - p for p in db_path.parent.iterdir() - if p.name.startswith(prefix) and not p.name.endswith(_DB_SIDECAR_SUFFIXES) - ] + found = [p for p in db_path.parent.iterdir() if p.name.startswith(prefix) and not p.name.endswith(_DB_SIDECAR_SUFFIXES)] except OSError: return [] return sorted(found, key=lambda p: p.name, reverse=True) @@ -541,48 +499,14 @@ def _prune_malformed_backups(db_path: Path, keep: int = _MAX_MALFORMED_BACKUPS) logger.warning("Could not prune stale DB backup %s: %s", victim, exc) -def _backup_free_space_error(db_path: Path) -> Optional[str]: - """Disk guard for the forensic copy: reason to refuse, or None. - - A full raw copy on a nearly-full volume (which a preceding repair loop may - itself have caused) can finish off the disk and every process on the - machine. Fails CLOSED on stat()/disk_usage() errors: the nearly-full volume - this guard exists for is exactly where they are most likely to fail, and - proceeding would take the copy that finishes off the disk. - """ - import shutil - - hint = _MANUAL_RECOVER_HINT.format(db_path=db_path) - try: - need = _bundle_bytes(db_path) - usage = shutil.disk_usage(db_path.parent) - headroom = _repair_backup_headroom_bytes(usage.total) - if usage.free - need >= headroom: - return None - return ( - f"only {usage.free / 1e9:.2f}GB free on {db_path.parent}; " - f"copying the damaged DB needs {need / 1e9:.2f}GB and must " - f"leave {headroom / 1e9:.2f}GB headroom. {hint}" - ) - except OSError as exc: - return ( - f"could not determine free space on {db_path.parent} ({exc}); " - f"refusing the forensic copy rather than risk filling the volume. {hint}" - ) - - def _publish_backup_bundle(db_path: Path, staging: Path, backup_path: Path) -> None: """Copy DB + sidecars to *staging* names, then rename each into place. - PUBLICATION ORDER MATTERS: the main DB name is the bundle's commit marker - (what ``_existing_malformed_backups`` counts), so sidecars go FIRST and the - main DB LAST; a failure partway then never leaves a countable main backup - over a missing sidecar. On any failure, unpublished staging files AND - anything already promoted are removed, so no official ``backup_path`` - survives a partial bundle. + ORDER MATTERS: the main DB name is the bundle's commit marker (what + ``_existing_malformed_backups`` counts), so sidecars publish FIRST and the + main DB LAST — a failure partway never leaves a countable backup over a + missing sidecar. On failure, staging files AND anything promoted are removed. """ - import shutil - pairs: "List[Tuple[Path, Path, Path]]" = [ (sidecar, staging.with_name(staging.name + suffix), backup_path.with_name(backup_path.name + suffix)) for suffix, sidecar in zip(_DB_SIDECAR_SUFFIXES, _sidecars(db_path)) @@ -597,10 +521,8 @@ def _publish_backup_bundle(db_path: Path, staging: Path, backup_path: Path) -> N os.replace(staged, dst) published.append(dst) except Exception: - for staged in (staging, *(s for _src, s, _d in pairs)): - _unlink_quiet(staged) - for dst in published: - _unlink_quiet(dst) + for victim in (staging, *(s for _src, s, _d in pairs), *published): + _unlink_quiet(victim) raise @@ -608,34 +530,29 @@ def _backup_db_file(db_path: Path) -> "Tuple[Optional[Path], Optional[str]]": """Raw-copy a (possibly malformed) DB plus sidecars to a timestamped backup. Raw bytes on purpose: the DB won't open cleanly, so preserve them exactly - for forensics / manual restore. Returns ``(backup_path, None)`` or - ``(None, reason)``; the repair path treats a refused backup as a HARD STOP - because the forensic bundle is the recovery path when every strategy fails. - Refuses while a connection to this DB is live in the process: reading the - file would ``close()`` a descriptor and cancel that connection's POSIX - advisory locks (see ``hermes_cli.sqlite_safe_read``) — a real case: one - SessionDB can enter repair while the gateway holds others. + for forensics. Returns ``(backup_path, None)`` or ``(None, reason)``; repair + treats a refused backup as a HARD STOP because the bundle is the recovery + path when every strategy fails. Refuses while a connection to this DB is + live in the process: the raw read would ``close()`` a descriptor and cancel + that connection's POSIX advisory locks (``hermes_cli.sqlite_safe_read``) — + real case: one SessionDB enters repair while the gateway holds others. - Dedupe: if the newest existing backup is byte-identical to the current - recovery image (``_backup_content_identity`` — NOT mtime, NOT - ``_db_fingerprint``), reuse it; a repair loop once copied the same damaged - bytes on every restart. The copy lands under a staging name OUTSIDE the - ``.malformed-backup-`` prefix: a staging name inside it counts as a backup, - sorts NEWEST (prune kept partials and deleted intact copies) and dedupe - could return it with no real forensic copy on disk. + Dedupe: reuse the newest backup when byte-identical to the current recovery + image (``_backup_content_identity`` — NOT mtime, NOT ``_db_fingerprint``); a + repair loop once re-copied the same bytes on every restart. Staging names + live OUTSIDE the ``.malformed-backup-`` prefix: inside it they count as a + backup, sort NEWEST (prune kept partials, deleted intact copies) and dedupe + could return one with no real forensic copy on disk. """ - import datetime - try: from hermes_cli.sqlite_safe_read import has_live_connection + live = has_live_connection(db_path) except ImportError: - has_live_connection = None # type: ignore[assignment] - - if has_live_connection is not None and has_live_connection(db_path): + live = False + if live: reason = ( f"a connection to {db_path} is still open in this process; " - "raw-copying it would cancel that connection's POSIX advisory " - "locks. Close all SessionDB handles first." + "raw-copying it would cancel that connection's POSIX advisory locks. Close all SessionDB handles first." ) logger.error("Refusing to raw-copy %s for backup: %s", db_path, reason) return None, reason @@ -648,30 +565,22 @@ def _backup_db_file(db_path: Path) -> "Tuple[Optional[Path], Optional[str]]": backup_path = db_path.with_name(f"{db_path.name}.malformed-backup-{stamp}_{seq}") seq += 1 try: - # Sweep staging debris from an earlier interrupted pass BEFORE the - # dedupe: leftover staging is byte-identical to the damaged DB, so - # dedupe would hand it back as a legitimate backup. Also sweeps the - # pre-merge ``.incomplete`` spelling, which prefix-matches as a backup, - # sorts NEWEST and would otherwise survive prune forever. + # Sweep staging debris from an interrupted pass BEFORE the dedupe (it is + # byte-identical to the damaged DB, so dedupe would hand it back as a + # backup). Also the old ``.incomplete`` spelling, which prefix-matches as + # a backup, sorts NEWEST and would otherwise survive prune forever. for pattern in (f"{db_path.name}.backup-staging-*", f"{db_path.name}.malformed-backup-*.incomplete*"): for old in db_path.parent.glob(pattern): _unlink_quiet(old) - try: - # Only hash the source when there is a candidate to compare - # against; hashing a multi-GB source right before copying it is - # pure waste on the common first-corruption pass. - existing_backups = _existing_malformed_backups(db_path)[:1] - if existing_backups: + with contextlib.suppress(OSError): + # Hash the source only when there is a candidate: hashing a multi-GB + # file right before copying it is waste on the first-corruption pass. + newest = _existing_malformed_backups(db_path)[:1] + if newest: src_id = _backup_content_identity(db_path) - for existing in existing_backups: - if src_id is not None and _backup_content_identity(existing) == src_id: - logger.info( - "Reusing existing forensic backup %s (identical to the " - "damaged DB).", existing, - ) - return existing, None - except OSError: - pass + if src_id is not None and _backup_content_identity(newest[0]) == src_id: + logger.info("Reusing existing forensic backup %s (identical to the damaged DB).", newest[0]) + return newest[0], None reason = _backup_free_space_error(db_path) if reason is not None: logger.error("Refusing forensic backup of %s: %s", db_path, reason) @@ -689,16 +598,17 @@ def preflight_db_writability(db_path: Path, *, db_label: str = "state.db") -> No """Refuse-or-repair read-only DB files BEFORE the first connection opens. A stray read-only ``state.db`` / ``-wal`` / ``-shm`` (sudo run, restored - backup, copied dotfiles) otherwise surfaces as an opaque "attempt to write - a readonly database" deep inside ``_init_schema``, and the obvious wrong - "fix" (deleting the ``-wal``) silently loses committed transactions. - Repairs with ``chmod u+rw`` only inside the Hermes home tree (Hermes owns - those files, and ``chmod`` fails on files the user doesn't own, which - bounds the repair exactly); otherwise fails fast naming the exact file and - ``chmod`` command. Never deletes or truncates a WAL sidecar — once - writable, the normal open path checkpoints its committed frames. - ``:memory:`` and ``file:`` URIs are skipped. Shared with ``kanban_db``. + backup, copied dotfiles) otherwise surfaces as an opaque "attempt to write a + readonly database" inside ``_init_schema``, and the obvious wrong "fix" + (deleting the ``-wal``) loses committed transactions. ``chmod u+rw`` repair + only inside the Hermes home tree (Hermes owns those files; ``chmod`` fails on + files the user doesn't own, bounding the repair exactly); otherwise fail fast + naming the file and command. Never deletes/truncates a WAL sidecar — once + writable, the normal open checkpoints it. ``:memory:``/``file:`` skipped. + Shared with ``kanban_db``. """ + import stat as _stat + raw = str(db_path) if raw == ":memory:" or raw.startswith("file:"): return @@ -707,31 +617,18 @@ def preflight_db_writability(db_path: Path, *, db_label: str = "state.db") -> No except Exception: # pragma: no cover - defensive home = None - def _in_repair_scope(p: Path) -> bool: - if home is None: - return False - try: - return p.resolve().is_relative_to(home) - except (OSError, ValueError): - return False - def _ensure_writable(p: Path, *, is_dir: bool = False) -> None: - import stat as _stat - if os.access(p, os.R_OK | os.W_OK): return - if _in_repair_scope(p): - try: + in_scope = False + with contextlib.suppress(OSError, ValueError): + in_scope = home is not None and p.resolve().is_relative_to(home) + if in_scope: add = _stat.S_IRUSR | _stat.S_IWUSR | (_stat.S_IXUSR if is_dir else 0) os.chmod(p, p.stat().st_mode | add) - except OSError: - pass - if os.access(p, os.R_OK | os.W_OK): - logger.info( - "%s preflight: repaired read-only %s (chmod u+rw%s)", - db_label, p, "x" if is_dir else "", - ) - return + if in_scope and os.access(p, os.R_OK | os.W_OK): + logger.info("%s preflight: repaired read-only %s (chmod u+rw%s)", db_label, p, "x" if is_dir else "") + return kind = "directory" if is_dir else "file" wal_note = ( " Do NOT delete the -wal file — it contains committed data that " @@ -739,19 +636,16 @@ def preflight_db_writability(db_path: Path, *, db_label: str = "state.db") -> No if p.name.endswith("-wal") else "" ) raise sqlite3.OperationalError( - f"{db_label} is not writable: {kind} {p} is read-only for this " - f"user. Hermes needs read-write access to open the database. " - f"Fix with: chmod u+rw{'x' if is_dir else ''} '{p}'" - f" (files owned by another user may need sudo/chown).{wal_note}" + f"{db_label} is not writable: {kind} {p} is read-only for this user. Hermes needs read-write access to " + f"open the database. Fix with: chmod u+rw{'x' if is_dir else ''} '{p}' (files owned by another user may " + f"need sudo/chown).{wal_note}" ) - parent = db_path.parent - if parent.is_dir(): - # SQLite needs a writable directory in every journal mode (WAL/SHM - # sidecars, or the rollback journal in DELETE mode). - _ensure_writable(parent, is_dir=True) - for suffix in ("", "-wal", "-shm"): - p = db_path.with_name(db_path.name + suffix) if suffix else db_path + # SQLite needs a writable directory in every journal mode (WAL/SHM + # sidecars, or the rollback journal in DELETE mode). + if db_path.parent.is_dir(): + _ensure_writable(db_path.parent, is_dir=True) + for p in (db_path, db_path.with_name(db_path.name + "-wal"), db_path.with_name(db_path.name + "-shm")): if p.is_file(): _ensure_writable(p) @@ -759,17 +653,13 @@ def preflight_db_writability(db_path: Path, *, db_label: str = "state.db") -> No def _connect_repair_durable(db_path: Path, *, timeout: float = 5.0) -> sqlite3.Connection: """``sqlite3.connect`` for the repair/probe paths, with macOS write barriers. - These paths open ``state.db`` directly (not via ``SessionDB`` / - :func:`apply_wal_with_fallback`), so they inherited ``synchronous=NORMAL`` - and no ``checkpoint_fullfsync`` — on Darwin, where ``fsync()`` guarantees - neither data-on-platter nor ordering, an interrupted rewrite leaves - half-written b-tree pages, and ``REINDEX``/``VACUUM``/``writable_schema`` - surgery rewrite nearly every page. Autocommit (``isolation_level=None``) - is preserved: DDL and ``VACUUM`` are illegal inside an implicit - transaction. Barriers are best-effort by necessity: SQLite loads the schema - before any statement, so on a malformed schema even ``PRAGMA - synchronous=FULL`` raises — and a malformed DB is this helper's input. - Whole-file rewrites call :func:`_reapply_durability_barriers` once the + These paths bypass ``SessionDB``/:func:`apply_wal_with_fallback`, so they + inherited ``synchronous=NORMAL`` and no ``checkpoint_fullfsync`` — on Darwin + an interrupted ``REINDEX``/``VACUUM``/``writable_schema`` rewrite leaves + half-written b-tree pages. Autocommit (``isolation_level=None``): DDL and + ``VACUUM`` are illegal inside an implicit transaction. Barriers are + best-effort: on a malformed schema even ``PRAGMA synchronous=FULL`` raises, + so whole-file rewrites call :func:`_reapply_durability_barriers` once the schema parses again. """ conn = sqlite3.connect(str(db_path), timeout=timeout, isolation_level=None) @@ -777,13 +667,20 @@ def _connect_repair_durable(db_path: Path, *, timeout: float = 5.0) -> sqlite3.C return conn -def _reapply_durability_barriers(conn: sqlite3.Connection) -> bool: - """Best-effort (re)application of the macOS write barriers. Never raises. +@contextmanager +def _repair_conn(db_path: Path, *, timeout: float = 5.0): + """A :func:`_connect_repair_durable` connection, closed on exit.""" + conn = _connect_repair_durable(db_path, timeout=timeout) + try: + yield conn + finally: + conn.close() - True when the pragmas were accepted. Call before ``VACUUM``/``REINDEX`` - once the schema parses: a connection opened on a malformed schema could - not take them at open time. - """ + +def _reapply_durability_barriers(conn: sqlite3.Connection) -> bool: + """Best-effort (re)application of the macOS write barriers; True if accepted. + Call before ``VACUUM``/``REINDEX`` once the schema parses: a connection opened + on a malformed schema could not take them at open time. Never raises.""" from hermes_state import _apply_macos_checkpoint_barrier, _enforce_macos_synchronous_full try: _apply_macos_checkpoint_barrier(conn) @@ -794,85 +691,79 @@ def _reapply_durability_barriers(conn: sqlite3.Connection) -> bool: def apply_durability_barriers(conn: sqlite3.Connection) -> bool: - """Apply state-store durability barriers without changing journal mode. - - Public entry point for secondary users of ``state.db`` that must inherit - its owner's journal mode. Also applies the configured - ``database.synchronous`` level, a per-connection pragma that otherwise - only rides on the journal-mode setup path guests must not run. - """ + """Durability barriers for guest users of ``state.db`` that must inherit its + owner's journal mode. Also applies the configured ``database.synchronous`` + level, a per-connection pragma that otherwise only rides on the journal-mode + setup path guests must not run.""" from hermes_state import _apply_synchronous_pragma ok = _reapply_durability_barriers(conn) - try: - # Local import: avoids a circular import with hermes_cli.config. - from hermes_cli.config import cfg_get, load_config_readonly + with contextlib.suppress(Exception): + from hermes_cli.config import cfg_get, load_config_readonly # local: avoids an import cycle - cfg = load_config_readonly() - raw_synchronous = cfg_get(cfg, "database", "synchronous", default=None) + raw_synchronous = cfg_get(load_config_readonly(), "database", "synchronous", default=None) if raw_synchronous is not None: _apply_synchronous_pragma(conn, raw_synchronous, db_label="state.db (guest)") - except Exception: - pass return ok def _close_unpinned(conn: sqlite3.Connection) -> None: """Leave EXCLUSIVE locking mode (so the file is never left pinned) and close.""" - try: + with contextlib.suppress(Exception): conn.execute("PRAGMA locking_mode=NORMAL") - except Exception: - pass conn.close() +def _open_exclusive(db_path: Path, begin: str) -> sqlite3.Connection: + """Zero-timeout connection holding ``locking_mode=EXCLUSIVE`` after a rolled-back + *begin*; closed (unpinned) and re-raised when exclusion cannot be taken.""" + conn = _connect_repair_durable(db_path, timeout=0.0) + try: + conn.execute("PRAGMA locking_mode=EXCLUSIVE") + conn.execute(begin) + conn.execute("ROLLBACK") + except BaseException: + _close_unpinned(conn) + raise + return conn + + @contextmanager def _exclusive_repair_db_guard(db_path: Path): - """Yield one live connection that excludes writers for repair surgery. + """Yield ``(conn, None)`` — one live connection that excludes writers for + repair surgery — or ``(None, exc)`` when exclusion could not be taken. ``locking_mode=EXCLUSIVE`` retains file-level exclusion after the short - ``BEGIN EXCLUSIVE`` is rolled back. The rollback is essential: + ``BEGIN EXCLUSIVE`` is rolled back; the rollback is essential because ``Connection.backup`` uses this connection as *source* and later as the - promotion *destination*, both of which require it transaction-free. It - stays open across the whole snapshot -> strategies -> promotion window, so - no other writer can commit a change promotion would overwrite. Existing - readers make acquisition fail rather than being disturbed: repair fails - closed unless this process owns the whole window. Timeout 0: the - cross-process repair lock already serializes repairers, and a partial - repair is less safe than an explicit "stop the gateway and retry". + promotion *destination*, both transaction-free. It stays open across the + snapshot -> strategies -> promotion window so no writer can commit a change + promotion would overwrite. Existing readers make acquisition fail rather + than being disturbed (fail closed). Timeout 0: the cross-process lock already + serializes repairers, and a partial repair is less safe than "stop the + gateway and retry". """ - guard: Optional[sqlite3.Connection] = None try: - guard = _connect_repair_durable(db_path, timeout=0.0) - guard.execute("PRAGMA locking_mode=EXCLUSIVE") - guard.execute("BEGIN EXCLUSIVE") - guard.execute("ROLLBACK") + guard = _open_exclusive(db_path, "BEGIN EXCLUSIVE") except (sqlite3.Error, OSError) as exc: - if guard is not None: - _close_unpinned(guard) yield None, exc return try: yield guard, None finally: - # Releasing the exclusive locks before close also keeps a close-time - # checkpoint from being mistaken for a repair write by callers that - # immediately reopen state.db. + # Releasing the exclusive locks before close keeps a close-time checkpoint + # from being mistaken for a repair write by callers that reopen immediately. _close_unpinned(guard) def _copy_database_snapshot( - source_path: Path, destination_path: Path, *, - source_connection: Optional[sqlite3.Connection] = None, + source_path: Path, destination_path: Path, *, source_connection: Optional[sqlite3.Connection] = None, destination_connection: Optional[sqlite3.Connection] = None, ) -> None: - """Copy one complete SQLite snapshot without replacing either file inode. - - The online backup API folds committed WAL frames into the source snapshot - and writes the destination in one transaction (rolled back if interrupted), - so ``state.db`` is never swapped out from under handles that refer to it. - """ - # Compute the deadline before opening an owned source connection: a - # sidecar vanishing mid-stat must not leak a just-opened descriptor. + """Copy one complete SQLite snapshot without replacing either file inode: + the online backup API folds committed WAL frames into the source snapshot and + writes the destination in one transaction (rolled back if interrupted), so + ``state.db`` is never swapped out from under handles that refer to it.""" + # Deadline first: a sidecar vanishing mid-stat must not leak a just-opened descriptor. deadline_seconds = _repair_snapshot_timeout_seconds(source_path) deadline = time.monotonic() + deadline_seconds source = source_connection or _connect_repair_durable(source_path) @@ -886,8 +777,8 @@ def _copy_database_snapshot( if destination is None: destination = _connect_repair_durable(destination_path) elif destination.in_transaction: - # sqlite3_backup needs a transaction-free destination; the exclusive - # guard retains exclusion via locking_mode, not a transaction. + # sqlite3_backup needs a transaction-free destination (the guard holds + # exclusion via locking_mode, not a transaction). raise sqlite3.ProgrammingError("SQLite repair backup destination has an active transaction") source.backup(destination, pages=256, progress=_check_deadline, sleep=_REPAIR_LOCK_POLL_SECONDS) finally: @@ -901,17 +792,16 @@ def _db_opens_cleanly(db_path: Path) -> Optional[str]: """Probe a DB on a fresh connection. Returns None if healthy, else a reason. Runs the first statement that trips the malformed-schema parse (``PRAGMA - journal_mode``), ``integrity_check``, a ``sessions`` read, FTS5 MATCH - probes, and a rolled-back ``messages`` write — so FTS5 index corruption, - which leaves reads and ``integrity_check`` passing while every ``INSERT - INTO messages`` fails through the FTS triggers, is reported as unhealthy. + journal_mode``), ``integrity_check``, a ``sessions`` read, FTS5 MATCH probes + and a rolled-back ``messages`` write — so FTS5 index corruption (reads and + ``integrity_check`` pass, every ``INSERT INTO messages`` fails through the + FTS triggers) is reported as unhealthy. """ from hermes_state import SessionDB, load_fts5_cjk_extension conn = _connect_repair_durable(db_path) try: - # Best-effort tokenizer load: messages_fts_cjk needs cjk_unicode61 - # before any statement (incl. the trigger-driven write probe) can touch - # it; tokenizer absence must never classify as corruption. + # Best-effort tokenizer load: messages_fts_cjk needs cjk_unicode61 before any + # statement can touch it; tokenizer absence must never classify as corruption. load_fts5_cjk_extension(conn) conn.execute("PRAGMA journal_mode").fetchone() rows = conn.execute("PRAGMA integrity_check").fetchall() @@ -920,29 +810,25 @@ def _db_opens_cleanly(db_path: Path) -> Optional[str]: return "; ".join(problems[:3]) conn.execute("SELECT COUNT(*) FROM sessions").fetchone() - # FTS5 read probe: partial shadow-table corruption makes MATCH / - # snippet / rank raise while check-only reports healthy. MATCH '""' - # (empty phrase) parses, scans zero rows and exercises the shadow-table - # read path; FTS5 rejects MATCH '' outright. + # FTS5 read probe: partial shadow-table corruption makes MATCH/snippet/rank + # raise while integrity_check reports healthy. MATCH '""' (empty phrase) + # parses, scans zero rows and exercises the shadow tables; FTS5 rejects MATCH ''. for fts_table in _FTS_TABLES: try: conn.execute(f"SELECT 1 FROM {fts_table} WHERE {fts_table} MATCH '\"\"' LIMIT 1").fetchone() - except sqlite3.OperationalError as exc: - # Builds without fts5 / trigram raise "no such module|tokenizer"; - # treating that as corruption would send the DB into repair, - # whose final fallback deletes the messages_fts% schema. - if SessionDB._is_fts5_unavailable_error(exc): - continue - msg = str(exc).lower() - if "no such table" in msg or "no such column" in msg: - continue # FTS5 not built yet (brand new file mid-init) - return f"fts5 read probe failed on {fts_table}: {exc}" except sqlite3.DatabaseError as exc: + # Builds without fts5/trigram raise "no such module|tokenizer"; calling + # that corruption would send the DB into repair, whose final fallback + # deletes messages_fts%. "no such table/column" = FTS5 not built yet. + msg = str(exc).lower() + if isinstance(exc, sqlite3.OperationalError) and ( + SessionDB._is_fts5_unavailable_error(exc) or "no such table" in msg or "no such column" in msg + ): + continue return f"fts5 read probe failed on {fts_table}: {exc}" # FTS write probe: drive a row through the messages_fts* triggers in a - # transaction that is always rolled back. Missing messages/sessions - # tables (brand new file mid-init) mean "not yet populated", not corruption. + # transaction that is always rolled back. probe_session_id = f"_hermes_fts_health_probe_{time.time_ns()}" try: conn.execute("BEGIN IMMEDIATE") @@ -951,23 +837,18 @@ def _db_opens_cleanly(db_path: Path) -> Optional[str]: (probe_session_id, "_health_probe", time.time()), ) conn.execute( - "INSERT INTO messages (session_id, role, content, timestamp) " - "VALUES (?, ?, ?, ?)", + "INSERT INTO messages (session_id, role, content, timestamp) VALUES (?, ?, ?, ?)", (probe_session_id, "user", "_fts_health_probe", time.time()), ) conn.execute("ROLLBACK") except sqlite3.OperationalError as exc: - try: + with contextlib.suppress(sqlite3.Error): conn.execute("ROLLBACK") - except sqlite3.Error: - pass msg = str(exc).lower() - if "no such table" in msg or "no such column" in msg: - return None - if "no such tokenizer: cjk_unicode61" in msg: - # This process couldn't load the cjk extension while the DB - # carries the cjk index — capability gap, not corruption. A - # tokenizer-less SessionDB self-heals by dropping the triggers. + # Missing messages/sessions tables = brand new file mid-init, not corruption. + # "no such tokenizer": this process lacks the cjk extension the DB's index + # needs — capability gap; a tokenizer-less SessionDB drops the triggers itself. + if "no such table" in msg or "no such column" in msg or "no such tokenizer: cjk_unicode61" in msg: return None return str(exc) return None @@ -981,39 +862,32 @@ def _live_writer_holds_db(db_path: Path) -> bool: """True when a connection outside this call still holds ``db_path`` open. Asks SQLite for what a repair needs and a live holder cannot grant: - ``PRAGMA locking_mode=EXCLUSIVE`` then ``BEGIN IMMEDIATE``. In WAL mode - that needs exclusive locks on the WAL index, so any other open connection - fails it with SQLITE_BUSY; neither statement parses the schema, so it - works on malformed DBs. Fails **open** (False) on anything but a positive - busy/locked signal — refusing to repair a DB nobody holds would strand the - self-heal path. - - Scope: WAL mode only. In ``journal_mode=DELETE`` a held reader takes only - SHARED and this returns False; repair is then serialised only by the - cross-process repairer lock. + ``locking_mode=EXCLUSIVE`` then ``BEGIN IMMEDIATE`` — in WAL mode that needs + exclusive WAL-index locks, so any other open connection fails it with + SQLITE_BUSY; neither statement parses the schema, so it works on malformed + DBs. Fails **open** (False) on anything but a positive busy/locked signal: + refusing to repair a DB nobody holds would strand the self-heal path. In + ``journal_mode=DELETE`` a held reader takes only SHARED and this returns + False; repair is then serialised only by the cross-process repairer lock. """ - probe = None try: - probe = _connect_repair_durable(db_path, timeout=0.0) - probe.execute("PRAGMA locking_mode=EXCLUSIVE") - probe.execute("BEGIN IMMEDIATE") - probe.execute("ROLLBACK") + probe = _open_exclusive(db_path, "BEGIN IMMEDIATE") + with contextlib.suppress(Exception): + _close_unpinned(probe) return False except sqlite3.OperationalError as exc: lowered = str(exc).lower() return "locked" in lowered or "busy" in lowered except Exception: # malformed/unreadable: no evidence of a live holder either way return False - finally: - if probe is not None: - try: - _close_unpinned(probe) - except Exception: - pass -def _repair_skip(report: Dict[str, Any], verb: str, error: str) -> Dict[str, Any]: - """Record *error* on *report* and log it as ``state.db repair ``.""" +def _repair_skip(report: Dict[str, Any], verb: str, error: str, exc: Optional[BaseException] = None) -> Dict[str, Any]: + """Record *error* on *report* and log it as ``state.db repair ``. An + *exc* proving deterministic corruption consumes the persistent repair budget + (private ``_repair_attempted`` marker, popped by the caller).""" + if exc is not None and _repair_failure_consumes_attempt(exc): + report["_repair_attempted"] = True report["error"] = error logger.error(f"state.db repair {verb}: %s", report["error"]) return report @@ -1026,24 +900,19 @@ def repair_state_db_schema(db_path: Path, *, backup: bool = True) -> Dict[str, A Two corruption classes: malformed schema / "duplicate object definition" (even ``PRAGMA`` fails), and FTS write-corruption (reads and ``integrity_check`` pass, writes fail through ``messages_fts*`` triggers). - Strategies run least-destructive first (see ``_REPAIR_STRATEGIES``) on a - complete SQLite snapshot; a successful result is copied back - transactionally, so canonical rows are never modified by a failed attempt. - A raw backup is taken first unless ``backup=False``. Surgery is serialised - across processes (:func:`_cross_process_repair_lock`): the gateway, Desktop - backend and CLI all open the same file, and concurrent ``writable_schema`` - surgery is itself a corruption source. - - Returns ``{repaired: bool, strategy: str|None, backup_path: str|None, - error: str|None}``. + ``_REPAIR_STRATEGIES`` run least-destructive first on a complete snapshot; + a success is copied back transactionally, so canonical rows are never + modified by a failed attempt. A raw backup is taken first unless + ``backup=False``. Serialised across processes (gateway, Desktop backend and + CLI open the same file; concurrent ``writable_schema`` surgery is itself a + corruption source). Returns ``{repaired, strategy, backup_path, error}``. """ from hermes_state import _cross_process_repair_lock, _db_opens_cleanly, _live_writer_holds_db, _persistent_repair_attempts_exhausted, _probe_journal_mode_for_repair, _record_repair_outcome, _repair_state_db_schema_locked report: Dict[str, Any] = {"repaired": False, "strategy": None, "backup_path": None, "error": None} - # Startup-watchdog progress lease: repair is I/O-bound (near-zero CPU), - # which the watchdog's CPU fallback would misread as a parked deadlock. A - # single lease (clamped to _MAX_LEASE_S=900) is deliberate: up to that much - # zombie time on a wedged repair beats per-chunk renewal complexity. + # Startup-watchdog lease: repair is I/O-bound (near-zero CPU), which the + # watchdog's CPU fallback would misread as a parked deadlock. One lease + # (clamped to _MAX_LEASE_S=900) beats per-chunk renewal complexity. report_startup_progress(900.0, phase="state_db_repair") db_path = Path(db_path) @@ -1051,80 +920,67 @@ def repair_state_db_schema(db_path: Path, *, backup: bool = True) -> Dict[str, A report["error"] = f"{db_path} does not exist" return report - # Cross-restart attempt cap: the in-memory claim bounds one process, but a - # class the strategies cannot heal (b-tree page damage) used to re-run the - # whole surgery, with a fresh forensic backup, on EVERY restart. + # Cross-restart cap: the in-memory claim bounds one process, but unhealable + # b-tree damage used to re-run surgery + a fresh backup on EVERY restart. if _persistent_repair_attempts_exhausted(db_path): return _repair_skip(report, "skipped", _persistent_repair_exhausted_error(db_path)) - result = report with _cross_process_repair_lock(db_path) as holding_lock: if not holding_lock: - # Another process is inside its critical section, or the lock file - # could not be opened. It may have healed the file already (long - # VACUUM after a successful strategy), so re-probe before failing. + # Another process holds the lock (or the lock file was unopenable); + # it may have healed the file already, so re-probe before failing. if _db_opens_cleanly(db_path) is None: - report["repaired"] = True - report["strategy"] = "repaired_by_other_process" + report["repaired"], report["strategy"] = True, "repaired_by_other_process" else: report["error"] = ( - "could not obtain the state.db repair lock (held by " - "another process, or the lock file was unopenable); " - "skipped schema surgery to avoid racing a concurrent " - "repairer" + "could not obtain the state.db repair lock (held by another process, or the lock file was " + "unopenable); skipped schema surgery to avoid racing a concurrent repairer" ) + return report + + result = report + # Recheck exhaustion after acquisition: a queued repairer can have + # recorded the final failure while this process waited. + if _persistent_repair_attempts_exhausted(db_path): + _repair_skip(report, "skipped", _persistent_repair_exhausted_error(db_path)) + # WAL-holder preflight: fail closed for active readers before a backup is + # taken. Not the race defence — the exclusive guard in the locked routine + # excludes writers through promotion and sees DELETE-mode readers too. + elif _live_writer_holds_db(db_path): + _repair_skip( + report, "skipped", + "a live writer still holds state.db; skipped schema surgery to avoid tearing b-tree pages under a " + "concurrent writer. Stop the gateway (hermes gateway stop) and retry.", + ) else: - # Recheck exhaustion after acquisition: a queued repairer can have - # recorded the final failure while this process waited. - if _persistent_repair_attempts_exhausted(db_path): - _repair_skip(report, "skipped", _persistent_repair_exhausted_error(db_path)) - # WAL-holder preflight: fail-closed for active readers before a - # forensic backup is taken. Not the race defence — the exclusive - # guard in the locked routine excludes writers through promotion and - # rejects DELETE-mode readers this probe cannot see. - elif _live_writer_holds_db(db_path): - _repair_skip( - report, "skipped", - "a live writer still holds state.db; skipped schema surgery " - "to avoid tearing b-tree pages under a concurrent writer. " - "Stop the gateway (hermes gateway stop) and retry.", - ) - else: - # Probe the journal mode BEFORE surgery: a rebuilt file comes - # back in the default (delete) mode and nothing else records - # the flip. The probe may fail on a damaged file; then - # database.journal_mode 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) - 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 mutates the - # snapshot; they are retriable, not proof a strategy was exhausted. - # Keep that private marker out of the public report. The ledger - # update stays under the same cross-process lock as surgery so two - # repairers cannot lose each other's updates; a queued loser must - # not record at all. - attempted = bool(result.pop("_repair_attempted", False)) - if attempted or result.get("repaired"): - _record_repair_outcome(db_path, repaired=bool(result.get("repaired"))) + # Probe journal mode BEFORE surgery: a rebuilt file comes back in the + # default (delete) mode and nothing else records the flip. Unprobeable + # (damaged file) -> database.journal_mode 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) + if result.get("repaired"): + result["journal_mode_before"] = before_mode + _restore_journal_mode_after_repair(db_path, before_mode) + # Environmental aborts (before a strategy mutates the snapshot) are + # retriable, not proof of exhaustion; the private marker stays out of the + # public report. The ledger update stays under the cross-process lock so + # two repairers cannot lose each other's updates; a queued loser must not + # record at all. + attempted = bool(result.pop("_repair_attempted", False)) + if attempted or result.get("repaired"): + _record_repair_outcome(db_path, repaired=bool(result.get("repaired"))) return result def _probe_journal_mode_for_repair(db_path: Path) -> Optional[str]: - """Best-effort journal-mode probe for a (possibly malformed) DB file. - - Returns ``wal``/``delete``, or ``None`` when the file cannot be opened or - probed (malformed header, concurrent opener's locks — both expected on - the repair path); callers then fall back to ``database.journal_mode``. - """ + """Best-effort journal-mode probe: ``wal``/``delete``, or ``None`` when the + file cannot be opened or probed (malformed header, concurrent opener's locks + — both expected on the repair path); callers then fall back to + ``database.journal_mode``.""" from hermes_state import _on_disk_journal_mode try: - conn = _connect_repair_durable(db_path) - try: + with _repair_conn(db_path) as conn: return _on_disk_journal_mode(conn) - finally: - conn.close() except (sqlite3.Error, OSError): return None @@ -1132,99 +988,80 @@ def _probe_journal_mode_for_repair(db_path: Path) -> Optional[str]: def _restore_journal_mode_after_repair(db_path: Path, before_mode: Optional[str]) -> None: """Re-apply the journal mode after schema surgery. - A rebuilt SQLite file comes back in the default (delete) mode; without - this, a corruption event silently moves a WAL store out of WAL (the - open-time WAL-reset gate never sees a flip made inside repair). Routed - through :func:`apply_wal_with_fallback`, not a direct pragma, so it - inherits the vulnerable-SQLite WAL-reset gate (on a vulnerable runtime the - gate deliberately keeps DELETE and the resulting journal_mode-changed - WARNING is expected there), the macOS-NFS silent-refusal handling, - and the WAL companions. ``before_mode`` (None if unprobeable) is only for - the log comparison; the target comes from ``database.journal_mode``. - Best-effort: the repair already succeeded, so failures log at WARNING. + A rebuilt file comes back in the default (delete) mode; without this a + corruption event silently moves a WAL store out of WAL (the open-time + WAL-reset gate never sees a flip made inside repair). Routed through + :func:`apply_wal_with_fallback`, not a direct pragma, so it inherits the + WAL-reset gate (a vulnerable runtime deliberately keeps DELETE; the + journal_mode-changed WARNING is expected there), the macOS-NFS silent-refusal + handling and the WAL companions. ``before_mode`` is only for the log + comparison; the target is ``database.journal_mode``. Best-effort: the repair + already succeeded, so failures log at WARNING. """ from hermes_state import apply_wal_with_fallback try: - conn = _connect_repair_durable(db_path) - try: + with _repair_conn(db_path) as conn: after = apply_wal_with_fallback(conn, db_label=db_path.name) - finally: - conn.close() if before_mode and after != before_mode: logger.warning( - "state.db repair changed journal_mode %r -> %r " - "(pre-surgery probe %r; restore resolved through " - "apply_wal_with_fallback per database.journal_mode and the " - "WAL-reset gate)", + "state.db repair changed journal_mode %r -> %r (pre-surgery probe %r; restore resolved through " + "apply_wal_with_fallback per database.journal_mode and the WAL-reset gate)", before_mode, after, before_mode, ) except (sqlite3.Error, OSError) as exc: logger.warning( "state.db repair at %s: post-surgery journal-mode restore " - "failed (%s); verify with PRAGMA journal_mode on the next open", - db_path, exc, + "failed (%s); verify with PRAGMA journal_mode on the next open", db_path, exc, ) def _repair_state_db_schema_locked(db_path: Path, *, backup: bool, report: Dict[str, Any]) -> Dict[str, Any]: - """Repair strategies for :func:`repair_state_db_schema`. + """Repair strategies for :func:`repair_state_db_schema`; caller holds the + cross-process repair lock. - Caller must hold the cross-process repair lock for *db_path*. Strategies - run on a SCRATCH COPY; the result is copied back through SQLite's - transactional backup API only once proven to open cleanly, so a failed - repair cannot modify or lose committed canonical data. (A WAL checkpoint of - already-committed frames on guard release is not a repair mutation.) - - WHY not in place: Strategy 2 ends in ``VACUUM``, which rebuilds the file - from the schema SQLite can still parse. When the damage IS in the schema - b-tree — the ``malformed database schema ()`` class handled here — every - table hanging off the unreadable part is silently dropped, the probe then - correctly reports STILL malformed, and repair returned ``repaired=False`` - having destroyed what it was asked to save. Not mutating the original is - the property that holds without a human in the loop. + Strategies run on a SCRATCH COPY, copied back through SQLite's transactional + backup API only once proven to open cleanly, so a failed repair cannot + modify or lose committed data (a WAL checkpoint of committed frames on guard + release is not a repair mutation). WHY not in place: the final strategy + ends in ``VACUUM``, which rebuilds the file from the schema SQLite can still + parse — when the damage IS in the schema b-tree (the ``malformed database + schema ()`` class) every table hanging off the unreadable part is silently + dropped, the probe still reports malformed, and repair returned + ``repaired=False`` having destroyed what it was asked to save. """ from hermes_state import _backup_db_file, _copy_database_snapshot, _db_opens_cleanly, _repair_scratch_space_error, _run_repair_strategies, _unlink_db_triple scratch = db_path.with_name(f"{db_path.name}.repair-scratch") cleanup_error = _unlink_db_triple(scratch) if cleanup_error is not None: return _repair_skip( - report, "aborted", - f"could not remove a stale repair snapshot before probing state.db: {cleanup_error}", + report, "aborted", f"could not remove a stale repair snapshot before probing state.db: {cleanup_error}", ) - # Re-probe under the lock: a process we queued behind may have just - # repaired the file; redoing surgery would undo its work (the - # repair/re-corrupt cascade this lock exists to break). + # Re-probe under the lock: a process we queued behind may have just repaired + # the file; redoing surgery would undo it (the repair/re-corrupt cascade). if _db_opens_cleanly(db_path) is None: - report["repaired"] = True - report["strategy"] = "already_healthy" + report["repaired"], report["strategy"] = True, "already_healthy" return report if backup: bpath, backup_error = _backup_db_file(db_path) report["backup_path"] = str(bpath) if bpath else None if bpath is None: - # HARD STOP: the forensic image is still required when corruption - # defeats every strategy, even though strategies run on a snapshot. + # HARD STOP: the forensic image is the recovery path when every strategy fails. return _repair_skip( - report, "aborted", - "pre-repair backup refused; aborting schema repair to avoid " + report, "aborted", "pre-repair backup refused; aborting schema repair to avoid " f"mutating the only copy of the damaged DB: {backup_error}", ) - # The forensic copy deliberately precedes this guard: its raw-copy safety - # checks inspect real live holders and would be poisoned by our exclusive - # connection. Everything affecting the repair image or live promotion - # happens only after writer exclusion is held. + # The forensic copy precedes this guard on purpose: its live-holder checks + # would be poisoned by our own exclusive connection. Everything touching the + # repair image or live promotion happens only under writer exclusion. with _exclusive_repair_db_guard(db_path) as (live_guard, guard_error): if live_guard is None: - if guard_error is not None and _repair_failure_consumes_attempt(guard_error): - report["_repair_attempted"] = True return _repair_skip( - report, "skipped", - "could not acquire exclusive state.db repair ownership; " + report, "skipped", "could not acquire exclusive state.db repair ownership; " "skipped schema surgery to avoid overwriting a concurrent " - f"writer. Stop the gateway and retry: {guard_error}", + f"writer. Stop the gateway and retry: {guard_error}", exc=guard_error, ) space_error = _repair_scratch_space_error(db_path) @@ -1232,63 +1069,56 @@ def _repair_state_db_schema_locked(db_path: Path, *, backup: bool, report: Dict[ return _repair_skip(report, "aborted", space_error) try: - # Reuse live_guard rather than a second source connection: the - # guard owns the exclusion, and a second connection could be - # blocked by our own EXCLUSIVE lock on some SQLite builds. + # Source = live_guard: it owns the exclusion, and a second connection + # could be blocked by our own EXCLUSIVE lock on some SQLite builds. _copy_database_snapshot(db_path, scratch, source_connection=live_guard) except (OSError, sqlite3.Error, TimeoutError) as exc: - if _repair_failure_consumes_attempt(exc): - report["_repair_attempted"] = True - _repair_skip( - report, "aborted", - f"could not stage a complete SQLite repair snapshot of {db_path}: {exc}", - ) _unlink_db_triple(scratch) - return report + return _repair_skip( + report, "aborted", f"could not stage a complete SQLite repair snapshot of {db_path}: {exc}", exc=exc, + ) try: - # Private marker consumed by the outer wrapper: a strategy failure - # consumes the persistent budget, but a later promotion failure is - # classified separately (disk/I/O/permission/lock = environmental). + # Private marker for the outer wrapper: a strategy failure consumes the + # persistent budget; a promotion failure is classified separately. report["_repair_attempted"] = True _run_repair_strategies(scratch, report) if report.get("repaired"): - try: - # Do not os.replace the live DB: Windows rejects replacement - # under open handles and POSIX would leave those handles on - # the old inode. The guard that staged the live image - # receives the promotion, keeping writer exclusion throughout. - _copy_database_snapshot(scratch, db_path, destination_connection=live_guard) - except (OSError, sqlite3.Error, TimeoutError) as exc: - report["repaired"] = False - report["strategy"] = None - report["_repair_attempted"] = _repair_failure_consumes_attempt(exc) - report["error"] = f"repaired snapshot could not be promoted transactionally: {exc}" - logger.error("state.db repair promotion failed: %s", exc) - else: - logger.warning( - "state.db repaired via '%s' and promoted transactionally: %s", - report.get("strategy"), db_path, - ) + _promote_repaired_snapshot(scratch, db_path, live_guard, report) if not report.get("repaired"): - # Logged HERE, not in the strategies: they see the scratch copy, - # and the one message a human acts on must not name a path - # that no longer exists by the time they read it. + # Logged HERE, not in the strategies: they see the scratch copy, and + # the message a human acts on must name a path that still exists. logger.error( - "state.db schema repair could not recover %s automatically " - "(no committed canonical data was modified or lost; backup: %s); " - "manual restore from backup may be required.", + "state.db schema repair could not recover %s automatically (no committed canonical data was " + "modified or lost; backup: %s); manual restore from backup may be required.", db_path, report["backup_path"], ) return report finally: - # Never leave a half-repaired file beside the DB for a later probe - # or human to mistake for the real thing. + # Never leave a half-repaired file beside the DB to be mistaken for the real thing. cleanup_error = _unlink_db_triple(scratch) if cleanup_error is not None: logger.warning("Could not remove state.db repair snapshot after repair: %s", cleanup_error) +def _promote_repaired_snapshot(scratch: Path, db_path: Path, live_guard: sqlite3.Connection, report: Dict[str, Any]) -> None: + """Copy the repaired *scratch* back into the live DB through *live_guard*. + + Never ``os.replace`` the live DB: Windows rejects replacement under open + handles and POSIX would leave those handles on the old inode. The guard + keeps writer exclusion throughout. On failure the report reverts to unrepaired. + """ + from hermes_state import _copy_database_snapshot + try: + _copy_database_snapshot(scratch, db_path, destination_connection=live_guard) + except (OSError, sqlite3.Error, TimeoutError) as exc: + report.update(repaired=False, strategy=None, _repair_attempted=_repair_failure_consumes_attempt(exc)) + report["error"] = f"repaired snapshot could not be promoted transactionally: {exc}" + logger.error("state.db repair promotion failed: %s", exc) + else: + logger.warning("state.db repaired via '%s' and promoted transactionally: %s", report.get("strategy"), db_path) + + def _unlink_db_triple(path: Path) -> Optional[str]: """Remove *path* and every SQLite sidecar; return any cleanup failure.""" from hermes_state import _IS_WINDOWS @@ -1296,33 +1126,35 @@ def _unlink_db_triple(path: Path) -> Optional[str]: for victim in (path, *_sidecars(path)): for attempt in range(10): try: - victim.unlink() - break - except FileNotFoundError: - break - except PermissionError as exc: - # Windows may retain a just-closed SQLite handle for a few - # scheduler ticks; bounded retry. A later open still fails - # safely if the handle truly remains live. - if _IS_WINDOWS and attempt < 9: + victim.unlink(missing_ok=True) + except OSError as exc: + # Windows may retain a just-closed SQLite handle for a few scheduler + # ticks; bounded retry (a later open still fails safely if it stays live). + if isinstance(exc, PermissionError) and _IS_WINDOWS and attempt < 9: time.sleep(0.05) continue failures.append(f"{victim}: {exc}") - break - except OSError as exc: - failures.append(f"{victim}: {exc}") - break + break return "; ".join(failures) or None # ── Repair strategies, least destructive first (each mutates its connection's DB) ── +def _edit_sqlite_master(conn: sqlite3.Connection, edit) -> None: + """Run *edit* under ``writable_schema=ON``; bump the schema cookie when it + reports a change so live peers discard their cached schema.""" + conn.execute("PRAGMA writable_schema=ON") + if edit(): + _bump_schema_cookie(conn) + conn.execute("PRAGMA writable_schema=OFF") + conn.commit() + + def _strategy_rebuild_fts(conn: sqlite3.Connection) -> None: """FTS5 'rebuild' rewrites each index from the content table: the least- destructive fix for an index that rejects writes while reads work.""" from hermes_state import load_fts5_cjk_extension - # The cjk index can only be rebuilt with its tokenizer loaded - # (best-effort; a tokenizer-less host skips it below). + # The cjk index can only be rebuilt with its tokenizer loaded (best-effort). load_fts5_cjk_extension(conn) for table_name in _FTS_TABLES: try: @@ -1334,8 +1166,7 @@ def _strategy_rebuild_fts(conn: sqlite3.Connection) -> None: def _strategy_reindex(conn: sqlite3.Connection) -> None: """integrity_check reports "wrong # of entries in index" when a B-tree index drifts from its base table; REINDEX rewrites it from canonical rows.""" - # REINDEX rewrites every index b-tree; take the barriers now that the - # schema parses, in case the open-time attempt was refused. + # REINDEX rewrites every index b-tree; take the barriers now that the schema parses. _reapply_durability_barriers(conn) conn.execute("REINDEX") conn.commit() @@ -1343,44 +1174,31 @@ def _strategy_reindex(conn: sqlite3.Connection) -> None: def _strategy_dedup_schema(conn: sqlite3.Connection) -> None: """De-duplicate sqlite_master (lowest rowid per type/name), keeping FTS.""" - conn.execute("PRAGMA writable_schema=ON") - dupes = conn.execute( - "SELECT type, name, COUNT(*) AS c, MIN(rowid) AS keep " - "FROM sqlite_master GROUP BY type, name HAVING c > 1" - ).fetchall() - for type_, name, _count, keep in dupes: - conn.execute( - "DELETE FROM sqlite_master " - "WHERE type IS ? AND name IS ? AND rowid <> ?", - (type_, name, keep), - ) - if dupes: - _bump_schema_cookie(conn) - conn.execute("PRAGMA writable_schema=OFF") - conn.commit() + def _dedup() -> bool: + dupes = conn.execute( + "SELECT type, name, COUNT(*) AS c, MIN(rowid) AS keep FROM sqlite_master GROUP BY type, name HAVING c > 1" + ).fetchall() + for type_, name, _count, keep in dupes: + conn.execute( + "DELETE FROM sqlite_master WHERE type IS ? AND name IS ? AND rowid <> ?", (type_, name, keep), + ) + return bool(dupes) + + _edit_sqlite_master(conn, _dedup) def _strategy_drop_fts_vacuum(conn: sqlite3.Connection) -> None: - """Drop all FTS schema and VACUUM; indexes rebuild on the next open. - - The destructive one, and why the strategies run on a scratch copy: on a - damaged schema b-tree VACUUM silently drops every table hanging off the - unreadable part (see _repair_state_db_schema_locked). - """ - conn.execute("PRAGMA writable_schema=ON") - conn.execute("DELETE FROM sqlite_master WHERE name LIKE 'messages_fts%'") - _bump_schema_cookie(conn) - conn.execute("PRAGMA writable_schema=OFF") - conn.commit() - # The schema parses now, so the barriers can finally stick — and VACUUM - # rewrites the entire file, the worst operation to lose halfway. + """Drop all FTS schema and VACUUM; indexes rebuild on the next open. The + destructive one, and why strategies run on a scratch copy: on a damaged + schema b-tree VACUUM silently drops every table hanging off the unreadable part.""" + _edit_sqlite_master(conn, lambda: conn.execute("DELETE FROM sqlite_master WHERE name LIKE 'messages_fts%'") or True) + # The schema parses now, so the barriers can stick — VACUUM rewrites the whole file. _reapply_durability_barriers(conn) conn.execute("VACUUM") -# (strategy name, body, success log, failure log) in escalation order. The -# first three log a warning on failure and fall through; the last records the -# failure in report["error"] for the caller. +# (name, body, success log, failure log) in escalation order. failure log None = +# final strategy: its failure lands in report["error"] instead of logged-and-skipped. _REPAIR_STRATEGIES = ( ("rebuild_fts", _strategy_rebuild_fts, "state.db FTS indexes rebuilt in place (schema preserved): %s", @@ -1391,50 +1209,32 @@ _REPAIR_STRATEGIES = ( ("dedup_schema", _strategy_dedup_schema, "state.db schema repaired by de-duplicating sqlite_master (FTS index preserved): %s", "state.db dedup repair pass failed: %s"), -) -_FINAL_STRATEGY = ( - "drop_fts_rebuild", _strategy_drop_fts_vacuum, - "state.db schema repaired by dropping FTS schema; indexes will rebuild from messages on next open: %s", + ("drop_fts_rebuild", _strategy_drop_fts_vacuum, + "state.db schema repaired by dropping FTS schema; indexes will rebuild from messages on next open: %s", + None), ) def _run_repair_strategies(db_path: Path, report: Dict[str, Any]) -> Dict[str, Any]: - """Escalating repair attempts, applied to *db_path* IN PLACE. - - Every strategy mutates its argument, so this is only ever called by - :func:`_repair_state_db_schema_locked` on a scratch copy nothing else - holds open — never on the user's database. The "could not recover" log - lives in the caller: it must name the user's database, not the scratch copy. - """ + """Escalating repair attempts, applied to *db_path* IN PLACE — only ever a + scratch copy nothing else holds open, never the user's database. The "could + not recover" log lives in the caller so it names the user's database.""" from hermes_state import _db_opens_cleanly - def _apply(body) -> Optional[str]: - conn = _connect_repair_durable(db_path) - try: - body(conn) - finally: - conn.close() - return _db_opens_cleanly(db_path) - - def _succeed(name: str, message: str) -> Dict[str, Any]: - report["repaired"] = True - report["strategy"] = name - logger.warning(message, db_path) - return report - for name, body, success_msg, failure_msg in _REPAIR_STRATEGIES: try: - if _apply(body) is None: - return _succeed(name, success_msg) + with _repair_conn(db_path) as conn: + body(conn) + reason = _db_opens_cleanly(db_path) except sqlite3.DatabaseError as exc: - logger.warning(failure_msg, exc) - - name, body, success_msg = _FINAL_STRATEGY - try: - reason = _apply(body) + reason = str(exc) + if failure_msg is not None: + logger.warning(failure_msg, exc) + continue if reason is None: - return _succeed(name, success_msg) - report["error"] = reason - except sqlite3.DatabaseError as exc: - report["error"] = str(exc) + report["repaired"], report["strategy"] = True, name + logger.warning(success_msg, db_path) + return report + if failure_msg is None: + report["error"] = reason return report diff --git a/hermes_state_wal.py b/hermes_state_wal.py index 006135f85b..cfda3f2592 100644 --- a/hermes_state_wal.py +++ b/hermes_state_wal.py @@ -1,13 +1,14 @@ -"""SQLite journal-mode and PRAGMA policy for state.db. +"""SQLite journal-mode and PRAGMA policy for state.db (split from hermes_state). -Split out of ``hermes_state.py``. Every name is re-imported there so -``hermes_state.`` keeps resolving, and tests that monkeypatch it keep -intercepting because intra-module calls to patched helpers go through a lazy -``from hermes_state import ...`` at call time. +Every name is re-imported into ``hermes_state``; intra-module calls to +patchable helpers go through a lazy ``from hermes_state import ...`` at call +time so monkeypatches there still intercept. """ from __future__ import annotations +import contextlib +import functools import logging import sqlite3 import sys @@ -15,54 +16,40 @@ import threading import time from typing import Any, Dict, Optional -from hermes_cli.sqlite_runtime import ( - is_sqlite_wal_reset_vulnerable as _is_sqlite_wal_reset_vulnerable, -) +from hermes_cli.sqlite_runtime import is_sqlite_wal_reset_vulnerable as _is_sqlite_wal_reset_vulnerable # Log-record parity with the origin module (caplog tests pin "hermes_state"). logger = logging.getLogger("hermes_state") -# --------------------------------------------------------------------------- -# WAL-compatibility fallback -# --------------------------------------------------------------------------- -# WAL needs mmap shared memory and fcntl byte-range locks, which network -# filesystems (NFS, SMB/CIFS, some FUSE, WSL1) don't provide reliably — there -# ``PRAGMA journal_mode=WAL`` raises ``locking protocol`` (SQLITE_PROTOCOL). -# ZFS instead corrupts the -shm file under concurrent connection bursts (COW + -# mmap), presenting as ``disk I/O error``. Either would silently break -# everything backed by state.db/kanban.db, so we fall back to -# ``journal_mode=DELETE`` (works on NFS/ZFS; readers block during a write). +# WAL needs mmap shared memory + fcntl byte-range locks. Network filesystems (NFS, +# SMB/CIFS, some FUSE, WSL1) raise ``locking protocol``; ZFS corrupts the -shm file +# under concurrent bursts (COW + mmap) -> ``disk I/O error``. Either would silently +# break everything on state.db/kanban.db, so fall back to DELETE (readers block on writes). _WAL_INCOMPAT_MARKERS = ( "locking protocol", # SQLITE_PROTOCOL on NFS/SMB "not authorized", # Some FUSE mounts block WAL pragma outright "disk i/o error", # ZFS SHM corruption under concurrent connections ) -# SQLite's default is -1 (unlimited), so state.db-wal would keep the high-water -# mark of the largest-ever transaction forever. See _apply_wal_size_limit(). +# SQLite's default journal_size_limit is -1 (unlimited); see _apply_wal_size_limit. _WAL_SIZE_LIMIT_BYTES = 64 * 1024 * 1024 # 64 MiB -# Once-per-process-per-db_label dedup sets: kanban_db.connect() runs on every -# kanban operation, so an undeduped log line would repeat per connection. -# Tests clear these through ``hermes_state.``; ``_warn_once`` resolves -# the set through hermes_state at call time for the same reason. +# Once-per-process-per-db_label dedup sets (kanban_db.connect() runs on every +# kanban operation, so an undeduped line would repeat per connection). Tests clear +# these via ``hermes_state.``; ``_warn_once`` resolves them there at call time. _wal_fallback_warned_paths: set[str] = set() _wal_fallback_warned_lock = threading.Lock() _wal_reset_bug_warned_paths: set[str] = set() _wal_reset_bug_warned_lock = threading.Lock() -# "configured delete overridden by on-disk WAL" ERROR. _delete_overridden_warned_paths: set[str] = set() _delete_overridden_warned_lock = threading.Lock() -# Dedup state for _log_journal_mode_upgrade_once. _journal_upgrade_warned_paths: set = set() _journal_upgrade_warned_lock = threading.Lock() _CANNOT_VERIFY_DELETE_MSG = ( - "could not verify journal mode before applying configured " - "journal_mode=delete (database is locked — possible " - "concurrent openers); refusing to downgrade a database " - "this process does not exclusively own" + "could not verify journal mode before applying configured journal_mode=delete (database is locked — possible " + "concurrent openers); refusing to downgrade a database this process does not exclusively own" ) @@ -83,13 +70,10 @@ def _mode_from_row(row) -> str: def _on_disk_journal_mode(conn: sqlite3.Connection) -> Optional[str]: - """Read the journal mode from the DB header; ``None`` if undeterminable. - - ``None`` (new DB, or PRAGMA failed) sends callers down their fail-closed - "unknown → refuse to downgrade" branch. ``disk i/o error`` can be transient - on virtualized block devices (XFS on cloud hosts), so it is retried a few - times first: transient EIO clears, deterministic filesystem errors do not. - """ + """Read the journal mode from the DB header; ``None`` if undeterminable + (new DB, or PRAGMA failed) -> callers take their fail-closed "refuse to + downgrade" branch. ``disk i/o error`` can be transient on virtualized block + devices (XFS on cloud hosts), so it is retried a few times first.""" last_exc: Optional[Exception] = None for _ in range(4): try: @@ -115,53 +99,37 @@ def _on_disk_journal_mode(conn: sqlite3.Connection) -> Optional[str]: def _apply_wal_size_limit(conn: sqlite3.Connection) -> None: - """Bound the WAL so it returns space to the OS after big transactions. - - SQLite's default ``journal_size_limit`` is -1: a checkpointed WAL is reused - in place, never truncated, so ``state.db-wal`` keeps the high-water mark - of the largest transaction ever run (a 3 GB optimize left a 3 GB WAL). - With a limit, each checkpoint truncates the WAL back to it; 64 MiB is - above normal transaction sizes while capping slack predictably. - Best-effort: failure only costs disk slack and must not prevent opening. - """ + """Bound the WAL so it returns space after big transactions. With the default + (-1) a checkpointed WAL is reused in place, never truncated, so ``state.db-wal`` + keeps the high-water mark of the largest transaction ever (a 3 GB optimize left + a 3 GB WAL). Best-effort: failure only costs disk slack.""" try: conn.execute(f"PRAGMA journal_size_limit={_WAL_SIZE_LIMIT_BYTES}") except sqlite3.OperationalError as exc: # pragma: no cover - defensive logger.debug("journal_size_limit not applied: %s", exc) -def _apply_macos_checkpoint_barrier(conn: sqlite3.Connection) -> None: - """Enable ``PRAGMA checkpoint_fullfsync`` on macOS (no-op elsewhere). - - Apple's ``fsync(2)`` guarantees neither data-on-platter nor write ordering, - so WAL's corruption-safety assumption fails on Darwin without ``F_FULLFSYNC``: - a launchd shutdown drops the page cache and a checkpoint that "reported" - durable can leave a malformed ``state.db``. The barrier applies only at - checkpoint boundaries (~+0.1 ms/commit vs ~+4 ms for ``fullfsync=1``). - Best-effort: never raises. - """ +def _darwin_pragma(conn: sqlite3.Connection, pragma: str) -> None: + """Best-effort PRAGMA on macOS only (no-op elsewhere, never raises).""" if sys.platform != "darwin": return - try: - conn.execute("PRAGMA checkpoint_fullfsync=1") - except sqlite3.OperationalError: - pass + with contextlib.suppress(sqlite3.OperationalError): + conn.execute(pragma) + + +def _apply_macos_checkpoint_barrier(conn: sqlite3.Connection) -> None: + """Enable ``PRAGMA checkpoint_fullfsync`` on macOS. Apple's ``fsync(2)`` + guarantees neither data-on-platter nor ordering, so without ``F_FULLFSYNC`` a + launchd shutdown can turn a "durable" checkpoint into a malformed ``state.db``. + Checkpoint boundaries only (~+0.1 ms/commit vs ~+4 ms for ``fullfsync=1``).""" + _darwin_pragma(conn, "PRAGMA checkpoint_fullfsync=1") def _enforce_macos_synchronous_full(conn: sqlite3.Connection) -> None: - """Enforce ``PRAGMA synchronous=FULL`` on macOS to prevent btree corruption. - - With NORMAL, a WAL checkpoint racing process termination can leave - half-written btree pages (``btreeInitPage error 11``). Called after every - successful WAL activation so a prior connection's NORMAL never sticks. - Best-effort: never raises. - """ - if sys.platform != "darwin": - return - try: - conn.execute("PRAGMA synchronous=FULL") - except sqlite3.OperationalError: - pass + """Enforce ``PRAGMA synchronous=FULL`` on macOS: with NORMAL a WAL checkpoint + racing process termination leaves half-written btree pages (``btreeInitPage + error 11``). Called after every WAL activation so a prior NORMAL never sticks.""" + _darwin_pragma(conn, "PRAGMA synchronous=FULL") def _apply_wal_companions(conn: sqlite3.Connection) -> None: @@ -190,100 +158,77 @@ def sqlite_source_id() -> str: conn.close() except sqlite3.Error: return "" - if not row or row[0] is None: - return "" - return str(row[0]) + return str(row[0]) if row and row[0] is not None else "" def _database_has_content(conn: sqlite3.Connection) -> bool: - """Whether the file already holds pages (existing vs brand-new DB). - - ``PRAGMA page_count`` is a lock-free header read. Fail-quiet: any error - answers False, because the only caller gates a warning on this and an - unknown-answer warning would fire on every fresh database. - """ + """Whether the file already holds pages (existing vs brand-new DB); lock-free + header read. Fail-quiet False: the only caller gates a warning on this and an + unknown-answer warning would fire on every fresh database.""" try: row = conn.execute("PRAGMA page_count").fetchone() - except sqlite3.Error: - return False - if not row or row[0] is None: - return False - try: - return int(row[0]) > 0 - except (TypeError, ValueError): + return bool(row) and row[0] is not None and int(row[0]) > 0 + except (sqlite3.Error, TypeError, ValueError): return False def resolve_journal_mode() -> str: - """Return the configured journal mode (``wal`` or ``delete``). - - ``database.journal_mode`` in config.yaml is the canonical operator setting; - ``wal`` is the default, ``delete`` is for filesystems without WAL-safe - durability (macOS virtiofs, NFS, SMB). Invalid values fail safe to ``wal``. - """ + """The configured ``database.journal_mode`` (``wal`` default; ``delete`` for + filesystems without WAL-safe durability: macOS virtiofs, NFS, SMB). Invalid + values fail safe to ``wal``.""" try: from hermes_cli.config import load_config_readonly - config = load_config_readonly() or {} - database = config.get("database", {}) - if not isinstance(database, dict): - return "wal" - raw = database.get("journal_mode", "wal") + database = (load_config_readonly() or {}).get("database", {}) + raw = database.get("journal_mode", "wal") if isinstance(database, dict) else "wal" except Exception: return "wal" - if not isinstance(raw, str): - return "wal" - mode = raw.strip().lower() + mode = raw.strip().lower() if isinstance(raw, str) else "" return mode if mode in ("wal", "delete") else "wal" class WalUnsupportedError(sqlite3.OperationalError): - """Raised by :func:`apply_wal_with_fallback` when ``require_wal=True`` and - the filesystem cannot provide WAL — whether SQLite *raised* - ``SQLITE_PROTOCOL`` or (macOS NFS) silently returned the still-effective - mode. Subclasses ``OperationalError`` so existing DB-init handlers still - catch it while WAL-mandating callers can catch the narrower type. - """ + """Raised by :func:`apply_wal_with_fallback` under ``require_wal=True`` when + the filesystem cannot provide WAL (SQLITE_PROTOCOL raised, or macOS-NFS silent + refusal). Subclasses ``OperationalError`` so DB-init handlers still catch it.""" -def apply_wal_with_fallback( - conn: sqlite3.Connection, *, db_label: str = "state.db", require_wal: bool = False -) -> str: +def _verify_configured_delete(actual: str) -> str: + """Raise unless SQLite reported ``delete`` for an explicit operator request.""" + if actual != "delete": + raise sqlite3.OperationalError( + f"could not set configured journal_mode=delete (got {actual or 'no result'})" + ) + return actual + + +def apply_wal_with_fallback(conn: sqlite3.Connection, *, db_label: str = "state.db", require_wal: bool = False) -> str: """Set ``journal_mode=WAL`` on ``conn``, falling back to DELETE on failure. - Returns the mode actually set (``"wal"`` or ``"delete"``). Shared by - :class:`SessionDB` and ``hermes_cli.kanban_db.connect``. On - WAL-incompatible filesystems SQLite either raises ``OperationalError`` - ("locking protocol" / "disk I/O error") or — macOS NFS / SMB / AgentFS — - silently refuses and leaves the DB in DELETE; either way we log at ERROR - (once per process per ``db_label``) and fall back to DELETE. - ``require_wal=True`` raises :class:`WalUnsupportedError` instead. - - WAL-reset-bug builds (https://sqlite.org/wal.html#walresetbug, fixed - 3.51.3+, backports 3.50.7 / 3.44.6) never enable WAL on fresh / non-WAL - databases; an already-WAL DB keeps WAL with a warning. This gate is - deliberately RETAINED: the attempt to revert it was confounded by a newer - SQLite, and re-measured on the bundled 3.50.4 there is no evidence WAL is - safer. + Returns the mode actually set. Shared by :class:`SessionDB` and + ``hermes_cli.kanban_db.connect``. WAL-incompatible filesystems either raise + ``OperationalError`` ("locking protocol" / "disk I/O error") or — macOS NFS / + SMB / AgentFS — silently refuse and stay in DELETE; either way log ERROR once + per process per ``db_label`` and fall back. ``require_wal=True`` raises + :class:`WalUnsupportedError` instead. WAL-reset-bug builds + (https://sqlite.org/wal.html#walresetbug) never enable WAL on non-WAL files; + an already-WAL DB keeps WAL with a warning. Gate deliberately RETAINED: + re-measured on the bundled 3.50.4 there is no evidence WAL is safer. Invariant on every path: never downgrade to DELETE if the on-disk header - reports WAL or the mode cannot be read — other gateway/cron/worker - connections may hold the DB open, and a live downgrade destroys their - committed-but-uncheckpointed transactions. + reports WAL or cannot be read — other gateway/cron/worker connections may + hold the DB open, and a live downgrade destroys their uncheckpointed commits. """ from hermes_state import is_sqlite_wal_reset_vulnerable, resolve_journal_mode configured = resolve_journal_mode() - # Vulnerable SQLite: never enable WAL on new/non-WAL files. Resolve the - # operator setting first so an explicit DELETE request still verifies SQLite - # accepted DELETE rather than silently returning MEMORY or another mode. + # Vulnerable SQLite: never enable WAL on non-WAL files. Configured mode is + # resolved first so an explicit DELETE request is still verified. if is_sqlite_wal_reset_vulnerable(): - return _apply_delete_for_wal_reset_bug( - conn, db_label=db_label, require_delete=configured == "delete" - ) + return _apply_delete_for_wal_reset_bug(conn, db_label=db_label, require_delete=configured == "delete") - # Read-only probe — no flock, no checkpoint, no WAL/SHM unlink — so - # WAL-init cannot unlink files other connections hold open. + # Read-only probe (no flock/checkpoint/WAL-SHM unlink): WAL-init must not + # unlink files other connections hold open. current_mode = _on_disk_journal_mode(conn) if current_mode == "wal": if configured == "delete": @@ -292,79 +237,53 @@ def apply_wal_with_fallback( _apply_wal_companions(conn) return "wal" - # Honor the canonical database.journal_mode setting (on-disk WAL DBs were - # returned above and are never live-downgraded). if configured == "delete": if current_mode is None: - # Probe failed (locked/busy): another process may hold this DB open - # in WAL, so ownership is not provably exclusive. Fail loudly — the - # operator asked for DELETE and we cannot verify it. + # Probe failed (locked/busy): ownership not provably exclusive. Fail loudly. raise sqlite3.OperationalError(_CANNOT_VERIFY_DELETE_MSG) - actual = _set_journal_mode_no_wait(conn, "DELETE") - if actual != "delete": - raise sqlite3.OperationalError( - f"could not set configured journal_mode=delete (got {actual or 'no result'})" - ) - return actual + return _verify_configured_delete(_set_journal_mode_no_wait(conn, "DELETE")) - # Decide BEFORE the flip whether it would overwrite a mode somebody chose: - # the probe and page_count are only readable while the file is untouched. - # A 0-page DB has no prior choice, and every caller reaches this before - # creating schema, so brand-new databases stay quiet. - _upgrading_existing_db = ( - current_mode is not None and current_mode != "wal" and _database_has_content(conn) - ) + return _enable_wal(conn, db_label, require_wal, current_mode) + + +def _enable_wal(conn: sqlite3.Connection, db_label: str, require_wal: bool, current_mode: Optional[str]) -> str: + """Flip a non-WAL, non-vulnerable connection to WAL, or fall back to DELETE.""" + # Decide BEFORE the flip whether it overwrites a mode somebody chose (probe and + # page_count are only readable while the file is untouched). A 0-page DB has + # no prior choice, and every caller reaches this before creating schema. + upgrading_existing_db = current_mode is not None and current_mode != "wal" and _database_has_content(conn) def _wal_activated() -> str: - if _upgrading_existing_db: + if upgrading_existing_db: _log_journal_mode_upgrade_once(db_label, current_mode) _apply_wal_companions(conn) return "wal" try: - # ``PRAGMA journal_mode=WAL`` RETURNS the resulting mode. Filesystems - # that refuse by *raising* SQLITE_PROTOCOL hit the except branch, but - # macOS NFS, SMB/CIFS and the AgentFS NFS overlay refuse WITHOUT raising - # and just return the still-effective mode. Trust the row, not the + # ``PRAGMA journal_mode=WAL`` RETURNS the resulting mode: macOS NFS, SMB/CIFS + # and the AgentFS overlay refuse WITHOUT raising. Trust the row, not the # absence of an exception. mode = _mode_from_row(conn.execute("PRAGMA journal_mode=WAL").fetchone()) if mode == "wal": return _wal_activated() - # Silent refusal: WAL was not honored, but nothing raised. silent_exc = WalUnsupportedError(f"journal_mode=WAL refused without raising (still {mode!r})") if require_wal: raise silent_exc _log_wal_fallback_once(db_label, silent_exc) return mode or "delete" except sqlite3.OperationalError as exc: - # The require_wal silent-refusal raise above lands here (subclass of - # OperationalError) — propagate unchanged, skip the marker logic. + # The require_wal silent-refusal raise above lands here — propagate unchanged. if isinstance(exc, WalUnsupportedError): raise msg = str(exc).lower() if not any(marker in msg for marker in _WAL_INCOMPAT_MARKERS): raise # unrelated OperationalError — don't silently swallow - # ``disk i/o error`` is ambiguous: deterministic WAL-incompatibility on - # ZFS / APFS-CoW, or a one-shot transient EIO. Treating a transient EIO - # as a permanent downgrade signal produced mixed-mode corruption (process - # A downgrades to DELETE while siblings set WAL), so retry the pragma: transient EIO clears and we return "wal"; - # deterministic cases keep failing into the guarded DELETE fallback. if "disk i/o error" in msg: - for _ in range(2): - time.sleep(0.05) - try: - row = conn.execute("PRAGMA journal_mode=WAL").fetchone() - except sqlite3.OperationalError as retry_exc: - if "disk i/o error" not in str(retry_exc).lower(): - raise - exc = retry_exc - continue - if _mode_from_row(row) == "wal": - return _wal_activated() - break - # Don't downgrade if another process already set WAL on disk, or if the - # mode cannot be read (probe blocked by a concurrent opener's locks) — - # ownership is not provably exclusive either way. + activated, exc = _retry_wal_after_eio(conn, exc) + if activated: + return _wal_activated() + # Never downgrade if WAL is on disk or the mode cannot be read (probe blocked + # by a concurrent opener) — ownership is not provably exclusive either way. existing = _on_disk_journal_mode(conn) if existing == "wal" or existing is None: raise @@ -375,64 +294,69 @@ def apply_wal_with_fallback( return "delete" +def _retry_wal_after_eio(conn: sqlite3.Connection, exc: sqlite3.OperationalError): + """Retry ``journal_mode=WAL`` twice after ``disk i/o error``: EIO is either + deterministic WAL-incompatibility (ZFS / APFS-CoW) or a one-shot transient, and + treating a transient as a permanent downgrade produced mixed-mode corruption + (A downgrades to DELETE while siblings set WAL). Returns ``(wal_activated, + last_exc)``; a non-EIO retry error propagates.""" + for _ in range(2): + time.sleep(0.05) + try: + row = conn.execute("PRAGMA journal_mode=WAL").fetchone() + except sqlite3.OperationalError as retry_exc: + if "disk i/o error" not in str(retry_exc).lower(): + raise + exc = retry_exc + continue + return _mode_from_row(row) == "wal", exc + return False, exc + + def _set_journal_mode_no_wait(conn: sqlite3.Connection, mode: str) -> str: """Execute ``PRAGMA journal_mode=`` without waiting on other openers. - The ONLY place a journal-mode switch may be issued for a non-WAL target. - Forces ``busy_timeout=0`` so SQLite's exclusivity requirement becomes a - concurrent-opener detector: leaving WAL needs exclusive access, so if ANY - other connection holds the DB the pragma fails immediately with ``database - is locked`` instead of sneaking the flip between a concurrent writer's - transactions (how committed-but-uncheckpointed WAL transactions die). - - Callers must treat a raised ``OperationalError`` as "not exclusively - owned: leave the journal mode alone", never as retryable. Returns SQLite's - reported mode (lowercase), or ``""`` if no row. + The ONLY place a non-WAL journal-mode switch may be issued. ``busy_timeout=0`` + turns SQLite's exclusivity requirement into a concurrent-opener detector: + leaving WAL needs exclusive access, so if ANY other connection holds the DB + the pragma fails immediately with ``database is locked`` instead of sneaking + the flip between a writer's transactions (how uncheckpointed WAL commits die). + Callers must treat a raised ``OperationalError`` as "not exclusively owned: + leave the mode alone", never as retryable. Returns the reported mode, ``""`` + if no row. """ - previous_timeout = 0 try: row = conn.execute("PRAGMA busy_timeout").fetchone() - if row and row[0] is not None: - previous_timeout = int(row[0]) + previous_timeout = int(row[0]) if row and row[0] is not None else 0 except (sqlite3.OperationalError, TypeError, ValueError): previous_timeout = 0 conn.execute("PRAGMA busy_timeout=0") try: return _mode_from_row(conn.execute(f"PRAGMA journal_mode={mode}").fetchone()) finally: - try: + with contextlib.suppress(sqlite3.OperationalError): conn.execute(f"PRAGMA busy_timeout={previous_timeout}") - except sqlite3.OperationalError: - pass -def _apply_delete_for_wal_reset_bug( - conn: sqlite3.Connection, *, db_label: str, require_delete: bool = False -) -> str: +def _apply_delete_for_wal_reset_bug(conn: sqlite3.Connection, *, db_label: str, require_delete: bool = False) -> str: """Avoid enabling WAL when the linked SQLite has the WAL-reset bug. - - Already-WAL on disk: leave WAL alone (no live downgrade) and warn. - - Mode unreadable (probe blocked by a concurrent opener's locks): not - provably exclusive — leave the mode alone and warn. Never treat "could - not read the mode" as "not WAL": that once flipped a live WAL state.db to - DELETE under a concurrent writer, destroying its uncheckpointed commits. - - Otherwise: set DELETE (refusing to wait out concurrent openers) and warn. - - For an explicit operator request, verify SQLite accepted DELETE. + Already-WAL on disk: keep WAL (no live downgrade) and warn. Mode unreadable + (probe blocked by a concurrent opener): not provably exclusive — leave it and + warn; treating "could not read" as "not WAL" once flipped a live WAL state.db + to DELETE under a writer, destroying its uncheckpointed commits. Otherwise set + DELETE without waiting out openers and warn; an explicit operator request + additionally verifies SQLite accepted DELETE. """ current = _on_disk_journal_mode(conn) if current == "wal": _log_wal_reset_bug_once(db_label, kept_wal=True) if require_delete: - # Upgrading SQLite (the warning above) doesn't help on a - # WAL-incompatible filesystem; emit the actionable message last. + # Upgrading SQLite doesn't help here; emit the actionable message last. _log_configured_delete_overridden_once(db_label) - # No TRUNCATE / journal_mode=DELETE while other processes may still - # hold this WAL DB open; same safety rule as the NFS path. _apply_wal_companions(conn) return "wal" if current is None: - # Probe failed — likely another opener's locks, and the DB may be in - # WAL under a live writer. Never flip a mode we cannot even read. if require_delete: raise sqlite3.OperationalError(_CANNOT_VERIFY_DELETE_MSG) _log_wal_reset_bug_once(db_label, kept_wal=True, indeterminate=True) @@ -445,30 +369,21 @@ def _apply_delete_for_wal_reset_bug( raise lowered = str(exc).lower() if "locked" in lowered or "busy" in lowered: - # A concurrent opener appeared between probe and flip (or already - # held the DB): SQLite refused the exclusive lock. Leave the mode as is. + # A concurrent opener appeared between probe and flip: leave the mode as is. _log_wal_reset_bug_once(db_label, kept_wal=True, indeterminate=True) return current or "delete" - # Best-effort for the automatic fallback: DELETE is normally already - # the default for new file-backed databases. - if require_delete and actual != "delete": - raise sqlite3.OperationalError( - "could not set configured journal_mode=delete " - f"(got {actual or 'no result'})" - ) + # Best-effort otherwise: DELETE is already the default for new file-backed DBs. + if require_delete: + _verify_configured_delete(actual) _log_wal_reset_bug_once(db_label, kept_wal=False) return "delete" def _wal_reset_repair_hint() -> str: - """Repair hint matching what ``hermes update`` can actually do for this - install type (uv-managed venv vs git/pip/docker/nix).""" + """Repair hint matching what ``hermes update`` can actually do for this install type.""" try: - from hermes_cli.config import ( - detect_install_method, - recommended_update_command_for_method, - get_project_root, - ) + from hermes_cli.config import detect_install_method, get_project_root, recommended_update_command_for_method + method = detect_install_method(get_project_root()) cmd = recommended_update_command_for_method(method) if method in {"git", "unknown"}: @@ -477,109 +392,86 @@ def _wal_reset_repair_hint() -> str: return f"update the container image with `{cmd}`" return cmd # nix/nixos except Exception: - pass - return ( - "install a Python build bundled with SQLite 3.51.3+ " - "(or backports 3.50.7 / 3.44.6) and restart Hermes" - ) + return "install a Python build bundled with SQLite 3.51.3+ (or backports 3.50.7 / 3.44.6) and restart Hermes" + + +# Once-per-(process, db_label) log table. Levels are deliberate: falling back to +# DELETE and an ignored ``journal_mode: delete`` are real losses (ERROR); a non-WAL +# -> WAL flip is normally desirable and only its invisibility was the problem (WARNING). +_WAL_RESET_BUG_ACTIONS = { + "indeterminate": ( + "journal mode could not be verified or exclusively switched (database is locked — possible concurrent " + "openers); leaving the journal mode untouched (no live downgrade under concurrent openers)" + ), + "kept_wal": ( + "is already in WAL mode — leaving WAL in place (no live downgrade under concurrent openers)" + ), + "delete": "using journal_mode=DELETE instead of enabling WAL", +} +_ONCE_LOGS = { + "wal_reset_bug": ( + _wal_reset_bug_warned_lock, "_wal_reset_bug_warned_paths", logging.WARNING, + # Install-type-aware so the warning never promises a repair path that + # doesn't exist for git/pip/system Python installs. + "%s: linked SQLite %s (interpreter %s) is vulnerable to the WAL-reset corruption bug " + "(https://sqlite.org/wal.html#walresetbug) — %s. Upgrade to SQLite 3.51.3+ (or backports 3.50.7 / 3.44.6); " + "%s. See `hermes doctor`. This warning fires once per process per database.", + ), + "journal_upgrade": ( + _journal_upgrade_warned_lock, "_journal_upgrade_warned_paths", logging.WARNING, + # journal_mode is a property of the FILE: switching an existing DB to WAL + # rewrites its header and outlives the process. Operators set DELETE on + # the file directly (the documented WAL-reset-bug mitigation) and nothing + # told them the next open would silently put WAL back. + "%s: on-disk journal_mode was %s and has been switched to WAL. This rewrites the database header and " + "persists after this process exits. If %s was a deliberate choice (for example the mitigation for the SQLite " + "WAL-reset bug, or a WAL-unsafe filesystem), setting it with PRAGMA on the file will not survive -- every " + "open re-applies the configured mode. Set `database.journal_mode: delete` in config.yaml to make it stick. " + "This message fires once per process per database.", + ), + "wal_fallback": ( + _wal_fallback_warned_lock, "_wal_fallback_warned_paths", logging.ERROR, + # Under kanban dispatcher + workers a DELETE-mode write blocks readers as SQLITE_BUSY. + "%s: WAL journal_mode unsupported on this filesystem (%s) — falling back to journal_mode=DELETE (slower " + "rollback-journal mode; reduces concurrency but works on NFS/SMB/FUSE/ZFS). See " + "https://www.sqlite.org/wal.html for details. This message fires once per process per database.", + ), + "delete_overridden": ( + _delete_overridden_warned_lock, "_delete_overridden_warned_paths", logging.ERROR, + # Never-live-downgrade keeps WAL; without this the operator never learns + # that ``database.journal_mode: delete`` had no effect. + "%s: database.journal_mode=delete is configured but the on-disk database is already WAL; keeping WAL (a live " + "downgrade under open connections can corrupt the DB). To apply journal_mode=DELETE, stop all connections to " + "this DB and run a one-time offline 'PRAGMA journal_mode=DELETE' on the file. This message fires once per " + "process per database.", + ), +} + + +def _log_once(kind: str, db_label: str, *args: Any) -> None: + """Emit ``_ONCE_LOGS[kind]`` once per (process, db_label). Callable *args* are + resolved only after the dedupe check, so install-method probes run once.""" + lock, set_name, level, message = _ONCE_LOGS[kind] + if _warn_once(lock, set_name, db_label): + logger.log(level, message, db_label, *(a() if callable(a) else a for a in args)) def _log_wal_reset_bug_once(db_label: str, *, kept_wal: bool, indeterminate: bool = False) -> None: """Log once per (process, db_label) about the WAL-reset vulnerability path.""" - if not _warn_once(_wal_reset_bug_warned_lock, "_wal_reset_bug_warned_paths", db_label): - return - if indeterminate: - action = ( - "journal mode could not be verified or exclusively switched " - "(database is locked — possible concurrent openers); leaving the " - "journal mode untouched (no live downgrade under concurrent " - "openers)" - ) - elif kept_wal: - action = ( - "is already in WAL mode — leaving WAL in place (no live " - "downgrade under concurrent openers)" - ) - else: - action = "using journal_mode=DELETE instead of enabling WAL" - # Install-type-aware so the warning never promises a repair path that - # doesn't exist for git/pip/system Python installs. - logger.warning( - "%s: linked SQLite %s (interpreter %s) is vulnerable to the WAL-reset " - "corruption bug (https://sqlite.org/wal.html#walresetbug) — %s. " - "Upgrade to SQLite 3.51.3+ (or backports 3.50.7 / 3.44.6); " - "%s. See `hermes doctor`. This warning fires once per " - "process per database.", - db_label, sqlite3.sqlite_version, sys.executable, action, _wal_reset_repair_hint(), - ) + action = _WAL_RESET_BUG_ACTIONS["indeterminate" if indeterminate else "kept_wal" if kept_wal else "delete"] + _log_once("wal_reset_bug", db_label, sqlite3.sqlite_version, sys.executable, action, _wal_reset_repair_hint) def _log_journal_mode_upgrade_once(db_label: str, previous_mode: str) -> None: - """Log a single WARNING per (process, db_label) about a non-WAL -> WAL flip. - - ``PRAGMA journal_mode`` is a property of the FILE: switching an existing DB - to WAL rewrites its header and outlives the process. Operators do set - DELETE on the file directly (the documented WAL-reset-bug mitigation), and - nothing told them the next open would silently put WAL back. WARNING, not - ERROR: this direction is normally desirable; only its invisibility was the - problem, so this names the durable setting without claiming a degradation. - """ - if not _warn_once(_journal_upgrade_warned_lock, "_journal_upgrade_warned_paths", db_label): - return - logger.warning( - "%s: on-disk journal_mode was %s and has been switched to WAL. This " - "rewrites the database header and persists after this process exits. " - "If %s was a deliberate choice (for example the mitigation for the " - "SQLite WAL-reset bug, or a WAL-unsafe filesystem), setting it with " - "PRAGMA on the file will not survive -- every open re-applies the " - "configured mode. Set `database.journal_mode: delete` in config.yaml " - "to make it stick. This message fires once per process per database.", - db_label, previous_mode, previous_mode, - ) + """Single WARNING per (process, db_label) about a non-WAL -> WAL flip.""" + _log_once("journal_upgrade", db_label, previous_mode, previous_mode) -def _log_wal_fallback_once(db_label: str, exc: Exception) -> None: - """Log a single ERROR per (process, db_label) about WAL fallback. - - ERROR, not WARNING: silently dropping to DELETE is a real concurrency loss - (under kanban dispatcher + workers a write blocks readers as SQLITE_BUSY). - """ - if not _warn_once(_wal_fallback_warned_lock, "_wal_fallback_warned_paths", db_label): - return - logger.error( - "%s: WAL journal_mode unsupported on this filesystem (%s) — " - "falling back to journal_mode=DELETE (slower rollback-journal " - "mode; reduces concurrency but works on NFS/SMB/FUSE/ZFS). See " - "https://www.sqlite.org/wal.html for details. This message " - "fires once per process per database.", - db_label, exc, - ) +# Single ERROR per (process, db_label): WAL fallback / configured delete ignored (DB already WAL). +_log_wal_fallback_once = functools.partial(_log_once, "wal_fallback") +_log_configured_delete_overridden_once = functools.partial(_log_once, "delete_overridden") -def _log_configured_delete_overridden_once(db_label: str) -> None: - """Log a single ERROR per (process, db_label) when the operator configured - ``journal_mode=delete`` but the on-disk DB is already WAL. - - Never-live-downgrade keeps WAL; without this the operator would never learn - that ``database.journal_mode: delete`` had no effect and that a one-time - offline ``PRAGMA journal_mode=DELETE`` (no open connections) is required. - """ - if not _warn_once(_delete_overridden_warned_lock, "_delete_overridden_warned_paths", db_label): - return - logger.error( - "%s: database.journal_mode=delete is configured but the on-disk " - "database is already WAL; keeping WAL (a live downgrade under open " - "connections can corrupt the DB). To apply journal_mode=DELETE, stop " - "all connections to this DB and run a one-time offline " - "'PRAGMA journal_mode=DELETE' on the file. This message fires once " - "per process per database.", - db_label, - ) - - -# --------------------------------------------------------------------------- -# Config-driven database pragmas -# --------------------------------------------------------------------------- # Operators write synchronous as a name; mapped here rather than passed through # so a typo becomes a warning instead of a silently different durability level. _SYNCHRONOUS_LEVELS: Dict[str, int] = {"OFF": 0, "NORMAL": 1, "FULL": 2, "EXTRA": 3} @@ -588,12 +480,9 @@ _SYNCHRONOUS_FULL = 2 def resolve_synchronous_level(raw_value: Any) -> Optional[int]: - """Map a configured ``database.synchronous`` value to its PRAGMA integer. - - Accepts SQLite's names (``OFF``/``NORMAL``/``FULL``/``EXTRA``, any case) or - ``0``-``3``. Anything else returns None so the caller warns and leaves the - level untouched — guessing at a malformed durability setting is worse. - """ + """Map ``database.synchronous`` (``OFF``/``NORMAL``/``FULL``/``EXTRA`` any + case, or ``0``-``3``) to its PRAGMA integer; None for anything else so the + caller warns and leaves the level untouched (guessing at durability is worse).""" if isinstance(raw_value, bool): # bool is an int subclass and YAML turns bare `on`/`off` into one. # "off" is a real durability choice; True is meaningless. @@ -601,68 +490,51 @@ def resolve_synchronous_level(raw_value: Any) -> Optional[int]: if isinstance(raw_value, int): return raw_value if raw_value in _SYNCHRONOUS_NAMES else None text = str(raw_value).strip() - if not text: - return None - upper = text.upper() - if upper in _SYNCHRONOUS_LEVELS: - return _SYNCHRONOUS_LEVELS[upper] - try: - value = int(text) - except (TypeError, ValueError): - return None - return value if value in _SYNCHRONOUS_NAMES else None + if text.upper() in _SYNCHRONOUS_LEVELS: + return _SYNCHRONOUS_LEVELS[text.upper()] + with contextlib.suppress(TypeError, ValueError): + return int(text) if int(text) in _SYNCHRONOUS_NAMES else None + return None def _apply_synchronous_pragma(conn: sqlite3.Connection, raw_value: Any, *, db_label: str) -> None: """Set ``PRAGMA synchronous`` from config, never below FULL on macOS. Kept out of the integer loop in :func:`apply_database_pragmas`: this PRAGMA - decides whether a commit is on the platter, so an unrecognised value must - not fall through to "SQLite default" the way a bad ``cache_size`` can. - Darwin floor: :func:`_enforce_macos_synchronous_full` runs during WAL - activation and this runs after it, so a configured ``NORMAL`` would - otherwise silently undo the macOS btree protection. Raising the level on - macOS is allowed; lowering it is refused out loud. + decides whether a commit is on the platter, so an unrecognised value must not + fall through to "SQLite default" the way a bad ``cache_size`` can. Darwin + floor: this runs after :func:`_enforce_macos_synchronous_full`, so a + configured ``NORMAL`` would silently undo the btree protection — raising is + allowed, lowering is refused out loud. """ level = resolve_synchronous_level(raw_value) if level is None: logger.warning( - "%s: ignoring unrecognized database.synchronous=%r " - "(expected OFF, NORMAL, FULL, EXTRA, or 0-3)", + "%s: ignoring unrecognized database.synchronous=%r (expected OFF, NORMAL, FULL, EXTRA, or 0-3)", db_label, raw_value, ) return if sys.platform == "darwin" and level < _SYNCHRONOUS_FULL: logger.warning( - "%s: refusing database.synchronous=%s on macOS; keeping FULL. " - "Darwin's fsync() does not guarantee write ordering, so a lower " - "level readmits the half-written btree pages FULL exists to " - "prevent.", + "%s: refusing database.synchronous=%s on macOS; keeping FULL. Darwin's fsync() does not guarantee write " + "ordering, so a lower level readmits the half-written btree pages FULL exists to prevent.", db_label, _SYNCHRONOUS_NAMES[level], ) return - try: + with contextlib.suppress(sqlite3.OperationalError): conn.execute(f"PRAGMA synchronous={level}") - except sqlite3.OperationalError: - pass def apply_database_pragmas(conn: sqlite3.Connection, *, db_label: str = "state.db") -> None: """Apply optional performance and WAL-sizing PRAGMAs from ``config.yaml``. - Journal mode is NOT handled here — ``database.journal_mode`` is owned by - :func:`resolve_journal_mode` inside :func:`apply_wal_with_fallback`. - - Keys under ``database:``: ``cache_size`` (negative = KiB, positive = - pages), ``mmap_size`` (bytes, 0 = disabled), ``temp_store`` (0-3), - ``wal_autocheckpoint`` (pages), ``journal_size_limit`` (bytes), and - ``synchronous`` (``OFF``/``NORMAL``/``FULL``/``EXTRA`` or ``0``-``3``). - Unset ``synchronous`` leaves SQLite's compile-time default, which differs - between bundled, distro and Homebrew builds. - - Best-effort: config load or pragma failures are ignored so DB init never - breaks on a malformed ``database:`` section. Applied to ALL connection - types: writer, read_only, WAL per-thread readers. + Journal mode is NOT handled here (owned by :func:`apply_wal_with_fallback`). + ``database:`` keys: ``cache_size`` (negative = KiB, positive = pages), + ``mmap_size`` (bytes, 0 = off), ``temp_store`` (0-3), ``wal_autocheckpoint`` + (pages), ``journal_size_limit`` (bytes), ``synchronous`` (unset leaves the + compile-time default, which differs between bundled/distro/Homebrew builds). + Best-effort: failures are ignored so DB init never breaks on a malformed + section. Applied to ALL connection types: writer, read_only, WAL readers. """ try: # Local import avoids a circular import with hermes_cli.config. @@ -680,12 +552,9 @@ def apply_database_pragmas(conn: sqlite3.Connection, *, db_label: str = "state.d except (TypeError, ValueError): logger.warning("%s: ignoring non-integer database.%s=%r", db_label, pragma_name, raw_value) continue - try: + with contextlib.suppress(sqlite3.OperationalError): conn.execute(f"PRAGMA {pragma_name}={value}") - except sqlite3.OperationalError: - pass - # Last: the sizing pragmas above cannot change durability, and the macOS - # enforcement ran earlier during WAL activation (see _apply_synchronous_pragma). + # Last: sizing pragmas cannot change durability (see _apply_synchronous_pragma). raw_synchronous = cfg_get(cfg, "database", "synchronous", default=None) if raw_synchronous is not None: _apply_synchronous_pragma(conn, raw_synchronous, db_label=db_label)