Fix session stickiness: advance last_activity_at on turns and explicit switches#183
Open
serxa wants to merge 2 commits into
Open
Fix session stickiness: advance last_activity_at on turns and explicit switches#183serxa wants to merge 2 commits into
serxa wants to merge 2 commits into
Conversation
mark_active() set connected_at and last_activity_at to the same value, and its callers pass the session's original connected_at when resuming an SDK client. That pinned last_activity_at to session start, so it never advanced across turns. last_activity_at drives channel stickiness (_is_within_sticky_period): once a session was older than sessions.sticky_period_minutes, the next inbound message on the channel minted a brand-new session even though a turn had just completed and the previous session was still alive — surfacing as an unexpected context reset on Telegram/web. Decouple the two: connected_at keeps the caller-supplied value (or now), last_activity_at always advances to the current time. Add regression tests for the unit behaviour and the resume->idle->next-message path. Co-Authored-By: Claude <noreply@anthropic.com>
set_active_session only rewrote the channel→session mapping. Channels without a per-message session id (Telegram) resolve the target via get_active_session, whose sticky-period check rejects a session idle longer than sessions.sticky_period_minutes and mints a fresh one — so selecting a long-idle session from /sessions and then sending a message silently routed into a NEW session instead of the chosen one. Mark the chosen session freshly active on switch (this also bumps updated_at, resetting the idle-archive clock) so the sticky check honours the deliberate choice. Add a regression test. Co-Authored-By: Claude <noreply@anthropic.com>
Member
Author
|
This is ready to be reviewed |
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.
Two related fixes to session-routing stickiness.
last_activity_atdrives_is_within_sticky_period, which decides whether an inbound message resumes the channel's mapped session or mints a fresh one.1.
last_activity_atfrozen at connect timemark_active()setconnected_atandlast_activity_atto the same value, and its callers pass the session's originalconnected_atwhen resuming an SDK client — solast_activity_atwas pinned to session start and never advanced across turns. Any session older thansessions.sticky_period_minuteswas then wrongly rotated onto a fresh session on the next message (an unexpected context reset on Telegram/web). Fix:connected_atkeeps the caller value (or now);last_activity_atalways advances to now.2. Explicit switches ignored past the sticky period
set_active_session()only rewrote the channel→session mapping. Channels without a per-message session id (Telegram) resolve the target viaget_active_session, whose sticky-period check rejects a session idle longer than the window and mints a fresh one — so selecting a long-idle session and then sending a message silently routed into a NEW session instead of the chosen one. Fix: an explicit switch marks the chosen session freshly active (this also bumpsupdated_at, resetting the idle-archive clock), so the deliberate choice is honoured.Tests
Regression tests for both:
mark_activeadvanceslast_activity_atdespite an oldconnected_at; a resumed long-lived session stays sticky after a turn; and an explicit switch to a long-idle session routes the next message there instead of forking. Both fail onmain, pass with the fix. Full suite: 1550 passed, 2 skipped. Backend-only.