Skip to content

fix(gateway): set ended_at on expired gateway sessions to bound state.db growth - #56603

Closed
adityachaudhary99 wants to merge 1 commit into
NousResearch:mainfrom
adityachaudhary99:fix/session-db-cleanup-54189
Closed

adityachaudhary99 wants to merge 1 commit into
NousResearch:mainfrom
adityachaudhary99:fix/session-db-cleanup-54189

Conversation

@adityachaudhary99

Copy link
Copy Markdown

What does this PR do?

Fixes unbounded state.db growth (659MB / 938 never-ended sessions in ~2 weeks). Expired gateway sessions were finalized in _session_expiry_watcher by closing the cached agent — but an idle session's agent is almost always soft-evicted first (cache eviction deliberately preserves the row for resume and never calls end_session). So when the expiry watcher runs there's no cached agent to close, and the DB row keeps ended_at IS NULL forever, accumulating without bound.

This calls the existing SessionDB.end_session(session_id, "session_expired") at the finalization point in _session_expiry_watcher, independent of whether a cached agent still exists. It's first-reason-wins and no-ops on an already-ended row, so it's safe alongside agent_close. I verified the cron / sync-subagent / async-subagent / one-shot / interactive-CLI paths already set ended_at, so the expiry watcher was the one real gap.

Scope is intentionally bounded to closing the lifecycle gap. A retention/prune policy for the now-eligible rows (there's already a maybe_auto_prune_and_vacuum hook) is left as a follow-up to avoid a design debate here.

Related Issue

Fixes #54189

Type of Change

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

Changes Made

  • gateway/run.py: in _session_expiry_watcher's per-entry finalization, call end_session(entry.session_id, "session_expired") (best-effort, logged on failure).

How to Test

  1. Create a gateway session; let its agent get soft-evicted (idle sweep) and then expire.
  2. Before: the state.db session row keeps ended_at IS NULL.
  3. After: the row has ended_at set with end_reason = "session_expired".

Added test_idle_expiry_sets_ended_at_in_db in tests/gateway/test_session_boundary_hooks.py (real SessionDB, empty agent cache to reproduce the leak, asserts ended_at is set after the watcher runs).

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

Expired gateway sessions were finalized in _session_expiry_watcher by closing the
cached agent, but an idle session's agent is almost always soft-evicted (which
preserves the row for resume, never calling end_session) before its reset policy
expires. So the DB row kept ended_at IS NULL forever — the unbounded state.db
growth in NousResearch#54189 (659MB / 938 never-ended sessions). Call the existing
end_session(session_id, "session_expired") helper at the finalization point,
independent of whether a cached agent still exists (first-reason-wins, no-op if
already ended). cron/subagent/one-shot/interactive paths already set ended_at.

A retention/prune policy for the now-eligible rows is left as a follow-up.

Fixes NousResearch#54189

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@adityachaudhary99
adityachaudhary99 force-pushed the fix/session-db-cleanup-54189 branch from 9fa9b6c to 6550d99 Compare July 9, 2026 14:59
@motoblurr

Copy link
Copy Markdown
Contributor

Current-main coordination update (no duplicate PR opened): I rebased the bounded expiry closeout onto current main (e589b739) at motoblurr@bca94f6. It keeps the existing first-reason-wins end_session(..., "session_expired") behavior and the real-DB regression for an already-soft-evicted agent.

Verification on the clean current-main branch: 379 passed (test_session_boundary_hooks.py, test_session_store_prune.py, test_hermes_state.py). Operational note: this fixes genuinely expired sessions; it intentionally does not affect installations configured with session_reset.mode: none until a session is otherwise expired.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for isolating a real lifecycle gap: current main's expiry watcher persists expiry_finalized at gateway/run.py:7843, but does not set the session row's ended_at.

Problems

  • The proposed call at gateway/run.py:7707 invokes synchronous session_store._db.end_session() on the async gateway loop. Current main routes loop-side database work through AsyncSessionDB (hermes_state.py:7196-7210), and tests/gateway/test_async_session_db.py:315-344 enforces that boundary.
  • The regression test targets the older synchronous watcher setup. Current main first awaits async_session_store at gateway/run.py:7753; the test needs the current async store/database facades to exercise the live path.

Suggested changes

  • Salvage the closeout through awaited self._session_db.end_session(entry.session_id, "session_expired"), preserving best-effort failure handling before set_expiry_finalized.
  • Update the test to wrap its temporary SessionDB in AsyncSessionDB and configure the async session-store path.

Automated hermes-sweeper review.

Comment thread gateway/run.py
_sess_db = getattr(self.session_store, "_db", None)
if _sess_db is not None and entry.session_id:
try:
_sess_db.end_session(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This runs inside the async gateway watcher, but _sess_db is the synchronous SessionDB. Current main requires loop-side database calls to go through the awaited AsyncSessionDB facade (await self._session_db.end_session(...)) so SQLite writes are offloaded rather than blocking the gateway event loop.

@teknium1 teknium1 added 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 area/sessions Session lifecycle, resume, persistence, history labels Jul 15, 2026
@teknium1

teknium1 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Already fixed on main: SessionStore.set_expiry_finalizedpromote_to_session_reset (9fc0074) sets ended_at on expiry. Thanks for the early report, @adityachaudhary99.

@teknium1 teknium1 closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history 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-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.

state.db unbounded growth: no session lifecycle/cleanup mechanism

4 participants