From 00a0d642867ce7f9e46c6bc7773fcdd7afba2a3f Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Fri, 16 Oct 2026 11:58:01 +0800 Subject: [PATCH] fix(cli): let archive reach never-ended sessions via --include-open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Desktop sessions never set ended_at, so the prune/archive selector's ended_at IS NOT NULL guard hid them from bulk archive entirely: explicit filters printed 'No sessions match' for sessions visible in the desktop sidebar (#139597). Add an archive-only --include-open flag that lifts the guard in the shared selector, and report open matches the default path skips. The delete path refuses include_open fail-closed — pruning open sessions stays impossible because deletion is not reversible. Fixes #139597 (cherry picked from commit f6b123111ef137c9048e5d023a59f55021259b78) --- hermes_cli/sessions_cmd.py | 13 ++++++++++-- hermes_cli/subcommands/sessions.py | 4 ++++ hermes_state_maintenance.py | 18 ++++++++++++---- tests/hermes_cli/test_sessions_delete.py | 27 +++++++++++++++++++++--- tests/hermes_state/test_hermes_state.py | 24 +++++++++++++++++++++ 5 files changed, 77 insertions(+), 9 deletions(-) diff --git a/hermes_cli/sessions_cmd.py b/hermes_cli/sessions_cmd.py index 856b8d7928..25b4dc8fa7 100644 --- a/hermes_cli/sessions_cmd.py +++ b/hermes_cli/sessions_cmd.py @@ -739,17 +739,26 @@ def _cmd_prune_or_archive(db, args, action): # Archive flips a compression lineage as a unit, matched through its tip (an old ancestor alone # never qualifies); the preview must show the same rows the archive will touch. filters["lineage_tips_only"] = not prune + # Desktop sessions never set ended_at, so the shared ended-only selector hides them from archive + # entirely (#139597); --include-open opts in for archive only — pruning is not reversible. + include_open = not prune and getattr(args, "include_open", False) + if include_open: + filters["include_open"] = True if not filters["include_pinned"]: _note_pinned_skipped(db, filters, action) # Prune deletes a compression lineage only as a unit; the preview must list the rows it deletes. candidates = db.list_prune_candidates(**filters, whole_lineages=prune) # Archive expands each matched tip to its compression lineage, so a direct-open count would # misdescribe its effect. - skipped_open = db.count_open_prune_matches(**filters) if prune else 0 - if skipped_open: + skipped_open = db.count_open_prune_matches(**filters) if (prune or not include_open) else 0 + if skipped_open and prune: print(f"Note: {skipped_open} open session{'' if skipped_open == 1 else 's'} also match these filters but " "will be skipped because prune only deletes ended sessions. Use `hermes sessions delete ` " "to remove one explicitly.") + elif skipped_open: + print(f"Note: {skipped_open} matching session{'' if skipped_open == 1 else 's'} still open — " + "skipped because archive defaults to ended sessions. Use --include-open to archive " + "them too.") if not candidates: print(f"No sessions match ({describe_filters(filters)}).") return diff --git a/hermes_cli/subcommands/sessions.py b/hermes_cli/subcommands/sessions.py index fdcaa19524..0309c3404e 100644 --- a/hermes_cli/subcommands/sessions.py +++ b/hermes_cli/subcommands/sessions.py @@ -142,6 +142,10 @@ def build_sessions_parser(subparsers, *, cmd_sessions: Callable) -> None: _add_session_filter_args( sessions_archive, "Only archive sessions older than AGE (duration like '5h'/'2d', " "bare number of days, or ISO timestamp)") + _flag(sessions_archive, "--include-open", + help="Also archive sessions that never ended (desktop app sessions never set an end " + "timestamp, so the default ended-only selector skips them); archive is reversible, " + "unlike prune") sessions_optimize = sessions_subparsers.add_parser( "optimize", help="Reclaim disk space: merge FTS5 segments + VACUUM (no data change)") diff --git a/hermes_state_maintenance.py b/hermes_state_maintenance.py index 889e5077c8..d37556a004 100644 --- a/hermes_state_maintenance.py +++ b/hermes_state_maintenance.py @@ -217,18 +217,21 @@ class SessionMaintenanceMixin: @staticmethod def _prune_filter_where(*, archived: Optional[bool] = None, include_pinned: bool = False, - lineage_tips_only: bool = False, **filters) -> tuple[str, list]: + lineage_tips_only: bool = False, include_open: bool = False, + **filters) -> tuple[str, list]: """Shared WHERE clause for bulk prune/archive selection (alias ``s``): ``_PRUNE_FILTERS`` AND together, only ended sessions are ever candidates, ``archived`` is tri-state (None = both), ``*_like`` are case-insensitive substrings, the rest exact. ``lineage_tips_only`` (bulk archive) drops compression ancestors: they are archived with their tip, never on their own age — matching an old ancestor would fan out over the lineage - and hide its OPEN, recently active tip (#115489).""" + and hide its OPEN, recently active tip (#115489). ``include_open`` lifts the ended guard + for reversible archive only — never for prune/delete (#139597: desktop sessions never set + ``ended_at``).""" unknown = set(filters) - _PRUNE_FILTER_NAMES if unknown: raise TypeError("SessionMaintenanceMixin._prune_filter_where() got an unexpected " f"keyword argument {min(unknown)!r}") - clauses = ["s.ended_at IS NOT NULL"] + clauses = [] if include_open else ["s.ended_at IS NOT NULL"] if lineage_tips_only: clauses.append("COALESCE(s.end_reason, '') <> 'compression'") params: list = [] @@ -288,7 +291,10 @@ class SessionMaintenanceMixin: def count_open_prune_matches(self, older_than_days: Optional[float] = None, source: str | None = None, **filters) -> int: - """Count open sessions a matching prune skips (``ended_at`` guard inverted); visibility-only.""" + """Count open sessions a matching prune/archive skips (``ended_at`` guard inverted); + visibility-only. With ``include_open`` the selection already contains them: nothing is skipped.""" + if filters.get("include_open"): + return 0 where, params = self._prune_where(older_than_days, source, filters) ended_guard = "s.ended_at IS NOT NULL" if not where.startswith(ended_guard): @@ -335,6 +341,10 @@ class SessionMaintenanceMixin: ancestor is deleted only together with every continuation after it (``whole_lineages``). ``deleted_ids`` (the owning gateway) receives the removed ids, so it can retire their in-memory routes and live sessions after the commit.""" + if filters.get("include_open"): + # Deletion is not reversible, so the ended-session guard is structural here: the + # archive-only escape hatch (#139597) must never reach the delete path. + raise ValueError("prune_sessions() never deletes open sessions: include_open is archive-only") where, where_params = self._prune_where(older_than_days, source, filters, whole_lineages=True) removed_ids: list[str] = [] def _do(conn): diff --git a/tests/hermes_cli/test_sessions_delete.py b/tests/hermes_cli/test_sessions_delete.py index 684d2abc0f..acd3f5b9de 100644 --- a/tests/hermes_cli/test_sessions_delete.py +++ b/tests/hermes_cli/test_sessions_delete.py @@ -69,8 +69,8 @@ def test_sessions_delete_with_unfinished_turn_points_to_discard(tmp_path, monkey reopened.close() -def _run_prune(monkeypatch, capsys, argv_tail, candidates=None, skipped_open=0): - """Run `hermes sessions prune ` against a FakeDB, capturing +def _run_prune(monkeypatch, capsys, argv_tail, candidates=None, skipped_open=0, sub="prune"): + """Run `hermes sessions ` (prune/archive) against a FakeDB, capturing the filter kwargs passed to list_prune_candidates. Auto-confirms.""" import hermes_cli.main as main_mod import hermes_state @@ -116,12 +116,15 @@ def _run_prune(monkeypatch, capsys, argv_tail, candidates=None, skipped_open=0): def prune_sessions(self, **kwargs): return len(rows) + def archive_sessions(self, **kwargs): + return len(rows) + def close(self): pass monkeypatch.setattr(hermes_state, "SessionDB", lambda *args, **kwargs: FakeDB()) monkeypatch.setattr( - sys, "argv", ["hermes", "sessions", "prune", *argv_tail] + sys, "argv", ["hermes", "sessions", sub, *argv_tail] ) monkeypatch.setattr("builtins.input", lambda _prompt="": "y") main_mod.main() @@ -136,3 +139,21 @@ def test_sessions_prune_bare_keeps_90_day_default(monkeypatch, capsys): assert filters["last_active_before"] == pytest.approx( _time.time() - 90 * 86400, abs=60 ) + + +def test_archive_include_open_flag_reaches_selection(monkeypatch, capsys): + """`sessions archive --include-open` lifts the ended-only selector: desktop sessions never set + ended_at, so without the flag they can never match any filter (#139597).""" + filters, _out = _run_prune( + monkeypatch, capsys, ["--title", "dupes", "--include-open", "--yes"], sub="archive") + assert filters["include_open"] is True + + +def test_archive_without_flag_reports_open_skips(monkeypatch, capsys): + """Without --include-open, matches that are still open are skipped silently no longer: the + CLI says how many and names the opt-in flag.""" + filters, out = _run_prune( + monkeypatch, capsys, ["--title", "dupes", "--yes"], skipped_open=2, sub="archive") + assert "include_open" not in filters + assert "2 matching sessions still open" in out + assert "--include-open" in out diff --git a/tests/hermes_state/test_hermes_state.py b/tests/hermes_state/test_hermes_state.py index 4946afb4f6..1a25e0fac7 100644 --- a/tests/hermes_state/test_hermes_state.py +++ b/tests/hermes_state/test_hermes_state.py @@ -1335,6 +1335,30 @@ class TestPruneSessions: older_than_days=90, source="cron", archived=False )} == {"ended"} + def test_archive_include_open_reaches_never_ended_sessions(self, db): + """Desktop sessions never set ended_at, so the shared ended-only selector hid them from + bulk archive entirely; archive is reversible, so include_open lifts the guard (#139597).""" + db.create_session(session_id="never-ended", source="desktop") + db.create_session(session_id="ended", source="desktop") + db.end_session("ended", "completed") + + assert db.archive_sessions(source="desktop") == 1 + assert db.get_session("ended")["archived"] == 1 + assert db.get_session("never-ended")["archived"] == 0 + + assert db.archive_sessions(source="desktop", include_open=True) == 1 + assert db.get_session("never-ended")["archived"] == 1 + assert db.count_open_prune_matches( + source="desktop", archived=False, include_open=True) == 0 + + def test_prune_rejects_include_open_fail_closed(self, db): + """include_open is the reversible-archive escape hatch; the delete path must refuse it + rather than silently drop the ended-session guard.""" + db.create_session(session_id="open", source="cli") + with pytest.raises(ValueError, match="archive-only"): + db.prune_sessions(include_open=True) + assert db.get_session("open") is not None + def test_negative_older_than_days_rejected_at_every_prune_boundary(self, db): """A negative bound builds a FUTURE cutoff that matches every ended session (and every never-active keyed row) — the SessionDB API must raise, naming the allowed range, instead