mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-25 17:18:11 +00:00
fix(checkpoints): don't prune a project whose volume is merely unmounted
Orphan pruning decides a project is gone from a single probe:
if delete_orphans and (not workdir or not Path(workdir).exists()):
reason = "orphan"
then deletes its ref, index, and metadata — the project's entire checkpoint
history. `Path.exists()` is False for a deleted directory, but it is equally
False for one whose storage is not attached right now: an unplugged external
drive, a share behind a downed VPN, a bind-mount absent from this container,
an offline Windows mapped drive. The project is fine; only our view of it is.
This is not an opt-in maintenance command. `maybe_auto_prune_checkpoints`
runs unattended at startup from both `cli.py` and `gateway/run.py`, with
`delete_orphans=True` by default. So starting Hermes once while the drive is
unplugged silently destroys the restore points for every project on it — the
one thing checkpoints exist to provide, and there is nothing to restore from
afterwards.
Reproduced against the real store: a project registered under an unmounted
path and one on local disk, then a startup prune —
prune: {'scanned': 2, 'deleted_orphan': 1}
unreachable project index still on disk: False
The legacy pre-v2 branch has the same flaw plus a second one: a
`HERMES_WORKDIR` marker that exists but cannot be read leaves `workdir = None`,
which the same condition treats as an orphan. Failing to read a file is not
evidence that a project was deleted.
Require corroboration before deleting: the workdir's parent must be present,
so its absence is something we actually observed. A missing parent means the
volume is not there and we know nothing, so the entry is left alone — and an
unreadable marker never deletes at all. Genuinely abandoned projects are still
reclaimed, both by the unchanged orphan path (parent present, project gone)
and by the retention/stale rule, which runs off `last_touch` rather than a
filesystem probe.
tests/tools/test_checkpoint_manager.py: a project whose whole mount disappears
keeps its history; controls prove a genuinely deleted project is still pruned
and a live project is untouched. The data-loss test fails on main; both
controls pass there. 81 passed across the checkpoint suites (2 failures in
test_checkpoint_manager.py are pre-existing and fail identically on clean
main).
This commit is contained in:
parent
a979ca2a67
commit
7d4e272cdf
2 changed files with 133 additions and 2 deletions
|
|
@ -4,6 +4,7 @@ import argparse
|
|||
import json
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import time
|
||||
import pytest
|
||||
|
|
@ -1156,3 +1157,89 @@ class TestClearFunctions:
|
|||
result = clear_all()
|
||||
assert result["deleted"] is False
|
||||
assert result["bytes_freed"] == 0
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# Orphan pruning must not act on an unreachable volume
|
||||
# =========================================================================
|
||||
|
||||
class TestOrphanPruneRequiresObservableDeletion:
|
||||
"""A missing workdir is ambiguous: deleted, or just not mounted right now.
|
||||
|
||||
Orphan pruning deletes a project's whole checkpoint history and runs
|
||||
unattended at startup (``maybe_auto_prune_checkpoints`` from the CLI and
|
||||
the gateway), so it must only fire when the deletion is something we
|
||||
actually observed — the parent directory present, the project gone. An
|
||||
unplugged drive, a share behind a downed VPN, or a bind-mount absent from
|
||||
this container must not cost the user their restore points.
|
||||
"""
|
||||
|
||||
def _project_with_history(self, work_dir, checkpoint_base, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
"tools.checkpoint_manager.CHECKPOINT_BASE", checkpoint_base
|
||||
)
|
||||
m = CheckpointManager(enabled=True, max_snapshots=10)
|
||||
m.ensure_checkpoint(str(work_dir), "initial")
|
||||
return m
|
||||
|
||||
def _history_survives(self, checkpoint_base, work_dir):
|
||||
store = _store_path(checkpoint_base)
|
||||
return _project_meta_path(
|
||||
store, _project_hash(str(work_dir))
|
||||
).exists()
|
||||
|
||||
def test_unreachable_volume_keeps_its_checkpoints(
|
||||
self, tmp_path, checkpoint_base, monkeypatch,
|
||||
):
|
||||
"""The whole mount is absent (parent missing too) → keep the history."""
|
||||
mount = tmp_path / "mnt" / "nas"
|
||||
work_dir = mount / "work" / "proj"
|
||||
work_dir.mkdir(parents=True)
|
||||
(work_dir / "main.py").write_text("print('x')\n")
|
||||
self._project_with_history(work_dir, checkpoint_base, monkeypatch)
|
||||
|
||||
# The volume goes away — project AND its parents disappear together.
|
||||
shutil.rmtree(tmp_path / "mnt")
|
||||
assert not work_dir.exists()
|
||||
|
||||
prune_checkpoints(
|
||||
retention_days=0, delete_orphans=True, checkpoint_base=checkpoint_base,
|
||||
)
|
||||
|
||||
assert self._history_survives(checkpoint_base, work_dir), (
|
||||
"checkpoint history was deleted for a project whose volume was "
|
||||
"merely unmounted"
|
||||
)
|
||||
|
||||
def test_genuinely_deleted_project_is_still_pruned(
|
||||
self, tmp_path, checkpoint_base, monkeypatch,
|
||||
):
|
||||
"""Control: parent present, project gone → a real orphan, still pruned."""
|
||||
parent = tmp_path / "projects"
|
||||
work_dir = parent / "proj"
|
||||
work_dir.mkdir(parents=True)
|
||||
(work_dir / "main.py").write_text("print('x')\n")
|
||||
self._project_with_history(work_dir, checkpoint_base, monkeypatch)
|
||||
|
||||
shutil.rmtree(work_dir)
|
||||
assert parent.exists() and not work_dir.exists()
|
||||
|
||||
result = prune_checkpoints(
|
||||
retention_days=0, delete_orphans=True, checkpoint_base=checkpoint_base,
|
||||
)
|
||||
|
||||
assert result["deleted_orphan"] >= 1
|
||||
assert not self._history_survives(checkpoint_base, work_dir)
|
||||
|
||||
def test_live_project_is_never_pruned_as_orphan(
|
||||
self, work_dir, checkpoint_base, monkeypatch,
|
||||
):
|
||||
"""Control: a present project keeps its history."""
|
||||
self._project_with_history(work_dir, checkpoint_base, monkeypatch)
|
||||
|
||||
result = prune_checkpoints(
|
||||
retention_days=0, delete_orphans=True, checkpoint_base=checkpoint_base,
|
||||
)
|
||||
|
||||
assert result["deleted_orphan"] == 0
|
||||
assert self._history_survives(checkpoint_base, work_dir)
|
||||
|
|
|
|||
|
|
@ -564,16 +564,21 @@ def _pre_v2_shadow_repos(base: Path) -> List[Dict]:
|
|||
if not (child / "HEAD").exists():
|
||||
continue
|
||||
workdir: Optional[str] = None
|
||||
marker_unreadable = False
|
||||
wd_marker = child / "HERMES_WORKDIR"
|
||||
if wd_marker.exists():
|
||||
try:
|
||||
workdir = wd_marker.read_text(encoding="utf-8").strip()
|
||||
except (OSError, UnicodeDecodeError):
|
||||
# The marker is there, we just could not read it. That is
|
||||
# not evidence the project is gone — never delete on it.
|
||||
workdir = None
|
||||
marker_unreadable = True
|
||||
out.append({
|
||||
"path": child,
|
||||
"workdir": workdir,
|
||||
"exists": bool(workdir) and Path(workdir).exists(),
|
||||
"marker_unreadable": marker_unreadable,
|
||||
})
|
||||
return out
|
||||
|
||||
|
|
@ -1288,6 +1293,41 @@ def _delete_ref(store: Path, ref: str) -> bool:
|
|||
return ok
|
||||
|
||||
|
||||
def _workdir_is_observably_gone(workdir: str) -> bool:
|
||||
"""True only when we can positively observe that ``workdir`` was removed.
|
||||
|
||||
``Path.exists()`` returns False for a deleted directory AND for one whose
|
||||
storage simply is not attached right now — an unplugged external drive, a
|
||||
network share behind a downed VPN, a bind-mount absent from this
|
||||
container, an offline Windows mapped drive. Orphan pruning deletes the
|
||||
project's entire checkpoint history, so treating that ambiguity as
|
||||
"deleted" throws away the user's restore points over a transient mount
|
||||
state, unattended, at startup.
|
||||
|
||||
Require corroboration: the parent directory must be present, so the
|
||||
absence of the project inside it is something we actually observed. When
|
||||
the parent is missing too, the volume is not there and we know nothing —
|
||||
leave the entry alone. Genuinely abandoned projects are still reclaimed by
|
||||
the retention/stale rule, which runs off ``last_touch`` rather than a
|
||||
filesystem probe.
|
||||
"""
|
||||
if not workdir:
|
||||
return False
|
||||
path = Path(workdir)
|
||||
try:
|
||||
if path.exists():
|
||||
return False
|
||||
parent = path.parent
|
||||
# A path whose parent is itself (a filesystem root) gives us nothing
|
||||
# to corroborate against.
|
||||
if parent == path:
|
||||
return False
|
||||
return parent.is_dir()
|
||||
except OSError:
|
||||
# Probe failed (permission, I/O error) — not evidence of deletion.
|
||||
return False
|
||||
|
||||
|
||||
def prune_checkpoints(
|
||||
retention_days: int = 7,
|
||||
delete_orphans: bool = True,
|
||||
|
|
@ -1380,7 +1420,11 @@ def prune_checkpoints(
|
|||
reason: Optional[str] = None
|
||||
if (
|
||||
delete_orphans
|
||||
and not repo["exists"]
|
||||
and not repo["marker_unreadable"]
|
||||
and (
|
||||
repo["workdir"] is None
|
||||
or _workdir_is_observably_gone(repo["workdir"])
|
||||
)
|
||||
and (orphan_allowlist is None or str(child) in orphan_allowlist)
|
||||
):
|
||||
reason = "orphan"
|
||||
|
|
@ -1423,7 +1467,7 @@ def prune_checkpoints(
|
|||
reason = None
|
||||
if (
|
||||
delete_orphans
|
||||
and (not workdir or not Path(workdir).exists())
|
||||
and (not workdir or _workdir_is_observably_gone(workdir))
|
||||
and (orphan_allowlist is None or dir_hash in orphan_allowlist)
|
||||
):
|
||||
reason = "orphan"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue