diff --git a/cli.py b/cli.py index b3e1c2fb8cb1..f4a1beb147f0 100644 --- a/cli.py +++ b/cli.py @@ -1927,10 +1927,15 @@ def _run_checkpoint_auto_maintenance() -> None: if not cfg.get("auto_prune", False): return from tools.checkpoint_manager import maybe_auto_prune_checkpoints + # delete_orphans is intentionally never honoured here: a missing + # workdir at startup is ambiguous (deleted project vs. an unmounted + # external volume / network share / VPN not yet up) and this sweep + # runs unattended. Orphan cleanup is only ever done via the explicit + # `hermes checkpoints prune` command, which the user has to invoke. maybe_auto_prune_checkpoints( retention_days=int(cfg.get("retention_days", 7)), min_interval_hours=int(cfg.get("min_interval_hours", 24)), - delete_orphans=bool(cfg.get("delete_orphans", True)), + delete_orphans=False, max_total_size_mb=int(cfg.get("max_total_size_mb", 500)), ) except Exception as exc: diff --git a/gateway/run.py b/gateway/run.py index dbfc3326cd05..1c67ac3bc737 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -3415,18 +3415,24 @@ def __init__(self, config: Optional[GatewayConfig] = None): except Exception as exc: logger.debug("state.db auto-maintenance skipped: %s", exc) - # Opportunistic shadow-repo cleanup — deletes orphan/stale - # checkpoint repos under ~/.hermes/checkpoints/. Opt-in via - # checkpoints.auto_prune, idempotent via .last_prune marker. + # Opportunistic shadow-repo cleanup — deletes stale checkpoint repos + # under ~/.hermes/checkpoints/. Opt-in via checkpoints.auto_prune, + # idempotent via .last_prune marker. try: from hermes_cli.config import load_config as _load_full_config _ckpt_cfg = (_load_full_config().get("checkpoints") or {}) if _ckpt_cfg.get("auto_prune", False): from tools.checkpoint_manager import maybe_auto_prune_checkpoints + # delete_orphans is intentionally never honoured here: a + # missing workdir at startup is ambiguous (deleted project + # vs. an unmounted external volume / network share / VPN + # not yet up) and this sweep runs unattended. Orphan cleanup + # is only ever done via the explicit `hermes checkpoints + # prune` command, which the user has to invoke. maybe_auto_prune_checkpoints( retention_days=int(_ckpt_cfg.get("retention_days", 7)), min_interval_hours=int(_ckpt_cfg.get("min_interval_hours", 24)), - delete_orphans=bool(_ckpt_cfg.get("delete_orphans", True)), + delete_orphans=False, max_total_size_mb=int(_ckpt_cfg.get("max_total_size_mb", 500)), ) except Exception as exc: diff --git a/hermes_cli/checkpoints.py b/hermes_cli/checkpoints.py index 2975553ae495..a5cdbe6daed3 100644 --- a/hermes_cli/checkpoints.py +++ b/hermes_cli/checkpoints.py @@ -109,20 +109,49 @@ def cmd_list(args: argparse.Namespace) -> int: def cmd_prune(args: argparse.Namespace) -> int: - from tools.checkpoint_manager import prune_checkpoints + from tools.checkpoint_manager import prune_checkpoints, store_status retention_days = args.retention_days max_size_mb = args.max_size_mb + delete_orphans = not args.keep_orphans + + if delete_orphans and not args.force: + info = store_status() + orphans = [ + p for p in info.get("projects", []) + if not p.get("exists") + ] + pre_v2_orphans = [ + p for p in info.get("pre_v2_projects", []) + if not p.get("exists") + ] + if orphans or pre_v2_orphans: + print(f"This will permanently delete {len(orphans) + len(pre_v2_orphans)} " + "orphan checkpoint project(s) whose workdir is not currently reachable:") + print() + for p in orphans: + wd = p.get("workdir") or "(unknown)" + print(f" {wd} ({p.get('commits', 0)} commit(s))") + for p in pre_v2_orphans: + wd = p.get("workdir") or "(unknown)" + print(f" {wd} (pre-v2 shadow repo)") + print() + print("A workdir can be unreachable because the project was deleted,") + print("or because an external volume / network share / VPN is down.") + print("Pass --keep-orphans to prune stale entries only.") + if not _confirm("Delete these orphan projects?"): + print("Aborted.") + return 1 print("Pruning checkpoint store…") print(f" retention_days: {retention_days}") - print(f" delete_orphans: {not args.keep_orphans}") + print(f" delete_orphans: {delete_orphans}") print(f" max_total_size_mb: {max_size_mb}") print() result = prune_checkpoints( retention_days=retention_days, - delete_orphans=not args.keep_orphans, + delete_orphans=delete_orphans, max_total_size_mb=max_size_mb, ) print(f"Scanned: {result['scanned']}") @@ -225,6 +254,8 @@ def register_cli(parser: argparse.ArgumentParser) -> None: "per project until total size <= this (default 500)") p_prune.add_argument("--keep-orphans", action="store_true", help="Skip deleting projects whose workdir no longer exists") + p_prune.add_argument("-f", "--force", action="store_true", + help="Skip the orphan-deletion confirmation prompt") p_prune.set_defaults(func=cmd_prune) p_clear = subs.add_parser( diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 23c1b7c66e31..4203bd27c01a 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1369,15 +1369,22 @@ def _ensure_hermes_home_managed(home: Path): "max_file_size_mb": 10, # Auto-maintenance: hermes sweeps the checkpoint base at startup # (at most once per ``min_interval_hours``) and: - # * deletes project entries whose workdir no longer exists (orphan) # * deletes project entries whose last_touch is older than # ``retention_days`` # * GCs the single shared store to reclaim unreachable objects # * enforces ``max_total_size_mb`` across remaining projects # * deletes ``legacy-*`` archives older than ``retention_days`` + # + # NOTE: this automatic sweep never deletes "orphan" entries (workdir + # no longer found on disk). A missing workdir at startup is + # ambiguous — it can mean the project was deleted, or that an + # external volume / network share / VPN is simply not mounted yet — + # and this sweep runs unattended, so it must never guess. Orphan + # cleanup is only available via the explicit + # ``hermes checkpoints prune`` command (add ``--keep-orphans`` to + # skip it), where a human is looking at the output. "auto_prune": True, "retention_days": 7, - "delete_orphans": True, "min_interval_hours": 24, }, diff --git a/tests/hermes_cli/test_checkpoints_prune.py b/tests/hermes_cli/test_checkpoints_prune.py new file mode 100644 index 000000000000..d597bc86f9c2 --- /dev/null +++ b/tests/hermes_cli/test_checkpoints_prune.py @@ -0,0 +1,208 @@ +"""Tests for `hermes checkpoints prune`'s orphan confirmation flow. + +Covers the P1 raised on PR #69141: the confirmation preview must cover +BOTH v2 projects (`store_status()["projects"]`) and pre-v2 shadow repos +(`store_status()["pre_v2_projects"]`), since `prune_checkpoints()` deletes +orphans from both layouts. Exercises decline / accept / --force across +pre-v2-only and mixed (v2 + pre-v2) stores. +""" + +from __future__ import annotations + +import argparse + +import pytest + + +def _ns(**kwargs) -> argparse.Namespace: + defaults = {"retention_days": 7, "max_size_mb": 500, "keep_orphans": False, "force": False} + defaults.update(kwargs) + return argparse.Namespace(**defaults) + + +def _prune_result(**kwargs) -> dict: + result = {"scanned": 0, "deleted_orphan": 0, "deleted_stale": 0, "errors": 0, "bytes_freed": 0} + result.update(kwargs) + return result + + +_V2_ORPHAN_ONLY_STATUS = { + "projects": [], + "pre_v2_projects": [], +} + +_PRE_V2_ONLY_STATUS = { + "projects": [], + "pre_v2_projects": [ + {"path": "/home/user/.hermes/checkpoints/deadbeefcafebabe", "workdir": None, "exists": False}, + ], +} + +_MIXED_STATUS = { + "projects": [ + {"hash": "abc123", "workdir": "/gone/v2-project", "exists": False, "commits": 4}, + ], + "pre_v2_projects": [ + {"path": "/home/user/.hermes/checkpoints/deadbeefcafebabe", "workdir": "/gone/pre-v2-project", "exists": False}, + ], +} + + +def _patch_checkpoint_manager(monkeypatch, status: dict, prune_calls: list): + import tools.checkpoint_manager as ckpt_mgr + + monkeypatch.setattr(ckpt_mgr, "store_status", lambda *a, **k: status) + + def _fake_prune(**kwargs): + prune_calls.append(kwargs) + return _prune_result( + deleted_orphan=len(status["projects"]) + len(status["pre_v2_projects"]), + ) + + monkeypatch.setattr(ckpt_mgr, "prune_checkpoints", _fake_prune) + + +# ─── pre-v2-only store ────────────────────────────────────────────────────── + + +def test_pre_v2_only_decline_aborts_without_deleting(monkeypatch, capsys): + import hermes_cli.checkpoints as checkpoints_cli + + prune_calls: list = [] + _patch_checkpoint_manager(monkeypatch, _PRE_V2_ONLY_STATUS, prune_calls) + monkeypatch.setattr("builtins.input", lambda _prompt: "n") + + rc = checkpoints_cli.cmd_prune(_ns()) + + assert rc == 1 + assert prune_calls == [] + out = capsys.readouterr().out + assert "pre-v2 shadow repo" in out + assert "Aborted" in out + + +def test_pre_v2_only_accept_deletes(monkeypatch, capsys): + import hermes_cli.checkpoints as checkpoints_cli + + prune_calls: list = [] + _patch_checkpoint_manager(monkeypatch, _PRE_V2_ONLY_STATUS, prune_calls) + monkeypatch.setattr("builtins.input", lambda _prompt: "y") + + rc = checkpoints_cli.cmd_prune(_ns()) + + assert rc == 0 + assert len(prune_calls) == 1 + assert prune_calls[0]["delete_orphans"] is True + + +def test_pre_v2_only_force_skips_prompt(monkeypatch, capsys): + import hermes_cli.checkpoints as checkpoints_cli + + prune_calls: list = [] + _patch_checkpoint_manager(monkeypatch, _PRE_V2_ONLY_STATUS, prune_calls) + + def _unexpected_input(_prompt): + raise AssertionError("input() must not be called when --force is passed") + + monkeypatch.setattr("builtins.input", _unexpected_input) + + rc = checkpoints_cli.cmd_prune(_ns(force=True)) + + assert rc == 0 + assert len(prune_calls) == 1 + + +# ─── mixed store (v2 + pre-v2) ────────────────────────────────────────────── + + +def test_mixed_store_decline_aborts_without_deleting(monkeypatch, capsys): + import hermes_cli.checkpoints as checkpoints_cli + + prune_calls: list = [] + _patch_checkpoint_manager(monkeypatch, _MIXED_STATUS, prune_calls) + monkeypatch.setattr("builtins.input", lambda _prompt: "n") + + rc = checkpoints_cli.cmd_prune(_ns()) + + assert rc == 1 + assert prune_calls == [] + out = capsys.readouterr().out + # Both layouts must appear in the preview, not just the v2 one. + assert "/gone/v2-project" in out + assert "/gone/pre-v2-project" in out + assert "This will permanently delete 2 orphan checkpoint project(s)" in out + + +def test_mixed_store_accept_deletes_both_layouts(monkeypatch, capsys): + import hermes_cli.checkpoints as checkpoints_cli + + prune_calls: list = [] + _patch_checkpoint_manager(monkeypatch, _MIXED_STATUS, prune_calls) + monkeypatch.setattr("builtins.input", lambda _prompt: "y") + + rc = checkpoints_cli.cmd_prune(_ns()) + + assert rc == 0 + assert len(prune_calls) == 1 + out = capsys.readouterr().out + assert "Deleted orphan: 2" in out + + +def test_mixed_store_force_skips_prompt_deletes_both(monkeypatch, capsys): + import hermes_cli.checkpoints as checkpoints_cli + + prune_calls: list = [] + _patch_checkpoint_manager(monkeypatch, _MIXED_STATUS, prune_calls) + + def _unexpected_input(_prompt): + raise AssertionError("input() must not be called when --force is passed") + + monkeypatch.setattr("builtins.input", _unexpected_input) + + rc = checkpoints_cli.cmd_prune(_ns(force=True)) + + assert rc == 0 + assert len(prune_calls) == 1 + assert prune_calls[0]["delete_orphans"] is True + + +# ─── --keep-orphans skips the prompt entirely, on either layout ─────────── + + +@pytest.mark.parametrize("status", [_PRE_V2_ONLY_STATUS, _MIXED_STATUS], ids=["pre_v2_only", "mixed"]) +def test_keep_orphans_skips_prompt(monkeypatch, capsys, status): + import hermes_cli.checkpoints as checkpoints_cli + + prune_calls: list = [] + _patch_checkpoint_manager(monkeypatch, status, prune_calls) + + def _unexpected_input(_prompt): + raise AssertionError("input() must not be called when --keep-orphans is passed") + + monkeypatch.setattr("builtins.input", _unexpected_input) + + rc = checkpoints_cli.cmd_prune(_ns(keep_orphans=True)) + + assert rc == 0 + assert len(prune_calls) == 1 + assert prune_calls[0]["delete_orphans"] is False + + +# ─── no orphans present: never prompts even without --force ─────────────── + + +def test_no_orphans_skips_prompt(monkeypatch, capsys): + import hermes_cli.checkpoints as checkpoints_cli + + prune_calls: list = [] + _patch_checkpoint_manager(monkeypatch, _V2_ORPHAN_ONLY_STATUS, prune_calls) + + def _unexpected_input(_prompt): + raise AssertionError("input() must not be called when there are no orphans") + + monkeypatch.setattr("builtins.input", _unexpected_input) + + rc = checkpoints_cli.cmd_prune(_ns()) + + assert rc == 0 + assert len(prune_calls) == 1 diff --git a/tools/checkpoint_manager.py b/tools/checkpoint_manager.py index b5ce04c6a64e..919655d3c8d7 100644 --- a/tools/checkpoint_manager.py +++ b/tools/checkpoint_manager.py @@ -545,6 +545,39 @@ def _list_projects(store: Path) -> List[Dict]: return out +def _pre_v2_shadow_repos(base: Path) -> List[Dict]: + """Return pre-v2 per-project shadow repos still directly under ``base``. + + Pre-v2 layout kept one shadow git repo per working directory directly + under ``CHECKPOINT_BASE`` (identified by a ``HEAD`` file). This is the + single source of truth for that scan so a preview built from it (e.g. + ``store_status``) always matches what ``prune_checkpoints`` deletes. + """ + out: List[Dict] = [] + if not base.exists(): + return out + for child in base.iterdir(): + if not child.is_dir(): + continue + if child.name == _STORE_DIRNAME or child.name.startswith(_LEGACY_PREFIX): + continue + if not (child / "HEAD").exists(): + continue + workdir: Optional[str] = None + wd_marker = child / "HERMES_WORKDIR" + if wd_marker.exists(): + try: + workdir = wd_marker.read_text(encoding="utf-8").strip() + except (OSError, UnicodeDecodeError): + workdir = None + out.append({ + "path": child, + "workdir": workdir, + "exists": bool(workdir) and Path(workdir).exists(), + }) + return out + + def _dir_file_count(path: str) -> int: """Quick file count estimate (stops early if over _MAX_FILES).""" count = 0 @@ -1325,22 +1358,16 @@ def prune_checkpoints( except OSError as exc: result["errors"] += 1 logger.warning("Failed to delete legacy archive %s: %s", child, exc) - continue - # Only count as a pre-v2 shadow repo if it has a HEAD. - if not (child / "HEAD").exists(): - continue + + # Pre-v2 per-project shadow repos. Scanned via the same helper + # `store_status()` uses for its orphan preview, so a confirmation prompt + # built from that preview always matches what gets deleted here. + for repo in _pre_v2_shadow_repos(base): + child = repo["path"] result["scanned"] += 1 reason: Optional[str] = None - if delete_orphans: - workdir: Optional[str] = None - wd_marker = child / "HERMES_WORKDIR" - if wd_marker.exists(): - try: - workdir = wd_marker.read_text(encoding="utf-8").strip() - except (OSError, UnicodeDecodeError): - workdir = None - if workdir is None or not Path(workdir).exists(): - reason = "orphan" + if delete_orphans and not repo["exists"]: + reason = "orphan" if reason is None and retention_days > 0: newest = 0.0 try: @@ -1572,7 +1599,14 @@ def store_status(checkpoint_base: Optional[Path] = None) -> Dict: ``{"base": path, "store_size_bytes": N, "legacy_size_bytes": N, "total_size_bytes": N, "project_count": N, "projects": [...], - "legacy_archives": [...]}`` + "pre_v2_projects": [...], "legacy_archives": [...]}`` + + ``pre_v2_projects`` covers shadow repos still on the pre-v2 per-project + layout (``base//HEAD``) — distinct from ``legacy_archives``, which + are already-migrated ``legacy-/`` dirs. Callers that preview an + orphan-deletion sweep must include both ``projects`` and + ``pre_v2_projects``, since ``prune_checkpoints`` deletes orphans from + both layouts. """ base = checkpoint_base or CHECKPOINT_BASE out: Dict = { @@ -1582,6 +1616,7 @@ def store_status(checkpoint_base: Optional[Path] = None) -> Dict: "total_size_bytes": 0, "project_count": 0, "projects": [], + "pre_v2_projects": [], "legacy_archives": [], } if not base.exists(): @@ -1613,6 +1648,15 @@ def store_status(checkpoint_base: Optional[Path] = None) -> Dict: }) out["project_count"] = len(out["projects"]) + out["pre_v2_projects"] = [ + { + "path": str(r["path"]), + "workdir": r["workdir"], + "exists": r["exists"], + } + for r in _pre_v2_shadow_repos(base) + ] + for child in base.iterdir(): if child.is_dir() and child.name.startswith(_LEGACY_PREFIX): try: diff --git a/website/docs/user-guide/checkpoints-and-rollback.md b/website/docs/user-guide/checkpoints-and-rollback.md index 1393060612e2..a71ec2fe5b79 100644 --- a/website/docs/user-guide/checkpoints-and-rollback.md +++ b/website/docs/user-guide/checkpoints-and-rollback.md @@ -94,12 +94,15 @@ checkpoints: max_file_size_mb: 10 # skip any single file larger than this # Auto-maintenance (on by default): sweep ~/.hermes/checkpoints/ at startup - # and delete project entries whose working directory no longer exists - # (orphans) or whose last_touch is older than retention_days. Runs at most - # once per min_interval_hours, tracked via a .last_prune marker. + # and delete project entries whose last_touch is older than retention_days. + # Runs at most once per min_interval_hours, tracked via a .last_prune + # marker. This sweep never deletes "orphan" entries (working directory not + # found) — a missing workdir at startup is ambiguous (deleted project vs. + # an unmounted external volume / network share / VPN not yet up), so + # orphan cleanup is only ever done via the explicit + # `hermes checkpoints prune` command below, with a confirmation prompt. auto_prune: true retention_days: 7 - delete_orphans: true min_interval_hours: 24 ``` diff --git a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/checkpoints-and-rollback.md b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/checkpoints-and-rollback.md index 472b30f930ab..dec3ddb259d9 100644 --- a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/checkpoints-and-rollback.md +++ b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/checkpoints-and-rollback.md @@ -94,12 +94,14 @@ checkpoints: max_file_size_mb: 10 # 跳过大于此值的单个文件 # 自动维护(默认开启):启动时扫描 ~/.hermes/checkpoints/, - # 删除工作目录已不存在的项目条目(孤立项)或 last_touch 超过 - # retention_days 的条目。通过 .last_prune 标记控制, - # 最多每 min_interval_hours 运行一次。 + # 删除 last_touch 超过 retention_days 的条目。通过 .last_prune + # 标记控制,最多每 min_interval_hours 运行一次。此扫描不会删除 + # “孤立”条目(工作目录未找到)——启动时工作目录缺失含义模糊 + # (项目被删除,还是外部卷/网络共享/VPN 尚未挂载),因此孤立项 + # 清理只能通过下方的 `hermes checkpoints prune` 命令显式触发, + # 并会要求确认。 auto_prune: true retention_days: 7 - delete_orphans: true min_interval_hours: 24 ```