Skip to content

Conversation

@inaku-Gyan
Copy link
Contributor

Exposed additional optional parameters in the call_tool and connect_to_server methods so that they can fully pass through supported arguments to the underlying session-level methods.

Replace the parameter args in call_tool with arguments so that it's aligned with the original ClientSession.call_tool method.
Use @overload and @deprecated to mark args as deprecated while still supporting its usage.

Motivation and Context

Previously, call_tool and connect_to_server ignored several optional parameters supported by the lower-level ClientSession and session methods.
As a result, certain advanced use cases (e.g., custom timeouts, callbacks, or client metadata) could only be supported by directly accessing lower-level APIs, which was cumbersome and inconsistent.

This change:

  • Adds new parameters to call_tool for timeouts, progress tracking, and metadata.
  • Adds session_params to connect_to_server to configure ClientSession creation directly.
  • Introduces ClientSessionParameters dataclass for clear typing.

How Has This Been Tested?

Pyright static type check and pytest.

Breaking Changes

  • Not strictly breaking, as old usage patterns are still supported.
  • Minor deprecation warning: The args parameter in call_tool is deprecated in favor of arguments.

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • 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

ClientSessionParameters uses dataclasses.dataclass instead of pydantic.BaseModel because BaseModel does not support Protocol-typed fields. Attempts with SkipValidation and arbitrary_types_allowed=True did not resolve the issue.

@maxisbey maxisbey added enhancement New feature or request needs more eyes Needs alignment among maintainers whether this is something we want to add P3 Nice to haves, rare edge cases labels Nov 4, 2025
@BetterAndBetterII
Copy link

we need this :)

@kilvil
Copy link

kilvil commented Nov 9, 2025

I need this function to refresh tools.

Copy link

@kylestratis kylestratis left a comment

Choose a reason for hiding this comment

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

Some feedback on the structure of this compared to the rest of the codebase and a proposed simplification.

Comment on lines 205 to 206
@overload
@deprecated("The 'args' parameter is deprecated. Use 'arguments' instead.")

Choose a reason for hiding this comment

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

Suggested change
@overload
@deprecated("The 'args' parameter is deprecated. Use 'arguments' instead.")

I'd avoid adding conventions that aren't used elsewhere in the SDK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your feedback!
Personally, I felt it might be more consistent to rename args to arguments, since the original method ClientSession.call_tool uses arguments as its parameter name, so I added the deprecation warning. But per your suggestion, I reverted the changes in commit 4b9546d (#1576).

return self._tools

async def call_tool(self, name: str, args: dict[str, Any]) -> types.CallToolResult:
@overload

Choose a reason for hiding this comment

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

Suggested change
@overload

Comment on lines 285 to 288
session_params: ClientSessionParameters | None = None,
) -> mcp.ClientSession:
"""Connects to a single MCP server."""
server_info, session = await self._establish_session(server_params)
server_info, session = await self._establish_session(server_params, session_params)

Choose a reason for hiding this comment

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

session_params should follow the pattern of server_params and be required or at least default to an empty session_params object (I prefer the former for consistency's sake). This will make _establish_session() able to take session_params as a required argument, removing the need to check if session_params() exists and having different calls to enter_async_context() in lines 322-328.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default value is changed into ClientSessionParameters() as required in commit 4ab9757 (#1576).

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

Labels

enhancement New feature or request needs more eyes Needs alignment among maintainers whether this is something we want to add P3 Nice to haves, rare edge cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants