Skip to content

feat: add configuration for transparent session re-init#760

Open
glicht wants to merge 4 commits intomodelcontextprotocol:mainfrom
binahm:feat/config-session-re-init
Open

feat: add configuration for transparent session re-init#760
glicht wants to merge 4 commits intomodelcontextprotocol:mainfrom
binahm:feat/config-session-re-init

Conversation

@glicht
Copy link
Contributor

@glicht glicht commented Mar 19, 2026

Motivation and Context

Providing control if to enable transparent session re-init. Added to StreamableHttpClientTransportConfig a new field: enable_reinit_on_expired_session to control if to do transparent session re-init.

How Has This Been Tested?

Added a unit test: test_session_expired_error_when_reinit_disabled and updated current unit test: test_transparent_reinitialization_on_session_expiry.

New config option is documented as part of the docs for StreamableHttpClientTransportConfig.

Breaking Changes

No

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

Closes #755

@glicht glicht requested a review from a team as a code owner March 19, 2026 18:39
@github-actions github-actions bot added T-dependencies Dependencies related changes T-test Testing related changes T-CI Changes to CI/CD workflows and configuration T-config Configuration file changes T-core Core library changes T-transport Transport layer changes labels Mar 19, 2026
@github-actions github-actions bot removed the T-CI Changes to CI/CD workflows and configuration label Mar 19, 2026
@glicht
Copy link
Contributor Author

glicht commented Mar 19, 2026

I originally added to ci.yml also running tests without the local feature as many tests weren't running during ci as they had: not(feature = "local"). But we have a failing test: test_custom_client_request_reaches_server.

So I reverted this addition. I've opened a separate PR for running the non-local tests here.

Copy link
Member

@DaleSeo DaleSeo left a comment

Choose a reason for hiding this comment

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

Thank you for adding this configurability, @glicht! I have a comment on the default value choice.

allow_stateless: true,
auth_header: None,
custom_headers: HashMap::new(),
enable_reinit_on_expired_session: false,
Copy link
Member

Choose a reason for hiding this comment

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

The MCP specification states:

When a client receives HTTP 404 in response to a request containing an Mcp-Session-Id, it MUST start a new session by sending a new InitializeRequest without a session ID attached.

https://spec.modelcontextprotocol.io/specification/2025-11-25/basic/transports/#session-management

I'm afraid that if we set enable_reinit_on_expired_session to false, the transport becomes spec-non-compliant out of the box.

Previously, the behavior was to always try to re-initialize transparently. Setting it to false is also a silent breaking change for existing users who upgrade without changing their configuration.

I recommend setting it to true to comply with the spec and maintain backward compatibility. Users who want to opt out of automatic re-initialization can explicitly set it to false.

Suggested change
enable_reinit_on_expired_session: false,
enable_reinit_on_expired_session: true,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Will change.

///
/// This recovery is best-effort and bounded to a single attempt. If recovery fails,
/// the original failure path is preserved and the error is returned to the caller.
pub enable_reinit_on_expired_session: bool,
Copy link
Member

Choose a reason for hiding this comment

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

for consistency with the existing style

Suggested change
pub enable_reinit_on_expired_session: bool,
pub reinit_on_expired_session: bool,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Will change.

@glicht
Copy link
Contributor Author

glicht commented Mar 20, 2026

Thank you for adding this configurability, @glicht! I have a comment on the default value choice.

I've updated following your comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-config Configuration file changes T-core Core library changes T-dependencies Dependencies related changes T-test Testing related changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configuration to enable/disable "transparent session re-init on HTTP 404"

2 participants