fix(agentserver): preserve MCP approval request links - #49037
JiahuiGu (Jiahui-Gu) wants to merge 2 commits into
Conversation
github-actions
Bot
added
Community Contribution
|
Thank you for your contribution JiahuiGu (@Jiahui-Gu)! We will review the pull request and get back to you soon. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 7 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 7 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
The async API lacks the new parameter, and the committed API snapshot is stale.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds MCP approval-request identity preservation to synchronous response streaming.
Changes:
- Adds and validates optional
approval_request_id. - Preserves it across added, done, and terminal output items.
- Adds focused tests and changelog documentation.
File summaries
| File | Description |
|---|---|
test_mcp_approval_link.py |
Tests approval-link retention and validation. |
CHANGELOG.md |
Documents the fix. |
_event_stream.py |
Exposes the new synchronous builder argument. |
_builders/_tools.py |
Validates and emits the approval ID. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| name: str, | ||
| *, | ||
| item_id: str | None = None, | ||
| approval_request_id: str | None = None, |
There was a problem hiding this comment.
Fixed in fd2ab09. The async factory accepts and forwards approval_request_id. The same parameterized tests now cover sync/async success and failure, added/done/terminal retention, absent IDs and empty-ID rejection; 33 builder/generator tests pass.
| :keyword approval_request_id: Explicit ID of the approval request for this MCP call. | ||
| :keyword type approval_request_id: str | None |
There was a problem hiding this comment.
Regenerated api.md through APIView 0.3.31 and the repository Export-APIViewMarkdown tool, then generated api.metadata.yml with extract_apiview_metadata.py. It includes sync/async factories and inherited builder signatures. Unrelated model-literal generator churn was excluded; the normalized content hash matches the committed API snapshot. Included in fd2ab09.
There was a problem hiding this comment.
🟢 Approval recommended
The implementation consistently covers both stream variants, terminal accumulation, validation, tests, and API documentation.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Description
Fixes #49066.
MCP call builders cannot accept the standard Responses
approval_request_idfield, so callers cannot preserve the relationship between an MCP execution and its approval request.Add an optional keyword argument to the synchronous and asynchronous
add_output_item_mcp_callfactories and their builders. Preserve the supplied ID inresponse.output_item.added,response.output_item.done, and accumulated terminal response output. Calls without an approval ID remain unchanged. The field is defined by the OpenAI Responses MCP contract.33 focused sync/async builder and generator tests pass, covering successful and failed calls, terminal output retention, absent links and empty-ID rejection. The API snapshot and hash metadata are updated with APIView 0.3.31.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines