Skip to content

Conversation

@SamMorrowDrums
Copy link

Motivation and Context

This PR is an experiment to see how using interfaces could look, so that the in and out types are in control of how their schema is generated, how their params are parsed and how their results are returned.

Related discussion and issues:

The primary change is to update NewServerTool and ToolHandlerFor to work with interfaces rather than pure generics:

// A ToolInput is a tool definition defining the input of a tool.
type ToolInput interface {
	// Schema returns the JSON schema for the tool's input.
	Schema() (*jsonschema.Schema, error)
	// SetParams sets the parameters for the tool's input from raw JSON.
	SetParams(json.RawMessage) error
}

// A ToolOutput is an interface defining the output of a tool.
type ToolOutput interface {
	// Schema returns the JSON schema for the tool's output.
	// Schema() (*jsonschema.Schema, error) // TODO implement output schema in future
	// Result returns the result of the tool's execution.
	Result() (*CallToolResult, error)
}

Note: this change is not fully fleshed out yet, as of opening this PR it is the minimum change to get everything building, and doesn't finish fleshing out all the ideas that become possible with this change, nor does it fully sell the benefits.

How Has This Been Tested?

All test cases have been updated

Breaking Changes

Yes, this is an extremely breaking change.

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

Additional context

@SamMorrowDrums SamMorrowDrums marked this pull request as draft June 26, 2025 22:24
@jba
Copy link
Contributor

jba commented Jun 27, 2025

Thinking about this.
It seems like a lot of work to define methods for each ToolInput and ToolOutput. Maybe this could be some sort of fallback if inference or explicitly writing the schema doesn't work out. Let's explore other options and come back to this.

return jsonschema.For[HiArgs]()
}

func (h *HiArgs) SetParams(raw json.RawMessage) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

When would this be any different? In other words, can we drop the interface method and just do this automatically?

Copy link
Author

Choose a reason for hiding this comment

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

I was thinking that some users might want to control the parsing more specifically, so the pure generic version was not flexible enough, it made the assumption that you would always parse the args into the type in this way - and that may not always be the case.

//
// TODO(jba): check that structured content is set in response.
func NewServerTool[In, Out any](name, description string, handler ToolHandlerFor[In, Out], opts ...ToolOption) *ServerTool {
func NewServerTool[In ToolInput, Out ToolOutput](name, description string, handler ToolHandlerFor[In, Out], opts ...ToolOption) *ServerTool {
Copy link
Contributor

Choose a reason for hiding this comment

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

My main concern is that this forces all the input and output types to implement the methods.
This would only be worth it if the type is used for more than one tool.
But how likely is that?

Copy link
Author

Choose a reason for hiding this comment

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

This would only be worth it if the type is used for more than one tool.

It's worth it for being able to define how the parsing and schema generation occurs. An example in the discussion is a request was to be able to order the schema in a specific way to encourage the LLM to provide the arguments in the same order. That kind of flexibility is not possible with the totally generic approach.

Also, for example I could have list GitHub Issues, and Search GitHub Issues, and could provide the same out type for both tools?

Pretty much the entire dance here was about inverting control back to the author to be able to change how the tool is represented directly.

@SamMorrowDrums
Copy link
Author

SamMorrowDrums commented Jun 27, 2025

It seems like a lot of work to define methods for each ToolInput and ToolOutput. Maybe this could be some sort of fallback if inference or explicitly writing the schema doesn't work out. Let's explore other options and come back to this.

@jba I think supporting both would be totally reasonable, this was my attempt to express the idea and show it concretely and see how we felt. I hope I did a good enough job of imagining how it could be. Not strongly tied to it - just wanted to have something to throw a stick at.

My personal goal is that I would like to be able to keep a concrete type in the return value of a handler, so handlers can wrap other handlers and not lose data in between. Too many SDKs assume that tools will always return the value, instead of a type that can serialize the value, but it makes composing new server tools out of existing server tools more difficult.

@jba
Copy link
Contributor

jba commented Jul 10, 2025

I think we went a different way with this. I'd like to close this PR, if that's all right with you @SamMorrowDrums. If there are use cases that our current API doesn't work for but this approach does, let's discuss them.

@SamMorrowDrums
Copy link
Author

Thanks @jba will close. It was more of a hypothetical to see what it would look and served its purpose.

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.

2 participants