Skip to content

Conversation

@Nyholm
Copy link
Contributor

@Nyholm Nyholm commented Dec 27, 2025

These are reserved for internal use. This will fix #168

Motivation and Context

The _session and _request are used internally, they are not meant to be injected via argument injection.
An alternative to this PR would be to remove them in ReferenceHandler::prepareArguments(), but that would throw an exception when the tool is called. By doing it in the SchemaGenerator we get an exception when the server was started

How Has This Been Tested?

I modified an example and tested.

Breaking Changes

No. I consider this a bug fix. See #168.

The "feature" of naming a variable $_session is not documented nor covered by tests.

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
  • [ x New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@Nyholm Nyholm added this to the 0.2.x milestone Dec 27, 2025
@chr-hertel
Copy link
Member

Nice hardening - could you please extend the SchemaGeneratorTest briefly to bake this in?

@Nyholm
Copy link
Contributor Author

Nyholm commented Dec 27, 2025

Sure, will do

@Nyholm
Copy link
Contributor Author

Nyholm commented Dec 27, 2025

I've updated the PR with a test

Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

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

Thanks @Nyholm!

@chr-hertel chr-hertel merged commit abd0ddf into modelcontextprotocol:main Dec 27, 2025
15 checks passed
@Nyholm
Copy link
Contributor Author

Nyholm commented Dec 27, 2025

Thank you for merging

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.

[Server] Schema not removed after injecting SessionInterface

2 participants