fix(sessions): don't let the empty-session sweep delete an archived transcript

[JoaoMarcos44]

Summary

Investigating #95868 I could not reproduce a hard delete on the reload path itself — and I want to state that plainly up front, because it changes where the fix belongs.

Driving the real code against a real state.db, the gateway-reload sequence the reporter describes (ws closed … code=1012 … detached_sessions=7 → plugin remount) preserves every row. _close_sessions_for_transport only re-points detached sessions at the drop sentinel; _teardown_popped_session → _finalize_session only calls end_session(). There is no DELETE FROM sessions anywhere on that path. What the reload does do is stamp ended_at + end_reason='ws_orphan_reap' on every detached session — and that stamp is the trigger for the actual defect.

Root cause: the empty-session sweep decides "empty" from a counter that two production paths deliberately zero on populated sessions.

SessionDB.count_empty_sessions() / delete_empty_sessions() — the dashboard's "Delete empty (N)" affordance, GET|DELETE /api/sessions/empty — selected on sessions.message_count = 0. That column is a denormalized counter over the live (active = 1) row set only. Two transcript-rewrite paths reset it on purpose while keeping every dropped turn on disk as active = 0:

Path	Why it archives instead of deleting	Reaches message_count = 0
replace_messages(..., archive_dropped=True)	The rewind / edit / regenerate mode from #82756 — a plain DELETE also evicts the rows from FTS, leaving nothing to recover from	Yes, in production. prompt.submit computes the surviving prefix with history_before_user_originated_turn; a confirmed ordinal-0 rewind (regenerating the very first user turn, gated behind confirm_empty_truncate) yields truncated == []
archive_and_compact	In-place compaction keeps the pre-compaction transcript under the same session id (#38763)	Only when the live set comes back empty — normally it publishes at least a summary row. Pinned as defense in depth

So a chat whose first user turn was regenerated reports message_count = 0 while still holding its entire recoverable history — and those soft-archived rows are the only copy that exists. Once the gateway reload stamps ended_at, the row satisfies the sweep's ended_at IS NOT NULL gate, gets counted in the "Delete empty (N)" badge, and is hard-deleted together with DELETE FROM messages. Silently, and with no log line — which is exactly the "rows gone, no delete_session in the logs" signature reported.

What makes this sting is whose guarantee it breaks. The archive_dropped=True call site says so in its own comment: "a rewind overwrites turns the user may not have meant to drop, and this write is the last step before they are gone — three reported incidents ended here with nothing to restore from (#70516, #80763, #82756). Soft-archiving keeps them on disk (active=0) and in the FTS index, so a mis-aimed cut is recoverable instead of terminal." The empty-session sweep silently deletes exactly the rows those three fixes were written to preserve.

The asymmetry that makes this a clear bug rather than a design call: every other emptiness test in hermes_state already refuses to trust the counter and pairs it with a real EXISTS (SELECT 1 FROM messages …) probe — delete_session_if_empty, prune_empty_ghost_sessions, list_never_active_keyed_sessions, find_recoverable_session. This sweep was the single destructive path still trusting it alone. Its own inline comment even anticipated the failure ("if a bookkeeping bug ever lets the counter drift below the real row count") without guarding against it.

The fix

One selector, SessionDB._EMPTY_SESSION_WHERE, shared by the count and the delete so the badge's N and the sweep it triggers can never disagree about what "empty" means:

message_count = 0
AND ended_at IS NOT NULL
AND archived = 0
AND NOT EXISTS (SELECT 1 FROM messages WHERE messages.session_id = sessions.id)

The counter stays as a cheap prefilter; the EXISTS probe is the authority. This is the same shape the four sibling guards already use, so it is an in-style narrowing rather than new machinery. Genuinely message-less rows are still swept — the feature is unchanged for the case it was built for.

Flow

%%{init: {'theme': 'dark', 'themeVariables': { 'primaryColor': '#8b0000', 'mainBkg': '#0a0204', 'primaryTextColor': '#ffccd5', 'primaryBorderColor': '#ff0038', 'lineColor': '#ff0038'}}}%%
graph TD
    A[🩸 Populated Desktop Chat] -->|"confirmed ordinal-0 rewind"| B["🔥 replace_messages(archive_dropped=True)"]
    A -->|"in-place compaction"| C[🔥 archive_and_compact]
    B --> D["⚰️ Rows kept as active = 0<br/>message_count reset to 0"]
    C --> D
    D -->|"gateway reload<br/>detached_sessions &gt; 0"| E["🕯️ ended_at stamped<br/>end_reason = ws_orphan_reap"]
    E --> F{"⚔️ Empty-Session Sweep"}
    F -->|"BEFORE: message_count = 0 only"| G["💀 Hard Delete<br/>sessions + messages<br/>no log, unrecoverable"]
    F -->|"AFTER: + NOT EXISTS messages"| H["🛡️ Transcript Preserved<br/>row survives, archived rows intact"]

    style G fill:#3b0008,stroke:#ff0038,color:#ffccd5
    style H fill:#062a12,stroke:#28d17c,color:#d6ffe6

Loading

Relationship to #95961

Deliberately disjoint — no overlapping files, no overlapping mechanism. #95961 addresses the soft half of the report (populated rows should not be stamped tui_shutdown/ended by a reload, plus delete-path logging) in tui_gateway/*. This PR addresses the hard half: the sweep that turns an ended_at stamp into an actual DELETE. Both can land independently; together they close the loop, since #95961 removes the stamp that makes rows eligible and this PR removes the sweep's ability to act on a populated row even when a stamp is legitimate.

Verification

Empirical, not inferred. Two harnesses were run against real SessionDB instances:

1. Reload path — 8 populated source='desktop' sessions bound to one transport, then _close_sessions_for_transport (reaped=0 detached=8, matching the report's shape) → forced ws_orphan_reap teardown → _shutdown_sessions → next-startup _run_state_db_auto_maintenance. Result: 8/8 rows survived, stamped ended_at + ws_orphan_reap. No hard-delete path exists there.
2. Counter drift — replace_messages(sid, [], archive_dropped=True) and archive_and_compact(sid, []) both leave (message_count=0, rows_on_disk=4, rows_active=0). On pre-fix code count_empty_sessions() reported 2 and delete_empty_sessions() destroyed both session rows and all 8 message rows.

The regression tests encode exactly that, and were confirmed red before / green after (3 of the 5 fail on unpatched hermes_state.py).

Test plan

• pytest tests/hermes_state/test_empty_sweep_archived_transcript_95868.py — 5 passed (3 fail without the fix)
• pytest tests/test_empty_session_hygiene.py tests/test_session_system_prompt_dedup.py tests/hermes_state — 202 passed
• pytest tests/hermes_cli/test_web_server.py::TestDeleteEmptySessionsEndpoint — 3 passed
• pytest tests/test_hermes_state.py tests/test_web_server_sessiondb_eventloop.py — 246 passed, 2 skipped
• ruff check on all changed files — clean
• Dashboard: rewind a chat to its first turn, close it, confirm "Delete empty" no longer counts it and the sweep leaves it resumable with history intact
• Dashboard: a genuinely message-less ended session is still counted and still swept

Fixes #95868

[kshitijk4poor]

Merged via #96401. Your commits were cherry-picked with authorship preserved (rebase-merge).

The fix is correct and well-tested — the NOT EXISTS probe pattern matches all four sibling emptiness guards in hermes_state.py. A follow-up commit trimmed the verbose comment block on _EMPTY_SESSION_WHERE to avoid triple-duplication across the constant and both method docstrings.

Thanks for the thorough root-cause analysis in the PR body — it made the review straightforward.