-
Notifications
You must be signed in to change notification settings - Fork 950
Add process_tool_call
hook to MCP servers to modify tool args, metadata, and return value
#1962
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
Conversation
Also noting that while we would love to merge this, we can't actually use the tip of pydantic-ai until we get streamable HTTP back |
0f05e72
to
28846c2
Compare
@hayk-corpusant Thanks for the PR! I think using Instead of an MCP-specific property on I imagine a new @dataclass
class MyDeps:
user_id: int
async def process_tool_call(ctx: RunContext[MyDeps], call_tool: Callable[[str, dict[str, Any]], CallToolResult], tool_name: str, args: dict[str, Any]) -> str:
return await call_tool(tool_name, args, {"user_id": ctx.deps.user_id})
server = MCPServerHTTP(..., process_tool_call=process_tool_call) This function would be called from inside the Would that work for you? |
2646f9f
to
80f57f6
Compare
How's this @DouweM? It doesn't seem like the types are quite right but the rest works. We thought it was strange that we couldn't find an existing way to pass through user-level auth for a persistent agent. Seems like a common use case if you're running an app serving many users and the agent is on the backend. |
80f57f6
to
64b593f
Compare
This would solve our issue in the creation of a AG-UI adapter, as it too requires metadata about the call, currently I have to workaround by renaming the method calls, which is far from ideal. I wonder if the default should be to always pass deps as metadata though, that way users can customise but for the majority of cases it will just work without any additional work from the developer. Thoughts? |
@hayk-corpusant if I can help get this over the line in any way let me know, as this would allow us get a good solution for tool call tracking. |
@hayk-corpusant Thanks, the implementation looks good! Can you please look add tests and docs, and look at the typing issue? Let me know if you'd like some guidance. |
@stevenh Passing all deps through has been suggested before (#1872), but (as I wrote there), I think it'd be unexpected to both users and MCP servers if we unconditionally passed all of the deps. Note also that deps may not be serializable. |
process_tool_call
hook to MCP servers to modify tool args, metadata, and return value
I also think passing through all deps is problematic, would rather it's explicit. @DouweM I'm quite busy this week so I'll get to it if I can, but I can't promise urgent action here so anyone else wanting to move it forward is welcome to |
I've created a PR off the back of this one, which I believe achieves what @hayk-corpusant set out to do, with tests and additional docs. You can find it here: #2000, its even got a nice round number 😄 |
@stevenh Thank you Steven! I'll close this PR in favor of that one then. @hayk-corpusant Thanks for making the start on this :) |
The goal here is to allow a persistent Agent to be used within a multi-user app where user-specific metadata/auth should get passed through to the MCP server in a way not visible to an LLM.
To do this, we use the _meta field of the request. See this discussion, method
2. Use the JSON‑RPC _meta field
https://github.com/orgs/modelcontextprotocol/discussions/309
In our app we directly call
agent.iter
and have verified that this works to then in a FastMCP server tool we can callctx.request_context.meta
to access the contents.Fixes #1872