From ba437934a0c3044da08b4d10191a6ff903df5f5b Mon Sep 17 00:00:00 2001 From: Royce Date: Sat, 8 Aug 2026 14:52:26 -0700 Subject: [PATCH] [verified] fix(backup): bound locked SQLite snapshots --- .../emails/royce.personalassistant@gmail.com | 2 + hermes_cli/backup.py | 103 +++++++++++++++--- tests/hermes_cli/test_backup_stability.py | 102 +++++++++++++++++ 3 files changed, 194 insertions(+), 13 deletions(-) create mode 100644 contributors/emails/royce.personalassistant@gmail.com diff --git a/contributors/emails/royce.personalassistant@gmail.com b/contributors/emails/royce.personalassistant@gmail.com new file mode 100644 index 0000000000000..ca9a3b147dcdd --- /dev/null +++ b/contributors/emails/royce.personalassistant@gmail.com @@ -0,0 +1,2 @@ +roycepersonalassistant +# PR #72966 diff --git a/hermes_cli/backup.py b/hermes_cli/backup.py index 98d0f41df1c1f..ff0f22d2e71ac 100644 --- a/hermes_cli/backup.py +++ b/hermes_cli/backup.py @@ -74,6 +74,23 @@ ".ruff_cache", } +# Chrome's dedicated CDP profile contains both durable operator state (cookies, +# logins) and regenerable runtime caches. Preserve the former, but never walk or +# snapshot the latter: Chrome helper processes can hold cache SQLite databases +# in a state where sqlite3.Connection.backup() reports SQLITE_BUSY forever. +_CHROME_REGENERABLE_CACHE_DIRS = { + "cache", + "cachestorage", + "code cache", + "dawngraphitecache", + "dawnwebgpucache", + "gpucache", + "graphitedawncache", + "grshadercache", + "shadercache", + "shared dictionary", +} + # File-name suffixes to skip _EXCLUDED_SUFFIXES = ( ".pyc", @@ -298,6 +315,14 @@ def _should_exclude(rel_path: Path) -> bool: """Return True if *rel_path* (relative to hermes root) should be skipped.""" parts = rel_path.parts + if "chrome-debug" in parts: + chrome_root_index = parts.index("chrome-debug") + if any( + part.casefold() in _CHROME_REGENERABLE_CACHE_DIRS + for part in parts[chrome_root_index + 1 :] + ): + return True + for part in parts: if part not in _EXCLUDED_DIRS: continue @@ -339,19 +364,67 @@ def _should_skip_backup_file(abs_path: Path, rel_path: Path, out_path: Path) -> # SQLite safe copy # --------------------------------------------------------------------------- -def _safe_copy_db(src: Path, dst: Path) -> bool: +_SQLITE_BACKUP_STALL_TIMEOUT_SECONDS = 5.0 +_SQLITE_BACKUP_PAGES_PER_STEP = 256 +_SQLITE_BACKUP_RETRY_SLEEP_SECONDS = 0.05 + + +def _safe_copy_db( + src: Path, + dst: Path, + *, + stall_timeout: float = _SQLITE_BACKUP_STALL_TIMEOUT_SECONDS, +) -> bool: """Copy a SQLite database safely using the backup() API. - Handles WAL mode — produces a consistent snapshot even while - the DB is being written to. Fail closed if a consistent snapshot cannot - be created: copying only the live main file can omit committed WAL data. + Handles WAL mode — produces a consistent snapshot even while the DB is + being written to. A progress callback bounds time without forward progress; + CPython's backup loop otherwise retries SQLITE_BUSY indefinitely and ignores + the source connection timeout. The deadline resets whenever the remaining + page count falls, so large healthy databases are not subject to a total + wall-clock limit. + + Fail closed if a consistent snapshot cannot be created: copying only the + live main file can omit committed WAL data. """ conn = None backup_conn = None try: - conn = sqlite3.connect(f"file:{src}?mode=ro", uri=True) + # A short connection busy timeout is load-bearing: SQLite applies it + # before the first backup progress callback, so the default 5 seconds + # would defeat a shorter stall_timeout and delay every locked probe. + conn = sqlite3.connect( + f"file:{src}?mode=ro", + uri=True, + timeout=_SQLITE_BACKUP_RETRY_SLEEP_SECONDS, + ) backup_conn = sqlite3.connect(str(dst)) - conn.backup(backup_conn) + last_progress_at = time.monotonic() + last_remaining: Optional[int] = None + + def _progress(status: int, remaining: int, total: int) -> None: + nonlocal last_progress_at, last_remaining + now = time.monotonic() + if status == sqlite3.SQLITE_DONE: + return + if status == sqlite3.SQLITE_OK and ( + last_remaining is None or remaining < last_remaining + ): + last_remaining = remaining + last_progress_at = now + if now - last_progress_at >= stall_timeout: + raise TimeoutError( + "SQLite backup made no progress for " + f"{stall_timeout:.1f}s (status={status}, " + f"remaining={remaining}, total={total})" + ) + + conn.backup( + backup_conn, + pages=_SQLITE_BACKUP_PAGES_PER_STEP, + progress=_progress, + sleep=_SQLITE_BACKUP_RETRY_SLEEP_SECONDS, + ) return True except Exception as exc: logger.warning("SQLite safe copy failed for %s: %s", src, exc) @@ -626,14 +699,13 @@ def _run_backup_locked(args, hermes_root: Path) -> None: dp = Path(dirpath) rel_dir = dp.relative_to(hermes_root) - # Prune excluded directories in-place so os.walk doesn't descend - # ``hermes-agent`` is only pruned at the root level; nested dirs - # with the same name (e.g. in skills/) must be preserved. - is_root = rel_dir == Path(".") + # Prune excluded directories in-place so os.walk doesn't descend. + # Route through the path-aware helper so Chrome cache subtrees are + # skipped while the rest of the persistent browser profile survives. orig_dirnames = dirnames[:] dirnames[:] = [ d for d in dirnames - if d not in _EXCLUDED_DIRS or (d == "hermes-agent" and not is_root) + if not _should_exclude(rel_dir / d) ] for removed in set(orig_dirnames) - set(dirnames): skipped_dirs.add(str(rel_dir / removed)) @@ -1654,8 +1726,13 @@ def _write_full_zip_backup_locked(out_path: Path, hermes_root: Path) -> Optional try: for dirpath, dirnames, filenames in os.walk(hermes_root, followlinks=False): dp = Path(dirpath) - # Prune excluded directories in-place so os.walk doesn't descend - dirnames[:] = [d for d in dirnames if d not in _EXCLUDED_DIRS] + rel_dir = dp.relative_to(hermes_root) + # Keep the full/update helper aligned with run_backup: avoid + # descending into regenerable cache trees, but retain durable + # browser profile state such as cookies and logins. + dirnames[:] = [ + d for d in dirnames if not _should_exclude(rel_dir / d) + ] for fname in filenames: fpath = dp / fname diff --git a/tests/hermes_cli/test_backup_stability.py b/tests/hermes_cli/test_backup_stability.py index d461d507220b4..4aeeefa0e5957 100644 --- a/tests/hermes_cli/test_backup_stability.py +++ b/tests/hermes_cli/test_backup_stability.py @@ -1,6 +1,9 @@ from __future__ import annotations import json +import sqlite3 +import time +import zipfile from pathlib import Path import pytest @@ -9,6 +12,8 @@ BackupInProgressError, _atomic_output_path, _backup_operation_lock, + _safe_copy_db, + _should_exclude, _write_full_zip_backup, create_quick_snapshot, list_quick_snapshots, @@ -103,3 +108,100 @@ def test_failed_automatic_backup_preserves_previous_archive(tmp_path, monkeypatc assert _write_full_zip_backup(archive, home) is None assert archive.read_bytes() == b"previous-valid-backup" assert list(tmp_path.glob(".*.partial")) == [] + + +def test_chrome_runtime_caches_are_excluded_but_profile_state_is_preserved() -> None: + assert _should_exclude(Path("chrome-debug/Default/GPUCache/cache.db")) + assert _should_exclude(Path("chrome-debug/Default/Code Cache/js/index")) + assert _should_exclude(Path("chrome-debug/ShaderCache/GPUCache/data_0")) + assert _should_exclude( + Path("profiles/coder/chrome-debug/Default/GPUCache/cache.db") + ) + assert not _should_exclude(Path("chrome-debug/Default/Cookies")) + assert not _should_exclude(Path("chrome-debug/Default/Login Data")) + assert not _should_exclude( + Path("profiles/coder/chrome-debug/Default/Login Data") + ) + + +def test_locked_database_stall_is_bounded_and_cleans_destination(tmp_path) -> None: + src = tmp_path / "locked-cache.db" + dst = tmp_path / "copy.db" + holder = sqlite3.connect(src, timeout=0) + holder.execute("CREATE TABLE cache (key TEXT)") + holder.commit() + holder.execute("BEGIN EXCLUSIVE") + + started = time.monotonic() + try: + result = _safe_copy_db(src, dst, stall_timeout=0.1) + finally: + holder.rollback() + holder.close() + + assert result is False + assert time.monotonic() - started < 2.0 + assert not dst.exists() + + +def test_nonadvancing_sqlite_ok_progress_is_bounded(tmp_path, monkeypatch) -> None: + from hermes_cli import backup + + src = tmp_path / "growing.db" + dst = tmp_path / "copy.db" + src.write_bytes(b"source") + dst.write_bytes(b"partial") + + class SourceConnection: + def backup(self, _destination, *, progress, **_kwargs) -> None: + until = time.monotonic() + 0.2 + while time.monotonic() < until: + progress(sqlite3.SQLITE_OK, 10, 10) + time.sleep(0.01) + + def close(self) -> None: + pass + + class DestinationConnection: + def close(self) -> None: + pass + + connections = iter((SourceConnection(), DestinationConnection())) + monkeypatch.setattr( + backup.sqlite3, + "connect", + lambda *_args, **_kwargs: next(connections), + ) + + assert _safe_copy_db(src, dst, stall_timeout=0.05) is False + assert not dst.exists() + + +def test_locked_chrome_cache_is_skipped_without_losing_profile_state(tmp_path) -> None: + home = tmp_path / ".hermes" + profile = home / "profiles" / "coder" / "chrome-debug" / "Default" + gpu_cache = profile / "GPUCache" + gpu_cache.mkdir(parents=True) + (home / "config.yaml").write_text("model: test\n", encoding="utf-8") + (profile / "Cookies").write_bytes(b"persistent-session-state") + + cache_db = gpu_cache / "cache.db" + holder = sqlite3.connect(cache_db, timeout=0) + holder.execute("CREATE TABLE cache (key TEXT)") + holder.commit() + holder.execute("BEGIN EXCLUSIVE") + + archive = tmp_path / "pre-update.zip" + started = time.monotonic() + try: + result = _write_full_zip_backup(archive, home) + finally: + holder.rollback() + holder.close() + + assert result == archive + assert time.monotonic() - started < 2.0 + with zipfile.ZipFile(archive) as zf: + names = set(zf.namelist()) + assert "profiles/coder/chrome-debug/Default/Cookies" in names + assert "profiles/coder/chrome-debug/Default/GPUCache/cache.db" not in names