From 64fe13a6476a6c0bf289a043464f2d06c7ea1a14 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 11 Sep 2026 10:38:20 +0530 Subject: [PATCH] fix(sessions): retire-capture integrity - backups skip capture dirs whole; short reads fail the capture - hermes backup excluded *.db-wal by suffix but not the retired-wal capture dirs, so it would ship the capture's main-image copy while dropping the captured -wal that is the artifact's point; exclude .retired-wal-* dirs whole (they must move as manifest+image+wal unit). - _copy_range treated a short read as success, yielding a truncated copy with a valid manifest while the unlinked inode still dies at exit; raise RetiredGenerationCaptureError and clean the .part file. - _quarantine_reason docstring no longer claims close() checks replaced before generation loss (close evaluates loss first and skips quarantine when lost). --- hermes_cli/backup.py | 10 +++++++++- hermes_state.py | 6 ++++-- hermes_state_dbfile.py | 35 ++++++++++++++++++++++++----------- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/hermes_cli/backup.py b/hermes_cli/backup.py index fd7b299996..6fa9f46f3d 100644 --- a/hermes_cli/backup.py +++ b/hermes_cli/backup.py @@ -19,6 +19,7 @@ from typing import Any, Dict, List, Optional, Tuple from hermes_constants import ( _get_platform_default_hermes_home, get_default_hermes_root, get_hermes_home, display_hermes_home, ) +from hermes_state_dbfile import RETIRED_GENERATION_DIR_SUFFIX from utils import ( _preserve_file_mode, _preserve_file_owner, _restore_file_mode, _restore_file_owner, atomic_replace, ) @@ -85,7 +86,14 @@ _EXCLUDED_NAMES = {".backup.lock", "gateway.pid", "cron.pid"} # The desktop updater's pre-flight drops ``state.db.pre-update-emergency-.bak`` at the root # — a backup artifact like ``backups/``. Prefix-matched because the name carries a timestamp; # a plain ``.bak`` suffix rule would drop user files. -_EXCLUDED_PREFIXES = ("state.db.pre-update-emergency-",) +# Retired-WAL capture dirs (``.retired-wal--/``) are excluded whole: a +# ``sqlite3.backup()`` snapshot of the live db paired with the captured ``-wal`` is exactly the +# torn-restore hazard the sidecar exclusion below exists to prevent, and the capture is an +# operator-recovery artifact that must move as a unit (manifest + image + WAL), never partially. +_EXCLUDED_PREFIXES = ( + "state.db.pre-update-emergency-", + f"state.db{RETIRED_GENERATION_DIR_SUFFIX}", +) # Files ``hermes import`` must never overwrite, matched by basename so root and named profiles are # both covered. They hold runtime state namespaced to the SOURCE machine: ``gateway_state.json`` diff --git a/hermes_state.py b/hermes_state.py index ca30558f98..bb73d5bac1 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -1190,8 +1190,10 @@ class SessionDB( def _quarantine_reason(self) -> Optional[str]: """Why this handle must not checkpoint or run in-file repair, or None. A corrupted image has torn B-trees; a replaced file or a deleted/replaced WAL generation would checkpoint under - wrong page numbers into the main DB -- the shutdown-time cause of #105670. Same precedence - as the halt path (replaced is checked before generation loss).""" + wrong page numbers into the main DB -- the shutdown-time cause of #105670. Precedence note: + close() evaluates generation loss BEFORE calling this (and skips it entirely when lost — + a lost generation settles through the capture path, not the quarantine advisory), while + the halt path checks replaced first.""" if self._db_corrupt: return f"structural corruption ({self._db_corrupt_reason})" if self._db_replaced: diff --git a/hermes_state_dbfile.py b/hermes_state_dbfile.py index 3e41307e55..c8efb7f2c5 100644 --- a/hermes_state_dbfile.py +++ b/hermes_state_dbfile.py @@ -250,20 +250,33 @@ def _own_descriptor_for_identity(identity) -> "Optional[int]": def _copy_range(read: Callable[[int, int], Optional[bytes]], dest: Path, *, size: int) -> Dict[str, Any]: - """Stream ``size`` bytes via ``read(offset, length)`` into ``dest`` (temp file, fsync, rename).""" + """Stream ``size`` bytes via ``read(offset, length)`` into ``dest`` (temp file, fsync, rename). + + A short read raises ``RetiredGenerationCaptureError``: a truncated copy with a valid-looking + manifest would let the caller settle the handle while the unlinked inode still dies at exit.""" digest = hashlib.sha256() part = dest.with_name(dest.name + ".part") offset = 0 - with open(part, "wb") as out: - while offset < size: - chunk = read(offset, min(_CAPTURE_CHUNK_BYTES, size - offset)) - if not chunk: - break - out.write(chunk) - digest.update(chunk) - offset += len(chunk) - out.flush() - os.fsync(out.fileno()) + try: + with open(part, "wb") as out: + while offset < size: + chunk = read(offset, min(_CAPTURE_CHUNK_BYTES, size - offset)) + if not chunk: + break + out.write(chunk) + digest.update(chunk) + offset += len(chunk) + out.flush() + os.fsync(out.fileno()) + except Exception: + part.unlink(missing_ok=True) + raise + if offset != size: + part.unlink(missing_ok=True) + raise RetiredGenerationCaptureError( + f"short read copying {dest.name}: {offset} of {size} bytes — the retired generation " + "is NOT fully captured; the handle stays open for a retry" + ) os.replace(part, dest) return {"file": dest.name, "bytes": offset, "sha256": digest.hexdigest()}