mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(cli): let archive reach never-ended sessions via --include-open
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)
This commit is contained in:
1 parent
edb78c6a52
commit
00a0d64286
5 files changed
+77
-9
No files matched your search
@@ -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 <id>` "
|
||||
"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
|
||||
|
||||
@@ -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)")
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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 <argv_tail>` against a FakeDB, capturing
|
||||
def _run_prune(monkeypatch, capsys, argv_tail, candidates=None, skipped_open=0, sub="prune"):
|
||||
"""Run `hermes sessions <sub> <argv_tail>` (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
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user