fix(gateway): notify resets finalized through the recovery fence - #89324
Open
liuhao1024 wants to merge 1 commit into
Open
fix(gateway): notify resets finalized through the recovery fence#89324liuhao1024 wants to merge 1 commit into
liuhao1024 wants to merge 1 commit into
Conversation
The session-reset expiry notice is delivered on the next inbound message, keyed off was_auto_reset on the routing entry. When the expiry watcher finalized the session in state.db and the sessions.json entry is missing or stale by the time that message arrives, recovery returns None — the session_reset boundary fences it (NousResearch#68539) — and the fresh-session path created the new session with was_auto_reset=False, silently dropping the notice the live-entry path delivers. When recovery comes back empty, look up the peer's latest reset boundary; a session_reset boundary flags the fresh session as auto-reset (reason per policy, mode:none still opts out). A manual /reset cannot be confused with this: it mints a fresh live row that recovery would have found, so an unrecoverable session_reset boundary means the reset was never followed by a new conversation yet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
The "◐ Session automatically reset …" notice is delivered on the next inbound message after the reset, keyed off
was_auto_reseton the routing entry. That works on the live-entry paths (stale routing drop and clean policy reset), but not when the reset was finalized through the recovery fence:end_reason=session_reset).sessions.jsonrouting entry is missing or stale — recovery runs instead.find_latest_gateway_session_for_peerreturnsNone: thesession_resetboundary fences recovery (Gateway session recovery resurrects ancient never-ended sessions after session_reset — nightly reset silently undone, stale context resurfaces #68539), by design.was_auto_reset=False— the notice is silently dropped, and the user's next reply lands in an unexplained empty conversation.This PR closes that gap: when recovery comes back empty, it looks up the peer's latest intentional reset boundary (
find_latest_reset_boundary_for_peer). Asession_resetboundary flags the fresh session as auto-reset (reason derived from the policy mode), carryingprev_session_idandreset_had_activitylike the live-entry paths — so the existing notice machinery in the gateway fires unchanged.Scoping guarantees:
/newor other non-session_resetboundary never flags (the end_reason check).session_reset.mode: nonenever flags (policy opt-out preserved, gateway: resume_pending_expired resets despite session_reset.mode=none and shows misleading 'inactive for 24h' notice #61052)./resetis not confused with an automatic one: it mints a fresh live row that recovery would have found, so reaching the fresh path with an unrecoverablesession_resetboundary means the reset was never followed by a new conversation yet.Related open PRs in the same function are complementary, not overlapping: #59597 flags
was_auto_resetwhen a stale routing entry is dropped (its trigger requires a dropped entry), and #66264 evaluates the reset policy on successfully recovered rows. Neither covers the entry-missing + fence-blocked shape this PR fixes.Related Issue
Fixes #89314
Type of Change
Changes Made
hermes_state.py— newfind_latest_reset_boundary_for_peer(session_key, source): the peer's most recent row with an end_reason in_RESET_END_REASONS, ordered byended_at; returns id/end_reason/ended_at/last_activity/message_count.gateway/session.py— inget_or_create_session's Phase 3, when recovery returned nothing and nothing setwas_auto_reset, consult the boundary; asession_resetboundary under a non-nonepolicy setswas_auto_reset,auto_reset_reason(idle/daily per mode),reset_had_activity(boundary row's message count), andprev_session_id.tests/gateway/test_session_store_runtime_stale_guard.py—TestResetBoundaryNotice: fencedsession_resetboundary flags the fresh session (fails on main withwas_auto_reset=False); anew_commandboundary never flags;mode=nonenever flags.How to Test
Fail-on-main check: with the two source files stashed,
test_fenced_reset_boundary_flags_auto_reset_on_fresh_sessionfails on main (was_auto_resetstays False); the two scoping pins pass on both.Manual repro from the issue: with
session_reset.mode: idle, let the watcher finalize the session (state.db row getsend_reason=session_reset), remove/stale the routing entry (or restart losing the mapping), then message the chat — on main the reply arrives with no notice; with this change the "◐ Session automatically reset (inactive for …)" notice is delivered.Checklist