Python: surface mid-run oauth_consent_request items from ResponsesHostServer - #7659
Python: surface mid-run oauth_consent_request items from ResponsesHostServer#7659Giles Odigwe (giles17) wants to merge 1 commit into
Conversation
…tServer `_to_outputs` had no branch for `oauth_consent_request` content, so a consent link produced after the agent was entered (for example by an on-behalf-of MCP server that needs a per-user token at tool-invocation time) fell into the catch-all and was dropped with "Content type 'oauth_consent_request' is not supported yet". The client saw a completed response with no consent prompt. Only connect-time consent failures raised by `_ensure_agent_ready` were surfaced as `oauth_consent_request` output items, and the inbound conversion (`_output_item_to_message`) already handled the item type, so the outbound direction was the missing half. - Emit `oauth_consent_request` added/done output items from `_to_outputs`, validating the link is an absolute HTTPS URL and reading `server_label` from the content's additional properties. - End the response as `incomplete` (instead of `completed`) when a consent request was emitted mid-run, in both the agent and workflow handlers, matching the connect-time path. - Factor the item emission, link validation, and incomplete reason into shared helpers reused by the connect-time path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 992057d8-78a8-45a7-9201-35af0919b071
There was a problem hiding this comment.
Pull request overview
Surfaces mid-run OAuth consent requests through Python’s Foundry Responses host.
Changes:
- Emits OAuth consent output items and marks affected responses incomplete.
- Adds consent-link validation and shared emission helpers.
- Adds streaming and non-streaming tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
_responses.py |
Handles and emits mid-run OAuth consent requests. |
test_responses.py |
Tests consent output, validation, and response status. |
Suppressed comments (1)
python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py:214
- The real Foundry OAuth parser does not populate
additional_properties:agent_framework_foundry/_oauth_helpers.py:59-62puts the upstream item (which carriesserver_label) inraw_representation. Consequently, actual mid-run OBO events always fall back toagent_framework; only this PR's synthetic test preserves the label. Fall back to the raw item'sserver_labelso clients receive the originating server identity.
def _consent_server_label(content: Content) -> str:
"""Return the server label to report for an ``oauth_consent_request`` content."""
label = content.additional_properties.get("server_label") if content.additional_properties else None
return label if isinstance(label, str) and label else "agent_framework"
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| parsed = urlparse(consent_link) | ||
| if parsed.scheme.lower() != "https" or not parsed.netloc: | ||
| logger.warning("Skipping oauth_consent_request with non-HTTPS consent_link.") | ||
| return None |
| for event in _emit_oauth_consent_item( | ||
| response_event_stream, | ||
| context.response_id, | ||
| consent_error.consent_url, | ||
| consent_error.name, |
| class TestMidRunOAuthConsentSurfacing: | ||
| """A tool can require consent after the agent has been entered (e.g. an on-behalf-of | ||
| MCP server needing a per-user token), in which case the consent link arrives as | ||
| ``oauth_consent_request`` content in the agent's stream rather than as a connect-time error. | ||
| """ |
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): b452dd73d248
Model: gpt-5.6-sol
Overview
The change consistently surfaces valid mid-run OAuth consent requests as output items and terminates affected agent and workflow responses as incomplete, with coverage for streaming, non-streaming, multiple requests, and common invalid links. Shared emit/reason helpers also keep the connect-time and mid-run paths aligned. Two residual gaps remain: malformed HTTPS syntax can fail the whole response, and real Foundry events do not propagate their required server label through the new hosting conversion.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py
| if not consent_link: | ||
| logger.warning("Received oauth_consent_request content without a consent_link; skipping.") | ||
| return None | ||
| parsed = urlparse(consent_link) |
There was a problem hiding this comment.
Malformed HTTPS syntax such as https://[ makes urlparse raise ValueError. Because this helper runs while consuming the agent stream, that turns an invalid consent item into response.failed instead of skipping it and allowing the turn to complete as the helper contract and invalid-link behavior require. Please handle parse/hostname validation errors and return None for every malformed URL.
|
|
||
| def _consent_server_label(content: Content) -> str: | ||
| """Return the server label to report for an ``oauth_consent_request`` content.""" | ||
| label = content.additional_properties.get("server_label") if content.additional_properties else None |
There was a problem hiding this comment.
Real Foundry OAuth events never populate this property: try_parse_oauth_consent_event stores the source item, including its required server_label, only in content.raw_representation. As a result, production requests are always re-emitted as agent_framework, losing the MCP server identity even though the synthetic test passes. Please preserve the label during parsing or read the validated label from the raw item here, and cover the parser-to-hosting path.
Motivation & Context
A customer running a hosted agent behind
ResponsesHostServerwith an on-behalf-of (OBO) MCP server never gets an OAuth consent card in Teams. The host logsContent type 'oauth_consent_request' is not supported yet. This is usually safe to ignore.and returns acompletedresponse with no consent item, so the user has no way to authorize the delegated token and the agent can never call the tool.They had already upgraded per #3950, which fixed the chat-client hop (
_oauth_helpers.try_parse_oauth_consent_eventnow converts the upstream event intoContent.from_oauth_consent_request(...)). The content is dropped one layer later, in the hosting layer, so no version bump ofagent-framework-core/agent-framework-foundryhelps._to_outputsinagent_framework_foundry_hosting/_responses.pyhas branches for text, reasoning, function call/result, image generation, MCP call/result, shell call/result, andfunction_approval_request, but none foroauth_consent_request, so that content hits the catch-allelseand is discarded. The host already emits the item for connect-time consent failures (when_ensure_agent_ready()raises andconsent_url_from_errorfinds a URL), and the inbound conversion in_output_item_to_messagealready understands the item type — the outbound mid-run half was simply missing. An OBO server that needs a per-user token at tool-invocation time connects fine and therefore never takes the connect-time path.This is a different root cause from #7227, which is about parsing connect-time gateway errors whose source type is
a2a_previewrather thanmcp.Description & Review Guide
What are the major changes?
_to_outputsgains anoauth_consent_requestbranch that emitsresponse.output_item.added/.donefor anOAuthConsentRequestOutputItem, carrying the consent link and aserver_labelread from the content's additional properties (defaulting toagent_framework). The link is validated as an absolute HTTPS URL; anything else is logged and skipped rather than emitted._handle_inner_agentand_handle_inner_workflownow terminate the response withresponse.incomplete(reasonOAuth consent required for N tool(s).) instead ofresponse.completedwhen at least one consent item was emitted mid-run, matching the existing connect-time behavior._emit_oauth_consent_item,_consent_link_from_content,_consent_server_label,_consent_incomplete_reason) that the pre-existing connect-time path now reuses, so both paths cannot drift apart.TestMidRunOAuthConsentSurfacingtests cover streaming, non-streaming, multiple consent contents in one update, and invalid links (empty,http://, non-URL) being skipped while the turn still completes.What is the impact of these changes?
oauth_consent_requestoutput item for mid-run consent, so a consent card can be rendered and the user can authorize and re-send the prompt. Automatic resumption of the interrupted turn remains separately tracked by Python: [Bug]: ResponsesHostServer has no turn suspension/resumption, so user must re-send message after OAuth consent #5594.incompleterather thancompleted. This mirrors the connect-time path that already returnedincomplete, and previously such a turn produced no output item at all.TestOAuthConsentSurfacingtests.What do you want reviewers to focus on?
response.incompleteis the right terminal status for a mid-run consent request, versus keepingcompletedand relying solely on the output item.Related Issue
Fixes #7658
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.