Skip to content

Conversation

RyoKusnadi
Copy link

Requirement
// TODO: use reflection to create the struct type to unmarshal into.
// Separate validation from assignment.

// Use default JSON marshalling for validation.
//
// This avoids inconsistent representation due to custom marshallers, such as
// time.Time (issue #449).
//
// Additionally, unmarshalling into a map ensures that the resulting JSON is
// at least {}, even if data is empty. For example, arguments is technically
// an optional property of callToolParams, and we still want to apply the
// defaults in this case.
//
// TODO(rfindley): in which cases can resolved be nil?

New Files:

  • mcp/reflection_validator.go - Validates JSON using Go struct types
  • mcp/schema_type_builder.go - Converts JSON schemas to Go types

Modified:

  • mcp/tool.go - Enhanced applySchema with reflection validation + fallback
  • mcp/tool_test.go - Added tests and benchmarks

@RyoKusnadi RyoKusnadi changed the title Using Reflection For ApplySchema mcp/tool: Using Reflection For ApplySchema Sep 21, 2025
@jba
Copy link
Contributor

jba commented Sep 22, 2025

Thanks for this big and impressive effort!

You should always file an issue before attempting this scale of work, even if there is a TODO in the code.

In this case, I'm not really sure what problem this is solving. Perhaps @findleyr has a better sense.

@RyoKusnadi
Copy link
Author

Thanks for this big and impressive effort!

You should always file an issue before attempting this scale of work, even if there is a TODO in the code.

In this case, I'm not really sure what problem this is solving. Perhaps @findleyr has a better sense.

ups sorry @jba @findleyr, this is my first time i contribute so i miss this
so in this case do i need to file an issue for this TODO?

@jba
Copy link
Contributor

jba commented Sep 22, 2025

so in this case do i need to file an issue for this TODO?

Let's start with my comment in mcp/reflection_validator.go. I don't understand what this PR provides.

@RyoKusnadi
Copy link
Author

the TODO "use reflection to create the struct type to unmarshal into"
so i'm thinking to remove map validation as below and change it into using reflection

v := make(map[string]any)
if len(data) > 0 {
	if err := json.Unmarshal(data, &v); err != nil {
		return nil, fmt.Errorf("unmarshaling arguments: %w", err)
	}
}
if err := resolved.ApplyDefaults(&v); err != nil {
	return nil, fmt.Errorf("applying schema defaults:\n%w", err)
}

So I create ReflectionValidator as the base/main wrapper for schema validation.
The ValidateAndApply function works like this:

  1. Extracts the schema from the provided *jsonschema.Resolved.
    • Returns a SchemaValidationError if no schema is found.
  2. Builds a Go type dynamically from the schema using SchemaTypeBuilder.
  3. Unmarshals the JSON into the typed struct for reflection-based type checking.
  4. Applies default values defined in the schema.
  5. Runs full schema validation on the enriched data.
  6. Returns the final JSON

For SchemaTypeBuilder:

  1. Input: *jsonschema.Schema
    • Returns an error if the schema is nil.
  2. Generates a unique cache key based on the schema structure.
    • Uses this key to reuse previously built types and avoid regeneration.
  3. Builds a Go reflect.Type from the schema definition:
    • Maps primitive type like string, integer, bool to their Go equivalents.
    • Recursively create struct types for object schemas.
    • Recursively create slice types for array schemas.
  4. Creates the final reflect.Type using reflect.StructOf (for objects) or reflect.SliceOf (for arrays).
  5. Stores the built type in the cache for future reuse.

here's the idea of the PR, let me know if need to change something or remove the cache key implementation @jba .

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