Skip to content

fix(core): preserve input server_params by using get() instead of pop()#66

Merged
grll merged 1 commit into
grll:mainfrom
fblacuttgottret-godaddy:main
Aug 3, 2025
Merged

fix(core): preserve input server_params by using get() instead of pop()#66
grll merged 1 commit into
grll:mainfrom
fblacuttgottret-godaddy:main

Conversation

@fblacuttgottret-godaddy

Copy link
Copy Markdown
Contributor

BREAKING CHANGE: None - this is a backwards-compatible fix

The previous implementation modified the input dictionary by removing the 'transport' key, causing issues for applications reusing the same config.

Fixes #65

@grll

grll commented Jul 31, 2025

Copy link
Copy Markdown
Owner

@fblacuttgottret-godaddy thanks a lot for proposing a fix for this.

Unfortunately it's not working. We remove the transport key because we pass serverparams directly to the client constructor and it doesnt support the "transport" kwarg. Check the failing test for more details. What could work would be to copy the serverparams object and pop the transport from the copy instead of the original.

BREAKING CHANGE: None - this is a backwards-compatible fix

The previous implementation modified the input dictionary by removing the
'transport' key, causing issues for applications reusing the same config.

Fixes grll#65
@fblacuttgottret-godaddy

Copy link
Copy Markdown
Contributor Author

@fblacuttgottret-godaddy thanks a lot for proposing a fix for this.

Unfortunately it's not working. We remove the transport key because we pass serverparams directly to the client constructor and it doesnt support the "transport" kwarg. Check the failing test for more details. What could work would be to copy the serverparams object and pop the transport from the copy instead of the original.

ah makes sense, that's why it was doing the pop in the first place. updated to make a copy and keep pop()

@grll grll merged commit 50153c1 into grll:main Aug 3, 2025
3 checks passed
amithkk pushed a commit to amithkk/mcpadapt that referenced this pull request Sep 6, 2025
BREAKING CHANGE: None - this is a backwards-compatible fix

The previous implementation modified the input dictionary by removing the
'transport' key, causing issues for applications reusing the same config.

Fixes grll#65
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.

Bug: MCPAdapt modifies input parameters causing subsequent connections to fail

3 participants