proposal: tool handler should set Content field if nil

[jba]

When ToolFor builds a handler, it sets CallToolResult.StructuredContent to the typed output of the handler. It does not touch CallToolResult.Content.

The spec says that tools which set StructuredContent should set a TextContent to the serialized JSON of StructuredContent. We don't do that now because we don't want to overwrite something that the handler has put there. But there's almost no reason not to do it if Content is nil.

The only downside is that we're marshaling StructuredContent twice: once for this, and once when it goes out on the wire. But perhaps we could stash the marshaled bytes and add a custom JSONMarshaler to use them, if we thought that performance warranted it.

[findleyr]

I don't understand why we couldn't just marshal once, and put a json.RawMessage into the StructuredContent result. That would avoid the double marshalling.

[jba]

We have to be OK with the client seeing a RawMessage.
I can make a PR to try out.

[jba]

Minor issue. Here's a tool handler we use a lot:

func SayHi(ctx context.Context, req *mcp.CallToolRequest, args SayHiParams) (*mcp.CallToolResult, any, error) {
      return &mcp.CallToolResult{
          Content: []mcp.Content{
              &mcp.TextContent{Text: "Hi " + args.Name},
          },
      }, nil, nil
 }

If we pass this to mcp.AddTool, what should StructuredContent be when it returns?

Before, when StructuredContent was any, it was set to nil and not marshaled.

After this change, it will be a raw message, and will have the value null.

I think that's right—the handler returned a nil, which marshals to null. But one could argue that StructuredContent should still be nil.

[findleyr]

Jonathan, I think you misunderstood my suggestion: I meant in the ToolFor wrapper, we can marshal the structured result into json.RawMessage, stick it in the text content, and then set the CallToolResult.StructuredContent field (which is still of type any to a json.RawMessage value, which will marshal as itself.