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 8 commits 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.
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
- Harden consent link validation. `urlparse` raises `ValueError` for malformed authorities such as `https://[broken`, which turned an unrenderable link into a failed response instead of skipping the item, and a non-empty `netloc` is not sufficient on its own (`https://@` has one but no host). Validation now catches the parse error and requires a hostname, in both the hosting layer and `agent_framework_foundry._oauth_helpers`, which had the same defect. - Preserve the server label. `try_parse_oauth_consent_event` only kept the upstream item in `raw_representation`, so every real Foundry consent event was re-emitted with the fallback label. The parser now copies `server_label` into `additional_properties`, and hosting falls back to the raw item's label before defaulting. - Validate connect-time consent links too. An entry-time consent error with no renderable link now produces `response.failed` rather than an `incomplete` carrying no link the user can act on, and the reported count reflects the items actually emitted. - Suppress duplicate consent prompts. `WorkflowAgent` replays the inner agent's content as workflow output, so the same consent request reached the host twice and produced two prompts. `_to_outputs` now takes the set of emitted `(consent_link, server_label)` pairs and skips repeats. - Cover the workflow hosting path, which was previously exercised only through the regular agent handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 992057d8-78a8-45a7-9201-35af0919b071
…onsent # Conflicts: # python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py
There was a problem hiding this comment.
MAF Automated Review — Iteration 2
Result: Findings reported
Scope: 12 net-new commit(s): 9a06fa3f426d, 9645d33cde44, ee27065359e0, 5fafa1856907, 8c4da3c3b9b8, 4aa737eee5da, ae7fa3389c8f, e2893200277b, e1e005f226a2, 12621e0a7465, 9b33db988a05, fbc4b985d6a9
Model: gpt-5.6-sol
Overview
The change surfaces mid-run OAuth consent as Responses output, preserves the MCP server label, suppresses workflow replay duplicates, and covers regular and workflow hosting paths. URL guards reject several unsafe forms, but rejected consent requirements are currently erased and some syntactically unusable HTTPS URLs still pass validation, leaving clients with either a false successful terminal state or an unusable incomplete response.
Reviewed the supplied incremental 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
|
Why didn't the consent item get emitted when the code enters the agent? |
…inks Addresses two review findings on the mid-run OAuth consent surfacing. Consent link validation was incomplete. `urlparse` only validates the port when it is read, so `https://host:bad` and `https://host:99999` passed the previous check, and a non-empty hostname was accepted even when it contained characters no URL client can resolve (`https://exa mple.com`). `urlparse` also silently strips tab and newline, letting control characters through. The validator now reads `port` inside the guarded block, checks the hostname against a permitted character set for both registered names and IPv6 literals, and rejects whitespace and control characters before parsing. The same rules are applied in the foundry parser and the hosting layer so the two agree. Dropping an unusable link also erased the consent requirement: nothing was recorded, so both host paths emitted `response.completed` and a blocked turn looked successful, reproducing the silent drop this feature exists to fix. Consent requests are now tracked in a `_ConsentTracker` holding the requests that were emitted and the ones whose link could not be surfaced. A response with at least one usable link still terminates as `incomplete`, and a response where consent was required but no link could be shown terminates as `response.failed`, matching the connect-time path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 992057d8-78a8-45a7-9201-35af0919b071
Tao Chen (@TaoChenOSU) Because at connect time there's nothing to emit. OBO consent is per-user and per-invocation: the gateway lists the tools fine with app credentials, then demands consent only when a tool is actually called for a specific user. That arrives mid-run as a streaming |
Share the consent-link validator and stop dropping unusable consent requests at the parser layer. - Add `agent_framework/_oauth.py` with `validate_oauth_consent_link`, the single definition of what makes a consent link renderable. The Foundry parser and the Foundry hosting layer had drifted copies of these rules; both now delegate to it while keeping their own empty string vs `None` return contracts. `foundry_hosting` depends on `agent-framework-core` but not on `agent-framework-foundry`, so core is the only module both packages can reach. - Extract `_finish_consent_response` so the agent and workflow paths share one definition of the terminal status precedence: a surfaced consent link ends the turn `incomplete`, an unusable one ends it `failed`, otherwise `completed`. Higher precedence request and session persistence failures still apply before it. - Always surface an `oauth_consent_request` marker from `try_parse_oauth_consent_event`, even when the link is missing or unusable. Returning empty contents meant the host never recorded the request as dropped, so the exact unusable link case this change exists to fail was reported as `response.completed` instead. Link validation stays in the parser for diagnostics, and the host remains the single authority on whether a link is renderable. Tests cover the shared validator in core, the preserved marker in the parser, and the existing host side failure path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 992057d8-78a8-45a7-9201-35af0919b071
Resolve conflicts with the resilient long-running agent refactor, which removed `_to_outputs` and folded its dispatch into an async `_OutputItemTracker.handle()`, and centralized terminal event emission in the caller instead of the two inner handlers. Every conflict takes main's side verbatim. The OAuth consent work is re-applied on top of the new structure: - `_ConsentTracker` state moves onto `_OutputItemTracker` as `consent`. The tracker is already created once per response and threaded through both the agent and workflow paths, so the `consent_tracker` parameter threading is no longer needed. - The `oauth_consent_request` branch moves into `_OutputItemTracker.handle()`, ahead of the fallback `else` that logs the unsupported-content warning. - `_finish_consent_response` hooks the single centralized terminal site, replacing `emit_completed(usage=tracker.usage)`, and now passes usage through. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 992057d8-78a8-45a7-9201-35af0919b071
A `WorkflowAgent` surfaces an OAuth consent request through `AgentExecutor.ctx.request_info(...)`, which parks the workflow on a pending `request_info` event. OAuth consent has no response content type, so nothing a later turn sends can answer that request. Ending the turn `incomplete` promised a continuation the workflow cannot honor, and the next turn on the same conversation restored the parked checkpoint and failed deep in the workflow machinery with `Unexpected content type while awaiting request info responses`. Report the block directly instead: - `_finish_consent_response` takes a `resumable` flag. The agent path stays `incomplete`, because a plain agent re-runs on the next turn and picks up the newly granted access. The workflow path ends `failed` with a message telling the user to grant consent and start a new conversation. - `_pending_consent_links` reads the consent requests a restored checkpoint is parked on straight off `WorkflowCheckpoint.pending_request_info_events`. The restore-only run does not replay them as agent response updates, so they are not observable from the update stream. - `_handle_inner_workflow` raises before starting the run when the restored checkpoint is parked on consent, letting the caller emit the single terminal failure event the same way an unresumable `previous_response_id` does. The consent link is still surfaced in both cases; only the terminal status changes. This does not make workflow consent resumable, which needs a matching input contract in the core workflow layer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 992057d8-78a8-45a7-9201-35af0919b071
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.