fix(gateway): honor the reset policy when recovering a session from state.db (#66255) by kimdzhekhon · Pull Request #66264 · NousResearch/hermes-agent

kimdzhekhon · 2026-07-17T11:21:13Z

What does this PR do?

get_or_create_session() only evaluates _should_reset() when the session key is already present in the in-memory SessionStore._entries map. When a session must be recovered from state.db instead — which happens on every routing lookup after a gateway restart, since _entries starts empty — the reset-policy check is skipped entirely:

_get_or_create_session_impl()'s Phase 1b only computes _reset_reason inside if _entry_for_checks is not None and _stale_session_id is not None: — both are None on the recovery path.
_create_entry_from_recovered_row() additionally stamps updated_at=now (the recovery moment) rather than the row's real last activity, which re-arms the idle clock from zero even if the check were reached later.

Net effect: with session_reset: {mode: idle/daily/both, ...} configured, a session that should have expired hours or days ago is silently resumed after a restart and can stay alive indefinitely, carrying its full history — including stale skill content injected earlier in the conversation that no longer reflects the on-disk SKILL.md.

Related Issue

Fixes #66255

Type of Change

🐛 Bug fix

Changes Made

hermes_state.py: add HermesStateDB.get_last_message_timestamp(session_id) — MAX(timestamp) over messages for a session.
gateway/session.py:
- _create_entry_from_recovered_row() now derives updated_at from the recovered row's real last message timestamp (falling back to created_at when there are no messages), instead of unconditionally using the recovery moment.
- _query_recoverable_session() now evaluates _should_reset() against the recovered entry before publishing it. If the row is past its reset deadline, it's ended in state.db (end_reason='session_reset') and the function returns (None, reset_reason, had_activity) so the caller falls through to the existing fresh-session-creation path — reusing the same was_auto_reset/reset_had_activity plumbing a live in-memory expiry already produces, so the user gets the same "your session was reset" notice either way.
tests/gateway/test_session_recovery_reset_policy.py (new): 4 regression tests — idle-expired recovered row is ended not resumed, a recently-active recovered row is still resumed, updated_at reflects the last message timestamp rather than the recovery moment, and mode="none" (the default) never rejects a recovered row.

mode="none" — the default reset policy — is unaffected: _should_reset() is a no-op for that mode, so recovery behavior only changes for deployments that explicitly configure session_reset.

How to Test

scripts/run_tests.sh tests/gateway/test_session_recovery_reset_policy.py tests/gateway/test_session.py tests/gateway/test_session_store_stale_prune.py tests/gateway/test_session_store_runtime_stale_guard.py tests/gateway/test_multiplex_phase0.py tests/gateway/test_session_store_lock_io.py tests/test_hermes_state.py tests/gateway/test_session_reset_notify.py tests/gateway/test_session_model_reset.py

Manual repro of the original bug (matches the issue):

# session_reset: {mode: both, idle_minutes: 60, at_hour: 4}
# 1. Create a gateway session, let the conversation end naturally (no /new).
# 2. Restart the gateway (SessionStore._entries starts empty).
# 3. Wait past idle_minutes, send another message from the same peer.
# Before this fix: the old session resumes with full history, idle clock reset.
# After this fix: _should_reset() fires on the recovered row -> fresh session.

What platforms you tested on

macOS (Darwin 25.5.0, arm64). No platform-specific code paths touched (pure SQLite + in-memory session-store logic).

Checklist

Conventional Commits (fix(gateway): ...)
Focused, single-purpose change (recovery-path reset-policy enforcement only)
Tested locally via scripts/run_tests.sh — 572/572 relevant tests pass
ruff check clean on changed files
No Windows-specific code paths touched

alt-glitch · 2026-07-17T11:29:07Z

This was generated by AI during triage.

Related to #66255 and the open reset/recovery fixes #61743, #62012, #62038, and #62477. This PR specifically covers DB-only recovery and idle-clock rearming; the others target distinct self-heal or reopen paths.

tonydwb

Code Review Summary

Verdict: Approved (LGTM)

Overview

Fixes gateway reset policy handling when recovering sessions from state.db. +233/0, 2 files.

Security

No hardcoded secrets or credentials

Code Quality

Clean fix for session recovery edge case
Proper reset policy application

Looks Good

Well-scoped bug fix

Reviewed by Hermes Agent

teknium1 · 2026-07-18T22:06:30Z

Thanks for isolating the DB-only recovery gap. Current main still stamps recovered entries with updated_at=now and publishes _query_recoverable_session() results without _should_reset() (gateway/session.py:1431, 1528-1535, 2027-2038).

Problems

The PR reopens the row before checking policy, then calls end_session(). On current main, end_session() only updates live rows (hermes_state.py:2346-2371); promote_to_session_reset() is the durable boundary API for recoverable agent_close / ws_orphan_reap rows (hermes_state.py:2373-2401). Check first, then promote an expired row; reopen only a valid resume candidate.
The sibling startup recovery path remains unguarded: _ensure_loaded_locked() calls _recover_session_from_db() (gateway/session.py:1203), which reopens and returns the recovered entry without a policy check (gateway/session.py:1480-1489).
Please add a real SessionDB + fresh SessionStore regression. The new mock-only tests do not execute the timestamp query or prove that an expired durable row is no longer recoverable.

Suggested changes

Share the recovered-row policy resolution between _query_recoverable_session() and _recover_session_from_db().

Automated hermes-sweeper review.

…tate.db (NousResearch#66255) get_or_create_session() only evaluated _should_reset() for sessions still present in the in-memory _entries map. A session recovered from state.db after a gateway restart (session_key missing from _entries) skipped the check entirely and was adopted unconditionally — with updated_at stamped at the recovery moment instead of the session's real last activity, which also re-armed the idle clock from zero. A session that should have expired hours or days ago could resume indefinitely, carrying its full history across restarts. Two changes: - _create_entry_from_recovered_row() now derives updated_at from the recovered row's real last message timestamp (new HermesStateDB.get_last_message_timestamp()), falling back to created_at when no messages exist. - _query_recoverable_session() evaluates _should_reset() against the recovered entry before publishing it. If the row is past its reset deadline, it's ended in state.db (end_reason='session_reset') and the caller falls through to the existing fresh-session path, reusing the same was_auto_reset/reset_had_activity notice a live in-memory expiry produces. mode="none" (the default) is unaffected — _should_reset() is a no-op for that policy, so recovery behavior is unchanged unless session_reset is explicitly configured.

…eal-DB tests Follow-up to the previous commit per maintainer review on PR NousResearch#66264: - Share the recovered-row reset-policy resolution between _query_recoverable_session() and _recover_session_from_db() via a new _resolve_recovered_session_row() helper, instead of only guarding the runtime get_or_create_session() path. The startup self-heal path (_prune_stale_sessions_locked -> _recover_session_from_db) now honors the reset policy too. - Stop reopening the row before checking policy. The policy check now runs first; only a valid (non-expired) resume candidate is reopened, and an expired row is promoted via promote_to_session_reset() (with an end_session() fallback for DBs predating that API) instead of being reopened and immediately re-ended. - Add SessionDB.reopen_recoverable_session(): a conditional reopen (WHERE ended_at IS NULL OR end_reason IN ('agent_close', 'ws_orphan_reap')) mirroring promote_to_session_reset()'s recoverable set. The policy check runs without holding a lock, so another thread could finalize the same row with an explicit boundary (session_reset/session_switch/compression) in between; an unconditional reopen would silently erase that boundary. When the conditional update affects no rows, recovery is declined instead of resurrecting the session. - Add two real-SessionDB regression tests (not mock-only): an expired agent_close row is durably promoted and no longer resurfaces via find_latest_gateway_session_for_peer, and get_last_message_timestamp drives the recovered entry's updated_at from actual message data. - Add a race-loss regression test: reopen_recoverable_session() returning False must decline recovery, not publish the row anyway. - Update existing mock-based tests in test_session_store_stale_prune.py and test_session_store_runtime_stale_guard.py to assert against reopen_recoverable_session (the method the recovery path now calls) instead of the now-bypassed plain reopen_session.

GottZ · 2026-08-03T17:27:25Z

This was generated by AI during triage.

Summary

Nine PRs address three linked session-recovery causes: #61242, #61489, #62672, #63068, and merged #65783 persist expiry boundaries; #66264 and #71530 apply reset policy using durable activity before DB recovery; #68617 and #68795 prevent recovery from crossing newer boundaries. The merged #65783 is the reference for #61220, while #66255 and #68539 each retain competing open implementations.

Related pull requests

fix(gateway): set end_reason='session_reset' when finalizing expired sessions #61242 partial — (+114/-0) — n/a: Adds reopen-then-end-as-session_reset at expiry finalization, but its shared error guard can skip end_session and its mock tests do not prove recovery exclusion. Despite the contributor keep_open review, merged fix(gateway): unify session reset boundaries vs recovery — stop resurrecting reset sessions #65783 provides the safer conditional transition and real-DB coverage, so fix(gateway): set end_reason='session_reset' when finalizing expired sessions #61242 is now relevant as an earlier duplicate.
fix(session): write end_reason='session_reset' to state.db on expiry finalization (#61220) #61489 [closed] fixes — (+2506/-45) — n/a: Its Bug: Session expiry finalization doesn't set end_reason='session_reset' — stale recovery reopens expired sessions with full history (#54878) #61220 slice uses separate error guards and real lifecycle tests, but the full diff bundles extensive unrelated work. It remains relevant as a closed duplicate whose reset-boundary fix was superseded by merged fix(gateway): unify session reset boundaries vs recovery — stop resurrecting reset sessions #65783.
fix(gateway): set end_reason='session_reset' on expiry finalization to prevent recovery loop #62672 [closed] partial — (+21/-0) — n/a: Applies the same unconditional reopen-then-session_reset transition as fix(gateway): set end_reason='session_reset' when finalizing expired sessions #61242, with a shared error guard and no real recovery regression. It remains relevant as a closed duplicate superseded by fix(gateway): unify session reset boundaries vs recovery — stop resurrecting reset sessions #65783.
fix(gateway): end finalized expired sessions as reset #63068 [closed] fixes — (+230/-0) — n/a: Introduces conditional promote_to_session_reset(), preserves existing explicit boundaries, and tests the real SessionDB recovery contract, addressing the contributor keep_open review. It remains relevant because this closed PR's implementation was salvaged into merged fix(gateway): unify session reset boundaries vs recovery — stop resurrecting reset sessions #65783.
fix(gateway): unify session reset boundaries vs recovery — stop resurrecting reset sessions #65783 [merged] best fix — (+839/-133) — n/a (recorded best fix for Bug: Session expiry finalization doesn't set end_reason='session_reset' — stale recovery reopens expired sessions with full history (#54878) #61220): Conditionally promotes live or recoverable accidental ends to durable intentional boundaries and prevents expiry cleanup from rewriting the boundary as agent_close. It remains relevant as the merged reference implementation subsuming fix(gateway): set end_reason='session_reset' when finalizing expired sessions #61242, fix(session): write end_reason='session_reset' to state.db on expiry finalization (#61220) #61489, fix(gateway): set end_reason='session_reset' on expiry finalization to prevent recovery loop #62672, and fix(gateway): end finalized expired sessions as reset #63068.
fix(gateway): honor the reset policy when recovering a session from state.db (#66255) #66264 best fix — (+448/-24) — n/a (recorded best fix for Gateway: DB session recovery bypasses the reset policy and re-arms the idle clock — sessions become immortal across restarts #66255): Reconstructs last activity, shares reset-policy evaluation across startup and runtime recovery, promotes expired rows, and conditionally reopens valid candidates with real-DB coverage. Its updated diff addresses the visible keep_open verdict's original gaps, so its recovery-policy mechanism remains salvageable.
fix(gateway): respect reset boundaries during recovery (#68539) #68617 best fix — (+120/-19) — n/a (recorded best fix for Gateway session recovery resurrects ancient never-ended sessions after session_reset — nightly reset silently undone, stale context resurfaces #68539): Selects the newest exact-key or peer row before testing recoverability, making reset-ended and empty newer rows authoritative while preserving permitted accidental-end recovery. This directly supports the high-salvageability keep_open verdict and is the focused boundary-selection implementation.
fix(gateway): respect reset boundaries during session recovery (#68539) #68795 partial — (+291/-16) — n/a: Uses enumerated reset fences and prune bypasses, but its fallback fence cannot connect an older missing-key candidate to a newer keyed boundary, newer empty or other nonrecoverable rows can still be skipped, and unrelated profile work is bundled. Despite its keep_open verdict, these diff-specific gaps favor fix(gateway): respect reset boundaries during recovery (#68539) #68617's recorded best-fix, newest-row-first semantics.
fix(gateway): stamp recovered sessions with their real last activity #71530 best fix — (+581/-79) — n/a (recorded best fix for Gateway: DB session recovery bypasses the reset policy and re-arms the idle clock — sessions become immortal across restarts #66255): Projects last_active and transcript presence in SQL, applies one pre-reopen policy decision across both recovery paths, carries reset metadata into a fresh session, and conditionally refuses close races with real-DB tests. Its updated diff addresses the visible keep_open review's stated gaps and remains a viable competing mechanism to fix(gateway): honor the reset policy when recovering a session from state.db (#66255) #66264.

Duplicates

#61242, #61489, #62672, and #63068 are variants of the #61220 expiry-finalization fix now represented by merged #65783. #66264 and #71530 are competing implementations of the #66255 recovery contract rather than exact duplicates at their current heads; #68795 substantially overlaps #68617 on #68539 but has narrower boundary semantics and unrelated bundled work.

Suggested consolidation

Close #61242 as duplicate of merged #65783; #61489, #62672, and #63068 are already closed members of that chain. Keep #66264 open with a salvage path for its shared last-activity, pre-reopen policy, conditional-reopen, and real-DB mechanism, while author action on #71530 should rebase onto main or split out the SQL-projection/reset-metadata implementation for maintainers to choose one #66255 design without discarding either recorded best-fix verdict. Keep #68617 open with a salvage path for focused newest-row-first boundary selection; close #68795 as duplicate of #68617 after splitting its unrelated profile change—despite #68795's keep_open verdict, its keyed-fallback gap and failure to make every newer durable row authoritative are shown by the diff.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I61220(["issue #61220 (closed)"])
    I66255(["issue #66255 (open)"])
    I68539(["issue #68539 (open)"])
    P66264["PR #66264 (open)"]
    P66264 -.->|partial| I61220
    P66264 -->|best fix| I66255
    P66264 -.->|partial| I68539
    class I61220 closed
    class I66255 open
    class I68539 open
    class P66264 open
    class P66264 best
    class P66264 target
    click I61220 "https://github.com/NousResearch/hermes-agent/issues/61220"
    click I66255 "https://github.com/NousResearch/hermes-agent/issues/66255"
    click I68539 "https://github.com/NousResearch/hermes-agent/issues/68539"
    click P66264 "https://github.com/NousResearch/hermes-agent/pull/66264"

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 9 pull requests and 3 issues in this complex. Each diff was read against this issue; Assessment working set: 287 kB of PR diffs, 40 kB of issue/PR text, 13 kB of discussion (22 comments), 18 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

…uperation depuis state.db Une session recuperee depuis state.db etait reconstruite avec updated_at=now et n'etait JAMAIS evaluee contre la politique de reset. Consequence: une conversation vieille de plusieurs jours etait ressuscitee intacte et son horloge d'inactivite reamorcee, annulant silencieusement session_reset. Observe en prod: 232 lignes ended_at IS NULL, dont une session courriel de 13 jours / 2,4M tokens. La politique elle-meme et le watcher d'expiration fonctionnent; c'est le chemin de recuperation DB qui les contournait. - _policy_expired(): extrait de _is_session_expired pour que le watcher et la barriere partagent une seule implementation et ne divergent pas. - _recovered_row_last_activity(): vraie derniere activite d'une ligne (dernier message -> ended_at -> started_at). ended_at est NULL precisement sur les lignes qui fuient, d'ou le recours au timestamp du message. - _recovered_row_is_expired(): la barriere, appelee AVANT reopen_session sur les DEUX chemins (_recover_session_from_db et _query_recoverable_session). Gater apres le reopen aurait re-orpheline la ligne a chaque refus. Echoue en mode ouvert: une erreur ici recupere comme avant plutot que de creer une session neuve a chaque tour. - _create_entry_from_recovered_row(): updated_at = vraie derniere activite au lieu de now, ce qui reamorcait l'horloge idle et repoussait indefiniment la branche quotidienne (at_hour). - SessionDB.get_last_message_timestamp() (hermes_state.py). N'affecte pas la reprise apres redemarrage: mark_resume_pending recupere quelques secondes/minutes apres l'interruption, tres en deca de idle_minutes (1440 par defaut). Ne pas resserrer ce seuil. Le cron horaire hermes-session-cleanup reste necessaire: il traite l'autre moitie (lignes orphelinees quand le gateway meurt en vol), que ce correctif ne couvre pas. Reference amont: PR NousResearch#66264 (non mergee) — a reconcilier plus tard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 17, 2026

tonydwb reviewed Jul 17, 2026

View reviewed changes

JoshHobbs mentioned this pull request Jul 18, 2026

fix(gateway): stabilize BlueBubbles DM session keys #67105

Closed

10 tasks

teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 18, 2026

kimdzhekhon added 2 commits July 19, 2026 08:32

kimdzhekhon force-pushed the fix/gateway-session-recovery-reset-policy branch from e2c6db2 to d1e34e0 Compare July 18, 2026 23:44

teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026

alt-glitch mentioned this pull request Jul 25, 2026

fix(gateway): stamp recovered sessions with their real last activity #71530

Closed

GottZ mentioned this pull request Jul 26, 2026

Gateway session recovery resurrects ancient never-ended sessions after session_reset — nightly reset silently undone, stale context resurfaces #68539

Open

GottZ mentioned this pull request Aug 3, 2026

fix(gateway): respect reset boundaries during session recovery (#68539) #68795

Open

GottZ mentioned this pull request Aug 3, 2026

fix(gateway): respect reset boundaries during recovery (#68539) #68617

Closed

5 tasks

liuhao1024 mentioned this pull request Aug 18, 2026

fix(gateway): notify resets finalized through the recovery fence #89324

Open

13 tasks

Navigation Menu

Uh oh!

fix(gateway): honor the reset policy when recovering a session from state.db (#66255) - #66264

fix(gateway): honor the reset policy when recovering a session from state.db (#66255)#66264
kimdzhekhon wants to merge 2 commits into
NousResearch:mainfrom
Library-Core:fix/gateway-session-recovery-reset-policy

kimdzhekhon commented Jul 17, 2026

Uh oh!

alt-glitch commented Jul 17, 2026

Uh oh!

tonydwb left a comment

Uh oh!

teknium1 commented Jul 18, 2026

Uh oh!

GottZ commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

kimdzhekhon commented Jul 17, 2026

What does this PR do?

Related Issue

Type of Change

Changes Made

How to Test

What platforms you tested on

Checklist

Uh oh!

alt-glitch commented Jul 17, 2026

Uh oh!

tonydwb left a comment

Choose a reason for hiding this comment

Code Review Summary

Overview

Security

Code Quality

Looks Good

Uh oh!

teknium1 commented Jul 18, 2026

Problems

Suggested changes

Uh oh!

GottZ commented Aug 3, 2026

Summary

Related pull requests

Duplicates

Suggested consolidation

Complex graph

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants