Skip to content

Python: Fix toolbox consent flow in hosted agent#6249

Open
TaoChenOSU wants to merge 4 commits into
mainfrom
feature/python-fix-toolbox-consent-flow
Open

Python: Fix toolbox consent flow in hosted agent#6249
TaoChenOSU wants to merge 4 commits into
mainfrom
feature/python-fix-toolbox-consent-flow

Conversation

@TaoChenOSU
Copy link
Copy Markdown
Contributor

@TaoChenOSU TaoChenOSU commented Jun 1, 2026

Motivation and Context

Changes needed following the recent updates in the Foundry Toolbox service.

Description

  1. The consent error code has changed back to -32006
  2. The consent url is now returned as part of a nested object

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible
  • Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.- [ ]

@TaoChenOSU TaoChenOSU self-assigned this Jun 1, 2026
Copilot AI review requested due to automatic review settings June 1, 2026 20:02
@github-actions github-actions Bot changed the title Fix toolbox consent flow in hosted agent Python: Fix toolbox consent flow in hosted agent Jun 1, 2026
@moonbox3 moonbox3 added the documentation Improvements or additions to documentation label Jun 1, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Python Foundry hosted-agent toolbox sample and the Foundry hosting responses implementation to accommodate recent Foundry Toolbox consent error format changes (error code and nested consent URL payload).

Changes:

  • Update hosted-agent consent handling to recognize the new consent error code and parse nested consent URL details (including multiple tool sources).
  • Enable/expand the toolbox provisioning sample manifest (parameters + connections + toolbox tools).
  • Refresh sample docs and requirements to align with the updated manifest and dependencies.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.

File Description
python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/requirements.txt Installs Agent Framework packages explicitly for the sample.
python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/README.md Clarifies how to switch the GitHub MCP connection between PAT and OAuth2.
python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/agent.manifest.yaml Uncomments/activates parameters, connections, and toolbox tool definitions for provisioning.
python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py Adjusts consent error code and parses nested consent details to emit OAuth consent items.

Comment thread python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py Outdated
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Reviewers: 4 | Confidence: 63% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Design Approach


Automated review by TaoChenOSU's agents

@moonbox3
Copy link
Copy Markdown
Contributor

moonbox3 commented Jun 1, 2026

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/foundry_hosting/agent_framework_foundry_hosting
   _responses.py7109087%186–189, 254, 314–315, 329, 332–333, 381–382, 392, 429, 484, 498, 551, 554–558, 577, 580, 586, 588, 642, 647, 680, 683, 688–694, 697–698, 700–701, 711–714, 993, 1006, 1472–1474, 1476, 1523–1524, 1526–1527, 1529–1530, 1532–1533, 1538, 1547, 1550–1552, 1554, 1568, 1581, 1626–1627, 1629, 1633–1637, 1639, 1646–1647, 1649–1650, 1656, 1658–1662, 1669, 1675, 1697, 1703
TOTAL37488436588% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
7457 34 💤 0 ❌ 0 🔥 1m 55s ⏱️

@TaoChenOSU TaoChenOSU marked this pull request as ready for review June 3, 2026 23:41
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Reviewers: 4 | Confidence: 90%

✓ Correctness

The PR correctly updates the consent flow to handle the new structured JSON error format from the Foundry MCP gateway. The error code change (-32007 → -32006), new ConsentError dataclass, JSON parsing logic, and iteration over multiple consent errors are all implemented correctly. Edge cases (missing JSON, malformed payloads, non-matching error types) are handled defensively. The only caller of consent_url_from_error has been updated to match the new return type. No correctness bugs found.

✓ Security Reliability

The PR adapts the consent flow to a new structured JSON error format from the Foundry MCP gateway. The implementation is well-defended: JSON parsing is wrapped in try/except, structure validation is thorough, and failure modes consistently return None (causing the original exception to re-raise). No new security or reliability issues introduced.

✓ Test Coverage

The PR updates consent error parsing from a single URL to a structured list of ConsentError objects, with tests updated accordingly. The primary test coverage gap is the absence of a test for multiple consent errors in a single payload — the core new capability enabled by the refactored return type. A secondary gap is the untested json.JSONDecodeError branch.

✗ Design Approach

The consent-parser change itself looks aligned with the Foundry gateway update, but the new design of surfacing one consent item per tool is not carried through the rest of the repo’s consent-content pipeline. As written, the tool-specific label is emitted at the host boundary and then dropped by existing response-to-content/frontend bridges, so the main new behavior is only partially realized.

Flagged Issues

  • The new per-tool server_label does not survive the existing consent-content pipeline: _responses.py:504 now sets it to the tool name, but _item_to_message() later rebuilds consent content from only oauth.consent_link (_responses.py:1396-140), and AG-UI emits only {"consent_link": content.consent_link} (_run_common.py:497-500). In any previous_response_id / Content / AG-UI path, clients still lose which tool needs consent. Please carry the tool name through the Content layer before switching the surfaced label semantics here.

Automated review by TaoChenOSU's agents

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants