-
Notifications
You must be signed in to change notification settings - Fork 31
Make server_context
optional for Tools and Prompts
#54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Make server_context
optional for Tools and Prompts
#54
Conversation
lib/mcp/server.rb
Outdated
def call_tool_with_args(tool, arguments) | ||
args = arguments.transform_keys(&:to_sym) | ||
|
||
# Check if the tool accepts server_context or has **kwargs | ||
parameters = tool.method(:call).parameters | ||
accepts_server_context = parameters.any? { |_type, name| name == :server_context } | ||
has_kwargs = parameters.any? { |type, _| type == :keyrest } | ||
|
||
if accepts_server_context || has_kwargs | ||
tool.call(**args, server_context: server_context).to_h | ||
else | ||
tool.call(**args).to_h | ||
end | ||
end | ||
|
||
def call_prompt_template_with_args(prompt, args) | ||
# Check if the prompt accepts server_context or has **kwargs | ||
parameters = prompt.method(:template).parameters | ||
accepts_server_context = parameters.any? { |_type, name| name == :server_context } | ||
has_kwargs = parameters.any? { |type, _| type == :keyrest } | ||
|
||
if accepts_server_context || has_kwargs | ||
prompt.template(args, server_context: server_context).to_h | ||
else | ||
prompt.template(args).to_h | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be simplified into a single function like
def accepts_server_context?(method_object)
parameters = method_object.parameters
# ...
accepts_server_context || has_kwargs
end
To have the signature checking logic be cleanly contained in a single place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also independent of that an old function tool_call_parameters
still exists which becomes unused by the current version of the change. So either deleting or adjusting that to include these changes would be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#56 explores the idea of making tools & prompts into simple data/config/value objects, and gets rid of the inheritance based approach in favour of the define
based approach.
One of the benefits of this is that the implementation of the tool/prompt always uses a block, and blocks implicitly handle omitting arguments, meaning we can get rid of all the logic to do with conditionally passing server_context:
.
- Updated `call` method in `MCP::Tool` and `MCP::Prompt` to accept `server_context` as an optional parameter. - Refactored `MCP::Server` to streamline argument handling for tool and prompt calls, introducing helper methods `call_tool_with_args` and `call_prompt_template_with_args`. - Adjusted tests to reflect changes in method signatures and ensure compatibility with optional `server_context` parameter.
8fcd48e
to
c10c0dd
Compare
This PR makes the
server_context
parameter truly optional for both tools and prompts, improving type checker compatibility and addressing developer experience concerns.Motivation and Context
The previous implementation required
server_context
as a keyword argument in the baseTool
andPrompt
classes, which caused several issues:server_context:
in its method signature, even when unusedHow Has This Been Tested?
Added comprehensive test coverage in
test/mcp/server_context_test.rb
covering all scenarios:For Tools:
**kwargs
for flexible parameter handlingFor Prompts:
All existing tests have been updated and continue to pass.
Breaking Changes
No breaking changes. This change maintains full backward compatibility:
server_context
parameters continue to work unchangedImplementation Details
1. Made server_context optional in base classes
2. Improved parameter detection
The new implementation properly inspects method parameters to determine if a tool/prompt accepts server_context:
This approach:
server_context
parameters**kwargs
for maximum flexibility3. Usage Examples
Now you can define tools and prompts more flexibly:
Types of changes
Checklist
Additional context
This PR addresses issue #39: Unintuitive use of meta programming for :server context in tool call by making the server_context handling explicit and transparent.
The change improves compatibility with Ruby type checkers (like Sorbet) and makes the gem more accessible to teams using static typing, while maintaining full backward compatibility for existing implementations.