fix(cli): recover detached background processes from checkpoint on CLI startup and shutdown - #69918
Open
mantenie wants to merge 2 commits into
Open
fix(cli): recover detached background processes from checkpoint on CLI startup and shutdown#69918mantenie wants to merge 2 commits into
mantenie wants to merge 2 commits into
Conversation
…I startup and shutdown Port the three approved process-registry recovery commits onto current main (origin/main @ de5ece9): 1. recover_from_checkpoint idempotency guard + recover_and_log wrapper 2. CLI startup recovery hook (_recover_background_processes_on_startup) 3. CLI shutdown checkpoint flush in _run_cleanup Preserves: - PID start-time / recycled-PID safety - sandbox entries never recovered - best-effort recovery that cannot block startup - no kill-on-CLI-exit behavior - Python 3.9 compatibility - prompt-cache and tool-schema stability Tests: 117/117 process_registry + 3/3 CLI recovery tests pass. Round-trip: spawn -> checkpoint -> clear -> recover -> list -> kill verified.
alt-glitch
added
type/bug
teknium1
reviewed
Jul 30, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for addressing a real CLI recovery gap.
Problems
tests/cli/test_cli_startup_recovery.py:22calls_recover_background_processes_on_startup()directly. It does not cover the production wiring inHermesCLI.run()(cli.py:14589), so the test would still pass if the new call site were removed.
Suggested changes
- Add a controlled startup-path test that exercises
HermesCLI.run()sufficiently to assert recovery is called once before the interactive loop begins.
This is an automated hermes-sweeper review.
| monkeypatch.setattr( | ||
| "tools.process_registry.process_registry.recover_and_log", | ||
| fake_recover_and_log, | ||
| ) |
Contributor
There was a problem hiding this comment.
This invokes the helper directly, so it cannot catch a regression where HermesCLI.run() stops calling it. Please add a controlled run-startup behavior test that asserts the wiring.
teknium1
added
the
sweeper:blast-moderate
This was referenced Aug 5, 2026
Open
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
3 participants
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.
Port the three approved process-registry recovery commits onto current main (origin/main @ de5ece9).
Changes
recover_from_checkpoint(skip sessions already tracked in live registry) + newrecover_and_logconvenience wrapper + new publicflush_checkpoint()method_recover_background_processes_on_startup()called inHermesCLI.run()+ final checkpoint flush in_run_cleanup()(now viaflush_checkpoint()instead of private_write_checkpoint())NIT fix (this round)
flush_checkpoint()as the public API onProcessRegistrycli.pynow callsprocess_registry.flush_checkpoint()instead ofprocess_registry._write_checkpoint()flush_checkpointinstead of_write_checkpointVerification
flush_checkpoint()) → clear → recover → list → kill — verifiedStatus