fix(checkpoints): never auto-delete orphans on unattended startup sweep - #1
Closed
JoaoMarcos44 wants to merge 3 commits into
Conversation
Builds on this PR's diagnosis by @Frowtek: a missing workdir is ambiguous (deleted project vs. an unmounted external volume / network share / VPN not yet up), so it's not safe evidence for a destructive GC sweep — especially one that runs unattended at startup. - cli.py / gateway/run.py: the startup auto-maintenance sweep now always passes delete_orphans=False to maybe_auto_prune_checkpoints(). It still prunes by retention_days, size cap, and legacy archives — none of which require guessing whether a project was deleted or is just temporarily unreachable. - hermes_cli/config.py: drop the now-unused delete_orphans default. - hermes_cli/checkpoints.py: `hermes checkpoints prune` (the explicit, human-invoked path) now previews the orphan project list and asks for confirmation before deleting, unless -f/--force is passed. - Docs updated (EN + zh-Hans) to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
store_status()["projects"] only ever covered v2 metadata, so the `hermes checkpoints prune` confirmation prompt was blind to pre-v2 base/<hash>/HEAD shadow repos that prune_checkpoints() deletes separately via shutil.rmtree — a pre-v2-only or mixed store could lose checkpoint history without ever hitting the confirmation. Extract the pre-v2 scan into _pre_v2_shadow_repos() and have both store_status() (preview, new pre_v2_projects key) and prune_checkpoints() (deletion) read from it, so the CLI prompt can no longer diverge from what actually gets removed. Addresses review from egilewski on NousResearch#69141. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
JoaoMarcos44
force-pushed
the
checkpoint-prune-startup-safety
branch
from
July 22, 2026 18:54
73319fb to
64b65dc
Compare
… and mixed stores Requested by egilewski on NousResearch#69141: the orphan confirmation flow had no test coverage at all before this. Exercises hermes_cli.checkpoints.cmd_prune directly against pre-v2-only and mixed (v2 + pre-v2) fake stores — decline aborts with nothing deleted, accept deletes both layouts, --force and --keep-orphans skip the prompt as expected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Author
|
Closing this one — it was an alternative direct-merge path onto your branch, offered in case that was preferred over routing through main. Since then, per your review on NousResearch#69141, we dropped NousResearch#69141 (NousResearch/hermes-agent) is the canonical path going forward — mergeable, no conflicts, will rebase onto main once NousResearch#69063 lands as discussed there. Thanks again for the diagnosis and reproduction that started all this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up on top of your fix, @Frowtek — this closes the root cause your diagnosis pointed at, not just the parent-dir mitigation.
Why
Path(workdir).exists() == Falseis ambiguous: deleted project vs. an unmounted external volume / network share / VPN not yet up. Your PR already makes that check less aggressive for the legacy marker path. This PR removes the ambiguity entirely from the place it's most dangerous: the unattended startup sweep.What changed
cli.py/gateway/run.py: the startup auto-maintenance sweep now always callsmaybe_auto_prune_checkpoints(delete_orphans=False, ...). It still prunes byretention_days, size cap, and legacy archives — none of which require guessing whether a project was deleted or is just temporarily unreachable.hermes_cli/config.py: dropped the now-unuseddelete_orphansconfig default.hermes_cli/checkpoints.py: the explicit, human-invokedhermes checkpoints prunecommand now previews the orphan project list (workdir + commit count) and asks for y/N confirmation before deleting any of them, unless-f/--forceis passed.--keep-orphansstill works for a stale-only sweep.No changes to
tools/checkpoint_manager.pyor its tests — this only touches the call sites, so it merges cleanly on top of your heuristic fix there.Full credit to @Frowtek for finding, reproducing, and diagnosing this — this PR only exists because of that report.