diff --git a/hermes_cli/backup.py b/hermes_cli/backup.py index 20ddb3c87d..dce199a5ab 100644 --- a/hermes_cli/backup.py +++ b/hermes_cli/backup.py @@ -793,9 +793,17 @@ def _prune_pre_update_backups(backup_dir: Path, keep: int) -> int: Returns the number of files deleted. Only touches files matching ``pre-update-*.zip`` so hand-made zips dropped in the same directory are never touched. + + ``keep`` is floored to 1 because this helper is only called immediately + after a fresh backup is written: deleting that backup right after the + user paid the disk/CPU cost to create it would leave them worse off + than no backup at all (and the wrapper in ``main.py`` would still print + a misleading ``Saved: `` 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. """ - if keep < 0: - keep = 0 + if keep < 1: + keep = 1 if not backup_dir.exists(): return 0 diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 98317a9043..0f34d98528 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1286,7 +1286,10 @@ DEFAULT_CONFIG = { # for a single update run. "pre_update_backup": False, # How many pre-update backup zips to retain. Older ones are pruned - # automatically after each successful backup. + # 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``. "backup_keep": 5, }, diff --git a/tests/hermes_cli/test_backup.py b/tests/hermes_cli/test_backup.py index 9a99a035fa..ab7ba21370 100644 --- a/tests/hermes_cli/test_backup.py +++ b/tests/hermes_cli/test_backup.py @@ -1374,6 +1374,53 @@ class TestPreUpdateBackup: from hermes_cli.backup import create_pre_update_backup assert create_pre_update_backup(hermes_home=tmp_path / "does-not-exist") is None + def test_keep_zero_does_not_delete_freshly_created_backup(self, hermes_home): + """Regression: ``backup_keep: 0`` previously triggered ``backups[0:]`` + in the pruner — wiping the just-created zip and leaving the user + with no recovery point. The floor (keep>=1) preserves the new file + regardless of misconfiguration; users who don't want backups should + set ``pre_update_backup: false`` instead. + """ + from hermes_cli.backup import create_pre_update_backup + out = create_pre_update_backup(hermes_home=hermes_home, keep=0) + assert out is not None + assert out.exists(), ( + "keep=0 silently deleted the freshly-created backup; floor " + "should preserve the just-written file." + ) + + def test_keep_negative_does_not_delete_freshly_created_backup(self, hermes_home): + """Mirror coverage: any value <1 should be floored, not literally + applied as a slice index.""" + from hermes_cli.backup import create_pre_update_backup + out = create_pre_update_backup(hermes_home=hermes_home, keep=-3) + assert out is not None + assert out.exists() + + def test_keep_zero_still_prunes_older_backups(self, hermes_home): + """The floor preserves the new backup but should NOT regress the + rotation behaviour for older zips: a third call with keep=0 must + still remove pre-existing backups beyond the (floored) limit of 1. + """ + import time as _t + from hermes_cli.backup import create_pre_update_backup + + first = create_pre_update_backup(hermes_home=hermes_home, keep=5) + _t.sleep(1.05) + second = create_pre_update_backup(hermes_home=hermes_home, keep=5) + _t.sleep(1.05) + third = create_pre_update_backup(hermes_home=hermes_home, keep=0) + + remaining = { + p.name for p in (hermes_home / "backups").iterdir() + if p.name.startswith("pre-update-") + } + assert third.name in remaining, "Floor must preserve the new backup" + assert first.name not in remaining and second.name not in remaining, ( + f"keep=0 floor of 1 should still prune older backups; " + f"remaining={remaining}" + ) + class TestRunPreUpdateBackup: """Tests for the ``_run_pre_update_backup`` wrapper in main.py —