Skip to content

fix(gateway): notify user and write correct end_reason for resume_pending_expired resets - #58935

Open
hejuntt1014 wants to merge 1 commit into
NousResearch:mainfrom
hejuntt1014:fix/resume-pending-expired-notification
Open

hejuntt1014 wants to merge 1 commit into
NousResearch:mainfrom
hejuntt1014:fix/resume-pending-expired-notification

Conversation

@hejuntt1014

Copy link
Copy Markdown
Contributor

What does this PR do?

When a gateway session has resume_pending=True (set after a drain-timeout restart) and API recovery fails — e.g. repeated timeouts on a large context — the resume_pending marker is never cleared. After gateway_auto_continue_freshness seconds the next inbound message silently creates a new session. Two implementation gaps existed in that path:

  1. No user notification. resume_pending_expired fell into the generic else branch in _handle_message_with_agent, which produced the wrong wording ("inactive for Xh") and, for session_reset.mode: none users, was gated behind policy.notify — which evaluates to False on that mode — so no notice was ever sent.
  2. Generic DB end_reason. The old session was always closed with the hardcoded string "session_reset", making it impossible to distinguish a restart-recovery timeout from a normal idle/daily reset in post-mortem analysis.

This PR fixes both gaps:

  • Adds an explicit resume_pending_expired case with correct agent system note and user notification (always fires, like suspended)
  • Passes auto_reset_reason as the DB end_reason so every auto-reset path is auditable

Related Issue

Fixes #58933

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • gateway/run.py (_handle_message_with_agent, _was_auto_reset block):

    • Added elif reset_reason == "resume_pending_expired": for the agent context note: "gateway restart recovery timed out"
    • Added "resume_pending_expired" to the should_notify always-on set (alongside "suspended") — restores history-context awareness without depending on session_reset policy config
    • Added elif reset_reason == "resume_pending_expired": for reason_text in the user-facing notice: "gateway restart recovery timed out"
  • gateway/session.py (get_or_create_session, SQLite section outside lock):

    • Replaced hardcoded "session_reset" with auto_reset_reason if auto_reset_reason else "session_reset", so resume_pending_expired, suspended, idle, and daily resets all record their specific reason in state.db
  • tests/gateway/test_session_reset_notify.py:

    • Added _make_db_mock() helper with safe defaults for all DB lookup methods (prevents MagicMock leaking into session_id via the compression-tip heal path)
    • Added TestResumePendingExpiredAutoReset class with 5 new tests:
      • test_stale_resume_pending_sets_auto_reset_reasonauto_reset_reason == "resume_pending_expired" when freshness expires
      • test_stale_resume_pending_had_activity_flagreset_had_activity reflects token usage
      • test_stale_resume_pending_db_end_reason_is_specificdb.end_session called with "resume_pending_expired" not "session_reset"
      • test_idle_reset_db_end_reason_reflects_idle — idle path non-regression
      • test_freshness_disabled_skips_resume_pending_expiredHERMES_AUTO_CONTINUE_FRESHNESS=0 disables the gate

How to Test

  1. Start gateway with a large-context session on any platform
  2. Restart gateway in a way that causes drain timeout (Skipping .clean_shutdown marker)
  3. Verify session is marked resume_pending=True in sessions.json
  4. Simulate API failures during recovery so resume_pending is never cleared
  5. Wait longer than gateway_auto_continue_freshness (default 3600s) then send a message
  6. Before this fix: silent new session, no notice, old session ends with "session_reset" in state.db
  7. After this fix: user receives ◐ Session automatically reset (gateway restart recovery timed out). ..., old session ends with "resume_pending_expired" in state.db

Or run the new unit tests:

pytest tests/gateway/test_session_reset_notify.py -v

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/gateway/test_session_reset_notify.py -v and all 21 tests pass
  • I've added tests for my changes (5 new tests in TestResumePendingExpiredAutoReset)
  • I've tested on my platform: Ubuntu 22.04 (Linux 6.8.0-124-generic), reproduced from production logs

Documentation & Housekeeping

  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A (no new config keys)
  • I've considered cross-platform impact — N/A (pure Python logic, no platform-specific code)
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Log evidence from two independent production profiles (dingding, bala) that triggered this bug simultaneously at ~19:22 on 2026-07-05 (~1h52m after the 17:30 restart):

# Profile A — before fix
17:30:09 Skipping .clean_shutdown marker (drain timeout)
17:30:52 [57182b] attempting auto-resume (history=2126, ~369k tokens)
17:41:06 Stream stale — killing connection
17:42:01 API call failed after 3 retries
# gap: resume_pending never cleared, user inactive
19:22:32 inbound msg='小可'
19:22:32 [3dc5bf3f] conversation turn: history=0   ← silent new session
19:22:36 title_generation → "召唤小可"           ← no hint a switch occurred

# Profile B — same pattern, 19:23
19:23:51 [d107459d] conversation turn: history=0
19:23:56 title_generation → "可爱的中文打招呼"

After this fix, both users would have received:

◐ Session automatically reset (gateway restart recovery timed out).
Conversation history cleared.
Use /resume to browse and restore a previous session.
Adjust reset timing in config.yaml under session_reset.

Made with Cursor

…ng_expired resets

When a gateway session with resume_pending=True is not recovered within the
auto-continue freshness window (e.g. because repeated API calls timed out on a
large context), get_or_create_session correctly creates a new session.  However
two gaps existed:

1. The user received no notification — resume_pending_expired fell through the
   generic "inactive for Xh" else-branch in run.py, which produces wrong wording
   and (for session_reset.mode: none users) is gated on policy.notify that
   evaluates to False.
2. The old session was ended in state.db with the hardcoded generic reason
   "session_reset", making it impossible to distinguish from a normal idle/daily
   reset in post-mortem analysis.

Fix:
- gateway/run.py: add an explicit resume_pending_expired case for the agent
  context note ("gateway restart recovery timed out") and the user-facing
  notification.  Always notify for this reason — like suspended — because the
  user had an active session that was silently replaced.
- gateway/session.py: pass auto_reset_reason as the DB end_reason instead of
  the hardcoded "session_reset", so all auto-reset paths are auditable.
- tests: extend TestResumePendingExpiredAutoReset in test_session_reset_notify.py
  with five new cases that cover the reason, activity flag, DB end_reason,
  non-regression of the idle path, and freshness-disabled bypass.

Closes NousResearch#58933

Co-authored-by: Cursor <cursoragent@cursor.com>
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused gateway fix. The premise is confirmed on current main: gateway/session.py:1881-1882 produces resume_pending_expired, while gateway/run.py:11094-11131 still treats that reason as ordinary inactivity and gateway/session.py:2004 persists the generic session_reset end reason.

Problems

  • The added tests cover storage-state and end_session arguments, but not either changed behavior in gateway/run.py. In particular, no regression proves that a resume_pending_expired reset with notifications disabled still sends the restart-recovery notice, or that the agent receives the corrected context note (gateway/run.py:11090-11149).

Suggested changes

  • Add an async handler-level regression with policy.notify=False that asserts the adapter notice and the context prompt passed to the agent both identify restart recovery timing out.
  • Salvage the DB-reason substitution into the current finalization block at gateway/session.py:2001-2006; the surrounding method was refactored after this PR's base.

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

This branch has not been deployed

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

Labels

comp/gateway Gateway runner, session dispatch, delivery 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.

[Bug]: resume_pending_expired silently creates new session without notifying user or properly closing the old one

3 participants