-
Notifications
You must be signed in to change notification settings - Fork 562
Tweak how McpClientTool.InvokeAsync exposes some content #941
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tweak how McpClientTool.InvokeAsync exposes some content #941
Conversation
|
Ah, this looks great for IChatClients with media tool result support. The tests made me think that ResourceLinks could be handled similar to how vs code does. Afaik it resolves those in a way where the message history retains that it's a link, and then it fetches the resource from the mcp server when needed, ie after tool call and on reconnect/resume. It's tangential to this PR I know, but having a resource link AIContent with an McpClient reference and dynamic resolution would replicate this I think. |
narasamdya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a small question in the PR; otherwise LGTM.
This means that logic like this can't work:
Yes, I'd addressed that as well as always converting, but it seems like I neglected to push my commit. |
Rather than always serializing to a JsonElement, which IChatClients that consume the tool won't be able to reason about, convert the content into AIContents that they can reason about.
1b6872b to
6a004f8
Compare
|
I think it's good. It has no way of combining AIContent conversion and structured output which might become an emerging pattern, given how it seems structured content is becoming a pseudo-metadata payload (ie OpenAI apps SDK). But one can do that conversion in middleware that will exist in such clients anyway. Converting to AIContent whenever possible, is a pattern I've become fonder of in general, so I am glad to see it repeat here. Preserving IsError is good. |
Enhanced
InvokeCoreAsyncinMcpClientTool.csto handle non-text content by converting it toAIContentwhen possible. This enables richer content handling for downstreamIChatClientswhile maintaining fallback serialization for unsupported cases.