Skip to content

fix(core): clear recently-cancelled request ids on connection close#892

Merged
devcrocod merged 2 commits into
mainfrom
fix/clear-cancelled-ids-on-close
Jul 16, 2026
Merged

fix(core): clear recently-cancelled request ids on connection close#892
devcrocod merged 2 commits into
mainfrom
fix/clear-cancelled-ids-on-close

Conversation

@devcrocod

Copy link
Copy Markdown
Contributor

Clear the recently-cancelled request id buffer in doClose so connection teardown fully resets per-request bookkeeping.

Motivation and Context

Follow-up to #884. doClose already resets _responseHandlers, _progressHandlers, and inFlightRequestJobs, but left recentlyCancelledRequestIds populated across a close. This aligns teardown to clear all Protocol-scoped per-request state.

Not a correctness bug: the buffer is bounded (256) and self-evicting, request ids are random UUIDs so stale entries cannot false-match a new connection, and the post-close onMessage guard drops late messages from the old transport before the buffer is consulted. The change removes the lone asymmetry in doClose.

How Has This Been Tested?

  • ./gradlew :kotlin-sdk-core:compileKotlinJvm
  • ./gradlew :kotlin-sdk-core:jvmTest --tests "*ProtocolTest*"

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

recentlyCancelledRequestIds is Protocol-scoped (like _responseHandlers/_progressHandlers), so it is reset in doClose rather than on the per-connection Connection object.

Copilot AI review requested due to automatic review settings July 15, 2026 11:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the core Protocol connection teardown path to fully reset Protocol-scoped per-request bookkeeping by clearing the buffer of recently-cancelled outbound request IDs during doClose. This aligns recentlyCancelledRequestIds cleanup with the existing teardown behavior for response handlers, progress handlers, and in-flight request jobs.

Changes:

  • Clear recentlyCancelledRequestIds during Protocol.doClose(...) to avoid carrying buffered cancellations across a close/reconnect.
  • Keep teardown symmetry with other Protocol-scoped request tracking structures (_responseHandlers, _progressHandlers, inFlightRequestJobs).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@devcrocod
devcrocod requested a review from e5l July 15, 2026 11:07

@e5l e5l left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

Copilot AI review requested due to automatic review settings July 16, 2026 11:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@devcrocod
devcrocod merged commit 5cb76fd into main Jul 16, 2026
21 checks passed
@devcrocod
devcrocod deleted the fix/clear-cancelled-ids-on-close branch July 16, 2026 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants