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
MCP call builders currently cannot preserve the standard approval_request_id relationship. Add an optional keyword argument to add_output_item_mcp_call and OutputItemMcpCallBuilder and retain it in added/done items and accumulated terminal response output. Calls without approvals remain unchanged.
This is the AgentServer portion of an end-to-end approval identity issue discovered while validating Inspector. Foundry Hosting also needs to translate or preserve source approval IDs consistently; this change does not claim to fix that producer mapping. The field is defined by the OpenAI Responses MCP contract: https://developers.openai.com/api/docs/guides/tools-connectors-mcp.
33 focused sync/async builder/generator tests pass, including successful and failed calls, terminal output retention, absent links and empty-ID rejection. The optional public API addition is submitted as Draft for design review.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines
B-line scope and dependencies
This is the independent first B-line deliverable. It preserves an existing OpenAI Responses field in both synchronous and asynchronous factories and builders; it introduces no new wire contract. API snapshots and their hash metadata are regenerated with APIView 0.3.31.
It does not depend on microsoft/agent-framework#8418 or Skylight UI changes. Hosting source/wire identity translation is downstream work and remains paused. C-line Inspector E2E proceeds independently and does not require single-card MCP behavior.