Skip to content

fix: preserve SSE resume cache and correct event replay - #1165

Open
freedom-winds wants to merge 3 commits into
modelcontextprotocol:mainfrom
freedom-winds:pr/09-rust-sdk-session-resume-and-cache-20260811200316-01
Open

fix: preserve SSE resume cache and correct event replay#1165
freedom-winds wants to merge 3 commits into
modelcontextprotocol:mainfrom
freedom-winds:pr/09-rust-sdk-session-resume-and-cache-20260811200316-01

Conversation

@freedom-winds

Copy link
Copy Markdown

Summary

Fix two issues in the Streamable HTTP server's SSE resume behavior:

  • The event identified by Last-Event-ID was replayed again.
  • Request-scoped SSE caches were deleted immediately after the final response, preventing clients from recovering responses after a disconnect.

Changes

  • Adjust the SSE resume offset to replay only events after Last-Event-ID.
  • Reject event IDs older than the retained cache window.
  • Mark completed request channels as complete while retaining their cached events.
  • Allow completed_cache_ttl to evict completed request caches.
  • Add regression coverage for retaining completed request channel state.

@freedom-winds
freedom-winds requested a review from a team as a code owner August 11, 2026 12:03
@github-actions github-actions Bot added T-core Core library changes T-transport Transport layer changes labels Aug 11, 2026
for resource in channel.resources {
if close {
let resources: Vec<_> = request_wise.resources.drain().collect();
request_wise.completed_at = Some(Instant::now());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The worker checks for eviction before it waits, so it checks this newly completed entry right away. That means the entry can outlive its TTL. If Resume is the next event, it gets served before the next eviction pass.

Comment on lines +338 to +345
if index < front_event_id.index {
return Err(SessionError::InvalidEventId);
}
// Last-Event-ID is the last event the client received, so resume only
// with events that follow it.
let sync_index = index
.saturating_sub(front_event_id.index)
.saturating_add(1);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the retained cache starts at event N and the client last received N - 1, every missed event is still available, but this check rejects the resume.

@github-actions github-actions Bot added the T-test Testing related changes label Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-test Testing related changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants