mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
fix(update): consolidate pre-update backups into one gated mechanism (#65754)
hermes update ran TWO separate pre-update backup mechanisms: the config-gated full zip (updates.pre_update_backup, default off) and an unconditional quick state snapshot added for #15733 that ignored the user's setting entirely. On a large state.db (observed: 24 GB) the 'cheap' snapshot silently added ~60s to every update and ate 24 GB of disk in state-snapshots/. Now there is ONE mechanism, gated by updates.pre_update_backup with three modes: - quick (new default): state snapshot of critical small files (pairing JSONs, cron jobs, config, auth, per-profile DBs). Files over 1 GiB are skipped with a warning so a bloated state.db can never stall the update again. - full: the quick snapshot plus the HERMES_HOME zip (old 'true' behavior; --backup forces it for one run). - off: nothing runs — an explicit opt-out now disables the quick snapshot too (--no-backup does the same per-run). Legacy booleans are honored: true -> full, false -> off. _run_pre_update_backup() now returns the quick-snapshot id so the post-update cron-jobs restore safety net (#34600) keeps working; the snapshot moved from the post-fetch site to the pre-mutation site, which also covers the zip-fallback update path it previously missed.
This commit is contained in:
parent
8462764367
commit
d0dcb9a5fd
9 changed files with 343 additions and 161 deletions
|
|
@ -794,18 +794,51 @@ def create_quick_snapshot(
|
|||
label: Optional[str] = None,
|
||||
hermes_home: Optional[Path] = None,
|
||||
keep: Optional[int] = None,
|
||||
max_file_size: Optional[int] = None,
|
||||
) -> Optional[str]:
|
||||
"""Create a quick state snapshot of critical files.
|
||||
|
||||
Copies STATE_FILES to a timestamped directory under state-snapshots/.
|
||||
Auto-prunes old snapshots beyond the keep limit.
|
||||
|
||||
Args:
|
||||
max_file_size: When set, individual files larger than this many bytes
|
||||
are skipped (with a printed warning) instead of copied. Used by
|
||||
the pre-update safety snapshot so a multi-GB ``state.db`` can
|
||||
never stall ``hermes update`` or silently eat disk — the small
|
||||
pairing/cron/config files the snapshot exists to protect are
|
||||
always captured. ``None`` (default) copies everything, which
|
||||
preserves manual ``/snapshot`` and ``hermes backup --quick``
|
||||
behavior.
|
||||
|
||||
Returns:
|
||||
Snapshot ID (timestamp-based), or None if no files found.
|
||||
"""
|
||||
home = hermes_home or get_hermes_home()
|
||||
root = _quick_snapshot_root(home)
|
||||
|
||||
def _too_large(path: Path, rel_name: str) -> bool:
|
||||
"""True (and warn) when ``path`` exceeds the max_file_size cap."""
|
||||
if max_file_size is None:
|
||||
return False
|
||||
try:
|
||||
size = path.stat().st_size
|
||||
except OSError:
|
||||
return False
|
||||
if size <= max_file_size:
|
||||
return False
|
||||
print(
|
||||
f" ⚠ Snapshot: skipping {rel_name} "
|
||||
f"({_format_size(size)} exceeds {_format_size(max_file_size)} limit)"
|
||||
)
|
||||
logger.warning(
|
||||
"Quick snapshot skipped %s: %d bytes exceeds %d byte limit",
|
||||
rel_name,
|
||||
size,
|
||||
max_file_size,
|
||||
)
|
||||
return True
|
||||
|
||||
ts = datetime.now(timezone.utc).strftime("%Y%m%d-%H%M%S")
|
||||
snap_id = f"{ts}-{label}" if label else ts
|
||||
snap_dir = root / snap_id
|
||||
|
|
@ -831,6 +864,8 @@ def create_quick_snapshot(
|
|||
# the board databases + their metadata to restore a board.
|
||||
if "/workspaces/" in f"/{sub_rel}/" or "/attachments/" in f"/{sub_rel}/":
|
||||
continue
|
||||
if _too_large(sub, sub_rel):
|
||||
continue
|
||||
dst = snap_dir / sub_rel
|
||||
dst.parent.mkdir(parents=True, exist_ok=True)
|
||||
try:
|
||||
|
|
@ -850,6 +885,9 @@ def create_quick_snapshot(
|
|||
if not src.is_file():
|
||||
continue
|
||||
|
||||
if _too_large(src, rel):
|
||||
continue
|
||||
|
||||
dst = snap_dir / rel
|
||||
dst.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
|
|
@ -1239,7 +1277,7 @@ def _prune_pre_update_backups(backup_dir: Path, keep: int) -> int:
|
|||
than no backup at all (and the wrapper in ``main.py`` would still print
|
||||
a misleading ``Saved: <path>`` line for a file that no longer exists).
|
||||
Operators who genuinely don't want a backup should set
|
||||
``updates.pre_update_backup: false`` in config — that gates creation.
|
||||
``updates.pre_update_backup: off`` in config — that gates creation.
|
||||
"""
|
||||
keep = max(keep, 1)
|
||||
if not backup_dir.exists():
|
||||
|
|
|
|||
|
|
@ -3110,21 +3110,29 @@ DEFAULT_CONFIG = {
|
|||
|
||||
# ``hermes update`` behaviour.
|
||||
"updates": {
|
||||
# Run a full ``hermes backup``-style zip of HERMES_HOME before every
|
||||
# ``hermes update``. Backups land in ``<HERMES_HOME>/backups/`` and
|
||||
# can be restored with ``hermes import <path>``. Off by default:
|
||||
# zipping a large HERMES_HOME (sessions DB, caches, skills) can add
|
||||
# minutes to every update. The #48200 incident — a ``hermes update
|
||||
# --yes`` run that computed a wrong path and silently wiped the
|
||||
# user's ``.env``, ``MEMORY.md``, ``kanban.db``, custom skills, and
|
||||
# scripts — is the reason this knob exists; enable it (here, or via
|
||||
# ``--backup`` for a single run) if you want that safety net.
|
||||
"pre_update_backup": False,
|
||||
# How many pre-update backup zips to retain. Older ones are pruned
|
||||
# automatically after each successful backup. Values below 1 are
|
||||
# floored to 1 — the backup just created is always preserved. To
|
||||
# disable backups entirely, set ``pre_update_backup: false`` above
|
||||
# rather than ``backup_keep: 0``.
|
||||
# Pre-update safety backup — ONE consolidated mechanism, three modes:
|
||||
#
|
||||
# quick (default) — snapshot critical small state files (pairing
|
||||
# JSONs, cron jobs, config.yaml, .env, auth.json, per-profile
|
||||
# DBs) into <HERMES_HOME>/state-snapshots/ before the update.
|
||||
# Files over 1 GiB (e.g. a bloated state.db) are skipped with a
|
||||
# warning so the snapshot stays fast. Restore via ``/snapshot``.
|
||||
# This is the #15733 (lost pairing data) / #34600 (emptied cron
|
||||
# jobs) safety net.
|
||||
# full — the quick snapshot PLUS a full ``hermes backup``-style zip
|
||||
# of HERMES_HOME into <HERMES_HOME>/backups/, restorable with
|
||||
# ``hermes import``. Can add minutes on large homes. This is the
|
||||
# #48200 (wrong-path wipe) safety net. ``--backup`` forces this
|
||||
# for a single run.
|
||||
# off — no pre-update backup of any kind. ``--no-backup`` forces
|
||||
# this for a single run.
|
||||
#
|
||||
# Legacy boolean values are honored: true -> full, false -> off.
|
||||
"pre_update_backup": "quick",
|
||||
# How many full pre-update backup zips to retain (mode ``full``).
|
||||
# Older ones are pruned automatically after each successful backup.
|
||||
# Values below 1 are floored to 1 — the backup just created is
|
||||
# always preserved. The quick snapshot always keeps exactly 1.
|
||||
"backup_keep": 5,
|
||||
# What `hermes update` does with uncommitted local changes to the
|
||||
# source tree when it runs NON-interactively — i.e. triggered from
|
||||
|
|
|
|||
|
|
@ -8908,23 +8908,30 @@ def _ensure_fhs_path_guard() -> None:
|
|||
print(" (reload your shell or run 'source ~/.bashrc' to pick it up)")
|
||||
|
||||
|
||||
def _run_pre_update_backup(args) -> None:
|
||||
"""Create a full zip backup of HERMES_HOME before running the update.
|
||||
_PRE_UPDATE_SNAPSHOT_KEEP = 1
|
||||
|
||||
Gated on ``updates.pre_update_backup`` in config (default false). Off
|
||||
by default because the zip can add minutes to every update on large
|
||||
HERMES_HOME directories. The ``--backup`` flag on ``hermes update``
|
||||
opts in for a single run; ``--no-backup`` forces it off when config
|
||||
has it enabled. Never raises — a backup failure should not block the
|
||||
update itself.
|
||||
# Per-file size cap for the pre-update quick snapshot. Anything larger is
|
||||
# skipped with a warning: the snapshot exists to protect small, hard-to-
|
||||
# regenerate state (pairing JSONs, cron jobs, config, auth) — not to copy a
|
||||
# multi-GB state.db on every update (observed: a 24 GB state.db added ~60s
|
||||
# of wall time and silently ate 24 GB of disk per update).
|
||||
_PRE_UPDATE_SNAPSHOT_MAX_FILE_SIZE = 1 << 30 # 1 GiB
|
||||
|
||||
|
||||
def _resolve_pre_update_backup_mode(args) -> str:
|
||||
"""Resolve the pre-update backup mode: ``"off"``, ``"quick"``, or ``"full"``.
|
||||
|
||||
CLI flags win over config; ``--no-backup`` beats ``--backup`` when both
|
||||
are set. Config accepts the mode strings plus legacy booleans:
|
||||
``true`` → ``full`` (the old zip behavior), ``false`` → ``off``
|
||||
(an explicit opt-out now disables the quick snapshot too — previously
|
||||
it ran unconditionally, ignoring the user's setting). A missing key
|
||||
defaults to ``quick``.
|
||||
"""
|
||||
# CLI flags win over config. --no-backup beats --backup if both are set.
|
||||
if getattr(args, "no_backup", False):
|
||||
print("◆ Pre-update backup: skipped (--no-backup)")
|
||||
print()
|
||||
return
|
||||
|
||||
force_backup = bool(getattr(args, "backup", False))
|
||||
return "off"
|
||||
if getattr(args, "backup", False):
|
||||
return "full"
|
||||
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
|
|
@ -8937,19 +8944,76 @@ def _run_pre_update_backup(args) -> None:
|
|||
cfg = {}
|
||||
|
||||
updates_cfg = cfg.get("updates", {}) if isinstance(cfg, dict) else {}
|
||||
# The default config ships with ``pre_update_backup: false`` (see
|
||||
# ``hermes_cli/config.py``). Fall back to false if the key is missing
|
||||
# so the default behaviour matches the shipped config: zipping a large
|
||||
# HERMES_HOME can add minutes to every update. Users who want the
|
||||
# #48200 safety net opt in via the config knob or ``--backup``.
|
||||
enabled = updates_cfg.get("pre_update_backup", False)
|
||||
keep = updates_cfg.get("backup_keep", 5)
|
||||
raw = updates_cfg.get("pre_update_backup", "quick")
|
||||
|
||||
if not enabled and not force_backup:
|
||||
# Silent by default — the backup is off, most users don't need to
|
||||
# hear about it on every update. They can opt in via --backup
|
||||
# or by flipping the config knob.
|
||||
return
|
||||
if raw is True:
|
||||
return "full"
|
||||
if raw is False:
|
||||
return "off"
|
||||
mode = str(raw).strip().lower()
|
||||
if mode in ("off", "false", "none", "disabled"):
|
||||
return "off"
|
||||
if mode in ("full", "zip", "true"):
|
||||
return "full"
|
||||
if mode == "quick":
|
||||
return "quick"
|
||||
logging.getLogger(__name__).warning(
|
||||
"Unknown updates.pre_update_backup value %r — using 'quick'", raw
|
||||
)
|
||||
return "quick"
|
||||
|
||||
|
||||
def _run_pre_update_backup(args) -> Optional[str]:
|
||||
"""Run the pre-update safety backup and return the quick-snapshot id.
|
||||
|
||||
Single consolidated mechanism gated on ``updates.pre_update_backup``:
|
||||
|
||||
- ``off`` — nothing runs. Explicit user opt-out is honored fully.
|
||||
- ``quick`` (default) — a state snapshot of critical small files
|
||||
(pairing JSONs, cron jobs, config, auth; see ``_QUICK_STATE_FILES``)
|
||||
under ``state-snapshots/``. Files over 1 GiB are skipped with a
|
||||
warning so a bloated state.db can never stall the update
|
||||
(issues #15733, #34600 are the reason this safety net exists).
|
||||
- ``full`` — the quick snapshot PLUS a full zip of HERMES_HOME under
|
||||
``backups/`` (restorable via ``hermes import``; the #48200 wrong-path
|
||||
wipe is the reason this level exists).
|
||||
|
||||
``--backup`` forces ``full`` for one run; ``--no-backup`` forces ``off``.
|
||||
Never raises — a backup failure should not block the update itself.
|
||||
|
||||
Returns the quick-snapshot id (used by the post-update cron-jobs
|
||||
restore safety net), or ``None`` when mode is ``off`` or the snapshot
|
||||
failed.
|
||||
"""
|
||||
mode = _resolve_pre_update_backup_mode(args)
|
||||
|
||||
if mode == "off":
|
||||
if getattr(args, "no_backup", False):
|
||||
print("◆ Pre-update backup: skipped (--no-backup)")
|
||||
print()
|
||||
# Config-level off is silent — the user opted out; don't spam them
|
||||
# on every update.
|
||||
return None
|
||||
|
||||
snapshot_id = None
|
||||
try:
|
||||
from hermes_cli.backup import create_quick_snapshot
|
||||
|
||||
snapshot_id = create_quick_snapshot(
|
||||
label="pre-update",
|
||||
keep=_PRE_UPDATE_SNAPSHOT_KEEP,
|
||||
max_file_size=_PRE_UPDATE_SNAPSHOT_MAX_FILE_SIZE,
|
||||
)
|
||||
if snapshot_id:
|
||||
print(f"◆ Pre-update snapshot: {snapshot_id}")
|
||||
except Exception as exc:
|
||||
# Never let a snapshot failure block an update.
|
||||
logging.getLogger(__name__).debug("Pre-update snapshot failed: %s", exc)
|
||||
|
||||
if mode != "full":
|
||||
if snapshot_id:
|
||||
print()
|
||||
return snapshot_id
|
||||
|
||||
try:
|
||||
from hermes_cli.backup import create_pre_update_backup
|
||||
|
|
@ -8958,24 +9022,31 @@ def _run_pre_update_backup(args) -> None:
|
|||
f"⚠ Pre-update backup: could not load backup module ({exc}); continuing update."
|
||||
)
|
||||
print()
|
||||
return
|
||||
return snapshot_id
|
||||
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
_keep = (load_config() or {}).get("updates", {}).get("backup_keep", 5)
|
||||
except Exception:
|
||||
_keep = 5
|
||||
|
||||
print("◆ Creating pre-update backup...")
|
||||
t0 = _time.monotonic()
|
||||
try:
|
||||
out_path = create_pre_update_backup(keep=int(keep))
|
||||
out_path = create_pre_update_backup(keep=int(_keep))
|
||||
except Exception as exc: # defensive — helper already swallows, but just in case
|
||||
print(f" ⚠ Backup failed: {exc}")
|
||||
print(" Continuing with update.")
|
||||
print()
|
||||
return
|
||||
return snapshot_id
|
||||
|
||||
elapsed = _time.monotonic() - t0
|
||||
|
||||
if out_path is None:
|
||||
print(" ⚠ Backup skipped (no files found or write failed); continuing update.")
|
||||
print()
|
||||
return
|
||||
return snapshot_id
|
||||
|
||||
try:
|
||||
size_bytes = out_path.stat().st_size
|
||||
|
|
@ -9004,9 +9075,9 @@ def _run_pre_update_backup(args) -> None:
|
|||
|
||||
print(f" Saved: {display_path} ({size_str}, {elapsed:.1f}s)")
|
||||
print(f" Restore: hermes import {out_path}")
|
||||
print(" Disable: omit --backup (backups are off by default)")
|
||||
print(" set updates.pre_update_backup: false in config.yaml")
|
||||
print(" Disable: set updates.pre_update_backup: quick (or off) in config.yaml")
|
||||
print()
|
||||
return snapshot_id
|
||||
|
||||
|
||||
def _write_update_planned_stop_marker(profile_path: Path, pid: int) -> bool:
|
||||
|
|
@ -9711,7 +9782,9 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
|
||||
# Pre-update backup — runs before any git/file mutation so users can
|
||||
# always roll back to the exact state they had before this update.
|
||||
_run_pre_update_backup(args)
|
||||
# Returns the quick-snapshot id (or None when disabled/failed); the
|
||||
# post-update cron-jobs safety net uses it to detect job loss.
|
||||
pre_update_snapshot_id = _run_pre_update_backup(args)
|
||||
|
||||
_windows_gateway_resume = _pause_windows_gateways_for_update()
|
||||
if _windows_gateway_resume:
|
||||
|
|
@ -9996,23 +10069,6 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
|
||||
print(f"→ Found {commit_count} new commit(s)")
|
||||
|
||||
# Snapshot critical state (state.db, config, pairing JSONs, etc.)
|
||||
# before pulling so a user can recover if something goes wrong.
|
||||
# Issue #15733 reported missing pairing data after an update; even
|
||||
# though `git pull` can't touch $HERMES_HOME, this is cheap
|
||||
# belt-and-suspenders insurance and gives the user something to
|
||||
# restore from via `/snapshot list` / `/snapshot restore <id>`.
|
||||
pre_update_snapshot_id = None
|
||||
try:
|
||||
from hermes_cli.backup import create_quick_snapshot
|
||||
|
||||
pre_update_snapshot_id = create_quick_snapshot(label="pre-update", keep=1)
|
||||
if pre_update_snapshot_id:
|
||||
print(f" ✓ Pre-update snapshot: {pre_update_snapshot_id}")
|
||||
except Exception as exc:
|
||||
# Never let a snapshot failure block an update.
|
||||
logger.debug("Pre-update snapshot failed: %s", exc)
|
||||
|
||||
print("→ Pulling updates...")
|
||||
update_succeeded = False
|
||||
# Capture the pre-pull SHA so we can auto-roll-back if the new code
|
||||
|
|
|
|||
|
|
@ -35,13 +35,13 @@ def build_update_parser(subparsers, *, cmd_update: Callable) -> None:
|
|||
"--no-backup",
|
||||
action="store_true",
|
||||
default=False,
|
||||
help="Skip the pre-update backup for this run (overrides updates.pre_update_backup)",
|
||||
help="Skip ALL pre-update backups for this run (both the quick state snapshot and the full zip; overrides updates.pre_update_backup)",
|
||||
)
|
||||
update_parser.add_argument(
|
||||
"--backup",
|
||||
action="store_true",
|
||||
default=False,
|
||||
help="Force a pre-update backup for this run (off by default; overrides updates.pre_update_backup=false)",
|
||||
help="Force a FULL pre-update backup (quick state snapshot + HERMES_HOME zip) for this run, regardless of updates.pre_update_backup",
|
||||
)
|
||||
update_parser.add_argument(
|
||||
"--yes",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue