Skip to content

feat: add Default and constructors to ServerSseMessage#794

Open
erenatas wants to merge 2 commits intomodelcontextprotocol:mainfrom
erenatas:fix/server-sse-message-constructors
Open

feat: add Default and constructors to ServerSseMessage#794
erenatas wants to merge 2 commits intomodelcontextprotocol:mainfrom
erenatas:fix/server-sse-message-constructors

Conversation

@erenatas
Copy link
Copy Markdown

@erenatas erenatas commented Apr 8, 2026

Motivation and Context

ServerSseMessage is #[non_exhaustive] but has no public constructor, Default impl, or builder, making it impossible for downstream SessionManager implementations to produce these values without unsafe.

This change adds #[derive(Default)] and two named constructors covering the common patterns:

  • ServerSseMessage::new(event_id, message) — JSON-RPC content with
    an event ID
  • ServerSseMessage::priming(event_id, retry) — priming event per
    SEP-1699
    Update all internal call sites in tower.rs and local.rs to use the new API.

How Has This Been Tested?

Tested in an rmcp based project. Pre-existing tests in tower.rs and local.rs continue to pass — these exercise the SSE stream paths that now use the new constructors.

Note: the existing http_header test module on main has a pre-existing compilation error (references a renamed function extract_scope_from_header); this is unrelated to this change.

Breaking Changes

None. This is a purely additive change:

Default is a new trait impl on an existing public type.
new() and priming() are new inherent methods.
No existing API surface is modified or removed.

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

@erenatas erenatas requested a review from a team as a code owner April 8, 2026 07:42
@github-actions github-actions bot added T-core Core library changes T-transport Transport layer changes labels Apr 8, 2026
Copy link
Copy Markdown
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.

Can we add lightweight tests that assert field values for ServerSseMessage::new, ServerSseMessage::priming and Default? It would guard against accidental regressions if the type or helpers evolve.

@erenatas
Copy link
Copy Markdown
Author

erenatas commented Apr 8, 2026

Can we add lightweight tests that assert field values for ServerSseMessage::new, ServerSseMessage::priming and Default? It would guard against accidental regressions if the type or helpers evolve.

Yes, thank you for the suggestion. On it

@github-actions github-actions bot added the T-test Testing related changes label Apr 8, 2026
@erenatas
Copy link
Copy Markdown
Author

erenatas commented Apr 8, 2026

I updated with basic tests, if you won't mind I fixed couple issues with tests that prevented me to run cargo test --all-features (--workspace still doesn't work, for another PR perhaps). Let me know if its a problem and I will revert my changes. Thanks!

@erenatas erenatas requested a review from DaleSeo April 8, 2026 15:04
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