Proposal: add a simpler version of AddTool that drops request and response

[jba]

Currently, mcp.AddTool takes a ToolHandlerFor[In, Out], which is defined as

func(context.Context, *CallToolRequest, In) (*CallToolResult, Out, error)

As argued on #386, the request and result are seldom used and confusing. In particular, it is unclear whether to use the StructuredOutput and IsError fields of CallToolResult, or to return results via the second and third return values.

The proposal is to change AddTool to take a handler with signature

func(context.Context, In) (Out, error)

That will satisfy the majority of tool authors and remove the confusion.

[jba]

An alternative is to leave AddTool as it is and define another function, AddStructuredTool, that has the simpler signature.

The argument for leaving AddTool alone is that, just as Server.AddTool provides the most generality for people who really want to roll their own, so AddTool provides the most generality when you add schema inference and error handling.

[findleyr]

I think accessing the Request is not that uncommon, for authors of distributed servers.

How about adding AddSimpleTool? A problem with AddStructuredTool is that AddTool can also add a structured tool.

In both of these scenarios, ToolFor stays the same, right?

[jba]

The only thing we have to decide now is whether to keep AddTool the same.

We can consider AddSimpleTool at our leisure, after v1. I think we should add doc and examples, and only provide AddSimpleTool if there is a lot of confusion.

[findleyr]

IMO, we should focus on docs and examples for v1. WDYT?

[findleyr]

Retitled accordingly, since I think we wouldn't modify AddTool at this point, but may add an alternative (AddSimpleTool).

[findleyr]

So, after writing up our documentation on tools (#498), and dealing with several bugs related to schema validation, and trying to write out a 'manual tool' from scratch. I have the following observations / realizations.

Observations

• We should absolutely expose a way to bind a tool handler that doesn't see the CallToolRequest or CallToolResult. We knew that.
• Most of the reasons users would want to access the CallToolRequest or CallToolResult is to implement cross-cutting concerns like authorization, logging, or pre/post-processing. The easiest way to implement those concerns is in a middleware anyway.
• Therefore, if we have nicer constructors like XXStructuredTool or XXXUnstructuredTool, that don't expose the request or result, they should return the callback, not add the tool to the server. So mcp.AddStructuredTool would be a bad thing, because it would expose the simple API but not any way to implement the important things like authorization, and therefore it would only be useable in toy scenarios.
• I'll note that these conflicting requirements (simplicity but retaining access to the full API) is why the signature of ToolHandlerFor is what it is: we were trying to have our cake and eat it to. Oh well: there's no harm in leaving mcp.AddTool as a sort of all-in-one package, though maybe we'll eventually deprecate it in favor of better APIs (see some ideas below).
• It's a darn good thing that we removed ToolFor, because of the observations above: if we're going to expose a tool constructor, it should accept the simple forms of tool handlers.
• It's almost impossible to 'roll your own' tool handler with the complexity of the current jsonschema APIs (validation, applying defaults, and parsing are nigh impossible to get right).

Proposal

In light of these observations, here is a tentative proposal that I think will greatly simplify our tool binding APIs.

Simplifying manual tool handling

Provide better building blocks in the jsonschema package.

• proposal: Schema.SubSchema google/jsonschema-go#10 allows easier mutation of schemas
• Expose something like the following helper methods on jsonschema.Resolved. Ummarshal validates and applies defaults before unmarshalling to v. Marshal and marshals.

package jsonschema

// Unmarshal validates and applies defaults to the given data, and then unmarshals into v.
func (*Resolved) Unmarshal(data []byte, v any, *UnmarshalOpts) error

// Marshal marshals v to JSON and validates.
func (*Resolved) Marshal(v any) ([]byte, error, *MarshalOpts)

type UnmarshalOpts struct {
  SkipValidation bool
  IgnoreDefaults bool
}

type MarshalOpts struct {
  SkipValidation bool
}

Simplifying / clarifying tool binding

Reintroduce the ServerTool concept from the original design, with multiple constructors. The advantage of ServerTool is that it admits batch apis (Server.AddTools), and organizes constructors in godoc.

package mcp

type ServerTool struct { Tool *Tool, Handler ToolHandler }

func NewStructuredTool[In, Out any](*Tool, func(ctx context.Context, In) (Out, error)) *ServerTool

func NewUnstructuredTool[In any](*Tool, func(ctx context.Context, In) ([]Content, error)) *ServerTool

func (*Server) AddTools(...*ServerTool)

In the future, we can also add Server.Tool to get the registered ServerTool by name, but let's not add that until it is needed.

CC @jba @katiehockman WDYT?

[jba]

We should absolutely expose a way to bind a tool handler that doesn't see the CallToolRequest or CallToolResult. We knew that.

Ignoring two arguments is a small price to pay for simplicity, IMO.

it would expose the simple API but not any way to implement the important things like authorization, and therefore it would only be useable in toy scenarios.

You don't need to expose the handler; you can use middleware without it, as I show in
https://github.com/modelcontextprotocol/go-sdk/blob/main/examples/server/middleware/main.go#L44.

jsonschema.(Un)marshal seem to combine two unrelated actions: serialization and defaults/validation. I don't understand why you'd want to do that.

The advantage of ServerTool is that it admits batch apis (Server.AddTools), and organizes constructors in godoc.

I don't think those are good enough reasons to add a second kind of Toolish thing. And then we'd have to change the other server features (prompts etc.)

[findleyr]

@jba I think the fact that writing out #498 was a sign that the current API is too confusing. The main cause of the confusion is that there is more than one way to return a result, and it's hard to differentiate between them. Another source of confusion is the magic any type argument that disables structured output. I think these are real API problems, and the solution is to differentiate structured/unstructured tools, and simplify their signatures. Do you agree with that, modulo uncertainty about names or ServerTool?

Ignoring two arguments is a small price to pay for simplicity, IMO.

A smaller API is not necessarily simpler. The fact that something like NewStructuredTool makes it absolutely unambguous what the input and output parameters mean is a big improvement in simplicity.

You don't need to expose the handler; you can use middleware without it

Hmm, I suppose.

jsonschema.(Un)marshal seem to combine two unrelated actions: serialization and defaults/validation. I don't understand why you'd want to do that.

It's because I want to avoid exposing the interstitial map[string]any to the user (because maybe eventually we can get rid of it), and so that the user doesn't have to remember to apply defaults before validating (or vice-versa--as I write this I can't remember which is right), and so that there's only one error to handle.

I don't think those are good enough reasons to add a second kind of Toolish thing. And then we'd have to change the other server features (prompts etc.)

I expected this feedback :). We could also have NewStructuredTool return (*Tool, Toolhandler), similar to what ToolFor did.

[jba]

We could also have NewStructuredTool return (*Tool, Toolhandler), similar to what ToolFor did.

I think AddStructuredTool is fine, since we can use ordinary middleware for wrapping.

[jba]

The jsonschema changes seem orthogonal and are best discussed on that repo.

[katiehockman]

Hey sorry for the delay @jba @findleyr

For clarity, that would turn the following example:

func main() {
	server := mcp.NewServer(&mcp.Implementation{ /* ... */ }, nil)
	mcp.AddTool(server, &mcp.Tool{
		Name:        "list_sessions",
		Description: "Lists all sessions",
	}, ListSessions)
	mcp.AddTool(server, &mcp.Tool{
		Name:        "get_session_details",
		Description: "Get session by ID",
	}, SessionByID)
}

type SessionsResult struct {
	Sessions []Session `json:"sessions"`
}

func ListSessions(ctx context.Context, _ *mcp.CallToolRequest, _ any) (*mcp.CallToolResult, SessionsResult, error) {
	// implementation
}

func SessionByID(ctx context.Context, _ *mcp.CallToolRequest, params SessionIDParams) (*mcp.CallToolResult, SessionsResult, error) {
	// implementation
}

into the following:

func main() {
	server := mcp.NewServer(&mcp.Implementation{ /* ... */ }, nil)
	listSessionsTool := mcp.NewStructuredTool(&mcp.Tool{
		Name:           "list_sessions",
		Description: "Lists all sessions",
	}, ListSessions)
	getSessionsTool := mcp.NewStructuredTool(&mcp.Tool{
		Name:           "get_session_details",
		Description: "Get session by ID",
	}, SessionByID)
	server.AddTools(listSessionsTool, getSessionsTool)
}

type SessionsResult struct {
	Sessions []Session `json:"sessions"`
}

func ListSessions(ctx context.Context, _ any) (SessionsResult, error) {
	return SessionsResult{Sessions: sessions()}, nil
}

func SessionByID(ctx context.Context, params SessionIDParams) (SessionsResult, error) {
	return SessionsResult{Sessions: session(params.ID)}, nil
}

Is that correct, or is there an edit you would make?

As far as feedback, if above is the correct interpretation:

• I like the fact that the mcp.CallToolRequest and mcp.CallToolResponse can be omitted, as that was my original ask
• I like the fact that AddTools is a method on *mcp.Server, instead of needing to pass it as a parameter. It makes more sense to have it as a method on the server anyway.
• I preferred adding the tools one at a time to the server as before, instead of needing to define a separate tool variable and then adding them all at once at the end. I suppose I could still do it individually with this syntax, but given that AddTools is plural, it feels like the expectation is to do it as written above.

So this is closer to what I would actually want:

func main() {
	server := mcp.NewServer(&mcp.Implementation{ /* ... */ }, nil)
	server.AddTool(&mcp.Tool{
		Name:        "list_sessions",
		Description: "Lists all sessions",
	}, ListSessions)
	server.AddTool(&mcp.Tool{
		Name:        "get_session_details",
		Description: "Get session by ID",
	}, SessionByID)
}

type SessionsResult struct {
	Sessions []Session `json:"sessions"`
}

func ListSessions(ctx context.Context, _ any) (SessionsResult, error) {
	return SessionsResult{Sessions: sessions()}, nil
}

func SessionByID(ctx context.Context, params SessionIDParams) (SessionsResult, error) {
	return SessionsResult{Sessions: session(params.ID)}, nil
}

[findleyr]

Thanks @katiehockman, the reasons we can't do exactly what you want are:

1. Go doesn't support generic methods, therefore the signature of the tool handler in the Server.AddTool you propose would have to be any, which would be confusing.
2. Sometimes people want to return unstructured content (structured content was after all only added in June).

Based on yours and @jba's feedback, I think what we're honing in on is:

func AddStructuredTool[In, Out any](*Tool, func(context.Context, In) (Out, error)

func AddUnstructuredTool[In any](*Tool, func(context.Context, In) ([]Content, error)

WDYT?