hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 28 Packages Projects Releases Wiki Activity

fix(backup): restore state.db through SQLite backup API so live connections see restored data (#65942)

Browse Source
This commit is contained in:
webtecnica
2026-07-16 18:50:19 -03:00
committed by Teknium
parent bc21808e6a
commit 88d55f31e4
3 changed files with 117 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
hermes_cli/backup.py
View File

@@ -635,6 +635,67 @@ def copy_db_and_verify(src: Path, dst: Path) -> bool:
return True
def _safe_restore_db(src: Path, dst: Path) -> bool:
"""Restore a SQLite database from snapshot *src* into live *dst*.
Uses SQLite's backup() API to write snapshot pages into the live
database file, preserving the file's inode and WAL state so that
any other process still holding the DB open (gateway, dashboard,
another CLI session) sees the restored data on the next read —
instead of continuing to serve stale cached pages from a replaced
inode.
The old approach was ``unlink() + move()``, which replaced the file
under any live connection. SQLite connections cache pages in
per-connection page caches keyed by inode; after an unlink+move the
old inode still existed (the live connection held a reference), so
that connection continued serving the pre-restore data while new
connections saw the restored snapshot — a partial/inconsistent
state (issue #65942).
By writing pages through the backup API the file inode is preserved,
the WAL journal is updated correctly, and all connections (old and
new) converge on the restored data.
Falls back to the unlink+move approach on failure so restore never
blocks on a transient error.
"""
try:
dst_conn = sqlite3.connect(str(dst))
try:
# Force a WAL checkpoint so the backup starts from a clean
# state rather than writing on top of a deep WAL.
dst_conn.execute("PRAGMA wal_checkpoint(TRUNCATE)")
except Exception:
pass
src_conn = sqlite3.connect(f"file:{src}?mode=ro", uri=True)
try:
src_conn.backup(dst_conn)
finally:
src_conn.close()
dst_conn.close()
# Restore original file permissions from the snapshot
try:
mode = src.stat().st_mode
dst.chmod(mode)
except Exception:
pass
return True
except Exception as exc:
logger.warning("SQLite safe restore failed for %s -> %s: %s", src, dst, exc)
# Fallback: unlink+move (the old approach). This still works for
# the common case where no other process holds the DB open.
try:
tmp = dst.parent / f".{dst.name}.snap_restore"
shutil.copy2(src, tmp)
dst.unlink(missing_ok=True)
shutil.move(str(tmp), str(dst))
return True
except Exception as exc2:
logger.error("Fallback restore also failed for %s -> %s: %s", src, dst, exc2)
return False
# ---------------------------------------------------------------------------
# Backup
# ---------------------------------------------------------------------------
@@ -1663,11 +1724,11 @@ def restore_quick_snapshot(
try:
if dst.suffix == ".db":
# Atomic-ish replace for databases
tmp = dst.parent / f".{dst.name}.snap_restore"
shutil.copy2(src, tmp)
dst.unlink(missing_ok=True)
shutil.move(str(tmp), str(dst))
# Restore through SQLite backup API so live connections
# (gateway, dashboard, another CLI session) see the
# restored data instead of continuing to serve stale
# cached pages from a replaced inode (issue #65942).
_safe_restore_db(src, dst)
else:
shutil.copy2(src, dst)
restored += 1

17
hermes_cli/cli_commands_mixin.py
View File

@@ -385,9 +385,24 @@ class CLICommandsMixin:
return
except ValueError:
pass
# Close our local SessionDB connection before restore so the
# backup-API restore doesn't contend with a live connection to
# state.db from this same process (issue #65942).
local_session_db = getattr(self, "_session_db", None)
if local_session_db is not None:
try:
local_session_db.close()
self._session_db = None
except Exception:
pass
if restore_quick_snapshot(snap_id):
print(f" Restored state from: {snap_id}")
print(" Restart recommended for state.db changes to take effect.")
print(
" Restart recommended for gateway/dashboard processes "
"to pick up state.db changes."
)
else:
print(f" Snapshot not found: {snap_id}")

35
tests/hermes_cli/test_backup.py
View File

@@ -1248,6 +1248,41 @@ class TestQuickSnapshot:
assert "state.db" not in data.get("files", {})
assert "state.db" in data.get("failed_dbs", [])
def test_restore_state_db_live_connection(self, hermes_home):
"""Restoring state.db must update data visible through a live connection.
Regression test for #65942: when state.db is open with a live SQLite
connection (as happens with the gateway, dashboard, or another CLI
session), the restore must write pages through the backup API so the
live connection sees the restored data instead of stale cached pages
from a replaced inode.
"""
from hermes_cli.backup import create_quick_snapshot, restore_quick_snapshot
snap_id = create_quick_snapshot(hermes_home=hermes_home)
# Open a live connection (simulating gateway/dashboard).
live_conn = sqlite3.connect(str(hermes_home / "state.db"))
live_conn.execute("PRAGMA journal_mode=wal")
# Insert data AFTER the snapshot — this is what must be reverted.
live_conn.execute("INSERT INTO sessions VALUES ('s2', 'new-data')")
live_conn.commit()
rows_before = live_conn.execute("SELECT * FROM sessions").fetchall()
assert len(rows_before) == 2
# Restore — the live connection stays open during restore.
result = restore_quick_snapshot(snap_id, hermes_home=hermes_home)
assert result is True
# The live connection must see the restored (single-row) state.
# A fresh connection would trivially work; the live one is the test.
rows_after = live_conn.execute("SELECT * FROM sessions").fetchall()
live_conn.close()
assert len(rows_after) == 1, (
f"Live connection still sees {len(rows_after)} rows after restore "
f"(expected 1); the extra row 's2' should have been reverted."
)