Skip to content

fix(state): purge gateway_routing entries when prune_sessions hard-deletes - #59512

Open
pierrenode wants to merge 1 commit into
NousResearch:mainfrom
pierrenode:fix/prune-sessions-routing-cleanup
Open

fix(state): purge gateway_routing entries when prune_sessions hard-deletes#59512
pierrenode wants to merge 1 commit into
NousResearch:mainfrom
pierrenode:fix/prune-sessions-routing-cleanup

Conversation

@pierrenode

Copy link
Copy Markdown
Contributor

What does this PR do?

prune_sessions() (hermes sessions prune) deletes sessions/messages rows and on-disk transcript files directly in hermes_state.py, bypassing gateway/session.py's SessionStore entirely. It never touched the gateway_routing table (#59203), so a routing entry (session_key -> full serialized SessionEntry) written before the prune stayed dangling, pointing at a session_id whose row no longer existed.

gateway/session.py's own stale-entry self-heal (_is_session_ended_in_db) does not catch this: its own docstring says a missing row returns False ("keep") — that check exists for a different case (an entry not yet persisted), not "the row was deleted out from under it". The next message on that session_key would rehydrate the stale entry, load an empty transcript via load_transcript(), and ensure_session()'s INSERT OR IGNORE would silently recreate a blank sessions row — the user sees the conversation lose all memory instead of either continuing normally or starting cleanly fresh.

This is the same failure class the gateway/session.py self-heal was originally built for (#54878, #52804: "stale sessions.json entry silently drops messages until restart") — now reappearing against the new gateway_routing table, and made more realistic to hit by #59327's --newer-than filters, which let an operator prune sessions from just hours ago instead of only 90-day-old ones.

Related Issue

No filed issue — found via cross-PR review of #59203 (gateway_routing table) and #59327 (expanded prune filters), both merged in the same window, neither cross-referencing the other's effect on this table.

Type of Change

  • 🐛 Bug fix (data consistency — stale routing entry causes silent conversation-history loss)

Changes Made

  • hermes_state.py: prune_sessions() now calls a new _purge_gateway_routing_for_sessions() helper after deleting session rows, which scans gateway_routing (scoped to sessions_dir, mirroring SessionStore._routing_scope()) for entries whose embedded session_id was just deleted, and drops them via the existing (previously unused) delete_gateway_routing_entries(). No-op when sessions_dir is not passed (can't compute scope) or when nothing was pruned — same graceful-degradation convention _remove_session_files already uses.
  • tests/test_hermes_state.py: two new regression tests — test_prune_sessions_drops_stale_gateway_routing_entries (verified: fails without the fix, passes with it) and test_prune_sessions_without_sessions_dir_skips_routing_cleanup (no-sessions_dir case doesn't crash or touch other scopes)

How to Test

python3.11 -m pytest tests/test_hermes_state.py -k "gateway_routing or prune_sessions" -v --override-ini="addopts="

Also re-ran the full file and the gateway session-routing suite to confirm no regressions: tests/test_hermes_state.py (324 passed), tests/gateway/test_session.py (100 passed).

Checklist

  • Read the Contributing Guide | Conventional Commits | No duplicate PR
  • Single logical fix | Tests added | Platform: macOS
  • Docs — N/A | Cross-platform — N/A

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P2 Medium — degraded but workaround exists labels Jul 6, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing a real consistency gap: current prune_sessions() deletes the session rows without coordinating routing state (hermes_state.py:6392-6402).

Problems

  • The proposed DB-only deletion does not invalidate an already-running SessionStore. Its runtime guard intentionally treats a missing DB row as keep (gateway/session.py:1637-1661), so get_or_create_session() will retain the in-memory entry and a later whole-index save can restore the DB row (gateway/session.py:1870, 1910-1920, 1239-1245).
  • The default sessions.json mirror remains intact. It is enabled by default (gateway/config.py:678-683) and is imported for DB-missing keys at startup (gateway/session.py:1080-1111), so a restart can rehydrate the stale mapping the patch removed from gateway_routing.

Suggested changes

  • Please rework this around an explicit prune invalidation path that coordinates the live store and both persisted routing representations, without repurposing the intentionally permissive generic missing-row check.
  • Add an integration regression covering an existing store and a restarted default-config store after CLI-style pruning.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@pierrenode
pierrenode force-pushed the fix/prune-sessions-routing-cleanup branch from 245431a to 04903b9 Compare July 26, 2026 23:48
…letes

prune_sessions() (hermes sessions prune) deletes sessions/messages rows and
on-disk transcript files directly in hermes_state.py, bypassing
gateway/session.py's SessionStore entirely. It never touched the
gateway_routing table (NousResearch#59203), so a routing entry (session_key -> full
serialized SessionEntry) written before the prune stayed dangling, pointing
at a session_id whose row no longer existed.

gateway/session.py's own stale-entry self-heal (_is_session_ended_in_db)
does not catch this: its own docstring says a missing row returns False
("keep") — that check exists for a different case (an entry not yet
persisted), not "the row was deleted out from under it". The next message
on that session_key would rehydrate the stale entry, load an empty
transcript via load_transcript(), and ensure_session()'s INSERT OR IGNORE
would silently recreate a blank sessions row — the user sees the
conversation lose all memory instead of either continuing or starting
cleanly fresh.

Purging gateway_routing alone leaves two more ways for the stale mapping
to resurrect itself:

1. The legacy sessions.json mirror (on by default) still has the entry.
   sessions.json fills any routing key the DB table doesn't have on the
   next SessionStore load, so a restarted gateway would silently
   re-import and re-persist exactly what was just purged. Now purged in
   the same pass, atomically written like the live SessionStore's own
   mirror writes.

2. A gateway process already running when the CLI prune executes has the
   stale entry cached in memory. Its per-request self-heal previously
   only checked whether the session was *ended* in state.db
   (_is_session_ended_in_db); a hard-deleted row reads identically to
   "not yet persisted" from that check alone, so it could never catch
   this case without also breaking the legitimate not-yet-persisted
   race. Added a second, narrower check (_is_routing_entry_purged) that
   asks the routing table directly whether it still carries the entry —
   a "no" is unambiguous once legacy sessions.json imports are persisted
   immediately on load (also fixed here) rather than left to the next
   incidental save.

This is the same failure class the gateway/session.py self-heal was built
for (NousResearch#54878, NousResearch#52804: "stale sessions.json entry silently drops messages
until restart") applied to the new gateway_routing table, exposed further
by NousResearch#59327's --newer-than filters, which make it realistic to prune
sessions from just hours ago rather than only 90-day-old ones.
@pierrenode
pierrenode force-pushed the fix/prune-sessions-routing-cleanup branch from 04903b9 to 6332226 Compare August 11, 2026 15:47
@pierrenode

Copy link
Copy Markdown
Contributor Author

Rebased onto current `upstream/main`. `gateway/session.py`'s stale-entry check was restructured into an explicit lock-read/no-lock-I/O/lock-write phase split since this PR's branch point — merged this PR's `_purged_by_prune` check into that structure (computed in the no-lock I/O phase alongside `_is_stale`, consistent with the refactor's own stated goal of keeping I/O out of the lock). `hermes_state.py`'s conflict was two independent, non-overlapping additions at the same insertion point — kept both.

The test-file conflict included a fragment of a pre-existing test (`test_force_new_skips_stale_check`) that turned out to belong to a class upstream has since restructured/renamed elsewhere in the same file — verified upstream's current `TestRuntimeStaleGuard` no longer ends with that test, so it wasn't something this PR removed; kept only this PR's own new test classes.

Mutation-verified: removing `_purged_by_prune` from the stale-entry condition breaks `test_purged_entry_creates_fresh_session`. Full `test_session_store_runtime_stale_guard.py` + `test_hermes_state.py` (245 tests) passes. Ruff clean.

(A broader `tests/gateway/test_session*.py tests/state/` sweep showed one unrelated failure — confirmed pre-existing test-order pollution by reproducing the identical failure against clean pre-fix `upstream/main` code with the same broad file selection.)

Squashed to a single commit on top of current `upstream/main`.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants