fix(sessions): address prune skip review notes
This commit is contained in:
@@ -942,16 +942,14 @@ def cmd_sessions(args, sessions_parser=None):
|
||||
filters["archived"] = False
|
||||
|
||||
candidates = db.list_prune_candidates(**filters)
|
||||
skipped_open = (
|
||||
db.count_open_prune_matches(**filters) if action == "prune" else 0
|
||||
)
|
||||
skipped_open = db.count_open_prune_matches(**filters)
|
||||
if skipped_open:
|
||||
suffix = "" if skipped_open == 1 else "s"
|
||||
print(
|
||||
f"Note: {skipped_open} open session{suffix} also match these "
|
||||
"filters but will be skipped because prune only deletes ended "
|
||||
"sessions. Use `hermes sessions delete <id>` to remove one "
|
||||
"explicitly."
|
||||
f"filters but will be skipped because bulk {action} only "
|
||||
"affects ended sessions. Use `hermes sessions delete <id>` "
|
||||
"to remove one explicitly."
|
||||
)
|
||||
verb = "Delete" if action == "prune" else "Archive"
|
||||
if not candidates:
|
||||
|
||||
+16
-16
@@ -11657,6 +11657,20 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
||||
clauses.append("s.archived = 0")
|
||||
return " AND ".join(clauses), params
|
||||
|
||||
@staticmethod
|
||||
def _apply_prune_age_filter(
|
||||
older_than_days: Optional[float], filters: Dict[str, Any]
|
||||
) -> None:
|
||||
"""Translate the legacy age window into the shared activity filter."""
|
||||
if (
|
||||
filters.get("last_active_before") is None
|
||||
and filters.get("started_before") is None
|
||||
and older_than_days is not None
|
||||
):
|
||||
filters["last_active_before"] = time.time() - (
|
||||
older_than_days * 86400
|
||||
)
|
||||
|
||||
def list_prune_candidates(
|
||||
self,
|
||||
older_than_days: Optional[float] = None,
|
||||
@@ -11674,14 +11688,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
||||
threshold: it uses the latest message timestamp, falling back to
|
||||
``started_at`` for sessions without messages.
|
||||
"""
|
||||
if (
|
||||
filters.get("last_active_before") is None
|
||||
and filters.get("started_before") is None
|
||||
and older_than_days is not None
|
||||
):
|
||||
filters["last_active_before"] = time.time() - (
|
||||
older_than_days * 86400
|
||||
)
|
||||
self._apply_prune_age_filter(older_than_days, filters)
|
||||
where, params = self._prune_filter_where(source=source, **filters)
|
||||
with self._lock:
|
||||
cursor = self._conn.execute(
|
||||
@@ -11711,14 +11718,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
||||
why an otherwise matching session was skipped without making live
|
||||
sessions eligible for destructive pruning.
|
||||
"""
|
||||
if (
|
||||
filters.get("last_active_before") is None
|
||||
and filters.get("started_before") is None
|
||||
and older_than_days is not None
|
||||
):
|
||||
filters["last_active_before"] = time.time() - (
|
||||
older_than_days * 86400
|
||||
)
|
||||
self._apply_prune_age_filter(older_than_days, filters)
|
||||
where, params = self._prune_filter_where(source=source, **filters)
|
||||
ended_guard = "s.ended_at IS NOT NULL"
|
||||
if not where.startswith(ended_guard):
|
||||
|
||||
@@ -39,8 +39,10 @@ def test_sessions_delete_accepts_unique_id_prefix(monkeypatch, capsys):
|
||||
assert "Deleted session '20260315_092437_c9a6ff'." in output
|
||||
|
||||
|
||||
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, action="prune"
|
||||
):
|
||||
"""Run `hermes sessions <action> <argv_tail>` against a FakeDB, capturing
|
||||
the filter kwargs passed to list_prune_candidates. Auto-confirms."""
|
||||
import hermes_cli.main as main_mod
|
||||
import hermes_state
|
||||
@@ -81,12 +83,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: FakeDB())
|
||||
monkeypatch.setattr(
|
||||
sys, "argv", ["hermes", "sessions", "prune", *argv_tail]
|
||||
sys, "argv", ["hermes", "sessions", action, *argv_tail]
|
||||
)
|
||||
monkeypatch.setattr("builtins.input", lambda _prompt="": "y")
|
||||
main_mod.main()
|
||||
@@ -124,6 +129,21 @@ def test_sessions_prune_surfaces_matching_open_sessions(monkeypatch, capsys):
|
||||
)
|
||||
|
||||
assert "2 open sessions also match these filters" in out
|
||||
assert "prune only deletes ended sessions" in out
|
||||
assert "bulk prune only affects ended sessions" in out
|
||||
assert "hermes sessions delete <id>" in out
|
||||
assert "No sessions match" in out
|
||||
|
||||
|
||||
def test_sessions_archive_surfaces_matching_open_sessions(monkeypatch, capsys):
|
||||
_filters, out = _run_prune(
|
||||
monkeypatch,
|
||||
capsys,
|
||||
["--source", "cron"],
|
||||
candidates=[],
|
||||
skipped_open=1,
|
||||
action="archive",
|
||||
)
|
||||
|
||||
assert "1 open session also match these filters" in out
|
||||
assert "bulk archive only affects ended sessions" in out
|
||||
assert "No sessions match" in out
|
||||
|
||||
@@ -1,17 +1,15 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { formatSessionPruneResult } from "./session-prune";
|
||||
import { formatSessionPruneResult } from './session-prune'
|
||||
|
||||
describe("formatSessionPruneResult", () => {
|
||||
it("reports open sessions skipped by the prune safety guard", () => {
|
||||
describe('formatSessionPruneResult', () => {
|
||||
it('reports open sessions skipped by the prune safety guard', () => {
|
||||
expect(formatSessionPruneResult({ removed: 0, skipped_open: 2 })).toBe(
|
||||
"Pruned 0 sessions. Skipped 2 open sessions; prune only removes ended sessions.",
|
||||
);
|
||||
});
|
||||
'Pruned 0 sessions. Skipped 2 open sessions; prune only removes ended sessions.'
|
||||
)
|
||||
})
|
||||
|
||||
it("keeps the existing success message when nothing was skipped", () => {
|
||||
expect(formatSessionPruneResult({ removed: 1, skipped_open: 0 })).toBe(
|
||||
"Pruned 1 session",
|
||||
);
|
||||
});
|
||||
});
|
||||
it('keeps the existing success message when nothing was skipped', () => {
|
||||
expect(formatSessionPruneResult({ removed: 1, skipped_open: 0 })).toBe('Pruned 1 session')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
interface SessionPruneResult {
|
||||
removed: number;
|
||||
skipped_open: number;
|
||||
removed: number
|
||||
skipped_open: number
|
||||
}
|
||||
|
||||
export function formatSessionPruneResult(result: SessionPruneResult): string {
|
||||
const removed = `Pruned ${result.removed} session${result.removed === 1 ? "" : "s"}`;
|
||||
if (!result.skipped_open) return removed;
|
||||
const removed = `Pruned ${result.removed} session${result.removed === 1 ? '' : 's'}`
|
||||
if (!result.skipped_open) return removed
|
||||
|
||||
return `${removed}. Skipped ${result.skipped_open} open session${result.skipped_open === 1 ? "" : "s"}; prune only removes ended sessions.`;
|
||||
}
|
||||
return `${removed}. Skipped ${result.skipped_open} open session${result.skipped_open === 1 ? '' : 's'}; prune only removes ended sessions.`
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user