Skip to content

Conversation

@jba
Copy link
Contributor

@jba jba commented Aug 18, 2025

Provide a non-generic form of tool handler, so users can wrap them.

Provide a non-generic form of tool handler, so users can wrap them.
@jba jba requested review from findleyr and samthanawalla August 18, 2025 14:57
@jba
Copy link
Contributor Author

jba commented Aug 18, 2025

/cc @rsc

@rsc
Copy link
Collaborator

rsc commented Aug 18, 2025

I think if you had different names you wouldn't need the helper. RawTool and AddRawTool sound like low-level things no one should use, and TypedTool returns a RawTool which is kind of odd.

What about

type ToolFunc = ToolFuncFor[json.RawMessage, any]

// Func takes a partially completed Tool description and a Go implementation function,
// updates the Tool schema to match the Go function, and returns the Tool and a
// canonical ToolHandler. The usual usage is:
//
//    s.AddTool(mcp.Func(tool, fn))
func Func[In, Out any](t *Tool, fn ToolFuncFor[In, Out]) (*Tool, ToolFunc)

and then delete the top-level AddTool entirely.

@rsc rsc mentioned this pull request Aug 18, 2025
@jba
Copy link
Contributor Author

jba commented Aug 18, 2025

I agree, the names could be better, but I don't think yours are quite right.

RawTool and AddRawTool sound like low-level things no one should use

They shouldn't. If you use that, you get no unmarshaling (which is obvious) and no schema validation (which is less so, and required by the spec). To do schema validation on a ToolFuncFor[json.RawMessage, any], we'd have to unmarshal into something, and then unmarshal again into the In of ToolFuncFor[In, Out].

Func is nice and short but we use Handler everywhere else; it would stand out.

Typed handlers are what almost everyone will use—I think the kind of ToolFunc wrapping you were talking about on our call is going to be relatively rare—so it seems backward to make the common case harder to write than the uncommon and error-prone one.

If you think the danger of the "raw" path can be mitigated with good doc, then we could remove the Raws and have

type ToolHandler = ToolHandlerFor[json.RawMessage, any]
Server.AddTool(*Tool, ToolHandler)

I guess we could rename TypedTool to just Handler, but again, we have lots of handlers.

@jba
Copy link
Contributor Author

jba commented Aug 21, 2025

Superseded by #325.

@jba jba closed this Aug 21, 2025
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