Skip to content

fix: remove stale session entry from sessions.json on idle-TTL eviction - #55232

Closed
Stoltemberg wants to merge 2 commits into
NousResearch:mainfrom
Stoltemberg:fix/session-store-stale-eviction
Closed

Stoltemberg wants to merge 2 commits into
NousResearch:mainfrom
Stoltemberg:fix/session-store-stale-eviction

Conversation

@Stoltemberg

Copy link
Copy Markdown
Contributor

What does this PR do?

When _sweep_idle_cached_agents() evicts an agent after the idle TTL (3600s), the session is ended in state.db but sessions.json was NOT updated. The stale session_key → session_id entry remained, routing all subsequent messages on that key into a closed session — silently dropping them until gateway restart (when _prune_stale_sessions_locked() fires).

Adds SessionStore.remove() method and calls it during idle eviction so sessions.json is cleaned up immediately.

Related Issue

Fixes #54878

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/session.py: Add SessionStore.remove(session_key) method that removes an entry from _entries and calls _save(), following the existing thread-safety pattern (lock + _ensure_loaded_locked)
  • gateway/run.py: In _sweep_idle_cached_agents(), call self.session_store.remove(key) after popping the agent from cache, wrapped in try/except to avoid blocking eviction on a save failure

How to Test

  1. Configure a gateway with a short idle TTL (or wait for the default 3600s)
  2. Send a message, wait for idle eviction
  3. Send another message on the same session key — should create a fresh session instead of being silently dropped

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 is not a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run ruff check gateway/session.py gateway/run.py and both pass
  • I've tested on my platform: Windows

Documentation & Housekeeping

  • I've updated relevant documentation — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — uses pathlib and threading, cross-platform safe
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

When _sweep_idle_cached_agents() evicts an agent after the idle TTL,
the session is ended in state.db but sessions.json was NOT updated.
The stale session_key -> session_id entry remained, routing all
subsequent messages on that key into a closed session — silently
dropping them until gateway restart.

Add SessionStore.remove() method and call it during idle eviction
so sessions.json is cleaned up immediately.

Fixes NousResearch#54878
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jun 29, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Competing with #54916 for the same issue (#54878). Same goal — clear the stale sessions.json entry on idle-TTL eviction so messages aren't silently dropped — but a different mechanism: this PR adds SessionStore.remove() and calls it directly in _sweep_idle_cached_agents, while #54916 adds _cleanup_evicted_agent_session + remove_ended_session/_is_session_active validation. Related family: #55045 / #54293 fix the memory-provider on_session_end loss on the same eviction path. Maintainer should pick one of the two sessions.json fixes.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: LGTM

Clean fix — removes stale session entries from sessions.json after idle-TTL eviction to prevent messages from routing into ended sessions. The SessionStore.remove() method is well-implemented with proper locking and save.

Correctness: The remove() call in _sweep_idle_cached_agents is the correct injection point.
Testing: Dedicated test file with clear assertions for the remove operation.


Reviewed by Hermes Agent

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: LGTM

Fixes stale session routing after idle-TTL eviction. Removes session entry from sessions.json when agent is evicted, preventing messages from routing into ended sessions.

  • Clean fix with dedicated test
  • Well-scoped: 3 files, 56 additions

Reviewed 3 files, 56 additions. Approved.

@Stoltemberg

Copy link
Copy Markdown
Contributor Author

Closing — duplicate detected by alt-glitch bot. Consolidating into earlier PRs. 🙏

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 P1 High — major feature broken, no workaround 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]: idle agent cache eviction ends session in DB but leaves sessions.json stale → messages silently dropped until restart

3 participants