Using field types with custom JSON format breaks schema inference

[johanbrandhorst]

Describe the bug
When using mcp.ToolHandlerFor and a field with a custom JSON format (implementing json.Marshaler, for example), the inferred schema will not match the resulting JSON output.

To Reproduce
Steps to reproduce the behavior:

1. Define a struct with a custom JSON format, such as time.Time:

   type MyToolParams struct {
   	MyTimestamp time.Time `json:"my_timestamp" jsonschema:"the timestamp for the tool call"`
   }

2. Use the struct in a tool call:

   func GetTime(ctx context.Context, cc *mcp.ServerSession, params *mcp.CallToolParamsFor[MyToolParams]) (*mcp.CallToolResultFor[MyToolParams], error) {
   	return &mcp.CallToolResultFor[MyToolParams]{
   		StructuredContent: params,
   	}
   }

When using the created endpoint, the schema will claim that my_timestamp is an object, but of course the response will have my_timestamp as a string.

Expected behavior
I'm not entirely sure what the solution is here, but this behavior was unexpected to me (since inference is mostly "just working").

Additional context
I think it's possible to work around this by specifying a custom InputSchema and OutputSchema when registering the tool. I'm mostly creating this so that we have a place to discuss this somewhat pointy edge that will likely hit many new users.

[jba]

Good observation. Of course there is no way to make inference work with custom marshalers. Perhaps we should detect that and fail (in For) or ignore (in ForLax).

And yes, you can certainly work around anything related to inference by just not using it!

[jba]

We could also add special cases for stdlib types like time.Time. I think that's a good idea for common types.

[jba]

/cc @samthanawalla @findleyr thoughts?

[jba]

Here is one thought:

For[T] takes an additional argument, a map[reflect.Type]*Schema. The map is consulted for T itself and for each type encountered while traversing T, and any schemas found there are used instead of following the usual algorithm.

There is a built-in map for types like time.Time that are in the standard library and have an established JSON representation (because they have a MarshalJSON method). The user's map overrides the built-in one if a key appears in both.

Since we just found the need for For[T] to have two additional degrees of freedom—the "lax" feature added in #147 and this—it may make sense to have

type ForOptions struct {
    Lax bool
    TypeSchemas map[reflect.Type]*Schema
}

func For[T any](opt *ForOptions) (*Schema, error)

[delete ForLax]

A simpler but less flexible alternative: instead of an argument to For[T], the package keeps a global registry of mappings from types to schemas. There would be a function like jsonschema.RegisterType[T](*Schema).

[johanbrandhorst]

I think I prefer the ForOptions option, it's more consistent with the rest of the API in this module, and it allows the caller to modify the schema per-invocation. What would it look like to use the TypeSchemas field though? Any option that wouldn't require the user to use reflect?

[jba]

Any option that wouldn't require the user to use reflect?

We could have a map[any]*Schema and you could pass a value of the type. For example:

map[any]*Schema{
    time.Time{}: &Schema{Type: "string"},
}

[jba]

Apologies to @bc-tayvs who already has a similar PR: #139.
That includes both a map from types to schemas, and also a function from types to schemas solely for the "additionalProperties" keyword.

[jba]

@johanbrandhorst have you tried the new API for this on main? If so and it works for you, please close this.

[johanbrandhorst]

Yep, that works for me, thanks!