Change type of Tool.InputSchema to be JsonElement #4
Conversation
| @@ -24,5 +25,5 @@ public class Tool | |||
| /// A JSON Schema object defining the expected parameters for the tool. | |||
| /// </summary> | |||
| [JsonPropertyName("inputSchema")] | |||
| public JsonSchema? InputSchema { get; set; } | |||
| public JsonElement InputSchema { get; set; } | |||
There was a problem hiding this comment.
Not all JsonElement are valid JSON schemas, and not all JSON schemas are valid root-level schemas per the MCP specification. We should try to insert baseline validation (e.g. using a wrapper type or validation on the setter) to make sure we conform.
There was a problem hiding this comment.
Roughly, this should be checking that the JsonElement is an object and also contains a "type" keyword that is set to "object". Everything else including the "properties" keyword is optional IIRC.
|
This seems to be causing hangs in the Ubuntu debug leg. |
There appear to be general reliability issues with our integration testing, although this is the first time I see a hang happening. |
eiriktsarpalis
force-pushed
the
fixschematype
branch
from
044a291 to
65a9f2e
Compare
eiriktsarpalis
force-pushed
the
fixschematype
branch
from
65a9f2e to
36333a0
Compare
|
The Ubuntu hand is happening on #9 as well, so whatever the issue is, it's already in main it seems. |
|
Adding validation to the property setter revealed a few bugs in the process :-) Merging since CI is borked right now. |
| @@ -481,8 +481,8 @@ private sealed class McpAIFunction(IMcpClient client, Tool tool) : AIFunction | |||
| ["type"] = "object", | |||
| ["title"] = tool.Name, | |||
| ["description"] = tool.Description ?? string.Empty, | |||
| ["properties"] = tool.InputSchema?.Properties ?? [], | |||
| ["required"] = tool.InputSchema?.Required ?? [] | |||
| ["properties"] = new Dictionary<string, object?>(), | |||
There was a problem hiding this comment.
I just noticed this. Your changes are leaving the properties and required keywords empty. Was this an intentional change?
There was a problem hiding this comment.
I've updated the code so that it always just forwards the JsonElement in the Tool. It is now always guaranteed to be populated.
No description provided.