From 7f51de9937eaa4085c1e72fc7787bd9cebdad7d0 Mon Sep 17 00:00:00 2001 From: Eduard van Valkenburg Date: Tue, 19 May 2026 16:30:55 +0200 Subject: [PATCH] Address review on Foundry Toolbox MCP samples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed feedback addressed: - Drop the branch-pinned `git+https://...@feature/...` entries from `04_foundry_toolbox/requirements.txt`; restore the simple comment + `mcp` runtime dep. The git pins were only useful while iterating on the PR and shouldn't ship. (eavanvalkenburg) - Fix the `/toolsets/` typo in both `04_foundry_toolbox/README.md` and `06_files/README.md`. Verified empirically against the research_toolbox in the test workspace: the toolbox MCP gateway lives at `/toolboxes/{name}/mcp?api-version=v1` and requires the `Foundry-Features: Toolboxes=V1Preview` header. `/toolsets/{name}/mcp` returns 403 with `preview_feature_required: Toolsets=V1Preview` (a different opt-in feature). - Wrap `httpx.AsyncClient(...)` in `async with ... as http_client:` in both samples so the connection pool is cleaned up. (Copilot reviewer) - Make the `TOOLBOX_NAME` env var consistent in both samples. Previously the tool name silently fell back to `"toolbox"` when `TOOLBOX_NAME` was unset, but `resolve_toolbox_endpoint()` still required `TOOLBOX_NAME` and would raise `KeyError`. The samples now resolve the endpoint once and derive the tool name from the resolved URL when `TOOLBOX_NAME` isn't set, so the local tool name always matches the upstream toolbox identity regardless of which env var the user set. (Copilot reviewer) - Rename `_responses.is_consent_error` to `consent_url_from_error`: the helper returns `str | None` (the consent URL), not a bool, so the new name matches behavior. Update the test class accordingly. (eavanvalkenburg) - Tighten `_handle_inner_agent`'s lazy-entry catch from `Exception` to `AgentFrameworkException`, the type the MCP layer actually wraps consent errors in via `MCPStreamableHTTPTool.__aenter__` → `ToolExecutionException(inner_exception=mcp_error)`. Network failures, cancellations, and other non-framework exceptions now propagate normally instead of being briefly caught and re-raised. The test helper `_make_consent_error` is updated to use `ToolExecutionException` so it matches the real-world wrapping. (eavanvalkenburg) - Clarify the `github_pat` description in `agent.manifest.yaml` to note it's only needed when the PAT-based connection (`github-mcp-pat-conn`) is chosen; users selecting the OAuth2 connection (`github-mcp-oauth-conn`) can leave it empty. (Copilot reviewer) Validation: ran both samples end-to-end against a real Foundry toolbox (`research_toolbox`) -- the samples connect successfully and the agent lists the toolbox's MCP tools (`api_specs___fetch_azure_rest_api_docs`, etc.). `uv run poe test -P foundry_hosting` passes (119 tests), pyright + mypy clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../_responses.py | 52 +++++++++------ .../foundry_hosting/tests/test_responses.py | 25 ++++--- .../responses/04_foundry_toolbox/README.md | 6 +- .../04_foundry_toolbox/agent.manifest.yaml | 7 +- .../responses/04_foundry_toolbox/main.py | 57 ++++++++-------- .../04_foundry_toolbox/requirements.txt | 7 +- .../responses/06_files/README.md | 6 +- .../responses/06_files/main.py | 65 ++++++++++--------- 8 files changed, 126 insertions(+), 99 deletions(-) diff --git a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py index dbaa75e458..49a461f9b1 100644 --- a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py +++ b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py @@ -26,6 +26,7 @@ from agent_framework import ( SupportsAgentRun, WorkflowAgent, ) +from agent_framework.exceptions import AgentFrameworkException from azure.ai.agentserver.responses import ( ResponseContext, ResponseEventStream, @@ -258,14 +259,21 @@ def _checkpoint_storage_for_context(root: str, context_id: str) -> FileCheckpoin CONSENT_ERROR_CODE = -32007 -def is_consent_error(exc: BaseException) -> str | None: - """Check if the exception is a consent error from the Foundry MCP gateway. +def consent_url_from_error(exc: BaseException) -> str | None: + """Return the consent URL when ``exc`` wraps a Foundry MCP gateway consent error. + + The Agent Framework MCP layer surfaces gateway consent failures by wrapping the underlying + ``McpError`` inside an :class:`AgentFrameworkException` (typically a ``ToolExecutionException`` + raised from ``MCPStreamableHTTPTool.__aenter__``). This helper inspects ``exc.args`` for a + wrapped ``McpError`` whose ``error.code`` is :data:`CONSENT_ERROR_CODE`; when found, the + consent link the gateway returned in ``error.message`` is returned. Returns ``None`` for + anything else, so callers can do ``if (url := consent_url_from_error(ex)) is None: raise``. Args: - exc: The exception to check. + exc: The exception to inspect. Returns: - The consent error message that is the URL if the exception is a consent error, otherwise None. + The consent URL if ``exc`` wraps a consent ``McpError``, otherwise ``None``. """ inner_exception = next((arg for arg in exc.args if isinstance(arg, McpError)), None) if inner_exception is not None and inner_exception.error.code == CONSENT_ERROR_CODE: @@ -424,26 +432,28 @@ class ResponsesHostServer(ResponsesAgentServerHost): else: run_kwargs["options"] = chat_options - # Lazy-enter the agent (and any MCP tools it owns). If this fails with an - # auth/consent error, surface the consent link to the client through the - # already-opened response stream instead of crashing the request. + # Lazy-enter the agent (and any MCP tools it owns). The MCP client wraps gateway + # consent failures (and other connection-time errors) in AgentFrameworkException; if + # one of those is a consent error we surface the consent link to the client through + # the already-opened response stream instead of crashing the request. Other exception + # types propagate normally so the host can handle / log them. try: await self._ensure_agent_ready() - except Exception as ex: - if consent_url := is_consent_error(ex): - logger.warning("OAuth consent required for Foundry MCP gateway.") - oauth_item = OAuthConsentRequestOutputItem( - id=IdGenerator.new_id("oacr"), - consent_link=consent_url, - server_label="Foundry Toolbox", - ) - builder = response_event_stream.add_output_item(oauth_item.id) - yield builder.emit_added(oauth_item) - yield builder.emit_done(oauth_item) - yield response_event_stream.emit_completed() - return - else: + except AgentFrameworkException as ex: + consent_url = consent_url_from_error(ex) + if consent_url is None: raise + logger.warning("OAuth consent required for Foundry MCP gateway.") + oauth_item = OAuthConsentRequestOutputItem( + id=IdGenerator.new_id("oacr"), + consent_link=consent_url, + server_label="Foundry Toolbox", + ) + builder = response_event_stream.add_output_item(oauth_item.id) + yield builder.emit_added(oauth_item) + yield builder.emit_done(oauth_item) + yield response_event_stream.emit_completed() + return # Track the current active output item builder for streaming; # lazily created on matching content, closed when a different type arrives. diff --git a/python/packages/foundry_hosting/tests/test_responses.py b/python/packages/foundry_hosting/tests/test_responses.py index 721a9484a7..46a3d7f8ef 100644 --- a/python/packages/foundry_hosting/tests/test_responses.py +++ b/python/packages/foundry_hosting/tests/test_responses.py @@ -38,7 +38,7 @@ from agent_framework_foundry_hosting._responses import ( InMemoryFunctionApprovalStorage, # pyright: ignore[reportPrivateUsage] _item_to_message, # pyright: ignore[reportPrivateUsage] _output_item_to_message, # pyright: ignore[reportPrivateUsage] - is_consent_error, + consent_url_from_error, ) @@ -2896,30 +2896,39 @@ class TestCheckpointContextPathValidation: def _make_consent_error(url: str = "https://consent.example.com/auth") -> Exception: - """Build an exception wrapping a Foundry MCP gateway consent error.""" + """Build an exception wrapping a Foundry MCP gateway consent error. + + Mirrors the real-world wrapping produced by ``MCPStreamableHTTPTool.__aenter__``, + which catches connection-time ``McpError``s and re-raises them as a + ``ToolExecutionException`` (an ``AgentFrameworkException`` subclass) with the + original error attached via ``inner_exception``. ``consent_url_from_error`` + then finds the wrapped ``McpError`` in ``exc.args``. + """ + from agent_framework.exceptions import ToolExecutionException + inner = McpError(ErrorData(code=CONSENT_ERROR_CODE, message=url)) - return Exception("MCP consent required", inner) + return ToolExecutionException("MCP consent required", inner_exception=inner) -class TestIsConsentError: +class TestConsentUrlFromError: def test_returns_consent_url_when_inner_arg_is_consent_mcp_error(self) -> None: exc = _make_consent_error("https://example.com/consent") - assert is_consent_error(exc) == "https://example.com/consent" + assert consent_url_from_error(exc) == "https://example.com/consent" def test_returns_none_when_no_mcp_error_in_args(self) -> None: - assert is_consent_error(Exception("boom")) is None + assert consent_url_from_error(Exception("boom")) is None def test_returns_none_when_mcp_error_has_different_code(self) -> None: inner = McpError(ErrorData(code=-32000, message="some other error")) exc = Exception("wrapped", inner) - assert is_consent_error(exc) is None + assert consent_url_from_error(exc) is None def test_returns_none_for_bare_mcp_error_without_wrapping(self) -> None: # `args` of a bare McpError holds the message string, not an McpError # instance, so it does not match the wrapping pattern produced by the # MCP client when it bubbles consent errors up. bare = McpError(ErrorData(code=CONSENT_ERROR_CODE, message="https://x")) - assert is_consent_error(bare) is None + assert consent_url_from_error(bare) is None class TestAgentLifecycle: diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/README.md b/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/README.md index 3c043d9a8f..11bf6f694f 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/README.md +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/README.md @@ -45,20 +45,20 @@ An extra environment variable must be set to point to the toolbox MCP endpoint. **Option A – Set `FOUNDRY_TOOLBOX_ENDPOINT` directly** (recommended for local development): ```bash -export FOUNDRY_TOOLBOX_ENDPOINT="https://.services.ai.azure.com/api/projects//toolsets//mcp?api-version=v1" +export FOUNDRY_TOOLBOX_ENDPOINT="https://.services.ai.azure.com/api/projects//toolboxes//mcp?api-version=v1" ``` Or in PowerShell: ```powershell -$env:FOUNDRY_TOOLBOX_ENDPOINT="https://.services.ai.azure.com/api/projects//toolsets//mcp?api-version=v1" +$env:FOUNDRY_TOOLBOX_ENDPOINT="https://.services.ai.azure.com/api/projects//toolboxes//mcp?api-version=v1" ``` **Option B – Set `TOOLBOX_NAME`** (used automatically by the Foundry hosting scaffolding after `azd provision`): The agent derives the endpoint at runtime as: ``` -{FOUNDRY_PROJECT_ENDPOINT}/toolsets/{TOOLBOX_NAME}/mcp?api-version=v1 +{FOUNDRY_PROJECT_ENDPOINT}/toolboxes/{TOOLBOX_NAME}/mcp?api-version=v1 ``` When deployed via `azd provision`, the scaffolding injects `TOOLBOX_NAME=agent-tools` and `FOUNDRY_PROJECT_ENDPOINT` automatically from the provisioned resources declared in [`agent.manifest.yaml`](agent.manifest.yaml). diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/agent.manifest.yaml b/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/agent.manifest.yaml index 3a8ec6e762..c8774c04f1 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/agent.manifest.yaml +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/agent.manifest.yaml @@ -26,9 +26,12 @@ template: # secret: false # description: URL of the public MCP server (e.g. https://gitmcp.io/Azure/azure-rest-api-specs) that does not require authentication # - name: github_pat -# # `azd ai agent init -m` will prompt for this value when initializing the agent manifest +# # `azd ai agent init -m` will prompt for this value when initializing the agent manifest. +# # Only needed when the GitHub MCP connection is configured to use the `github-mcp-pat-conn` +# # PAT-based connection below; if you use the `github-mcp-oauth-conn` OAuth2 connection +# # instead, you can leave this empty. # secret: true -# description: GitHub Personal Access Token used to authenticate with the GitHub MCP server (press Enter if OAuth2 is used instead) +# description: GitHub Personal Access Token used to authenticate with the GitHub MCP server (only needed when using the PAT connection; press Enter if using OAuth2 instead) # - name: language_mcp_entra_audience # secret: false # description: Entra ID audience for the Azure Language MCP server (e.g. https://cognitiveservices.azure.com/) diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/main.py b/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/main.py index e8f0a31510..c9f13109bc 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/main.py +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/main.py @@ -48,38 +48,43 @@ async def main(): # Create the toolbox token_provider = get_bearer_token_provider(credential, "https://ai.azure.com/.default") - http_client = httpx.AsyncClient( + # Resolve the endpoint once and derive the tool name from the same source: when + # ``TOOLBOX_NAME`` isn't explicitly set, parse it out of the resolved URL so the + # tool's local name and the upstream toolbox always agree. + toolbox_endpoint = resolve_toolbox_endpoint() + toolbox_name = os.environ.get("TOOLBOX_NAME") or toolbox_endpoint.rsplit("/mcp", 1)[0].rsplit("/", 1)[-1] + + async with httpx.AsyncClient( auth=ToolboxAuth(token_provider), headers={"Foundry-Features": "Toolboxes=V1Preview"}, timeout=120.0, - ) + ) as http_client: + toolbox = MCPStreamableHTTPTool( + name=toolbox_name, + url=toolbox_endpoint, + http_client=http_client, + load_prompts=False, + ) - toolbox = MCPStreamableHTTPTool( - name=os.environ.get("TOOLBOX_NAME", "toolbox"), - url=resolve_toolbox_endpoint(), - http_client=http_client, - load_prompts=False, - ) + # Create the chat client + client = FoundryChatClient( + project_endpoint=os.environ["FOUNDRY_PROJECT_ENDPOINT"], + model=os.environ["AZURE_AI_MODEL_DEPLOYMENT_NAME"], + credential=credential, + ) - # Create the chat client - client = FoundryChatClient( - project_endpoint=os.environ["FOUNDRY_PROJECT_ENDPOINT"], - model=os.environ["AZURE_AI_MODEL_DEPLOYMENT_NAME"], - credential=credential, - ) + agent = Agent( + client=client, + instructions="You are a friendly assistant. Keep your answers brief.", + tools=toolbox, + # History will be managed by the hosting infrastructure, thus there + # is no need to store history by the service. Learn more at: + # https://developers.openai.com/api/reference/resources/responses/methods/create + default_options={"store": False}, + ) - agent = Agent( - client=client, - instructions="You are a friendly assistant. Keep your answers brief.", - tools=toolbox, - # History will be managed by the hosting infrastructure, thus there - # is no need to store history by the service. Learn more at: - # https://developers.openai.com/api/reference/resources/responses/methods/create - default_options={"store": False}, - ) - - server = ResponsesHostServer(agent) - await server.run_async() + server = ResponsesHostServer(agent) + await server.run_async() if __name__ == "__main__": diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/requirements.txt b/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/requirements.txt index 1d0c4801df..eaa894b7c4 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/requirements.txt +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/04_foundry_toolbox/requirements.txt @@ -1,9 +1,4 @@ # agent-framework # agent-framework-foundry-hosting -git+https://github.com/microsoft/agent-framework.git@feature/add-more-foundry-toolbox-mcp-auth-methods-in-sample#egg=agent-framework-core&subdirectory=python/packages/core -git+https://github.com/microsoft/agent-framework.git@feature/add-more-foundry-toolbox-mcp-auth-methods-in-sample#egg=agent-framework-openai&subdirectory=python/packages/openai -git+https://github.com/microsoft/agent-framework.git@feature/add-more-foundry-toolbox-mcp-auth-methods-in-sample#egg=agent-framework-foundry&subdirectory=python/packages/foundry -git+https://github.com/microsoft/agent-framework.git@feature/add-more-foundry-toolbox-mcp-auth-methods-in-sample#egg=agent-framework-foundry-hosting&subdirectory=python/packages/foundry_hosting - -mcp>=1.24.0,<2 \ No newline at end of file +mcp>=1.24.0,<2 diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/06_files/README.md b/python/samples/04-hosting/foundry-hosted-agents/responses/06_files/README.md index 0df9b1508a..82005970f5 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/06_files/README.md +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/06_files/README.md @@ -35,20 +35,20 @@ An extra environment variable must be set to point to the toolbox MCP endpoint. **Option A – Set `FOUNDRY_TOOLBOX_ENDPOINT` directly** (recommended for local development): ```bash -export FOUNDRY_TOOLBOX_ENDPOINT="https://.services.ai.azure.com/api/projects//toolsets//mcp?api-version=v1" +export FOUNDRY_TOOLBOX_ENDPOINT="https://.services.ai.azure.com/api/projects//toolboxes//mcp?api-version=v1" ``` Or in PowerShell: ```powershell -$env:FOUNDRY_TOOLBOX_ENDPOINT="https://.services.ai.azure.com/api/projects//toolsets//mcp?api-version=v1" +$env:FOUNDRY_TOOLBOX_ENDPOINT="https://.services.ai.azure.com/api/projects//toolboxes//mcp?api-version=v1" ``` **Option B – Set `TOOLBOX_NAME`** (used automatically by the Foundry hosting scaffolding after `azd provision`): The agent derives the endpoint at runtime as: ``` -{FOUNDRY_PROJECT_ENDPOINT}/toolsets/{TOOLBOX_NAME}/mcp?api-version=v1 +{FOUNDRY_PROJECT_ENDPOINT}/toolboxes/{TOOLBOX_NAME}/mcp?api-version=v1 ``` When deployed via `azd provision`, the scaffolding injects `TOOLBOX_NAME=agent-tools` and `FOUNDRY_PROJECT_ENDPOINT` automatically from the provisioned resources declared in [`agent.manifest.yaml`](agent.manifest.yaml). diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/06_files/main.py b/python/samples/04-hosting/foundry-hosted-agents/responses/06_files/main.py index 5c3f9c927c..06c35efd87 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/06_files/main.py +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/06_files/main.py @@ -76,41 +76,46 @@ async def main(): # Create the toolbox token_provider = get_bearer_token_provider(credential, "https://ai.azure.com/.default") - http_client = httpx.AsyncClient( + # Resolve the endpoint once and derive the tool name from the same source: when + # ``TOOLBOX_NAME`` isn't explicitly set, parse it out of the resolved URL so the + # tool's local name and the upstream toolbox always agree. + toolbox_endpoint = resolve_toolbox_endpoint() + toolbox_name = os.environ.get("TOOLBOX_NAME") or toolbox_endpoint.rsplit("/mcp", 1)[0].rsplit("/", 1)[-1] + + async with httpx.AsyncClient( auth=ToolboxAuth(token_provider), headers={"Foundry-Features": "Toolboxes=V1Preview"}, timeout=120.0, - ) + ) as http_client: + toolbox = MCPStreamableHTTPTool( + name=toolbox_name, + url=toolbox_endpoint, + http_client=http_client, + load_prompts=False, + ) - toolbox = MCPStreamableHTTPTool( - name=os.environ.get("TOOLBOX_NAME", "toolbox"), - url=resolve_toolbox_endpoint(), - http_client=http_client, - load_prompts=False, - ) + # Create the chat client + client = FoundryChatClient( + project_endpoint=os.environ["FOUNDRY_PROJECT_ENDPOINT"], + model=os.environ["AZURE_AI_MODEL_DEPLOYMENT_NAME"], + credential=credential, + ) - # Create the chat client - client = FoundryChatClient( - project_endpoint=os.environ["FOUNDRY_PROJECT_ENDPOINT"], - model=os.environ["AZURE_AI_MODEL_DEPLOYMENT_NAME"], - credential=credential, - ) - - agent = Agent( - client=client, - instructions=( - "You are a friendly assistant. Keep your answers brief. " - "Make sure all mathematical calculations are performed using the code interpreter " - "instead of mental arithmetic." - ), - tools=[get_cwd, list_files, read_file, toolbox], - # History will be managed by the hosting infrastructure, thus there - # is no need to store history by the service. Learn more at: - # https://developers.openai.com/api/reference/resources/responses/methods/create - default_options={"store": False}, - ) - server = ResponsesHostServer(agent) - await server.run_async() + agent = Agent( + client=client, + instructions=( + "You are a friendly assistant. Keep your answers brief. " + "Make sure all mathematical calculations are performed using the code interpreter " + "instead of mental arithmetic." + ), + tools=[get_cwd, list_files, read_file, toolbox], + # History will be managed by the hosting infrastructure, thus there + # is no need to store history by the service. Learn more at: + # https://developers.openai.com/api/reference/resources/responses/methods/create + default_options={"store": False}, + ) + server = ResponsesHostServer(agent) + await server.run_async() if __name__ == "__main__":