-
-
Notifications
You must be signed in to change notification settings - Fork 11.8k
[Frontend] Add MCP type support infrastructure to Responses API #30054
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
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.
Code Review
This pull request adds support for MCP (Multi-turn Conversation Protocol) tool calls to the Responses API. The changes introduce parsing logic for MCP calls in both complete and streaming responses, update the protocol definitions, and add corresponding tests. My review identified a critical issue in the streaming logic that could lead to incorrect behavior for certain built-in tools, and a high-severity issue regarding code duplication that impacts maintainability. I've provided suggestions to address both. The rest of the changes, including the new tests and protocol updates, are well-implemented.
4da4f60 to
82adc20
Compare
|
/cc @yeqcharlotte @qandrew PTAL |
9ee6816 to
b6bbde0
Compare
|
Hi @daniel-salib, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
|
This pull request has merge conflicts that must be resolved before it can be |
b6bbde0 to
7fa2e11
Compare
yeqcharlotte
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.
LGTM @qandrew @daniel-salib can we create a follow-up PR to update examples?
Head branch was pushed to by a user without write access
cec9f47 to
f450c23
Compare
18cd84c to
bd2d381
Compare
|
Documentation preview: https://vllm--30054.org.readthedocs.build/en/30054/ |
04b9d64 to
687ca45
Compare
Signed-off-by: Daniel Salib <danielsalib@meta.com>
687ca45 to
8f24ea8
Compare
…-project#30054) Signed-off-by: Daniel Salib <danielsalib@meta.com>
Add MCP type support infrastructure to Responses API
Summary
Add MCP type support infrastructure to Responses API
Changes
McpCalltype from openai library_parse_mcp_call()function to parse MCP tool calls into properly typed output itemsparse_output_message()to route unknown recipients (notbrowser.*,functions.*, or built-in tools) to MCP parsingparse_remaining_state()to handle in-progress MCP callsserver_labelfield toMcpCallobjects (extracted from recipient name)Test Plan
Unit tests:
python3 -m pytest tests/entrypoints/test_harmony_utils.py -k "mcp" -vManual test - Before this change (missing
server_labelfield):Response:
{"error":{"message":"unhandled errors in a TaskGroup (1 sub-exception)","type":"BadRequestError","param":null,"code":400}}After this change:
Same curl command now returns:
{ "output": [ ...reasoning items..., { "id": "mcp_bc8c1958cd96823c", "arguments": "{\"path\": \"/tmp\"}", "name": "filesystem", "server_label": "filesystem", "type": "mcp_call", "status": "completed" } ] }